diff mbox series

[RFC,v3,net-next,10/10] docs: devicetree: add documentation for the VSC7512 SPI device

Message ID 20210814025003.2449143-11-colin.foster@in-advantage.com
State New
Headers show
Series add support for VSC75XX control over SPI | expand

Commit Message

Colin Foster Aug. 14, 2021, 2:50 a.m. UTC
Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 .../devicetree/bindings/net/dsa/ocelot.txt    | 92 +++++++++++++++++++
 1 file changed, 92 insertions(+)

Comments

Colin Foster Aug. 14, 2021, 6:40 p.m. UTC | #1
On Sat, Aug 14, 2021 at 02:47:21PM +0300, Vladimir Oltean wrote:
> On Fri, Aug 13, 2021 at 07:50:03PM -0700, Colin Foster wrote:
> > +* phy_mode = "sgmii": on ports 0, 1, 2, 3
> 
> > +			port@0 {
> > +				reg = <0>;
> > +				ethernet = <&mac>;
> > +				phy-mode = "sgmii";
> > +
> > +				fixed-link {
> > +					speed = <100>;
> > +					full-duplex;
> > +				};
> > +			};
> 
> Your driver is unconditionally setting up the NPI port at gigabit and
> you claim it works, yet the device tree sees a 100Mbps fixed-link? Which
> one is right, what speed does the port operate at?

Good catch!

I made the change to ocelot_spi_vsc7512 yesterday to set it up as
gigabit, tested it, and it still works. Previously for my testing I'd
had it hard-coded to 100, because the Beaglebone I'm using only supports
100Mbps on eth0.

# phytool print swp1/0

ieee-phy: id:0x00070540

   ieee-phy: reg:BMCR(0x00) val:0x1040
      flags:          -reset -loopback +aneg-enable -power-down -isolate -aneg-restart -collision-test
      speed:          1000-half

   ieee-phy: reg:BMSR(0x01) val:0x796d
      capabilities:   -100-b4 +100-f +100-h +10-f +10-h -100-t2-f -100-t2-h
      flags:          +ext-status +aneg-complete -remote-fault +aneg-capable +link -jabber +ext-register


Of course I understand that "it works" is not the same as "it's correct"

What I wanted to accomplish was to use the speed parameter and set up
the link based on that. I looked through all the DSA drivers and
couldn't find anything that seems to do that. The closest thing I saw
was in mt7531_cpu_port_config where they set the speed to either 2500 or
1000 based on the interface. But nothing that I saw would explicitly set
the speed based on this parameter.

So I think there's something I'm missing. The fixed-link speed should apply to 
the CPU port on the switch (port@0)? Then eth0 can be manually set to a
specific speed, but if it doesn't match the fixed-link speed I'd be out
of luck? Or should an ip link or ethtool command to eth0 modify the
speeds of both sides of the connection? It feels like setting port@0 to
the fastest speed and letting it negotiate down to eth0 makes sense...

To ask the same question a different way:

I can currently run "ethtool -s eth0 speed 10 duplex full autoneg on" 
and the link at eth0 drops to 10Mbps. Pinging my desktop jumps from 
about 400us to about 600us when I do that.

Should I not be able to do that? It should be fixed at 100Mbps without
autoneg, end of story? Because in the current configuration it feels
like the fixed-link settings are more a suggestion than a rule...
Vladimir Oltean Aug. 14, 2021, 7:08 p.m. UTC | #2
On Sat, Aug 14, 2021 at 11:40:40AM -0700, Colin Foster wrote:
> On Sat, Aug 14, 2021 at 02:47:21PM +0300, Vladimir Oltean wrote:
> > On Fri, Aug 13, 2021 at 07:50:03PM -0700, Colin Foster wrote:
> > > +* phy_mode = "sgmii": on ports 0, 1, 2, 3
> > 
> > > +			port@0 {
> > > +				reg = <0>;
> > > +				ethernet = <&mac>;
> > > +				phy-mode = "sgmii";
> > > +
> > > +				fixed-link {
> > > +					speed = <100>;
> > > +					full-duplex;
> > > +				};
> > > +			};
> > 
> > Your driver is unconditionally setting up the NPI port at gigabit and
> > you claim it works, yet the device tree sees a 100Mbps fixed-link? Which
> > one is right, what speed does the port operate at?
> 
> Good catch!
> 
> I made the change to ocelot_spi_vsc7512 yesterday to set it up as
> gigabit, tested it, and it still works. Previously for my testing I'd
> had it hard-coded to 100, because the Beaglebone I'm using only supports
> 100Mbps on eth0.
> 
> # phytool print swp1/0

Why are you showing the PHY registers of swp1? Why are these relevant at all?

> 
> ieee-phy: id:0x00070540
> 
>    ieee-phy: reg:BMCR(0x00) val:0x1040
>       flags:          -reset -loopback +aneg-enable -power-down -isolate -aneg-restart -collision-test
>       speed:          1000-half

Also, 1000/half sounds like an odd speed to end negotiation at.

> 
>    ieee-phy: reg:BMSR(0x01) val:0x796d
>       capabilities:   -100-b4 +100-f +100-h +10-f +10-h -100-t2-f -100-t2-h
>       flags:          +ext-status +aneg-complete -remote-fault +aneg-capable +link -jabber +ext-register
> 
> 
> Of course I understand that "it works" is not the same as "it's correct"
> 
> What I wanted to accomplish was to use the speed parameter and set up
> the link based on that. I looked through all the DSA drivers and
> couldn't find anything that seems to do that. The closest thing I saw
> was in mt7531_cpu_port_config where they set the speed to either 2500 or
> 1000 based on the interface. But nothing that I saw would explicitly set
> the speed based on this parameter.

As I mentioned in the other email, .phylink_mac_link_up is the function
you are looking for. Phylink parses the fixed-link and calls that
function for fixed-link ports with the speed and duplex specified. Check
and see if felix_phylink_mac_link_up is not in fact called with
link_an_mode == MLO_AN_FIXED, speed == SPEED_100 and duplex == DUPLEX_FULL,
then what you are doing with that and if it makes sense for what you are
trying to do.

> 
> So I think there's something I'm missing. The fixed-link speed should apply to 
> the CPU port on the switch (port@0)?

Is this a question? It is under port@0, the port with the 'ethernet'
property i.e. the CPU port, so why should it not?

> Then eth0 can be manually set to a specific speed, but if it doesn't
> match the fixed-link speed I'd be out of luck? Or should an ip link or
> ethtool command to eth0 modify the speeds of both sides of the
> connection? It feels like setting port@0 to the fastest speed and
> letting it negotiate down to eth0 makes sense...
> 
> To ask the same question a different way:
> 
> I can currently run "ethtool -s eth0 speed 10 duplex full autoneg on" 
> and the link at eth0 drops to 10Mbps. Pinging my desktop jumps from 
> about 400us to about 600us when I do that.

If eth0 is also a fixed-link, you should not be able to do that, no.
But the fact that you are able to do that means it's not a fixed-link,
you have a pair of PHYs that freely auto-negotiate the speed between the
BeagleBone and the switch.

> 
> Should I not be able to do that? It should be fixed at 100Mbps without
> autoneg, end of story? Because in the current configuration it feels
> like the fixed-link settings are more a suggestion than a rule...
> 

It should describe the hardware configuration, of course. It is
incorrect to describe one side of a copper PHY connection as fixed-link
and the other as having a phy-handle, and it sounds like this is what
you're doing. We need to see the device tree binding for eth0, and
maybe a picture of your setup if that is possible. How do you connect
the switch board to the BeagleBone? Is it an RJ45 cable or some sort of
PCIe-style connector with fingers for an SGMII SERDES lane, in which the
board is plugged?

The device tree says SGMII, the behavior says RJ45.
Rob Herring Aug. 17, 2021, 10:08 p.m. UTC | #3
On Fri, Aug 13, 2021 at 07:50:03PM -0700, Colin Foster wrote:
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> ---
>  .../devicetree/bindings/net/dsa/ocelot.txt    | 92 +++++++++++++++++++
>  1 file changed, 92 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/ocelot.txt b/Documentation/devicetree/bindings/net/dsa/ocelot.txt
> index 7a271d070b72..edf560a50803 100644
> --- a/Documentation/devicetree/bindings/net/dsa/ocelot.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/ocelot.txt
> @@ -8,6 +8,7 @@ Currently the switches supported by the felix driver are:
>  
>  - VSC9959 (Felix)
>  - VSC9953 (Seville)
> +- VSC7511, VSC7512, VSC7513, VSC7514 via SPI
>  
>  The VSC9959 switch is found in the NXP LS1028A. It is a PCI device, part of the
>  larger ENETC root complex. As a result, the ethernet-switch node is a sub-node
> @@ -211,3 +212,94 @@ Example:
>  		};
>  	};
>  };
> +
> +The VSC7513 and VSC7514 switches can be controlled internally via the MIPS
> +processor. The VSC7511 and VSC7512 don't have this internal processor, but all
> +four chips can be controlled externally through SPI with the following required
> +properties:
> +
> +- compatible:
> +	Can be "mscc,vsc7511", "mscc,vsc7512", "mscc,vsc7513", or
> +	"mscc,vsc7514".
> +
> +Supported phy modes for all chips are:
> +
> +* phy_mode = "sgmii": on ports 0, 1, 2, 3
> +
> +The VSC7512 and 7514 also support:
> +
> +* phy_mode = "sgmii": on ports 4, 5, 6, 7
> +* phy_mode = "qsgmii": on ports 7, 8, 10
> +
> +Example for control from a BeagleBone Black
> +
> +&spi0 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ethernet-switch@0 {
> +		compatible = "mscc,vsc7512";
> +		spi-max-frequency = <250000>;
> +		reg = <0>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +				ethernet = <&mac>;
> +				phy-mode = "sgmii";
> +
> +				fixed-link {
> +					speed = <100>;
> +					full-duplex;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				label = "swp1";
> +				phy-handle = <&sw_phy1>;
> +				phy-mode = "sgmii";
> +			};
> +
> +			port@2 {
> +				reg = <2>;
> +				label = "swp2";
> +				phy-handle = <&sw_phy2>;
> +				phy-mode = "sgmii";
> +			};
> +
> +			port@3 {
> +				reg = <3>;
> +				label = "swp3";
> +				phy-handle = <&sw_phy3>;
> +				phy-mode = "sgmii";
> +			};
> +		};
> +
> +		mdio {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			sw_phy1: ethernet-phy@1 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				reg = <0x1>;
> +			};
> +
> +			sw_phy2: ethernet-phy@2 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				reg = <0x2>;
> +			};
> +
> +			sw_phy3: ethernet-phy@3 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				reg = <0x3>;
> +			};
> +		};
> +	};
> +};

If you want a whole new example, then convert this to DT schema. But is 
there anything really new or different here to warrant another example?

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/dsa/ocelot.txt b/Documentation/devicetree/bindings/net/dsa/ocelot.txt
index 7a271d070b72..edf560a50803 100644
--- a/Documentation/devicetree/bindings/net/dsa/ocelot.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ocelot.txt
@@ -8,6 +8,7 @@  Currently the switches supported by the felix driver are:
 
 - VSC9959 (Felix)
 - VSC9953 (Seville)
+- VSC7511, VSC7512, VSC7513, VSC7514 via SPI
 
 The VSC9959 switch is found in the NXP LS1028A. It is a PCI device, part of the
 larger ENETC root complex. As a result, the ethernet-switch node is a sub-node
@@ -211,3 +212,94 @@  Example:
 		};
 	};
 };
+
+The VSC7513 and VSC7514 switches can be controlled internally via the MIPS
+processor. The VSC7511 and VSC7512 don't have this internal processor, but all
+four chips can be controlled externally through SPI with the following required
+properties:
+
+- compatible:
+	Can be "mscc,vsc7511", "mscc,vsc7512", "mscc,vsc7513", or
+	"mscc,vsc7514".
+
+Supported phy modes for all chips are:
+
+* phy_mode = "sgmii": on ports 0, 1, 2, 3
+
+The VSC7512 and 7514 also support:
+
+* phy_mode = "sgmii": on ports 4, 5, 6, 7
+* phy_mode = "qsgmii": on ports 7, 8, 10
+
+Example for control from a BeagleBone Black
+
+&spi0 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ethernet-switch@0 {
+		compatible = "mscc,vsc7512";
+		spi-max-frequency = <250000>;
+		reg = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				ethernet = <&mac>;
+				phy-mode = "sgmii";
+
+				fixed-link {
+					speed = <100>;
+					full-duplex;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				label = "swp1";
+				phy-handle = <&sw_phy1>;
+				phy-mode = "sgmii";
+			};
+
+			port@2 {
+				reg = <2>;
+				label = "swp2";
+				phy-handle = <&sw_phy2>;
+				phy-mode = "sgmii";
+			};
+
+			port@3 {
+				reg = <3>;
+				label = "swp3";
+				phy-handle = <&sw_phy3>;
+				phy-mode = "sgmii";
+			};
+		};
+
+		mdio {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			sw_phy1: ethernet-phy@1 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x1>;
+			};
+
+			sw_phy2: ethernet-phy@2 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x2>;
+			};
+
+			sw_phy3: ethernet-phy@3 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x3>;
+			};
+		};
+	};
+};