[PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[Luiz Angelo Daros de Luca]

Compatible strings are used to help the driver find the chip ID/version
register for each chip family. After that, the driver can setup the
switch accordingly. Keep only the first supported model for each family
as a compatible string and reference other chip models in the
description.

The removed compatible strings have never been used in a released kernel.

CC: devicetree@vger.kernel.org
Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 .../devicetree/bindings/net/dsa/realtek.yaml  | 35 ++++++++-----------
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/realtek.yaml b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
index 8756060895a8..99ee4b5b9346 100644
--- a/Documentation/devicetree/bindings/net/dsa/realtek.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
@@ -27,32 +27,25 @@ description:
   The realtek-mdio driver is an MDIO driver and it must be inserted inside
   an MDIO node.
 
+  The compatible string is only used to identify which (silicon) family the
+  switch belongs to. Roughly speaking, a family is any set of Realtek switches
+  whose chip identification register(s) have a common location and semantics.
+  The different models in a given family can be automatically disambiguated by
+  parsing the chip identification register(s) according to the given family,
+  avoiding the need for a unique compatible string for each model.
+
 properties:
   compatible:
     enum:
       - realtek,rtl8365mb
-      - realtek,rtl8366
       - realtek,rtl8366rb
-      - realtek,rtl8366s
-      - realtek,rtl8367
-      - realtek,rtl8367b
-      - realtek,rtl8367rb
-      - realtek,rtl8367s
-      - realtek,rtl8368s
-      - realtek,rtl8369
-      - realtek,rtl8370
     description: |
-      realtek,rtl8365mb: 4+1 ports
-      realtek,rtl8366: 5+1 ports
-      realtek,rtl8366rb: 5+1 ports
-      realtek,rtl8366s: 5+1 ports
-      realtek,rtl8367:
-      realtek,rtl8367b:
-      realtek,rtl8367rb: 5+2 ports
-      realtek,rtl8367s: 5+2 ports
-      realtek,rtl8368s: 8 ports
-      realtek,rtl8369: 8+1 ports
-      realtek,rtl8370: 8+2 ports
+      realtek,rtl8365mb:
+        Use with models RTL8363NB, RTL8363NB-VB, RTL8363SC, RTL8363SC-VB,
+        RTL8364NB, RTL8364NB-VB, RTL8365MB, RTL8366SC, RTL8367RB-VB, RTL8367S,
+        RTL8367SB, RTL8370MB, RTL8310SR
+      realtek,rtl8366rb:
+        Use with models RTL8366RB, RTL8366S
 
   mdc-gpios:
     description: GPIO line for the MDC clock line.
@@ -335,7 +328,7 @@ examples:
             #size-cells = <0>;
 
             switch@29 {
-                    compatible = "realtek,rtl8367s";
+                    compatible = "realtek,rtl8365mb";
                     reg = <29>;
 
                     reset-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>;
-- 
2.35.1

[PATCH net v2 2/2] net: dsa: realtek: remove realtek,rtl8367s string

[Luiz Angelo Daros de Luca]

There is no need to add new compatible strings for each new supported
chip version. The compatible string is used only to select the subdriver
(rtl8365mb.c or rtl8366rb.c). Once in the subdriver, it will detect the
chip model by itself, ignoring which compatible string was used.

Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/realtek/realtek-mdio.c | 1 -
 drivers/net/dsa/realtek/realtek-smi.c  | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
index 31e1f100e48e..c58f49d558d2 100644
--- a/drivers/net/dsa/realtek/realtek-mdio.c
+++ b/drivers/net/dsa/realtek/realtek-mdio.c
@@ -267,7 +267,6 @@ static const struct of_device_id realtek_mdio_of_match[] = {
 #endif
 #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 6cec559c90ce..45992f79ec8d 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -551,10 +551,6 @@ static const struct of_device_id realtek_smi_of_match[] = {
 		.compatible = "realtek,rtl8365mb",
 		.data = &rtl8365mb_variant,
 	},
-	{
-		.compatible = "realtek,rtl8367s",
-		.data = &rtl8365mb_variant,
-	},
 #endif
 	{ /* sentinel */ },
 };
-- 
2.35.1

Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[Andrew Lunn]

On Mon, Apr 18, 2022 at 08:35:57PM -0300, Luiz Angelo Daros de Luca wrote:
> Compatible strings are used to help the driver find the chip ID/version
> register for each chip family. After that, the driver can setup the
> switch accordingly. Keep only the first supported model for each family
> as a compatible string and reference other chip models in the
> description.
> 
> The removed compatible strings have never been used in a released kernel.
> 
> CC: devicetree@vger.kernel.org
> Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net v2 2/2] net: dsa: realtek: remove realtek,rtl8367s string

[Andrew Lunn]

On Mon, Apr 18, 2022 at 08:35:58PM -0300, Luiz Angelo Daros de Luca wrote:
> There is no need to add new compatible strings for each new supported
> chip version. The compatible string is used only to select the subdriver
> (rtl8365mb.c or rtl8366rb.c). Once in the subdriver, it will detect the
> chip model by itself, ignoring which compatible string was used.
> 
> Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net v2 2/2] net: dsa: realtek: remove realtek,rtl8367s string

[Arınç ÜNAL]

On 19/04/2022 02:35, Luiz Angelo Daros de Luca wrote:
> There is no need to add new compatible strings for each new supported
> chip version. The compatible string is used only to select the subdriver
> (rtl8365mb.c or rtl8366rb.c). Once in the subdriver, it will detect the
> chip model by itself, ignoring which compatible string was used.
> 
> Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Acked-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Cheers.
Arınç

Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[Arınç ÜNAL]

On 19/04/2022 02:35, Luiz Angelo Daros de Luca wrote:
> Compatible strings are used to help the driver find the chip ID/version
> register for each chip family. After that, the driver can setup the
> switch accordingly. Keep only the first supported model for each family
> as a compatible string and reference other chip models in the
> description.
> 
> The removed compatible strings have never been used in a released kernel.
> 
> CC: devicetree@vger.kernel.org
> Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>

Do we know the chip ID and version of all of the switches this driver 
_can_ support? So we have all the switches actually supported under a 
single compatible string.

The chip ID seems to be the same across all the switches under this 
defacto rtl8367c family.

Alvin, could your contacts at Realtek provide the chip ID and version 
for the switches we don’t know:
RTL8363NB, RTL8363NB-VB, RTL8363SC, RTL8363SC-VB, RTL8364NB, 
RTL8364NB-VB, RTL8366SC, RTL8367SB, RTL8370MB, RTL8310SR

The switch chip IP/versions currently defined:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/realtek/rtl8365mb.c?id=a997157e42e3119b13c644549a3d8381a1d825d6#n104

Other than that:

Acked-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Cheers.
Arınç

> ---
>   .../devicetree/bindings/net/dsa/realtek.yaml  | 35 ++++++++-----------
>   1 file changed, 14 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/realtek.yaml b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
> index 8756060895a8..99ee4b5b9346 100644
> --- a/Documentation/devicetree/bindings/net/dsa/realtek.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
> @@ -27,32 +27,25 @@ description:
>     The realtek-mdio driver is an MDIO driver and it must be inserted inside
>     an MDIO node.
>   
> +  The compatible string is only used to identify which (silicon) family the
> +  switch belongs to. Roughly speaking, a family is any set of Realtek switches
> +  whose chip identification register(s) have a common location and semantics.
> +  The different models in a given family can be automatically disambiguated by
> +  parsing the chip identification register(s) according to the given family,
> +  avoiding the need for a unique compatible string for each model.
> +
>   properties:
>     compatible:
>       enum:
>         - realtek,rtl8365mb
> -      - realtek,rtl8366
>         - realtek,rtl8366rb
> -      - realtek,rtl8366s
> -      - realtek,rtl8367
> -      - realtek,rtl8367b
> -      - realtek,rtl8367rb
> -      - realtek,rtl8367s
> -      - realtek,rtl8368s
> -      - realtek,rtl8369
> -      - realtek,rtl8370
>       description: |
> -      realtek,rtl8365mb: 4+1 ports
> -      realtek,rtl8366: 5+1 ports
> -      realtek,rtl8366rb: 5+1 ports
> -      realtek,rtl8366s: 5+1 ports
> -      realtek,rtl8367:
> -      realtek,rtl8367b:
> -      realtek,rtl8367rb: 5+2 ports
> -      realtek,rtl8367s: 5+2 ports
> -      realtek,rtl8368s: 8 ports
> -      realtek,rtl8369: 8+1 ports
> -      realtek,rtl8370: 8+2 ports
> +      realtek,rtl8365mb:
> +        Use with models RTL8363NB, RTL8363NB-VB, RTL8363SC, RTL8363SC-VB,
> +        RTL8364NB, RTL8364NB-VB, RTL8365MB, RTL8366SC, RTL8367RB-VB, RTL8367S,
> +        RTL8367SB, RTL8370MB, RTL8310SR
> +      realtek,rtl8366rb:
> +        Use with models RTL8366RB, RTL8366S
>   
>     mdc-gpios:
>       description: GPIO line for the MDC clock line.
> @@ -335,7 +328,7 @@ examples:
>               #size-cells = <0>;
>   
>               switch@29 {
> -                    compatible = "realtek,rtl8367s";
> +                    compatible = "realtek,rtl8365mb";
>                       reg = <29>;
>   
>                       reset-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>;

Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[Alvin Šipraga]

On Tue, Apr 19, 2022 at 03:47:40AM +0300, Arınç ÜNAL wrote:
> On 19/04/2022 02:35, Luiz Angelo Daros de Luca wrote:
> > Compatible strings are used to help the driver find the chip ID/version
> > register for each chip family. After that, the driver can setup the
> > switch accordingly. Keep only the first supported model for each family
> > as a compatible string and reference other chip models in the
> > description.
> > 
> > The removed compatible strings have never been used in a released kernel.
> > 
> > CC: devicetree@vger.kernel.org
> > Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
> > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> 
> Do we know the chip ID and version of all of the switches this driver _can_
> support? So we have all the switches actually supported under a single
> compatible string.
> 
> The chip ID seems to be the same across all the switches under this defacto
> rtl8367c family.

Some of them will have a different chip ID, but we haven't had anyone try those
switches yet. Currently they will be unsupported, and I wouldn't want to claim
otherwise though because nobody has actually tested. There are small differences
per switch but in general these differences disappear if they have the same chip
ID. To give a more precise answer will require a lot more detail which I don't
think is relevant, but if you are curious, you can check how the vendor driver
does the detection and what the different parameters then are:

https://github.com/openwrt/openwrt/blob/aae7af4219e56c2787f675109d9dd1a44a5dcba4/target/linux/mediatek/files-5.10/drivers/net/phy/rtk/rtl8367c/rtk_switch.c#L712

> 
> Alvin, could your contacts at Realtek provide the chip ID and version for
> the switches we don’t know:
> RTL8363NB, RTL8363NB-VB, RTL8363SC, RTL8363SC-VB, RTL8364NB, RTL8364NB-VB,
> RTL8366SC, RTL8367SB, RTL8370MB, RTL8310SR

I'll ask my contact at Realtek if he can give me a full list of chip/version
values per switch, but given that the vendor driver is already doing similar
auto-detection, I think it is safe to assume that we don't require an additional
compatible string for the switches listed in Luiz' patch. The vendor driver
simply doesn't have a very granular check - presumably because the switches in
this family share many similarities - so it's not possible to infer the
chip/version ID per switch.

Kind regards,
Alvin

Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[Alvin Šipraga]

On Mon, Apr 18, 2022 at 08:35:57PM -0300, Luiz Angelo Daros de Luca wrote:
> Compatible strings are used to help the driver find the chip ID/version
> register for each chip family. After that, the driver can setup the
> switch accordingly. Keep only the first supported model for each family
> as a compatible string and reference other chip models in the
> description.
> 
> The removed compatible strings have never been used in a released kernel.
> 
> CC: devicetree@vger.kernel.org
> Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> ---
>  .../devicetree/bindings/net/dsa/realtek.yaml  | 35 ++++++++-----------
>  1 file changed, 14 insertions(+), 21 deletions(-)

I think this is OK now. I'll follow up with whatever reply I get from Realtek
regarding the revision ID register values for switches we do not own.

Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon, 18 Apr 2022 20:35:57 -0300 you wrote:
> Compatible strings are used to help the driver find the chip ID/version
> register for each chip family. After that, the driver can setup the
> switch accordingly. Keep only the first supported model for each family
> as a compatible string and reference other chip models in the
> description.
> 
> The removed compatible strings have never been used in a released kernel.
> 
> [...]

Here is the summary with links:
  - [net,v2,1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings
    https://git.kernel.org/netdev/net-next/c/6f2d04ccae9b
  - [net,v2,2/2] net: dsa: realtek: remove realtek,rtl8367s string
    https://git.kernel.org/netdev/net-next/c/fcd30c96af95

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[Luiz Angelo Daros de Luca]

> This series was applied to netdev/net-next.git (master)
> by David S. Miller <davem@davemloft.net>:
>
> On Mon, 18 Apr 2022 20:35:57 -0300 you wrote:
> > Compatible strings are used to help the driver find the chip ID/version
> > register for each chip family. After that, the driver can setup the
> > switch accordingly. Keep only the first supported model for each family
> > as a compatible string and reference other chip models in the
> > description.
> >
> > The removed compatible strings have never been used in a released kernel.
> >
> > [...]
>
> Here is the summary with links:
>   - [net,v2,1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings
>     https://git.kernel.org/netdev/net-next/c/6f2d04ccae9b
>   - [net,v2,2/2] net: dsa: realtek: remove realtek,rtl8367s string
>     https://git.kernel.org/netdev/net-next/c/fcd30c96af95
>

Hi David,

I was expecting to get those patches merged to net as well. Otherwise,
the "realtek,rtl8367s" we are removing will get into a released
kernel.

Regards,

Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[Jakub Kicinski]

On Wed, 20 Apr 2022 17:29:00 -0300 Luiz Angelo Daros de Luca wrote:
> > This series was applied to netdev/net-next.git (master)
> > by David S. Miller <davem@davemloft.net>:
> >
> > On Mon, 18 Apr 2022 20:35:57 -0300 you wrote:  
> > > Compatible strings are used to help the driver find the chip ID/version
> > > register for each chip family. After that, the driver can setup the
> > > switch accordingly. Keep only the first supported model for each family
> > > as a compatible string and reference other chip models in the
> > > description.
> > >
> > > The removed compatible strings have never been used in a released kernel.
> > >
> > > [...]  
> >
> > Here is the summary with links:
> >   - [net,v2,1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings
> >     https://git.kernel.org/netdev/net-next/c/6f2d04ccae9b
> >   - [net,v2,2/2] net: dsa: realtek: remove realtek,rtl8367s string
> >     https://git.kernel.org/netdev/net-next/c/fcd30c96af95
> >  
> 
> I was expecting to get those patches merged to net as well. Otherwise,
> the "realtek,rtl8367s" we are removing will get into a released
> kernel.

Seems reasonable. Unless someone objects I'll "yolo it" and apply 
the patches to net as well.

Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[Rob Herring]

On Mon, Apr 18, 2022 at 08:35:57PM -0300, Luiz Angelo Daros de Luca wrote:
> Compatible strings are used to help the driver find the chip ID/version
> register for each chip family. After that, the driver can setup the
> switch accordingly. Keep only the first supported model for each family
> as a compatible string and reference other chip models in the
> description.

The power supplies needing power before you can actually read the ID
registers are the same for all the variations?

The RTL8366s has a serdes power supply while the RTL8370 does not. Maybe 
that doesn't matter as the PHYs probably don't need power to access 
registers, but I didn't look at more than 2 datasheets. If there's *any* 
differences in power sequencing then you need specific compatibles.

Rob

Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings

[Luiz Angelo Daros de Luca]

> On Mon, Apr 18, 2022 at 08:35:57PM -0300, Luiz Angelo Daros de Luca wrote:
> > Compatible strings are used to help the driver find the chip ID/version
> > register for each chip family. After that, the driver can setup the
> > switch accordingly. Keep only the first supported model for each family
> > as a compatible string and reference other chip models in the
> > description.
>
> The power supplies needing power before you can actually read the ID
> registers are the same for all the variations?
>
> The RTL8366s has a serdes power supply while the RTL8370 does not. Maybe
> that doesn't matter as the PHYs probably don't need power to access
> registers, but I didn't look at more than 2 datasheets. If there's *any*
> differences in power sequencing then you need specific compatibles.

Hi Rob,

I don't believe we would need to deal with any special power sequences
in order to read the switch registers.
Anyway, if we face a situation where we do need to handle a special
device differently, it is always less drastic to add a new compatible
string than to remove one already in use.

Regards,

Luiz