From 59fafac4d646548904d801b1a824c106251a703e Mon Sep 17 00:00:00 2001 From: Alexander Boettcher Date: Thu, 3 Dec 2020 14:26:43 +0100 Subject: [PATCH] platform_drv: increase readability by adding convenience functions to make code easier readable Issue #3963 --- .../drivers/platform/spec/x86/pci_device.cc | 39 +++++---------- .../platform/spec/x86/pci_device_component.h | 48 ++++++++++++------- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/repos/os/src/drivers/platform/spec/x86/pci_device.cc b/repos/os/src/drivers/platform/spec/x86/pci_device.cc index 668bae1026..5e203fb3df 100644 --- a/repos/os/src/drivers/platform/spec/x86/pci_device.cc +++ b/repos/os/src/drivers/platform/spec/x86/pci_device.cc @@ -218,36 +218,27 @@ bool Platform::Device_component::_setup_msi(Genode::uint16_t const msi_cap) addr_t const msi_address = _irq_session->msi_address(); uint32_t const msi_value = _irq_session->msi_data(); - uint16_t msi = _device_config.read(_config_access, - msi_cap + 2, - Platform::Device::ACCESS_16BIT); + uint16_t msi = _read_config_16(msi_cap + 2); - _device_config.write(_config_access, msi_cap + 0x4, msi_address, - Platform::Device::ACCESS_32BIT); + _write_config_32(msi_cap + 0x4, msi_address); if (msi & CAP_MSI_64) { uint32_t upper_address = sizeof(msi_address) > 4 ? uint64_t(msi_address) >> 32 : 0UL; - _device_config.write(_config_access, msi_cap + 0x8, - upper_address, - Platform::Device::ACCESS_32BIT); - _device_config.write(_config_access, msi_cap + 0xc, - msi_value, - Platform::Device::ACCESS_16BIT); + _write_config_16(msi_cap + 0x8, upper_address); + _write_config_16(msi_cap + 0xc, msi_value); } else - _device_config.write(_config_access, msi_cap + 0x8, msi_value, - Platform::Device::ACCESS_16BIT); + _write_config_16(msi_cap + 0x8, msi_value); /* enable MSI */ _device_config.write(_config_access, msi_cap + 2, msi ^ MSI_ENABLED, Platform::Device::ACCESS_8BIT); - msi = _device_config.read(_config_access, - msi_cap + 2, - Platform::Device::ACCESS_16BIT); + msi = _read_config_16(msi_cap + 2); + return msi & MSI_ENABLED; } catch (...) { } @@ -266,16 +257,11 @@ bool Platform::Device_component::_setup_msix(Genode::uint16_t const msix_cap) addr_t const msi_address = _irq_session->msi_address(); uint32_t const msi_value = _irq_session->msi_data(); - uint16_t ctrl = _device_config.read(_config_access, - msix_cap + 2, - Platform::Device::ACCESS_16BIT); - - uint32_t const table = _device_config.read(_config_access, - msix_cap + 4, - Platform::Device::ACCESS_32BIT); + uint16_t ctrl = _read_config_16(msix_cap + 2); uint32_t const slots = Msix_ctrl::Slots::get(ctrl) + 1; + uint32_t const table = _read_config_32(msix_cap + 4); uint8_t const table_bir = Table_pba::Bir::masked(table); uint32_t const table_off = Table_pba::Offset::masked(table); @@ -320,13 +306,12 @@ bool Platform::Device_component::_setup_msix(Genode::uint16_t const msix_cap) /* enable MSI-X */ Msix_ctrl::Fmask::set(ctrl, 0); Msix_ctrl::Enable::set(ctrl, 1); - _device_config.write(_config_access, msix_cap + 2, ctrl, - Platform::Device::ACCESS_16BIT); + _write_config_16(msix_cap + 2, ctrl); }); /* check back that MSI-X got enabled */ - ctrl = _device_config.read(_config_access, msix_cap + 2, - Platform::Device::ACCESS_16BIT); + ctrl = _read_config_16(msix_cap + 2); + return Msix_ctrl::Enable::get(ctrl); } catch (Genode::Out_of_caps) { Genode::warning("Out_of_caps during MSI-X enablement"); } diff --git a/repos/os/src/drivers/platform/spec/x86/pci_device_component.h b/repos/os/src/drivers/platform/spec/x86/pci_device_component.h index f9456c9478..2459beae67 100644 --- a/repos/os/src/drivers/platform/spec/x86/pci_device_component.h +++ b/repos/os/src/drivers/platform/spec/x86/pci_device_component.h @@ -107,6 +107,32 @@ class Platform::Device_component : public Genode::Rpc_object, inline static access_t read(Genode::uint8_t t) { return t; }; }; + /** + * Convenience functions to increase readability of code + */ + uint16_t _read_config_16(uint16_t const cap) + { + return _device_config.read(_config_access, cap, + Platform::Device::ACCESS_16BIT); + } + + void _write_config_16(uint16_t const cap, uint16_t const value) + { + _device_config.write(_config_access, cap, value, + Platform::Device::ACCESS_16BIT); + } + + uint32_t _read_config_32(uint16_t const cap) + { + return _device_config.read(_config_access, cap, + Platform::Device::ACCESS_32BIT); + } + + void _write_config_32(uint16_t const cap, uint32_t const value) + { + _device_config.write(_config_access, cap, value, + Platform::Device::ACCESS_32BIT); + } /** * Read out msi capabilities of the device. @@ -127,19 +153,14 @@ class Platform::Device_component : public Genode::Rpc_object, { enum { PCI_STATUS = 0x6, PCI_CAP_OFFSET = 0x34 }; - Status::access_t status = Status::read(_device_config.read(_config_access, - PCI_STATUS, - Platform::Device::ACCESS_16BIT)); + Status::access_t status = Status::read(_read_config_16(PCI_STATUS)); if (!Status::Capabilities::get(status)) return 0; - Genode::uint8_t cap = _device_config.read(_config_access, - PCI_CAP_OFFSET, - Platform::Device::ACCESS_8BIT); + uint8_t cap = _read_config_16(PCI_CAP_OFFSET); for (Genode::uint16_t val = 0; cap; cap = val >> 8) { - val = _device_config.read(_config_access, cap, - Platform::Device::ACCESS_16BIT); + val = _read_config_16(cap); if ((val & 0xff) != target_cap) continue; @@ -174,8 +195,7 @@ class Platform::Device_component : public Genode::Rpc_object, } if (msi_cap) { - uint16_t msi = _device_config.read(_config_access, msi_cap + 2, - Platform::Device::ACCESS_16BIT); + uint16_t msi = _read_config_16(msi_cap + 2); if (msi & MSI_ENABLED) /* disable MSI */ @@ -185,16 +205,12 @@ class Platform::Device_component : public Genode::Rpc_object, } if (msix_cap) { - uint16_t msix = _device_config.read(_config_access, - msix_cap + 2, - Platform::Device::ACCESS_16BIT); + uint16_t msix = _read_config_16(msix_cap + 2); if (Msix_ctrl::Enable::get(msix)) { Msix_ctrl::Enable::set(msix, 0); - _device_config.write(_config_access, msix_cap + 2, - msix, - Platform::Device::ACCESS_16BIT); + _write_config_16(msix_cap + 2, msix); } }