diff mbox series

[net-next,v2,12/13] net: dsa: realtek: rtl8365mb: add RTL8367S support

Message ID 20211218081425.18722-13-luizluca@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v2,01/13] dt-bindings: net: dsa: realtek-smi: mark unsupported switches | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter warning Series does not have a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 2 this patch: 2
netdev/cc_maintainers warning 2 maintainers not CCed: kuba@kernel.org davem@davemloft.net
netdev/build_clang success Errors and warnings before: 2 this patch: 2
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 2 this patch: 2
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 82 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Luiz Angelo Daros de Luca Dec. 18, 2021, 8:14 a.m. UTC 
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@bang-olufsen.dk>.

Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.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(-)

Comments

Alvin Šipraga Dec. 19, 2021, 9:43 p.m. UTC | #1 
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@bang-olufsen.dk>.
> 
> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.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);
>
Luiz Angelo Daros de Luca Dec. 31, 2021, 12:18 a.m. UTC | #2 
> > @@ -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);
> >
>
diff mbox series

Patch

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.
 
 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
+#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
 
 /* Family-specific data and limits */
 #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
+#define RTL8365MB_MAX_NUM_PORTS		10
 
 /* 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);