Re: Move MT7530 phy muxing from DSA to PHY driver

From: Florian Fainelli
Date: Sun Sep 18 2022 - 18:58:46 EST




On 9/18/2022 4:28 AM, Arınç ÜNAL wrote:
On 17.09.2022 18:07, Andrew Lunn wrote:
Where in the address range is the mux register? Officially, PHY
drivers only have access to PHY registers, via MDIO. If the mux
register is in the switch address space, it would be better if the
switch did the mux configuration. An alternative might be to represent
the mux in DT somewhere, and have a mux driver.

I don't know this part very well but it's in the register for hw trap
modification which, I think, is in the switch address space.

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/mt7530.c?id=1f9a6abecf538cc73635f6082677a2f4dc9c89a4#n941

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/mt7530.h?id=1f9a6abecf538cc73635f6082677a2f4dc9c89a4#n500

Like you said, I don't think we can move away from the DSA driver, and would
rather keep the driver do the mux configuration.

We could change the check for phy muxing to define the phy muxing bindings
in the DSA node instead. If I understand correctly, the mdio address for
PHYs is fake, it's for the sole purpose of making the driver check if
there's request for phy muxing and which phy to mux. I'm saying this because
the MT7530 switch works fine at address 0 while also using phy0 as a slave
interface.

A property could be introduced on the DSA node for the MT7530 DSA driver:

     mdio {
         #address-cells = <1>;
         #size-cells = <0>;

         switch@0 {
             compatible = "mediatek,mt7530";
             reg = <0>;

             reset-gpios = <&pio 33 0>;

             core-supply = <&mt6323_vpa_reg>;
             io-supply = <&mt6323_vemc3v3_reg>;

             mt7530,mux-phy = <&sw0_p0>;

             ethernet-ports {
                 #address-cells = <1>;
                 #size-cells = <0>;

                 sw0_p0: port@0 {
                     reg = <0>;
                 };
             };
         };
     };

This would also allow using the phy muxing feature with any ethernet mac.
Currently, phy muxing check wants the ethernet mac to be gmac1 of a MediaTek
SoC. However, on a standalone MT7530, the switch can be wired to any SoC's
ethernet mac.

For the port which is set for PHY muxing, do not bring it as a slave
interface, just do the phy muxing operation.

Do not fail because there's no CPU port (ethernet property) defined when
there's only one port defined and it's set for PHY muxing.

I don't know if the ethernet mac needs phy-handle defined in this case.

 From mediatek,mt7530.yaml:

   Port 5 modes/configurations:
   1. Port 5 is disabled and isolated: An external phy can interface to the 2nd
      GMAC of the SOC.
      In the case of a build-in MT7530 switch, port 5 shares the RGMII bus with 2nd
      GMAC and an optional external phy. Mind the GPIO/pinctl settings of the SOC!
   2. Port 5 is muxed to PHY of port 0/4: Port 0/4 interfaces with 2nd GMAC.
      It is a simple MAC to PHY interface, port 5 needs to be setup for xMII mode
      and RGMII delay.
   3. Port 5 is muxed to GMAC5 and can interface to an external phy.
      Port 5 becomes an extra switch port.
      Only works on platform where external phy TX<->RX lines are swapped.
      Like in the Ubiquiti ER-X-SFP.
   4. Port 5 is muxed to GMAC5 and interfaces with the 2nd GAMC as 2nd CPU port.
      Currently a 2nd CPU port is not supported by DSA code.

So this mux has a scope bigger than the switch, it also affects one of
the SoCs MACs.

The phy-handle should have all the information you need, but it is
scattered over multiple locations. It could be in switch port 5, or it
could be in the SoC GMAC node.

Although the mux is in the switches address range, could you have a
tiny driver using that address range. Have this tiny driver export a
function to set the mux. Both the GMAC and the DSA driver make use of
the function, which should be enough to force the tiny driver to load
first. The GMAC and the DSA driver can then look at there phy-handle,
and determine how the mux should be set. The GMAC should probably do
that before register_netdev. The DSA driver before it registers the
switch with the DSA core.

Does that solve all your ordering issues?

I believe it does.


By using the phy-handle, you don't need any additional properties, so
backwards compatibility should not be a problem. You can change driver
code as much as you want, but ABI like DT is fixed.

Understood, thanks Andrew!

Yes this seems like a reasonably good idea, I would be a bit concerned about possibly running into issues with fw_devlink=on and whichever driver is managing the PHY device not being an actual PHY device driver provider and thus preventing the PHY device consumers from probing. This is not necessarily an issue right now though because 'phy-handle' is not (yet again) part of of_supplier_bindings.

Maybe what you can do is just describe that mux register space using a dedicated DT node, and use a syscon phandle for both the switch and/or the MAC and have them use an exported symbol routine that is responsible for configuring the mux in an atomic and consistent way.
--
Florian