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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6e04fbc91a924201aac672165a938bf76cf873f1.camel@pengutronix.de>
Date:   Thu, 24 Mar 2022 11:08:15 +0100
From:   Philipp Zabel <p.zabel@...gutronix.de>
To:     Clément Léger <clement.leger@...tlin.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Allan Nielsen <allan.nielsen@...rochip.com>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 0/2] add fwnode support to reset subsystem

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