netdev - Re: [PATCH 4/4] net: dsa: realtek: rtl8365mb: Add SGMII and HSGMII support

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 11 May 2022 20:44:13 -0300
From:   Luiz Angelo Daros de Luca <luizluca@...il.com>
To:     Alvin Šipraga <ALSI@...g-olufsen.dk>
Cc:     Vladimir Oltean <olteanv@...il.com>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH 4/4] net: dsa: realtek: rtl8365mb: Add SGMII and HSGMII support

> On Tue, May 10, 2022 at 08:29:10PM +0300, Vladimir Oltean wrote:
> > On Mon, May 09, 2022 at 12:48:48AM +0200, Hauke Mehrtens wrote:
> > > @@ -983,14 +1295,25 @@ static bool rtl8365mb_phy_mode_supported(struct dsa_switch *ds, int port,
> > >  static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port,
> > >                                    struct phylink_config *config)
> > >  {
> > > -   if (dsa_is_user_port(ds, port))
> > > +   int ext_int = rtl8365mb_extint_port_map[port];
> > > +
> > > +   config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
> > > +                              MAC_10 | MAC_100 | MAC_1000FD;
> > > +
> > > +   if (dsa_is_user_port(ds, port)) {
> > >             __set_bit(PHY_INTERFACE_MODE_INTERNAL,
> > >                       config->supported_interfaces);
> > > -   else if (dsa_is_cpu_port(ds, port))
> > > +   } else if (dsa_is_cpu_port(ds, port)) {
> >
> > What does the quality of being a user port or a CPU port have to do with
> > which interfaces are supported?
>
> Right, I think this function was actually broken already in a few ways. The
> switch will have ports with integrated PHYs, and ports with extension interfaces
> like RGMII or SGMII etc. But which of those ports one uses as a CPU port, user
> port, or (one day) DSA port, is of no concern to the switch. The supported
> interface of a given port is a static property and simply a function of the port
> number and switch model. All switch models in the family have between 1 and 2
> ports with an extension interface.
>
> Luiz introduced this map:
>
> /* valid for all 6-port or less variants */
> static const int rtl8365mb_extint_port_map[]  = { -1, -1, -1, -1, -1, -1, 1, 2, -1, -1};
>
> ... which I think is actually what we ought to test on. It can be improved, but
> currently it is correct for all supported models.

I was playing some time ago with a more detailed description of
ports/supported modes I manually extracted from Realtek docs:

https://github.com/luizluca/linux/commit/d64201989803274bf6ba8bb784e2bf500114cff5

It might have more details than the driver really needs. Although
there are a lot of new lines with duplicated parts, I don't believe
that this family will grow too much.
I can get this upstream-ready if it looks the way to go.

Another approach would be to check if the switch can describe its
capabilities programmatically. The Realtek rtl8367c code has lots and
comes and goes, reads mysterious registers, but maybe deep down there
might be a way the switch can tell how many ports it has, which one
are really in use and what modes each port does support.

Regards,

Luiz