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]

Message-ID: <4b53b688-3769-c378-ec35-3286b3229303@gmail.com>
Date:   Wed, 9 Feb 2022 19:30:19 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Luiz Angelo Daros de Luca <luizluca@...il.com>,
        netdev@...r.kernel.org
Cc:     linus.walleij@...aro.org, andrew@...n.ch, vivien.didelot@...il.com,
        olteanv@...il.com, davem@...emloft.net, kuba@...nel.org,
        alsi@...g-olufsen.dk, arinc.unal@...nc9.com
Subject: Re: [PATCH net-next] net: dsa: realtek: rtl8365mb: irq with
 realtek-mdio



On 2/9/2022 2:45 PM, Luiz Angelo Daros de Luca wrote:
> realtek-smi creates a custom ds->slave_mii_bus and uses a mdio
> device-tree subnode to associates the interrupt-controller to each port.
> However, with realtek-mdio, ds->slave_mii_bus is created and configured
> by the switch with no device-tree settings. With no interruptions, the
> switch falls back to polling the port status.
> 
> This patch adds a new ds_ops->port_setup() to configure each phy_device
> interruption. It is only used by realtek-mdio but it could probably be
> used by realtek-smi as well, removing the need for a mdio subnode in the
> realtek device-tree node.
> 
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@...il.com>
> ---
>   drivers/net/dsa/realtek/rtl8365mb.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
> index 2ed592147c20..45afe57a5d31 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb.c
> @@ -1053,6 +1053,23 @@ static void rtl8365mb_phylink_mac_link_up(struct dsa_switch *ds, int port,
>   	}
>   }
>   
> +static int rtl8365mb_port_setup(struct dsa_switch *ds, int port)
> +{
> +	struct realtek_priv *priv = ds->priv;
> +	struct phy_device *phydev;
> +
> +	if (priv->irqdomain && ds->slave_mii_bus->irq[port] == PHY_POLL) {
> +		phydev = mdiobus_get_phy(ds->slave_mii_bus, port);

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

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.
-- 
Florian