Re: [PATCH 0/2] add fwnode support to reset subsystem

[Philipp Zabel <p.zabel@pengutronix.de>]

On Mi, 2022-03-23 at 17:05 +0100, Clément Léger wrote:
[...]
> 
> As you noticed, the initial goal of the primary series was to add
> fwnode support in order to allow registering devices with software
> nodes. Since a lot of subsystem are of-centric, It was needed to
> modify them to use fwnode and thus accept the use of software nodes.
> 
> The device I'm trying to support is a PCIe card that uses a lan9662
> SoC. This card is meant to be used an ethernet switch with 2 x RJ45
> ports and 2 x 10G SFPs. The lan966x SoCs can be used in two different
> ways:
> 
>  - It can run Linux by itself, on ARM64 cores included in the SoC. This
>    use-case of the lan966x is currently being upstreamed, using a
>    traditional Device Tree representation of the lan996x HW blocks [1]
>    A number of drivers for the different IPs of the SoC have already
>    been merged in upstream Linux.
> 
>  - It can be used as a PCIe endpoint, connected to a separate platform
>    that acts as the PCIe root complex. In this case, all the devices
>    that are embedded on this SoC are exposed through PCIe BARs and the
>    ARM64 cores of the SoC are not used. Since this is a PCIe card, it
>    can be plugged on any platform, of any architecture supporting PCIe.
> 
> Appart from adding software node support, the fwnode API would also
> allow to add ACPI support more easily later.

Thank you for the explanation. So this would be used by the sparx5
switch reset driver to provide the microchip,lan966x-switch-reset
controller via software node?

If that needs to be converted to fwnode anyway, it would be nice to
include the conversion in this series as an example.

[...]
> On that side, I must say I'm not really competent regarding ACPI
> which I do not know enough to answer you on that point.
> 
> The discussions we had with Mark Brown regarding fwnode ACPI support
> pointed out the fact that we should not create unwanted ACPI support
> by using the same descriptions/specifications that exists for the
> device-tree. In order to avoid that, we suggested to explicitely left
> out ACPI with this fwnode support. This will allow to specify that
> support later and integrate it in the subsystem that have been
> converted to fwnode.

Ok.

> > 
> > On the other hand, I think it would be good to avoid the direct of_node
> > assignment, possibly by letting devm_reset_controller_register()
> > initialize of_node or fwnode from the device for most cases, and by
> > adding of_reset_controller_register() and
> > fwnode_reset_controller_register() variants that take the node as an
> > argument for the rest.
> > That could allow to eventually get rid of the of_node pointer.
> 
> Ok, I see that. Do you want this to be done in this series ?

Just thinking out loudly, before starting to drop the
rcdev->of_node assigment from drivers en masse, I'd like to use the
opportunity and turn reset_controller_register() and friends into
macros that provide the module owner as a parameter, so the explicit
rcdev->owner = THIS_MODULE assignment can be removed from the drivers
as well.

I think that is better done separately.

> > For those drivers that provide their own .of_xlate, I'm not sure it
> > would make sense to force them to use .fwnode_xlate if they don't
> > already have a reason to use fwnode on their own.
> 
> No indeed and that's why I added the fwnode_xlate -> of_xlate
> translation function, this will allow to keep the existing of_xlate
> support.

Ok.

regards
Philipp