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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220416105753.ks3ztjvqkyatehqy@bang-olufsen.dk>
Date:   Sat, 16 Apr 2022 10:57:54 +0000
From:   Alvin Šipraga <ALSI@...g-olufsen.dk>
To:     Luiz Angelo Daros de Luca <luizluca@...il.com>
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>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
        "arinc.unal@...nc9.com" <arinc.unal@...nc9.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH net 1/2] dt-bindings: net: dsa: realtek: cleanup
 compatible strings

On Sat, Apr 16, 2022 at 03:25:03AM -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.
> 
> CC: devicetree@...r.kernel.org
> Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@...il.com>
> ---
>  .../devicetree/bindings/net/dsa/realtek.yaml  | 33 +++++++------------
>  1 file changed, 12 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/realtek.yaml b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
> index 8756060895a8..9bf862abb496 100644
> --- a/Documentation/devicetree/bindings/net/dsa/realtek.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
> @@ -27,32 +27,23 @@ description:
>    The realtek-mdio driver is an MDIO driver and it must be inserted inside
>    an MDIO node.
>  
> +  The compatibility string is used only to find an identification register,

s/compatibility/compatible/

> +  (chip ID and version) which is at a different MDIO base address in different
> +  switch families. The driver then uses the chip ID/version to device how to
> +  drive the switch.

This applies to Marvell nicely but it is not accurate for Realtek, so
either make it less precise (don't mention MDIO base address), or be
more precise. Here is my suggestion:

| 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, obviating the need for a
| unique compatible string for each model.

This also makes it clear when and why a new compatible string should be
added.

> +
>  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,rtl8367rb:

s/67rb/66rb/?

> +        Use with models RTL8366RB, RTL8366S
>  
>    mdc-gpios:
>      description: GPIO line for the MDC clock line.
> @@ -335,7 +326,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
>