[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJq09z7qaHTW03QTP5JyU7U+rQP2guybVhw5OsX5FH1VvWukJA@mail.gmail.com>
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
Powered by blists - more mailing lists