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
| ||
|
Message-ID: <87k0e1wzay.fsf@bang-olufsen.dk> Date: Fri, 11 Feb 2022 09:03:02 +0000 From: Alvin Šipraga <ALSI@...g-olufsen.dk> To: Luiz Angelo Daros de Luca <luizluca@...il.com> CC: Florian Fainelli <f.fainelli@...il.com>, "open list:NETWORKING DRIVERS" <netdev@...r.kernel.org>, Linus Walleij <linus.walleij@...aro.org>, Andrew Lunn <andrew@...n.ch>, Vivien Didelot <vivien.didelot@...il.com>, Vladimir Oltean <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Arınç ÜNAL <arinc.unal@...nc9.com> Subject: Re: [PATCH net-next] net: dsa: realtek: rtl8365mb: irq with realtek-mdio Hi Luiz, Luiz Angelo Daros de Luca <luizluca@...il.com> writes: > Thanks Alvin, > >> > As far as I know, for supported models, yes. I'm not sure about models >> > rtl8363nb and rtl8364nb because they have only 2 user ports at 1 and >> > 3. >> > Anyway, they are not supported yet. >> >> I think the port number as defined in the device tree is always going to >> be the same as its PHY address on the internal bus. I had a look at the >> Realtek code and this seems to be the assumption there too. > > One of the realtek-smi.txt examples (that I also copied to > realtek.yaml) does not respect that: > > phy4: phy@4 { > reg = <4>; > interrupt-parent = <&switch_intc>; > interrupts = <12>; > }; > > I don't know if 12 is a typo here. Oh sorry, I thought we were talking about the rtl8365mb driver and the family it supports. I did not check datasheets for RTL8366RB and such. My statement was only regarding switches supported by the '65mb driver, where I still believe this is the case based on my reading of the Realtek vendor code. > > It would only matter if I do create a default association when the > specific device tree-entry is missing. It wasn't supposed to > completely remove the device-tree declaration but to make it optional. > For now, I'll put this option aside. > >> >> We could also change the DSA framework's way of creating the >> >> MDIO bus so as to be OF-aware. > > It worked like a charm. I'll send it in reply to this email. I still > have some questions about it. > >> We are not the only ones doing this. mv88e6xxx is another example. So >> Florian's suggestion seems like a good one, but we should be careful to >> maintain compatibility with older device trees. In some cases it is >> based on child node name (e.g. "mdio"), in others it is based on the >> child node compatible string (e.g. "realtek,smi-mdio", >> "marvell,mv88e6xxx-mdio-external"). > > The name "mdio" seems to be the de facto name. I'll use it. However, > it might be confusing with mdio-connected switches as you'll have an > mdio inside a switch inside another mdio. But it is exactly what it > is. > > It would not affect drivers that are already allocating slave_mii_bus > by themselves. If the driver is fine with that, that's the end of the > case. > > However, if a driver doesn't need any special properties inside the > mdio node, it might want to migrate to the default dsa slave_mii_bus. > For those already using "mdio" node name, they just need to drop their > code and move phy_read/write to dsa_switch_ops. Now, those using > different node names (like when they check the compatible strings) > will have a little more job. I believe we cannot rename a node "on the > fly". So, if the matched node name is not mdio, they still need to > allocate the slave_mii_bus. They will also need a different > dsa_switch_ops for each case because dsa_switch_ops->phy_read cannot > coexist with an externally allocated slave_mii_bus. Each driver needs > to plan their own migration path if they want to migrate. > > For realtek-smi, the code does not require the node to be named "mdio" > but doc "realtek-smi.txt" and my new "realtek.yaml" does require it. The fact that the required property is documented as "mdio" probably lets us do away with the compatible string parsing and switch to a generic implementation in the realtek drivers - although I'm no device tree lawyer, so I could be wrong here. I agree with your analysis. > If I assume that, I could simply drop the code and migrate to > read/write to dsa_switch_ops. > If not, we need to maintain both code paths and warn the user for a > couple of releases until we drop the compatible string match. > >> > If possible, I would like to define safe default values (like assuming >> > 1:1 mapping between the port number and its PHY address) for this >> > driver when interrupt-controller is present but >> > slave_mii_bus node is missing. >> >> You could just require the phy nodes to be described in the device >> tree. Then you don't need this extra port_setup code. Seems better IMO, >> or am I missing something? > > Upstream devs seem to prefer more code than more device-tree confs. I > just wanted to reduce some device-tree copy/paste. I'm ok with using a > device-tree node. Do you have an example of such a statement from an upstream dev? I am asking for my own education :-) If we require an mdio node to begin with it also obviates the whole discussion about 1:1 mapping between DSA port number and PHY address. Kind regards, Alvin
Powered by blists - more mailing lists