Re: [RFC 00/10] add support for fwnode in i2c mux system and sfp

["Clément Léger" <clement.leger@bootlin.com>]

Le Mon, 21 Feb 2022 19:41:24 +0200,
Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit :

> > 
> > We thought about adding CONFIG_OF to x86 and potentially describe this
> > card using device-tree overlays but it introduce other problems that
> > also seems difficult to solve (overlay loading without base
> > device-tree, fixup of IRQs, adresses, and so on) and CONFIG_OF is not
> > often enabled on x86 to say the least.  
> 
> Why it can't be described by SSDT overlay (if the x86 platform in question is
> ACPI based)?

This devices uses a SoC for which drivers are already available but are
meant to be used by a device-tree description. These drivers uses the
following subsystems:
- reset (no ACPI support ?)
- clk (no ACPI support ?)
- pinctrl (no ACPI support ?)
- syscon (no ACPI support ?)
- gpio
- phy
- mdio

Converting existing OF support to fwnode support and thus allowing
drivers and subsystems to be compatible with software nodes seemed like
the easiest way to do what I needed by keeping all existing drivers.
With this support, the driver is completely self-contained and does
allow the card to be plugged on whatever platform the user may have.

Again, the PCI card is independent of the platform, I do not really see
why it should be described using platform description language.

> > 
> > This series introduce a number of changes in multiple subsystems to
> > allow registering and using devices that are described with a
> > software_node description attached to a mfd_cell, making them usable
> > with the fwnode API. It was needed to modify many subsystem where
> > CONFIG_OF was tightly integrated through the use of of_xlate()
> > functions and other of_* calls. New calls have been added to use fwnode
> > API and thus be usable with a wider range of nodes. Functions that are
> > used to get the devices (pinctrl_get, clk_get and so on) also needed
> > to be changed to use the fwnode API internally.
> > 
> > For instance, the clk framework has been modified to add a
> > fwnode_xlate() callback and a new named fwnode_clk_add_hw_provider()
> > has been added. This function will register a clock using
> > fwnode_xlate() callback. Note that since the fwnode API is compatible
> > with devices that have a of_node member set, it will still be possible
> > to use the driver and get the clocks with CONFIG_OF enabled
> > configurations.  
> 
> How does this all is compatible with ACPI approaches?
> I mean we usually do not reintroduce 1:1 DT schemas in ACPI.

For the moment, I only added fwnode API support as an alternative to
support both OF and software nodes. ACPI is not meant to be handled by
this code "as-is". There is for sure some modifications to be made and
I do not know how clocks are handled when using ACPI. Based on some
thread dating back to 2018 [1], it seem it was even not supported at
all.

To be clear, I added the equivalent of the OF support but using
fwnode API because I was interested primarly in using it with software
nodes and still wanted OF support to work. I did not planned it to be
"ACPI compliant" right now since I do not have any knowledge in that
field.

> 
> I think the CCF should be converted to use fwnode APIs and meanwhile
> we may discuss how to deal with clocks on ACPI platforms, because
> it may be a part of the power management methods.

Ok, before going down that way, should the fwnode support be the "only"
one, ie remove of_clk_register and others and convert them to
fwnode_clk_register for instance or should it be left to avoid
modifying all clock drivers ?

> 
> > In some subsystems, it was possible to keep OF related function by
> > wrapping the fwnode ones. It is not yet sure if both support
> > (device-tree and fwnode) should still continue to coexists. For instance
> > if fwnode_xlate() and of_xlate() should remain since the fwnode version
> > also supports device-tree. Removing of_xlate() would of course require
> > to modify all drivers that uses it.
> > 
> > Here is an excerpt of the lan966x description when used as a PCIe card.
> > The complete description is visible at [2]. This part only describe the
> > flexcom controller and the fixed-clock that is used as an input clock.
> > 
> > static const struct property_entry ddr_clk_props[] = {
> >         PROPERTY_ENTRY_U32("clock-frequency", 30000000),  
> 
> >         PROPERTY_ENTRY_U32("#clock-cells", 0),  
> 
> Why this is used?
> 

These props actually describes a fixed-clock properties. When adding
fwnode support to clk framework, it was needed to add the
equivalent of of_xlate() for fwnode (fwnode_xlate()). The number of
cells used to describe a reference is still needed to do the
translation using fwnode_property_get_reference_args() and give the
correct arguments to fwnode_xlate().

[1]
https://lore.kernel.org/lkml/914341e7-ca94-054d-6127-522b745006b4@arm.com/T/
-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com