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: <CAJq09z7Hu-dswU41km=L2YFbKyHUJ9JkDjUGwQN5RQqowY0=1A@mail.gmail.com> Date: Fri, 11 Feb 2022 01:51:16 -0300 From: Luiz Angelo Daros de Luca <luizluca@...il.com> To: Alvin Šipraga <ALSI@...g-olufsen.dk> 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 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. 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. 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. --- Luiz
Powered by blists - more mailing lists