linux-dev/drivers/net/dsa/realtek, branch linus/master

net: dsa: realtek: rtl8365mb: fix GMII caps for ports with internal PHY

2022-06-09T04:03:51Z

Since commit a18e6521a7d9 ("net: phylink: handle NA interface mode in phylink_fwnode_phy_connect()"), phylib defaults to GMII when no phy-mode or phy-connection-type property is specified in a DSA port node of the device tree. The same commit caused a regression in rtl8365mb whereby phylink would fail to connect, because the driver did not advertise support for GMII for ports with internal PHY. It should be noted that the aforementioned regression is not because the blamed commit was incorrect: on the contrary, the blamed commit is correcting the previous behaviour whereby unspecified phy-mode would cause the internal interface mode to be PHY_INTERFACE_MODE_NA. The rtl8365mb driver only worked by accident before because it _did_ advertise support for PHY_INTERFACE_MODE_NA, despite NA being reserved for internal use by phylink. With one mistake fixed, the other was exposed. Commit a5dba0f207e5 ("net: dsa: rtl8365mb: add GMII as user port mode") then introduced implicit support for GMII mode on ports with internal PHY to allow a PHY connection for device trees where the phy-mode is not explicitly set to "internal". At this point everything was working OK again. Subsequently, commit 6ff6064605e9 ("net: dsa: realtek: convert to phylink_generic_validate()") broke this behaviour again by discarding the usage of rtl8365mb_phy_mode_supported() - where this GMII support was indicated - while switching to the new .phylink_get_caps API. With the new API, rtl8365mb_phy_mode_supported() is no longer needed. Remove it altogether and add back the GMII capability - this time to rtl8365mb_phylink_get_caps() - so that the above default behaviour works for ports with internal PHY again. Fixes: 6ff6064605e9 ("net: dsa: realtek: convert to phylink_generic_validate()") Signed-off-by: Alvin Šipraga Reviewed-by: Russell King (Oracle) Link: https://lore.kernel.org/r/20220607184624.417641-1-alvin@pqrs.dk Signed-off-by: Jakub Kicinski

net: dsa: realtek: rtl8366rb: Serialize indirect PHY register access

2022-05-16T20:09:46Z

Lock the regmap during the whole PHY register access routines in rtl8366rb. Signed-off-by: Alvin Šipraga Tested-by: Linus Walleij Signed-off-by: Linus Walleij Reviewed-by: Vladimir Oltean Link: https://lore.kernel.org/r/20220513213618.2742895-1-linus.walleij@linaro.org Signed-off-by: Jakub Kicinski

net: dsa: remove port argument from ->change_tag_protocol()

2022-05-12T23:38:55Z

DSA has not supported (and probably will not support in the future either) independent tagging protocols per CPU port. Different switch drivers have different requirements, some may need to replicate some settings for each CPU port, some may need to apply some settings on a single CPU port, while some may have to configure some global settings and then some per-CPU-port settings. In any case, the current model where DSA calls ->change_tag_protocol for each CPU port turns out to be impractical for drivers where there are global things to be done. For example, felix calls dsa_tag_8021q_register(), which makes no sense per CPU port, so it suppresses the second call. Let drivers deal with replication towards all CPU ports, and remove the CPU port argument from the function prototype. Signed-off-by: Vladimir Oltean Acked-by: Luiz Angelo Daros de Luca Signed-off-by: Jakub Kicinski

net: dsa: realtek: remove realtek,rtl8367s string

2022-04-20T10:02:28Z

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 Reviewed-by: Alvin Šipraga Reviewed-by: Florian Fainelli Reviewed-by: Andrew Lunn Acked-by: Arınç ÜNAL Signed-off-by: David S. Miller

net: dsa: realtek: don't parse compatible string for RTL8366S

2022-04-13T13:40:59Z

This switch is not even supported, but if someone were to actually put this compatible string "realtek,rtl8366s" in their device tree, they would be greeted with a kernel panic because the probe function would dereference NULL. So let's just remove it. Link: https://lore.kernel.org/all/CACRpkdYdKZs0WExXc3=0yPNOwP+oOV60HRz7SRoGjZvYHaT=1g@mail.gmail.com/ Signed-off-by: Alvin Šipraga Reviewed-by: Andrew Lunn Signed-off-by: David S. Miller

net: dsa: realtek: fix Kconfig to assure consistent driver linkage

2022-04-13T13:30:31Z

The kernel test robot reported a build failure: or1k-linux-ld: drivers/net/dsa/realtek/realtek-smi.o:(.rodata+0x16c): undefined reference to `rtl8366rb_variant' ... with the following build configuration: CONFIG_NET_DSA_REALTEK=y CONFIG_NET_DSA_REALTEK_SMI=y CONFIG_NET_DSA_REALTEK_RTL8365MB=y CONFIG_NET_DSA_REALTEK_RTL8366RB=m The problem here is that the realtek-smi interface driver gets built-in, while the rtl8366rb switch subdriver gets built as a module, hence the symbol rtl8366rb_variant is not reachable when defining the OF device table in the interface driver. The Kconfig dependencies don't help in this scenario because they just say that the subdriver(s) depend on at least one interface driver. In fact, the subdrivers don't depend on the interface drivers at all, and can even be built even in their absence. Somewhat strangely, the interface drivers can also be built in the absence of any subdriver, BUT, if a subdriver IS enabled, then it must be reachable according to the linkage of the interface driver: effectively what the IS_REACHABLE() macro achieves. If it is not reachable, the above kind of linker error will be observed. Rather than papering over the above build error by simply using IS_REACHABLE(), we can do a little better and admit that it is actually the interface drivers that have a dependency on the subdrivers. So this patch does exactly that. Specifically, we ensure that: 1. The interface drivers' Kconfig symbols must have a value no greater than the value of any subdriver Kconfig symbols. 2. The subdrivers should by default enable both interface drivers, since most users probably want at least one of them; those interface drivers can be explicitly disabled however. What this doesn't do is prevent a user from building only a subdriver, without any interface driver. To that end, add an additional line of help in the menu to guide users in the right direction. Link: https://lore.kernel.org/all/202204110757.XIafvVnj-lkp@intel.com/ Reported-by: kernel test robot Fixes: aac94001067d ("net: dsa: realtek: add new mdio interface for drivers") Signed-off-by: Alvin Šipraga Signed-off-by: David S. Miller

net: dsa: realtek: make interface drivers depend on OF

2022-03-25T00:50:32Z

The kernel test robot reported build warnings with a randconfig that built realtek-{smi,mdio} without CONFIG_OF set. Since both interface drivers are using OF and will not probe without, add the corresponding dependency to Kconfig. Link: https://lore.kernel.org/all/202203231233.Xx73Y40o-lkp@intel.com/ Link: https://lore.kernel.org/all/202203231439.ycl0jg50-lkp@intel.com/ Fixes: aac94001067d ("net: dsa: realtek: add new mdio interface for drivers") Fixes: 765c39a4fafe ("net: dsa: realtek: convert subdrivers into modules") Signed-off-by: Alvin Šipraga Reviewed-by: Andrew Lunn Acked-by: Luiz Angelo Daros de Luca Link: https://lore.kernel.org/r/20220323124225.91763-1-alvin@pqrs.dk Signed-off-by: Jakub Kicinski

net: dsa: realtek: rtl8365mb: add support for rtl8_4t

2022-03-05T11:04:25Z

The trailing tag is also supported by this family. The default is still rtl8_4 but now the switch supports changing the tag to rtl8_4t. Reintroduce the dropped cpu in struct rtl8365mb (removed by 6147631). Signed-off-by: Luiz Angelo Daros de Luca Reviewed-by: Alvin Šipraga Reviewed-by: Vladimir Oltean Signed-off-by: David S. Miller

net: dsa: pass extack to .port_bridge_join driver methods

2022-02-27T11:06:14Z

As FDB isolation cannot be enforced between VLAN-aware bridges in lack of hardware assistance like extra FID bits, it seems plausible that many DSA switches cannot do it. Therefore, they need to reject configurations with multiple VLAN-aware bridges from the two code paths that can transition towards that state: - joining a VLAN-aware bridge - toggling VLAN awareness on an existing bridge The .port_vlan_filtering method already propagates the netlink extack to the driver, let's propagate it from .port_bridge_join too, to make sure that the driver can use the same function for both. Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller

net: dsa: realtek: rtl8365mb: serialize indirect PHY register access

2022-02-23T12:24:29Z

Realtek switches in the rtl8365mb family can access the PHY registers of the internal PHYs via the switch registers. This method is called indirect access. At a high level, the indirect PHY register access method involves reading and writing some special switch registers in a particular sequence. This works for both SMI and MDIO connected switches. Currently the rtl8365mb driver does not take any care to serialize the aforementioned access to the switch registers. In particular, it is permitted for other driver code to access other switch registers while the indirect PHY register access is ongoing. Locking is only done at the regmap level. This, however, is a bug: concurrent register access, even to unrelated switch registers, risks corrupting the PHY register value read back via the indirect access method described above. Arınç reported that the switch sometimes returns nonsense data when reading the PHY registers. In particular, a value of 0 causes the kernel's PHY subsystem to think that the link is down, but since most reads return correct data, the link then flip-flops between up and down over a period of time. The aforementioned bug can be readily observed by: 1. Enabling ftrace events for regmap and mdio 2. Polling BSMR PHY register for a connected port; it should always read the same (e.g. 0x79ed) 3. Wait for step 2 to give a different value Example command for step 2: while true; do phytool read swp2/2/0x01; done On my i.MX8MM, the above steps will yield a bogus value for the BSMR PHY register within a matter of seconds. The interleaved register access it then evident in the trace log: kworker/3:4-70 [003] ....... 1927.139849: regmap_reg_write: ethernet-switch reg=1004 val=bd phytool-16816 [002] ....... 1927.139979: regmap_reg_read: ethernet-switch reg=1f01 val=0 kworker/3:4-70 [003] ....... 1927.140381: regmap_reg_read: ethernet-switch reg=1005 val=0 phytool-16816 [002] ....... 1927.140468: regmap_reg_read: ethernet-switch reg=1d15 val=a69 kworker/3:4-70 [003] ....... 1927.140864: regmap_reg_read: ethernet-switch reg=1003 val=0 phytool-16816 [002] ....... 1927.140955: regmap_reg_write: ethernet-switch reg=1f02 val=2041 kworker/3:4-70 [003] ....... 1927.141390: regmap_reg_read: ethernet-switch reg=1002 val=0 phytool-16816 [002] ....... 1927.141479: regmap_reg_write: ethernet-switch reg=1f00 val=1 kworker/3:4-70 [003] ....... 1927.142311: regmap_reg_write: ethernet-switch reg=1004 val=be phytool-16816 [002] ....... 1927.142410: regmap_reg_read: ethernet-switch reg=1f01 val=0 kworker/3:4-70 [003] ....... 1927.142534: regmap_reg_read: ethernet-switch reg=1005 val=0 phytool-16816 [002] ....... 1927.142618: regmap_reg_read: ethernet-switch reg=1f04 val=0 phytool-16816 [002] ....... 1927.142641: mdio_access: SMI-0 read phy:0x02 reg:0x01 val:0x0000 <- ?! kworker/3:4-70 [003] ....... 1927.143037: regmap_reg_read: ethernet-switch reg=1001 val=0 kworker/3:4-70 [003] ....... 1927.143133: regmap_reg_read: ethernet-switch reg=1000 val=2d89 kworker/3:4-70 [003] ....... 1927.143213: regmap_reg_write: ethernet-switch reg=1004 val=be kworker/3:4-70 [003] ....... 1927.143291: regmap_reg_read: ethernet-switch reg=1005 val=0 kworker/3:4-70 [003] ....... 1927.143368: regmap_reg_read: ethernet-switch reg=1003 val=0 kworker/3:4-70 [003] ....... 1927.143443: regmap_reg_read: ethernet-switch reg=1002 val=6 The kworker here is polling MIB counters for stats, as evidenced by the register 0x1004 that we are writing to (RTL8365MB_MIB_ADDRESS_REG). This polling is performed every 3 seconds, but is just one example of such unsynchronized access. In Arınç's case, the driver was not using the switch IRQ, so the PHY subsystem was itself doing polling analogous to phytool in the above example. A test module was created [see second Link] to simulate such spurious switch register accesses while performing indirect PHY register reads and writes. Realtek was also consulted to confirm whether this is a known issue or not. The conclusion of these lines of inquiry is as follows: 1. Reading of PHY registers via indirect access will be aborted if, after executing the read operation (via a write to the INDIRECT_ACCESS_CTRL_REG), any register is accessed, other than INDIRECT_ACCESS_STATUS_REG. 2. The PHY register indirect read is only complete when INDIRECT_ACCESS_STATUS_REG reads zero. 3. The INDIRECT_ACCESS_DATA_REG, which is read to get the result of the PHY read, will contain the result of the last successful read operation. If there was spurious register access and the indirect read was aborted, then this register is not guaranteed to hold anything meaningful and the PHY read will silently fail. 4. PHY writes do not appear to be affected by this mechanism. 5. Other similar access routines, such as for MIB counters, although similar to the PHY indirect access method, are actually table access. Table access is not affected by spurious reads or writes of other registers. However, concurrent table access is not allowed. Currently this is protected via mib_lock, so there is nothing to fix. The above statements are corroborated both via the test module and through consultation with Realtek. In particular, Realtek states that this is simply a property of the hardware design and is not a hardware bug. To fix this problem, one must guard against regmap access while the PHY indirect register read is executing. Fix this by using the newly introduced "nolock" regmap in all PHY-related functions, and by aquiring the regmap mutex at the top level of the PHY register access callbacks. Although no issue has been observed with PHY register _writes_, this change also serializes the indirect access method there. This is done purely as a matter of convenience and for reasons of symmetry. Fixes: 4af2950c50c8 ("net: dsa: realtek-smi: add rtl8365mb subdriver for RTL8365MB-VC") Link: https://lore.kernel.org/netdev/CAJq09z5FCgG-+jVT7uxh1a-0CiiFsoKoHYsAWJtiKwv7LXKofQ@mail.gmail.com/ Link: https://lore.kernel.org/netdev/871qzwjmtv.fsf@bang-olufsen.dk/ Reported-by: Arınç ÜNAL Reported-by: Luiz Angelo Daros de Luca Signed-off-by: Alvin Šipraga Reviewed-by: Vladimir Oltean Reviewed-by: Linus Walleij Signed-off-by: David S. Miller