netdev - Re: [PATCH net-next] net: dsa: realtek: rtl8365mb: irq with realtek-mdio

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]

Date:   Thu, 10 Feb 2022 19:15:58 -0300
From:   Luiz Angelo Daros de Luca <luizluca@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     "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>,
        Alvin Šipraga <alsi@...g-olufsen.dk>,
        Arınç ÜNAL <arinc.unal@...nc9.com>
Subject: Re: [PATCH net-next] net: dsa: realtek: rtl8365mb: irq with realtek-mdio

> This assumes a 1:1 mapping between the port number and its PHY address
> on the internal MDIO bus, is that always true?

Thanks Florian,

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.

> It seems to me like we are resisting as much as possible the creating of
> the MDIO bus using of_mdiobus_register() and that seems to be forcing
> you to jump through hoops to get your per-port PHY interrupts mapped.
>
> Maybe this needs to be re-considered and you should just create that
> internal MDIO bus without the help of the DSA framework and reap the
> benefits? We could also change the DSA framework's way of creating the
> MDIO bus so as to be OF-aware.

That looks like a nice idea.

I do not have any problem duplicating the mdio setup from realtek-smi
into realtek-mdio.
However, it is just 3 copies of the same code (and I believe there are
a couple more of them):

1) dsa_switch_setup()+dsa_slave_mii_bus_init()
2) realtek_smi_setup_mdio()
3) realtek_mdio_setup_mdio() (NEW)

And realtek_smi_setup_mdio only exists as a way to reference the
OF-node. And OF-node is only needed because it needs to associate the
interrupt-parent and interrupts with each phy.
I think the best solution would be a way that the
dsa_slave_mii_bus_init could look for a specific subnode. Something
like:

dsa_slave_mii_bus_init(struct dsa_switch *ds)
{
        struct device_node *dn;
...
        dn = of_get_child_by_name(ds->dn, "slave_mii_bus");
        if (dn) {
                ds->slave_mii_bus->dev.of_node = dn;
        }
...
}

It would remove the realtek_smi_setup_mdio().

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.

Does it sound ok?

--
Luiz