| 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
| ||
|
Date: Thu, 30 Dec 2021 21:18:23 -0300
From: Luiz Angelo Daros de Luca <luizluca@...il.com>
To: Alvin Šipraga <ALSI@...g-olufsen.dk>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.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>,
"olteanv@...il.com" <olteanv@...il.com>,
"arinc.unal@...nc9.com" <arinc.unal@...nc9.com>
Subject: Re: [PATCH net-next v2 12/13] net: dsa: realtek: rtl8365mb: add
RTL8367S support
> > @@ -31,7 +31,10 @@ config NET_DSA_REALTEK_RTL8365MB
> > depends on NET_DSA_REALTEK_SMI || NET_DSA_REALTEK_MDIO
> > select NET_DSA_TAG_RTL8_4
> > help
> > - Select to enable support for Realtek RTL8365MB
> > + Select to enable support for Realtek RTL8365MB-VC and RTL8367S. This subdriver
> > + might also support RTL8363NB, RTL8363NB-VB, RTL8363SC, RTL8363SC-VB, RTL8364NB,
> > + RTL8364NB-VB, RTL8366SC, RTL8367RB-VB, RTL8367SB, RTL8370MB, RTL8310SR
> > + in the future.
>
> Not sure how useful this marketing is when I am configuring my kernel.
>
I'll clean it, mentioning only the supported drivers.
> > /* Chip-specific data and limits */
> > #define RTL8365MB_CHIP_ID_8365MB_VC 0x6367
> > -#define RTL8365MB_LEARN_LIMIT_MAX_8365MB_VC 2112
>
> The learn limit actually seems to be chip-specific and not family
> specific, that's why I placed it here to begin with. For example,
> something called RTL8370B has a limit of 4160...
>
>From Realtek rtl8367c driver:
typedef enum switch_chip_e
{
CHIP_RTL8367C = 0,
CHIP_RTL8370B,
CHIP_RTL8364B,
CHIP_RTL8363SC_VB,
CHIP_END
}switch_chip_t;
and
switch (data)
{
case 0x0276:
case 0x0597:
case 0x6367:
*pSwitchChip = CHIP_RTL8367C;
halCtrl = &rtl8367c_hal_Ctrl;
break;
case 0x0652:
case 0x6368:
*pSwitchChip = CHIP_RTL8370B;
halCtrl = &rtl8370b_hal_Ctrl;
break;
case 0x0801:
case 0x6511:
if( (regValue & 0x00F0) == 0x0080)
{
*pSwitchChip = CHIP_RTL8363SC_VB;
halCtrl = &rtl8363sc_vb_hal_Ctrl;
}
else
{
*pSwitchChip = CHIP_RTL8364B;
halCtrl = &rtl8364b_hal_Ctrl;
}
break;
default:
return RT_ERR_FAILED;
}
RTL8370B does not seem to be a real device, but another "(sub)family",
like RTL8367C. I can leave it as chip_version specific but the
RTL8365MB is, for now, about RTL8367C chips. I think it is better to
leave it as a family limit.
It would make sense to have it specific for each chip family if all
Realtek DSA drivers get merged into a single one.
> > +#define RTL8365MB_CHIP_VER_8365MB_VC 0x0040
> > +
> > +#define RTL8365MB_CHIP_ID_8367S 0x6367
> > +#define RTL8365MB_CHIP_VER_8367S 0x00A0
> > +
> > +#define RTL8365MB_LEARN_LIMIT_MAX 2112
>
> But anyways, if you are going to make it family-specific rather than
> chip-specific, place it ...
OK
>
> >
> > /* Family-specific data and limits */
>
> ... somewhere under here.
>
> > #define RTL8365MB_PHYADDRMAX 7
> > #define RTL8365MB_NUM_PHYREGS 32
> > #define RTL8365MB_PHYREGMAX (RTL8365MB_NUM_PHYREGS - 1)
> > -#define RTL8365MB_MAX_NUM_PORTS 7
> > +// RTL8370MB and RTL8310SR, possibly suportable by this driver, have 10 ports
>
> C style comments :-)
>
> > +#define RTL8365MB_MAX_NUM_PORTS 10
>
> Did you mess up the indentation here? Also seems unrelated to RTL8367S
> support...
It looks like...
>
> >
> > /* Chip identification registers */
> > #define RTL8365MB_CHIP_ID_REG 0x1300
> > @@ -1964,9 +1970,22 @@ static int rtl8365mb_detect(struct realtek_priv *priv)
> >
> > switch (chip_id) {
> > case RTL8365MB_CHIP_ID_8365MB_VC:
> > - dev_info(priv->dev,
> > - "found an RTL8365MB-VC switch (ver=0x%04x)\n",
> > - chip_ver);
> > + switch (chip_ver) {
> > + case RTL8365MB_CHIP_VER_8365MB_VC:
> > + dev_info(priv->dev,
> > + "found an RTL8365MB-VC switch (ver=0x%04x)\n",
> > + chip_ver);
> > + break;
> > + case RTL8365MB_CHIP_VER_8367S:
> > + dev_info(priv->dev,
> > + "found an RTL8367S switch (ver=0x%04x)\n",
> > + chip_ver);
> > + break;
> > + default:
> > + dev_err(priv->dev, "unrecognized switch version (ver=0x%04x)",
> > + chip_ver);
> > + return -ENODEV;
> > + }
> >
> > priv->num_ports = RTL8365MB_MAX_NUM_PORTS;
> >
> > @@ -1974,7 +1993,7 @@ static int rtl8365mb_detect(struct realtek_priv *priv)
> > mb->chip_id = chip_id;
> > mb->chip_ver = chip_ver;
> > mb->port_mask = GENMASK(priv->num_ports - 1, 0);
> > - mb->learn_limit_max = RTL8365MB_LEARN_LIMIT_MAX_8365MB_VC;
> > + mb->learn_limit_max = RTL8365MB_LEARN_LIMIT_MAX;
> > mb->jam_table = rtl8365mb_init_jam_8365mb_vc;
> > mb->jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc);
> >
>
Powered by blists - more mailing lists