From 19f50a9a45126b35289a30a6a71fae3f28dc9d14 Mon Sep 17 00:00:00 2001 From: Stefan Kalkowski Date: Fri, 13 May 2022 13:54:28 +0200 Subject: [PATCH] platform_drv: enhance coding practice * more constness where possible * hide device reporter functionality in Device_reporter interface --- repos/os/src/drivers/platform/common.h | 26 +++++++-- repos/os/src/drivers/platform/device.cc | 55 +++++++++---------- repos/os/src/drivers/platform/device.h | 44 +++++++++------ .../drivers/platform/device_model_policy.cc | 2 +- repos/os/src/drivers/platform/pci.cc | 17 +++--- repos/os/src/drivers/platform/pci.h | 8 +-- .../src/drivers/platform/session_component.cc | 13 ++--- .../src/drivers/platform/session_component.h | 9 ++- 8 files changed, 99 insertions(+), 75 deletions(-) diff --git a/repos/os/src/drivers/platform/common.h b/repos/os/src/drivers/platform/common.h index 04beb3e3dd..559a4de6e9 100644 --- a/repos/os/src/drivers/platform/common.h +++ b/repos/os/src/drivers/platform/common.h @@ -15,7 +15,7 @@ namespace Driver { class Common; }; -class Driver::Common +class Driver::Common : Device_reporter { private: @@ -24,7 +24,7 @@ class Driver::Common Attached_rom_dataspace _devices_rom { _env, _rom_name.string() }; Heap _heap { _env.ram(), _env.rm() }; Sliced_heap _sliced_heap { _env.ram(), _env.rm() }; - Device_model _devices { _heap, _dev_reporter }; + Device_model _devices { _heap, *this }; Signal_handler _dev_handler { _env.ep(), *this, &Common::_handle_devices }; Driver::Root _root; @@ -36,14 +36,21 @@ class Driver::Common public: + Common(Genode::Env & env, + Attached_rom_dataspace const & config_rom); + Heap & heap() { return _heap; } Device_model & devices() { return _devices; } - void handle_config(Xml_node config); void announce_service(); + void handle_config(Xml_node config); - Common(Genode::Env & env, - Attached_rom_dataspace const & config_rom); + + /********************* + ** Device_reporter ** + *********************/ + + void update_report() override; }; @@ -51,10 +58,19 @@ void Driver::Common::_handle_devices() { _devices_rom.update(); _devices.update(_devices_rom.xml()); + update_report(); _root.update_policy(); } +void Driver::Common::update_report() +{ + if (_dev_reporter.constructed()) + _dev_reporter->generate([&] (Xml_generator & xml) { + _devices.generate(xml); }); +} + + void Driver::Common::handle_config(Xml_node config) { config.for_each_sub_node("report", [&] (Xml_node const node) { diff --git a/repos/os/src/drivers/platform/device.cc b/repos/os/src/drivers/platform/device.cc index c661a7807b..27962a06cf 100644 --- a/repos/os/src/drivers/platform/device.cc +++ b/repos/os/src/drivers/platform/device.cc @@ -37,7 +37,7 @@ void Driver::Device::acquire(Session_component & sc) _power_domain_list.for_each([&] (Power_domain & p) { bool ok = false; - sc.devices().powers().apply(p.name, [&] (Driver::Power &power) { + _model.powers().apply(p.name, [&] (Driver::Power &power) { power.on(); ok = true; }); @@ -49,7 +49,7 @@ void Driver::Device::acquire(Session_component & sc) _reset_domain_list.for_each([&] (Reset_domain & r) { bool ok = false; - sc.devices().resets().apply(r.name, [&] (Driver::Reset &reset) { + _model.resets().apply(r.name, [&] (Driver::Reset &reset) { reset.deassert(); ok = true; }); @@ -61,7 +61,7 @@ void Driver::Device::acquire(Session_component & sc) _clock_list.for_each([&] (Clock &c) { bool ok = false; - sc.devices().clocks().apply(c.name, [&] (Driver::Clock &clock) { + _model.clocks().apply(c.name, [&] (Driver::Clock &clock) { if (c.parent.valid()) clock.parent(c.parent); @@ -81,7 +81,7 @@ void Driver::Device::acquire(Session_component & sc) pci_enable(sc.env(), sc.device_pd(), *this); sc.update_devices_rom(); - sc.devices().update_report(); + _model.device_status_changed(); } @@ -94,36 +94,35 @@ void Driver::Device::release(Session_component & sc) _reset_domain_list.for_each([&] (Reset_domain & r) { - sc.devices().resets().apply(r.name, [&] (Driver::Reset &reset) { + _model.resets().apply(r.name, [&] (Driver::Reset &reset) { reset.assert(); }); }); _power_domain_list.for_each([&] (Power_domain & p) { - sc.devices().powers().apply(p.name, [&] (Driver::Power &power) { + _model.powers().apply(p.name, [&] (Driver::Power &power) { power.off(); }); }); _clock_list.for_each([&] (Clock & c) { - sc.devices().clocks().apply(c.name, [&] (Driver::Clock &clock) { + _model.clocks().apply(c.name, [&] (Driver::Clock &clock) { clock.disable(); }); }); _owner = Owner(); sc.update_devices_rom(); - sc.devices().update_report(); + _model.device_status_changed(); } -void Driver::Device::report(Xml_generator & xml, Device_model & devices, - bool info) +void Driver::Device::generate(Xml_generator & xml, bool info) const { xml.node("device", [&] () { xml.attribute("name", name()); xml.attribute("type", type()); xml.attribute("used", _owner.valid()); - _io_mem_list.for_each([&] (Io_mem & io_mem) { + _io_mem_list.for_each([&] (Io_mem const & io_mem) { xml.node("io_mem", [&] () { if (!info) return; @@ -131,14 +130,14 @@ void Driver::Device::report(Xml_generator & xml, Device_model & devices, xml.attribute("size", String<16>(Hex(io_mem.range.size))); }); }); - _irq_list.for_each([&] (Irq & irq) { + _irq_list.for_each([&] (Irq const & irq) { xml.node("irq", [&] () { if (!info) return; xml.attribute("number", irq.number); }); }); - _io_port_range_list.for_each([&] (Io_port_range & io_port_range) { + _io_port_range_list.for_each([&] (Io_port_range const & io_port_range) { xml.node("io_port_range", [&] () { if (!info) return; @@ -146,35 +145,33 @@ void Driver::Device::report(Xml_generator & xml, Device_model & devices, xml.attribute("size", String<16>(Hex(io_port_range.size))); }); }); - _property_list.for_each([&] (Property & p) { + _property_list.for_each([&] (Property const & p) { xml.node("property", [&] () { xml.attribute("name", p.name); xml.attribute("value", p.value); }); }); - _clock_list.for_each([&] (Clock &c) { - devices.clocks().apply(c.name, [&] (Driver::Clock &clock) { + _clock_list.for_each([&] (Clock const & c) { + _model.clocks().apply(c.name, [&] (Driver::Clock &clock) { xml.node("clock", [&] () { xml.attribute("rate", clock.rate().value); xml.attribute("name", c.driver_name); }); }); }); - _pci_config_list.for_each([&] (Pci_config &pci) { + _pci_config_list.for_each([&] (Pci_config const & pci) { xml.node("pci-config", [&] () { xml.attribute("vendor_id", String<16>(Hex(pci.vendor_id))); xml.attribute("device_id", String<16>(Hex(pci.device_id))); xml.attribute("class", String<16>(Hex(pci.class_code))); }); }); - - _report_platform_specifics(xml, devices); }); } -Driver::Device::Device(Name name, Type type) -: _name(name), _type(type) { } +Driver::Device::Device(Device_model & model, Name name, Type type) +: _model(model), _name(name), _type(type) { } Driver::Device::~Device() @@ -184,18 +181,20 @@ Driver::Device::~Device() } -void Driver::Device_model::update_report() +void Driver::Device_model::device_status_changed() { - if (_reporter.constructed()) - _reporter->generate([&] (Xml_generator & xml) { - for_each([&] (Device & device) { - device.report(xml, *this, true); }); - }); + _reporter.update_report(); +}; + + +void Driver::Device_model::generate(Xml_generator & xml) const +{ + for_each([&] (Device const & device) { + device.generate(xml, true); }); } void Driver::Device_model::update(Xml_node const & node) { _model.update_from_xml(*this, node); - update_report(); } diff --git a/repos/os/src/drivers/platform/device.h b/repos/os/src/drivers/platform/device.h index ca32ef8aff..828022311b 100644 --- a/repos/os/src/drivers/platform/device.h +++ b/repos/os/src/drivers/platform/device.h @@ -33,6 +33,7 @@ namespace Driver { using namespace Genode; class Device; + struct Device_reporter; struct Device_model; class Session_component; struct Irq_update_policy; @@ -173,38 +174,38 @@ class Driver::Device : private List_model::Element bridge(bridge) {} }; - Device(Name name, Type type); + Device(Device_model & model, Name name, Type type); virtual ~Device(); - Name name() const; - Type type() const; + Name name() const; + Type type() const; Owner owner() const; virtual void acquire(Session_component &); virtual void release(Session_component &); - template void for_each_irq(FN const & fn) + template void for_each_irq(FN const & fn) const { unsigned idx = 0; _irq_list.for_each([&] (Irq const & irq) { fn(idx++, irq.number, irq.type, irq.polarity, irq.mode); }); } - template void for_each_io_mem(FN const & fn) + template void for_each_io_mem(FN const & fn) const { unsigned idx = 0; _io_mem_list.for_each([&] (Io_mem const & iomem) { fn(idx++, iomem.range); }); } - template void for_each_io_port_range(FN const & fn) + template void for_each_io_port_range(FN const & fn) const { unsigned idx = 0; _io_port_range_list.for_each([&] (Io_port_range const & ipr) { fn(idx++, ipr.addr, ipr.size); }); } - template void for_pci_config(FN const & fn) + template void for_pci_config(FN const & fn) const { /* * we allow only one PCI config per device, @@ -221,19 +222,17 @@ class Driver::Device : private List_model::Element }); } - void report(Xml_generator &, Device_model &, bool); + void generate(Xml_generator &, bool) const; protected: - virtual void _report_platform_specifics(Xml_generator &, - Device_model &) {} - friend class Driver::Device_model; friend class List_model; friend class List; - Name _name; - Type _type; + Device_model & _model; + Name const _name; + Type const _type; Owner _owner {}; List_model _io_mem_list {}; List_model _irq_list {}; @@ -252,26 +251,34 @@ class Driver::Device : private List_model::Element }; +struct Driver::Device_reporter +{ + virtual ~Device_reporter() {} + + virtual void update_report() = 0; +}; + + class Driver::Device_model : public List_model::Update_policy { private: Heap & _heap; + Device_reporter & _reporter; List_model _model { }; Clocks _clocks { }; Resets _resets { }; Powers _powers { }; - Constructible & _reporter; - public: - void update_report(); + void generate(Xml_generator & xml) const; void update(Xml_node const & node); + void device_status_changed(); Device_model(Heap & heap, - Constructible & reporter) + Device_reporter & reporter) : _heap(heap), _reporter(reporter) { } ~Device_model() { @@ -280,6 +287,9 @@ class Driver::Device_model : template void for_each(FN const & fn) { _model.for_each(fn); } + template + void for_each(FN const & fn) const { _model.for_each(fn); } + /*********************** ** Update_policy API ** diff --git a/repos/os/src/drivers/platform/device_model_policy.cc b/repos/os/src/drivers/platform/device_model_policy.cc index cda8df53cc..652e28eec5 100644 --- a/repos/os/src/drivers/platform/device_model_policy.cc +++ b/repos/os/src/drivers/platform/device_model_policy.cc @@ -66,7 +66,7 @@ Device & Device_model::create_element(Genode::Xml_node node) { Device::Name name = node.attribute_value("name", Device::Name()); Device::Type type = node.attribute_value("type", Device::Type()); - return *(new (_heap) Device(name, type)); + return *(new (_heap) Device(*this, name, type)); } diff --git a/repos/os/src/drivers/platform/pci.cc b/repos/os/src/drivers/platform/pci.cc index f37394f2e6..45df1afd52 100644 --- a/repos/os/src/drivers/platform/pci.cc +++ b/repos/os/src/drivers/platform/pci.cc @@ -24,14 +24,14 @@ using namespace Pci; struct Config_helper { - Driver::Device & _dev; + Driver::Device const & _dev; Driver::Device::Pci_config const & _cfg; Attached_io_mem_dataspace _io_mem; Config _config { (addr_t)_io_mem.local_addr() }; Config_helper(Env & env, - Driver::Device & dev, + Driver::Device const & dev, Driver::Device::Pci_config const & cfg) : _dev(dev), _cfg(cfg), _io_mem(env, cfg.addr, 0x1000) { } @@ -70,21 +70,23 @@ struct Config_helper }; -void Driver::pci_enable(Env & env, Device_pd & pd, Device & dev) +void Driver::pci_enable(Env & env, Device_pd & pd, Device const & dev) { dev.for_pci_config([&] (Device::Pci_config const & pc) { Config_helper(env, dev, pc).enable(pd); }); } -void Driver::pci_disable(Env & env, Device & dev) +void Driver::pci_disable(Env & env, Device const & dev) { dev.for_pci_config([&] (Device::Pci_config const & pc) { Config_helper(env, dev, pc).disable(); }); } -void Driver::pci_msi_enable(Env & env, addr_t cfg_space, Irq_session::Info info) +void Driver::pci_msi_enable(Env & env, + addr_t cfg_space, + Irq_session::Info const info) { Attached_io_mem_dataspace io_mem { env, cfg_space, 0x1000 }; Config config { (addr_t)io_mem.local_addr() }; @@ -140,7 +142,8 @@ pci_class_code_alias(uint32_t class_code) } -bool Driver::pci_device_matches(Session_policy const & policy, Device & dev) +bool Driver::pci_device_matches(Session_policy const & policy, + Device const & dev) { bool ret = false; @@ -153,7 +156,7 @@ bool Driver::pci_device_matches(Session_policy const & policy, Device & dev) vendor_t vendor_id = node.attribute_value("vendor_id", 0); device_t device_id = node.attribute_value("device_id", 0); - dev.for_pci_config([&] (Device::Pci_config cfg) + dev.for_pci_config([&] (Device::Pci_config const cfg) { if ((pci_class_code_alias(cfg.class_code) == class_code) || (vendor_id == cfg.vendor_id && device_id == cfg.device_id)) diff --git a/repos/os/src/drivers/platform/pci.h b/repos/os/src/drivers/platform/pci.h index a5d88975c5..c5b9290ad5 100644 --- a/repos/os/src/drivers/platform/pci.h +++ b/repos/os/src/drivers/platform/pci.h @@ -22,12 +22,12 @@ namespace Driver { class Device; class Device_pd; - void pci_enable(Genode::Env & env, Device_pd & pd, Device & dev); - void pci_disable(Genode::Env & env, Device & dev); + void pci_enable(Genode::Env & env, Device_pd & pd, Device const & dev); + void pci_disable(Genode::Env & env, Device const & dev); void pci_msi_enable(Genode::Env & env, addr_t cfg_space, - Genode::Irq_session::Info info); + Genode::Irq_session::Info const info); bool pci_device_matches(Genode::Session_policy const & policy, - Device & dev); + Device const & dev); } #endif /* _SRC__DRIVERS__PLATFORM__PCI_H_ */ diff --git a/repos/os/src/drivers/platform/session_component.cc b/repos/os/src/drivers/platform/session_component.cc index bd9e220ea1..5931f7293e 100644 --- a/repos/os/src/drivers/platform/session_component.cc +++ b/repos/os/src/drivers/platform/session_component.cc @@ -49,7 +49,7 @@ void Session_component::_free_dma_buffer(Dma_buffer & buf) } -bool Session_component::matches(Device & dev) const +bool Session_component::matches(Device const & dev) const { bool ret = false; @@ -80,7 +80,7 @@ void Session_component::update_policy(bool info, Policy_version version) _device_registry.for_each([&] (Device_component & dc) { Device_state state = AWAY; - _devices.for_each([&] (Device & dev) { + _devices.for_each([&] (Device const & dev) { if (dev.name() != dc.device()) return; state = (dev.owner() == _owner_id) ? UNCHANGED : CHANGED; @@ -107,17 +107,14 @@ void Session_component::produce_xml(Xml_generator &xml) if (_version.valid()) xml.attribute("version", _version); - _devices.for_each([&] (Device & dev) { - if (matches(dev)) dev.report(xml, _devices, _info); }); + _devices.for_each([&] (Device const & dev) { + if (matches(dev)) dev.generate(xml, _info); }); } Genode::Env & Session_component::env() { return _env; } -Driver::Device_model & Session_component::devices() { return _devices; } - - Genode::Heap & Session_component::heap() { return _md_alloc; } @@ -217,7 +214,7 @@ Genode::addr_t Session_component::dma_addr(Ram_dataspace_capability ram_cap) if (!ram_cap.valid()) return ret; - _buffer_registry.for_each([&] (Dma_buffer & buf) { + _buffer_registry.for_each([&] (Dma_buffer const & buf) { if (buf.cap.local_name() == ram_cap.local_name()) ret = _env.pd().dma_addr(buf.cap); }); diff --git a/repos/os/src/drivers/platform/session_component.h b/repos/os/src/drivers/platform/session_component.h index fa4f1d7fd6..f4a0d63124 100644 --- a/repos/os/src/drivers/platform/session_component.h +++ b/repos/os/src/drivers/platform/session_component.h @@ -56,12 +56,11 @@ class Driver::Session_component ~Session_component(); - Env & env(); - Heap & heap(); - Device_model & devices(); - Device_pd & device_pd(); + Env & env(); + Heap & heap(); + Device_pd & device_pd(); - bool matches(Device &) const; + bool matches(Device const &) const; void update_devices_rom(); Ram_quota_guard & ram_quota_guard() { return _ram_quota_guard(); }