| Message ID | 20220418233558.13541-1-luizluca@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [net,v2,1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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
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>;
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
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>
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!
> 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,
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.
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
> 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
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>;
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(-)