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: <67543b6f.df0a0220.3bd32.6d5d@mx.google.com>
Date: Sat, 7 Dec 2024 13:11:23 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	upstream@...oha.com
Subject: Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port
 Gigabit DSA Switch driver

On Fri, Dec 06, 2024 at 01:57:09AM +0200, Vladimir Oltean wrote:
> On Thu, Dec 05, 2024 at 08:36:30PM +0100, Christian Marangi wrote:
> > > I guess the non-hack solution would be to permit MDIO buses to have
> > > #size-cells = 1, and MDIO devices to acquire a range of the address
> > > space, rather than just one address. Though take this with a grain of
> > > salt, I have a lot more to learn.
> > 
> > I remember this was an idea when PHY Package API were proposed and was
> > rejected as we wanted PHY to be single reg.
> 
> Would that effort have helped with MDIO devices, in the way it was proposed?
> Why did it die out?
> 
> > > If neither of those are options, in principle the hack with just
> > > selecting, randomly, one of the N internal PHY addresses as the central
> > > MDIO address should work equally fine regardless of whether we are
> > > talking about the DSA switch's MDIO address here, or the MFD device's
> > > MDIO address.
> > > 
> > > With MFD you still have the option of creating a fake MDIO controller
> > > child device, which has mdio-parent-bus = <&host_bus>, and redirecting
> > > all user port phy-handles to children of this bus. Since all regmap I/O
> > > of this fake MDIO bus goes to the MFD driver, you can implement there
> > > your hacks with page switching etc etc, and it should be equally
> > > safe.
> > 
> > I wonder if a node like this would be more consistent and descriptive?
> > 
> > mdio_bus: mdio-bus {
> >     #address-cells = <1>;
> >     #size-cells = <0>;
> > 
> >     ...
> > 
> >     mfd@1 {
> >             compatible = "airoha,an8855-mfd";
> >             reg = <1>;
> > 
> >             nvmem_node {
> >                     ...
> >             };
> > 
> >             switch_node {
> >                 ports {
> >                         port@0 {
> >                                 phy-handle = <&phy>;
> >                         };
> > 
> >                         port@1 {
> >                                 phy-handle = <&phy_2>;
> >                         }
> >                 };
> >             };
> > 
> >             phy: phy_node {
> > 
> >             };
> >     };
> > 
> >     phy_2: phy@2 {
> >         reg = <2>;
> >     }
> > 
> >     phy@3 {
> >         reg = <3>;
> >     }
> > 
> >     ..
> > };
> > 
> > No idea how to register that single phy in mfd... I guess a fake mdio is
> > needed anyway... What do you think of this node example? Or not worth it
> > and better have the fake MDIO with all the switch PHY in it?
> 
> Could you work with something like this? dtc seems to swallow it without
> any warnings...
> 
> mdio_bus: mdio {
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         soc@1 {
>                 compatible = "airoha,an8855";
>                 reg = <1>, <2>, <3>, <4>;
>                 reg-names = "phy0", "phy1", "phy2", "phy3";
> 
>                 nvmem {
>                         compatible = "airoha,an8855-nvmem";
>                 };
> 
>                 ethernet-switch {
>                         compatible = "airoha,an8855-switch";
> 
>                         ethernet-ports {
>                                 #address-cells = <1>;
>                                 #size-cells = <0>;
> 
>                                 ethernet-port@0 {
>                                         reg = <0>;
>                                         phy-handle = <&phy0>;
>                                         phy-mode = "internal";
>                                 };
> 
>                                 ethernet-port@1 {
>                                         reg = <1>;
>                                         phy-handle = <&phy1>;
>                                         phy-mode = "internal";
>                                 };
> 
>                                 ethernet-port@2 {
>                                         reg = <2>;
>                                         phy-handle = <&phy2>;
>                                         phy-mode = "internal";
>                                 };
> 
>                                 ethernet-port@3 {
>                                         reg = <3>;
>                                         phy-handle = <&phy3>;
>                                         phy-mode = "internal";
>                                 };
>                         };
>                 };
> 
>                 mdio {
>                         compatible = "airoha,an8855-mdio";
>                         mdio-parent-bus = <&host_mdio>;
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         phy0: ethernet-phy@1 {
>                                 reg = <1>;
>                         };
> 
>                         phy1: ethernet-phy@2 {
>                                 reg = <2>;
>                         };
> 
>                         phy2: ethernet-phy@3 {
>                                 reg = <3>;
>                         };
> 
>                         phy3: ethernet-phy@4 {
>                                 reg = <4>;
>                         };
>                 };
>         };
> };

I finished testing and this works, I'm not using mdio-parent-bus tho as
the mdio-mux driver seems overkill for the task and problematic for PAGE
handling. (mdio-mux doesn't provide a way to give the current addr that
is being accessed)

My big concern is dt_binding_check and how Rob might take this
implementation. We recently had another case with a MFD node and Rob
found some problems in having subnode with compatible but maybe for this
particular complex case it will be O.K.

Still have to check if it's ok to have multiple reg in the mfd root node
(for mdio schema)

-- 
	Ansuel

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ