| Message ID | 20211216201342.25587-1-luizluca@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | net: dsa: realtek: MDIO interface and RTL8367S | expand |
Here's insight to what I've tested. I tested an RTL8365MB-VC switch connected through SMI on an Asus RT-AC88U router. I applied this patch series on net-next of 17 December 2021 using the OpenWrt master branch with slight modifications to run the net-next kernel. First, I enabled every option introduced with this patch series to make sure there's nothing wrong with the compilation process. Then, I disabled the MDIO & rtl8366rb subdriver options along with the now optional rtl4_a tag option. Therefore compiling the kernel with only the SMI, rtl8367c subdriver & rtl8_4 tag options enabled. Everything compiles and the driver works as expected in both cases. Cheers. Arınç On 16/12/2021 23:13, luizluca@gmail.com wrote: > This series refactors the current Realtek DSA driver to support MDIO > connected switchesand RTL8367S. RTL8367S is a 5+2 10/100/1000M Ethernet > switch, with one of those 2 external ports supporting SGMII/High-SGMII. > > The old realtek-smi driver was linking subdrivers into a single > realtek-smi.ko After this series, each subdriver will be an independent > module required by either realtek-smi (platform driver) or the new > realtek-mdio (mdio driver). Both interface drivers (SMI or MDIO) are > independent, and they might even work side-by-side, although it will be > difficult to find such device. The subdriver can be individually > selected but only at buildtime, saving some storage space for custom > embedded systems. > > The subdriver rtl8365mb was renamed to rtl8367c. rtl8367c is not a real > model, but it is the name Realtek uses for their driver that supports > RTL8365MB-VC, RTL8367S and other siblings. The subdriver name was not > exposed to userland, but now it will be used as the module name. If > there is a better name, this is the last opportunity to rename it again > without affecting userland. > > Existing realtek-smi devices continue to work untouched during the > tests. The realtek-smi was moved into a realtek subdirectory, but it > normally does not break things. > > I couldn't identify a fixed relation between port numbers (0..9) and > external interfaces (0..2), and I'm not sure if it is fixed for each > chip version or a device configuration. Until there is more info about > it, there is a new port property "realtek,ext-int" that can inform the > external interface. > > The rtl8367c still can only use those external interface ports as a > single CPU port. Eventually, a different device could use one of those > ports as a downlink to a second switch or as a second CPU port. RTL8367S > has an SGMII external interface, but my test device (TP-Link Archer > C5v4) uses only the second RGMII interface. We need a test device with > more external ports in use before implementing those features. > > The rtl8366rb subdriver was not tested with this patch series, but it > was only slightly touched. It would be nice to test it, especially in an > MDIO-connected switch. > > Best, > > Luiz > >
On Thu, 16 Dec 2021 17:13:29 -0300 luizluca@gmail.com wrote: > This series refactors the current Realtek DSA driver to support MDIO > connected switchesand RTL8367S. RTL8367S is a 5+2 10/100/1000M Ethernet > switch, with one of those 2 external ports supporting SGMII/High-SGMII. nit: plenty of warnings in patch 3 and patch 8, please try building patch-by-patch with C=1 and W=1. Also some checkpatch warnings that should be fixed (scripts/checkpatch.pl).
> On Thu, 16 Dec 2021 17:13:29 -0300 luizluca@gmail.com wrote: > > This series refactors the current Realtek DSA driver to support MDIO > > connected switchesand RTL8367S. RTL8367S is a 5+2 10/100/1000M Ethernet > > switch, with one of those 2 external ports supporting SGMII/High-SGMII. > > nit: plenty of warnings in patch 3 and patch 8, please try building > patch-by-patch with C=1 and W=1. Also some checkpatch warnings that > should be fixed (scripts/checkpatch.pl). Yes, I got those problems fixed. I'll resend as soon as the rtl8365mb/rtl8367c name discussion settles. Or should I already send the patches before that one? For now, here is my repo: https://github.com/luizluca/linux/commits/realtek_mdio_refactor
On Fri, Dec 17, 2021 at 05:53:49AM -0300, Luiz Angelo Daros de Luca wrote: > > On Thu, 16 Dec 2021 17:13:29 -0300 luizluca@gmail.com wrote: > > > This series refactors the current Realtek DSA driver to support MDIO > > > connected switchesand RTL8367S. RTL8367S is a 5+2 10/100/1000M Ethernet > > > switch, with one of those 2 external ports supporting SGMII/High-SGMII. > > > > nit: plenty of warnings in patch 3 and patch 8, please try building > > patch-by-patch with C=1 and W=1. Also some checkpatch warnings that > > should be fixed (scripts/checkpatch.pl). > > Yes, I got those problems fixed. I'll resend as soon as the > rtl8365mb/rtl8367c name discussion settles. > Or should I already send the patches before that one? > > For now, here is my repo: > https://github.com/luizluca/linux/commits/realtek_mdio_refactor Please give people time to comment on the patches. Don't do a resend in less than 24 hours. It is really annoying to do a review on v1 and then find lower down in your mailbox v2. Also, comments made to v1 sometimes get lost and never make it into v3. Andrew
Thanks Andrew,
I was not planning on reposting too soon. The new version only fixes
formatting issues, adds module author/description and this code
change:
--- a/drivers/net/dsa/realtek/rtl8366rb.c
+++ b/drivers/net/dsa/realtek/rtl8366rb.c
@@ -1030,10 +1030,12 @@ static int rtl8366rb_setup(struct dsa_switch *ds)
if (ret)
dev_info(priv->dev, "no interrupt support\n");
- ret = priv->setup_interface(ds);
- if (ret) {
- dev_info(priv->dev, "could not set up MDIO bus\n");
- return -ENODEV;
+ if (priv->setup_interface) {
+ ret = priv->setup_interface(ds);
+ if (ret) {
+ dev_err(priv->dev, "could not set up MDIO bus\n");
+ return -ENODEV;
+ }
}
return 0;
Anyway, there is my github repo if someone wants to take a fresh look.
I think the module name issue will take some time.
---
Luiz Angelo Daros de Luca
luizluca@gmail.com
Em sex., 17 de dez. de 2021 às 06:26, Andrew Lunn <andrew@lunn.ch> escreveu:
>
> On Fri, Dec 17, 2021 at 05:53:49AM -0300, Luiz Angelo Daros de Luca wrote:
> > > On Thu, 16 Dec 2021 17:13:29 -0300 luizluca@gmail.com wrote:
> > > > This series refactors the current Realtek DSA driver to support MDIO
> > > > connected switchesand RTL8367S. RTL8367S is a 5+2 10/100/1000M Ethernet
> > > > switch, with one of those 2 external ports supporting SGMII/High-SGMII.
> > >
> > > nit: plenty of warnings in patch 3 and patch 8, please try building
> > > patch-by-patch with C=1 and W=1. Also some checkpatch warnings that
> > > should be fixed (scripts/checkpatch.pl).
> >
> > Yes, I got those problems fixed. I'll resend as soon as the
> > rtl8365mb/rtl8367c name discussion settles.
> > Or should I already send the patches before that one?
> >
> > For now, here is my repo:
> > https://github.com/luizluca/linux/commits/realtek_mdio_refactor
>
> Please give people time to comment on the patches. Don't do a resend
> in less than 24 hours. It is really annoying to do a review on v1 and
> then find lower down in your mailbox v2. Also, comments made to v1
> sometimes get lost and never make it into v3.
>
> Andrew
This series refactors the current Realtek DSA driver to support MDIO connected switchesand RTL8367S. RTL8367S is a 5+2 10/100/1000M Ethernet switch, with one of those 2 external ports supporting SGMII/High-SGMII. The old realtek-smi driver was linking subdrivers into a single realtek-smi.ko After this series, each subdriver will be an independent module required by either realtek-smi (platform driver) or the new realtek-mdio (mdio driver). Both interface drivers (SMI or MDIO) are independent, and they might even work side-by-side, although it will be difficult to find such device. The subdriver can be individually selected but only at buildtime, saving some storage space for custom embedded systems. Existing realtek-smi devices continue to work untouched during the tests. The realtek-smi was moved into a realtek subdirectory, but it normally does not break things. I couldn't identify a fixed relation between port numbers (0..9) and external interfaces (0..2), and I'm not sure if it is fixed for each chip version or a device configuration. Until there is more info about it, there is a new port property "realtek,ext-int" that can inform the external interface. The rtl8365mb still can only use those external interface ports as a single CPU port. Eventually, a different device could use one of those ports as a downlink to a second switch or as a second CPU port. RTL8367S has an SGMII external interface, but my test device (TP-Link Archer C5v4) uses only the second RGMII interface. We need a test device with more external ports in use before implementing those features. The rtl8366rb subdriver was not tested with this patch series, but it was only slightly touched. It would be nice to test it, especially in an MDIO-connected switch. Best, Luiz
As I finish the code side, I'm sending the patches to review. The doc part might still require the yaml conversion and forward to the corresponding maillist. The rename was removed and I applied the suggested changes. I hope I didn't forget any Reviewed-By. Thank you all. Luiz
On 12/18/21 09:14, Luiz Angelo Daros de Luca wrote: > This series refactors the current Realtek DSA driver to support MDIO > connected switchesand RTL8367S. RTL8367S is a 5+2 10/100/1000M Ethernet > switch, with one of those 2 external ports supporting SGMII/High-SGMII. Hi Luiz, Please send v3 as a separate thread and not a follow-up. I don't speak for others, but it is disorienting for me to navigate the thread when posted as a follow-up. It is also appreciated if you give a brief changelog for each series in your cover letter. Just whatever changed between v2 and v3. Thanks! Kind regards, Alvin