netdev - Re: [PATCH net-next 2/6] software node: allow named software node to be created

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: <YtXE0idsKe6FZ+n4@shell.armlinux.org.uk>
Date:   Mon, 18 Jul 2022 21:38:42 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Vladimir Oltean <olteanv@...il.com>, Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Alvin __ipraga <alsi@...g-olufsen.dk>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Daniel Scally <djrscally@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        DENG Qingfang <dqfext@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        George McCollister <george.mccollister@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Landen Chao <Landen.Chao@...iatek.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-acpi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org,
        Matthias Brugger <matthias.bgg@...il.com>,
        netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Sean Wang <sean.wang@...iatek.com>,
        UNGLinuxDriver@...rochip.com,
        Vivien Didelot <vivien.didelot@...il.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        Marek Behún <kabel@...nel.org>
Subject: Re: [PATCH net-next 2/6] software node: allow named software node to
 be created

On Mon, Jul 18, 2022 at 11:07:30PM +0300, Andy Shevchenko wrote:
> On Mon, Jul 18, 2022 at 08:11:40PM +0100, Russell King (Oracle) wrote:
> > Good point - I guess we at least need to attach the swnode parent to the
> > device so its path is unique, because right now that isn't the case. I'm
> > guessing that:
> > 
> >         new_port_fwnode = fwnode_create_software_node(port_props, NULL);
> > 
> > will create something at the root of the swnode tree, and then:
> > 
> >         fixed_link_fwnode = fwnode_create_named_software_node(fixed_link_props,
> >                                                               new_port_fwnode,
> >                                                               "fixed-link");
> > 
> > will create a node with a fixed name. I guess it in part depends what
> > pathname the first node gets (which we don't specify.) I'm not familiar
> > with the swnode code to know what happens with the naming for the first
> > node.
> 
> First node's name will be unique which is guaranteed by IDA framework. If we
> have already 2B nodes, then yes, it would be problematic (but 2^31 ought to be
> enough :-).
> 
> > However, it seems sensible to me to attach the first node to the device
> > node, thus giving it a unique fwnode path. Does that solve the problem
> > in swnode land?
> 
> Yes, but in the driver you will have that as child of the device, analogue in DT
> 
>   my_root_node { // equal the level of device node you attach it to
> 	  fixed-link {
> 	  }
>   }
> 
> (Sorry, I don't know the DT syntax by heart, but I hope you got the idea.)

Yes, that looks about right.

What we're attempting to do here is create the swnode equivalent of this
DT description:

	some_node {
		phy-mode = "foo";

		fixed-link {
			speed = X;
			full-duplex;
		};
	};

and the some_node fwnode handle gets passed into phylink for it to
parse - we never attach it to the firmware tree itself. Once phylink
has parsed it, we destroy the swnode tree since it's no longer useful.

This would get used in this situation as an example:

	switch@4 {
		compatible = "marvell,mv88e6085";

		ports {
			port@0 {
				reg = <0>;
				phy-mode = "internal";
				phy-handle = <&sw_phy_0>;
			};
			...
			port@5 {
				reg = <5>;
				label = "cpu";
				ethernet = <&eth1>;
			};
		};
	};

The DSA driver knows the capabilities of the chip, so it knows what the
fastest "phy-mode" and speed would be, and whether full or half duplex
are supported.

We need to get this information into phylink some how, and my initial
approach was to add a new function to phylink to achieve that.

We would normally have passed the "port@5" node to phylink, just as we
pass the "port@0" node. However, because the "port@5" operates as a
fixed-link as determined by the hardware/driver, we need some way to
get that into phylink.

So, Vladimir's approach is to create a swnode tree that reflects the
DT layout, and rather than passing the "port@5" as a fwnode to phylink,
we instead pass that "some_node" swnode instead. Phylink then uses
normal fwnode APIs to parse the swnode tree it's been given, resulting
in it picking up the fixed-link specification as if it had been in the
original DT.

We don't augment the existing firmware tree for "port@5", we are
effectively creating a small sub-tree and using it as a subsitute
description.

I hope that clarifies what is going on here and why.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!