Hi Andrew and Vladimir, pon., 16 sty 2023 o 23:04 Andrew Lunn <andrew@xxxxxxx> napisał(a): > > On Mon, Jan 16, 2023 at 08:16:18PM +0200, Vladimir Oltean wrote: > > On Mon, Jan 16, 2023 at 05:50:13PM +0000, Russell King (Oracle) wrote: > > > On Mon, Jan 16, 2023 at 06:34:14PM +0100, Marcin Wojtas wrote: > > > > fixed-link PHYs API is used by DSA and a number of drivers > > > > and was depending on of_. Switch to fwnode_ so to make it > > > > hardware description agnostic and allow to be used in ACPI > > > > world as well. > > > > > > Would it be better to let the fixed-link PHY die, and have everyone use > > > the more flexible fixed link implementation in phylink? > > > > Would it be even better if DSA had some driver-level prerequisites to > > impose for ACPI support - like phylink support rather than adjust_link - > > and we would simply branch off to a dsa_shared_port_link_register_acpi() > > function, leaving the current dsa_shared_port_link_register_of() alone, > > with all its workarounds and hacks? I don't believe that carrying all > > that logic over to a common fwnode based API is the proper way forward. In the past couple of years, a number of subsystems have migrated to a more generic HW description abstraction (e.g. a big chunk of network, pinctrl, gpio). ACPI aside, with this patchset one can even try to describe the switch topology with the swnode (I haven't tried that though). I fully agree that there should be no 0-day baggage in the DSA ACPI binding (FYI the more fwnode- version of the dsa_shared_port_validate_of() cought one issue in the WIP ACPI description in my setup). On the other hand, I find fwnode_/device_ APIs really helpful for most of the cases - ACPI/OF/swnode differences can be hidden to a generic layer and the need of maintaining separate code paths related to the hardware description on the driver/subsystem level is minimized. An example could be found in v1 of this series, the last 4 patches in [1] show that it can be done in a simple / seamless way, especially given the ACPI (fwnode) PHY description in phylink is already settled and widely used. I am aware at the end of the day, after final review all this can be more complex. I expect that the actual DSA ACPI support acceptance will require a lot of discussions and decisions, on whether certain solutions are worth migrating from OF world or require spec modification. For now my goal was to migrate to a more generic HW description API, and so to allow possible follow-up ACPI-related modifications, and additions to be extracted and better tracked. > > I agree with you there, here is little attempt to make a clean ACPI > binding. Most of the attempts to add ACPI support seem to try to take > the short cut for just search/replace of_ with fwnode_. And we then > have to push back and say no, and generally it then goes quiet. In most cases, the devices' description is pretty straightforward: * a node (single or with some children), resources (mem, irqs), mmio address space, optionally address on a bus and a couple of properties The DSDT/SSDT tables are very well suited for this. In case of separate, contained drivers that is also really easy to maintain. However, I fully understand your concerns and caution before blessing any change related to subsystem/generic code. Therefore ACPI support addition was split after v1 (refer to discussion in [1]) and will require ACPI maintainers' input and guidelines. > > Marcin, please approach this from the other end. Please document in > Documentation/firmware-guide/acpi/dsd what a clean binding should look > like, and then try to implement it. > This is how I initially approached this (original submission: [2]; a bit updated version, working on top of the current patchset: [3]). We then agreed that in order to remove a bit hacky mitigation of the double ACPI scan problem, an MDIOSerialBus _CRS method should be defined in the ACPI spec, similar to the I2CSerialBus/SPISerialBus/UARTSerialBus. I am going to submit the first version for review in the coming days. The DSA purely ACPI-related changes would be updated and submitted, once the method is accepted. Best regards, Marcin [1] https://www.spinics.net/lists/netdev/msg827337.html [2] https://www.spinics.net/lists/netdev/msg827345.html [3] https://github.com/semihalf-wojtas-marcin/Linux-Kernel/commit/e017e69c0eda18747029bfe0c335df204670ba59
