| 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
| ||
|
Message-ID: <c4f4aef5-aeb5-047b-d3e1-78e7b4c2c968@bang-olufsen.dk>
Date: Sun, 19 Dec 2021 21:43:21 +0000
From: Alvin Šipraga <ALSI@...g-olufsen.dk>
To: Luiz Angelo Daros de Luca <luizluca@...il.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "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
On 12/18/21 09:14, Luiz Angelo Daros de Luca wrote:
> Realtek's RTL8367S, a 5+2 port 10/100/1000M Ethernet switch.
> It shares the same driver family (RTL8367C) with other models
> as the RTL8365MB-VC. Its compatible string is "realtek,rtl8367s".
>
> It was tested only with MDIO interface (realtek-mdio), although it might
> work out-of-the-box with SMI interface (using realtek-smi).
>
> This patch was based on an unpublished patch from Alvin Šipraga
> <alsi@...g-olufsen.dk>.
>
> Tested-by: Arınç ÜNAL <arinc.unal@...nc9.com>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@...il.com>
> ---
> drivers/net/dsa/realtek/Kconfig | 5 ++++-
> drivers/net/dsa/realtek/realtek-mdio.c | 1 +
> drivers/net/dsa/realtek/realtek-smi.c | 4 ++++
> drivers/net/dsa/realtek/rtl8365mb.c | 31 +++++++++++++++++++++-----
> 4 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig
> index 73b26171fade..16521500a888 100644
> --- a/drivers/net/dsa/realtek/Kconfig
> +++ b/drivers/net/dsa/realtek/Kconfig
> @@ -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.
>
> config NET_DSA_REALTEK_RTL8366RB
> tristate "Realtek RTL8366RB switch subdriver"
> diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
> index 08d13bb94d91..0acb95142b7e 100644
> --- a/drivers/net/dsa/realtek/realtek-mdio.c
> +++ b/drivers/net/dsa/realtek/realtek-mdio.c
> @@ -248,6 +248,7 @@ static const struct of_device_id realtek_mdio_of_match[] = {
> { .compatible = "realtek,rtl8366s", .data = NULL, },
> #if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8365MB)
> { .compatible = "realtek,rtl8365mb", .data = &rtl8365mb_variant, },
> + { .compatible = "realtek,rtl8367s", .data = &rtl8365mb_variant, },
> #endif
> { /* sentinel */ },
> };
> diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
> index 32690bd28128..0fc096b355c5 100644
> --- a/drivers/net/dsa/realtek/realtek-smi.c
> +++ b/drivers/net/dsa/realtek/realtek-smi.c
> @@ -511,6 +511,10 @@ static const struct of_device_id realtek_smi_of_match[] = {
> .compatible = "realtek,rtl8365mb",
> .data = &rtl8365mb_variant,
> },
> + {
> + .compatible = "realtek,rtl8367s",
> + .data = &rtl8365mb_variant,
> + },
> #endif
> { /* sentinel */ },
> };
> diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
> index b79a4639b283..e58dd4d1e7b8 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb.c
> @@ -103,13 +103,19 @@
>
> /* 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...
> +#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 ...
>
> /* 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...
>
> /* 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