| Message ID | 20240809233840.59953-2-Tristram.Ha@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: microchip: add SGMII port support to KSZ9477 switch | expand |
On Fri, 09 Aug 2024 16:38:37 -0700, Tristram.Ha@microchip.com wrote: > From: Tristram Ha <tristram.ha@microchip.com> > > The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct > connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. > > SFP is typically used so the default is 1. The driver can detect > 10/100/1000 SFP and change the mode to 2. For direct connect this mode > has to be explicitly set to 0 as driver cannot detect that > configuration. > > Signed-off-by: Tristram Ha <tristram.ha@microchip.com> > --- > .../devicetree/bindings/net/dsa/microchip,ksz.yaml | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/dsa/microchip,ksz.example.dtb: switch@0: Unevaluated properties are not allowed ('sgmii-mode' was unexpected) from schema $id: http://devicetree.org/schemas/net/dsa/microchip,ksz.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240809233840.59953-2-Tristram.Ha@microchip.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 10/08/2024 01:38, Tristram.Ha@microchip.com wrote: > From: Tristram Ha <tristram.ha@microchip.com> > > The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct > connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. Binding should say it, not commit msg. But aren't you duplicating something like phy-connection-type? > > SFP is typically used so the default is 1. The driver can detect > 10/100/1000 SFP and change the mode to 2. For direct connect this mode > has to be explicitly set to 0 as driver cannot detect that > configuration. > > Signed-off-by: Tristram Ha <tristram.ha@microchip.com> > --- > .../devicetree/bindings/net/dsa/microchip,ksz.yaml | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml > index 52acc15ebcbf..b4a9746556bf 100644 > --- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml > @@ -71,6 +71,13 @@ properties: > enum: [2000, 4000, 8000, 12000, 16000, 20000, 24000, 28000] > default: 8000 > > + microchip,sgmii-mode: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + SGMII mode to use for the SGMII port > + enum: [0, 1, 2] > + default: 1 > + > interrupts: > maxItems: 1 > > @@ -137,6 +144,7 @@ examples: > compatible = "microchip,ksz9477"; > reg = <0>; > reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; > + sgmii-mode = <1>; It does not look like you tested the bindings, at least after quick look. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Maybe you need to update your dtschema and yamllint. Best regards, Krzysztof
On Fri, Aug 09, 2024 at 04:38:37PM -0700, Tristram.Ha@microchip.com wrote: > From: Tristram Ha <tristram.ha@microchip.com> > > The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct > connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. > > SFP is typically used so the default is 1. The driver can detect > 10/100/1000 SFP and change the mode to 2. For direct connect this mode > has to be explicitly set to 0 as driver cannot detect that > configuration. Could you explain this in more detail. Other SGMII blocks don't need this. Why is this block special? Has this anything to do with in-band signalling? Andrew
> > From: Tristram Ha <tristram.ha@microchip.com> > > > > The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct > > connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. > > > > SFP is typically used so the default is 1. The driver can detect > > 10/100/1000 SFP and change the mode to 2. For direct connect this mode > > has to be explicitly set to 0 as driver cannot detect that > > configuration. > > Could you explain this in more detail. Other SGMII blocks don't need > this. Why is this block special? > > Has this anything to do with in-band signalling? There are 2 ways to program the hardware registers so that the SGMII module can communicate with either 1000Base-T/LX/SX SFP or 10/100/1000Base-T SFP. When a SFP is plugged in the driver can try to detect which type and if it thinks 10/100/1000Base-T SFP is used it changes the mode to 2 and program appropriately.
On Tue, Aug 13, 2024 at 09:14:34PM +0000, Tristram.Ha@microchip.com wrote: > > > From: Tristram Ha <tristram.ha@microchip.com> > > > > > > The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct > > > connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. > > > > > > SFP is typically used so the default is 1. The driver can detect > > > 10/100/1000 SFP and change the mode to 2. For direct connect this mode > > > has to be explicitly set to 0 as driver cannot detect that > > > configuration. > > > > Could you explain this in more detail. Other SGMII blocks don't need > > this. Why is this block special? > > > > Has this anything to do with in-band signalling? > > There are 2 ways to program the hardware registers so that the SGMII > module can communicate with either 1000Base-T/LX/SX SFP or > 10/100/1000Base-T SFP. When a SFP is plugged in the driver can try to > detect which type and if it thinks 10/100/1000Base-T SFP is used it > changes the mode to 2 and program appropriately. What should happen here is that phylink will read the SFP EEPROM and determine what mode should be used. It will then tell the MAC or PCS how to configure itself, 1000BaseX, or SGMII. Look at the mac_link_up() callback, parameter interface. Andrew
> > > > From: Tristram Ha <tristram.ha@microchip.com> > > > > > > > > The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct > > > > connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. > > > > > > > > SFP is typically used so the default is 1. The driver can detect > > > > 10/100/1000 SFP and change the mode to 2. For direct connect this mode > > > > has to be explicitly set to 0 as driver cannot detect that > > > > configuration. > > > > > > Could you explain this in more detail. Other SGMII blocks don't need > > > this. Why is this block special? > > > > > > Has this anything to do with in-band signalling? > > > > There are 2 ways to program the hardware registers so that the SGMII > > module can communicate with either 1000Base-T/LX/SX SFP or > > 10/100/1000Base-T SFP. When a SFP is plugged in the driver can try to > > detect which type and if it thinks 10/100/1000Base-T SFP is used it > > changes the mode to 2 and program appropriately. > > What should happen here is that phylink will read the SFP EEPROM and > determine what mode should be used. It will then tell the MAC or PCS > how to configure itself, 1000BaseX, or SGMII. Look at the > mac_link_up() callback, parameter interface. I am not sure the module can retrieve SFP EEPROM information.
> On 10/08/2024 01:38, Tristram.Ha@microchip.com wrote: > > From: Tristram Ha <tristram.ha@microchip.com> > > > > The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct > > connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. > > Binding should say it, not commit msg. But aren't you duplicating > something like phy-connection-type? The sgmii-mode parameter is just used internally. I am not sure using phy-connection-type or phy-mode is appropriate. > > @@ -137,6 +144,7 @@ examples: > > compatible = "microchip,ksz9477"; > > reg = <0>; > > reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; > > + sgmii-mode = <1>; > > It does not look like you tested the bindings, at least after quick > look. Please run `make dt_binding_check` (see > Documentation/devicetree/bindings/writing-schema.rst for instructions). > Maybe you need to update your dtschema and yamllint. Sorry, I missed the example part.
On 14/08/2024 01:09, Tristram.Ha@microchip.com wrote: >> On 10/08/2024 01:38, Tristram.Ha@microchip.com wrote: >>> From: Tristram Ha <tristram.ha@microchip.com> >>> >>> The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct >>> connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. >> >> Binding should say it, not commit msg. But aren't you duplicating >> something like phy-connection-type? > > The sgmii-mode parameter is just used internally. I am not sure using This does not matter. > phy-connection-type or phy-mode is appropriate. Depends on what this property expressed in terms of hardware. Looks like you want to say which SGMII mode is being used? Best regards, Krzysztof
On Tue, Aug 13, 2024 at 10:17:03PM +0000, Tristram.Ha@microchip.com wrote: > > > > > From: Tristram Ha <tristram.ha@microchip.com> > > > > > > > > > > The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct > > > > > connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. > > > > > > > > > > SFP is typically used so the default is 1. The driver can detect > > > > > 10/100/1000 SFP and change the mode to 2. For direct connect this mode > > > > > has to be explicitly set to 0 as driver cannot detect that > > > > > configuration. > > > > > > > > Could you explain this in more detail. Other SGMII blocks don't need > > > > this. Why is this block special? > > > > > > > > Has this anything to do with in-band signalling? > > > > > > There are 2 ways to program the hardware registers so that the SGMII > > > module can communicate with either 1000Base-T/LX/SX SFP or > > > 10/100/1000Base-T SFP. When a SFP is plugged in the driver can try to > > > detect which type and if it thinks 10/100/1000Base-T SFP is used it > > > changes the mode to 2 and program appropriately. > > > > What should happen here is that phylink will read the SFP EEPROM and > > determine what mode should be used. It will then tell the MAC or PCS > > how to configure itself, 1000BaseX, or SGMII. Look at the > > mac_link_up() callback, parameter interface. > > I am not sure the module can retrieve SFP EEPROM information. The board should be designed such that the I2C bus pins of the SFP cage are connected to an I2C controller. There are also a few pins which ideally should be connected to GPIOs, LOS, Tx disable etc. You can then put a node in DT describing the SFP cage: Documentation/devicetree/bindings/net/sff,sfp.yaml sfp2: sfp { compatible = "sff,sfp"; i2c-bus = <&sfp_i2c>; los-gpios = <&cps_gpio1 28 GPIO_ACTIVE_HIGH>; mod-def0-gpios = <&cps_gpio1 27 GPIO_ACTIVE_LOW>; pinctrl-names = "default"; pinctrl-0 = <&cps_sfpp0_pins>; tx-disable-gpios = <&cps_gpio1 29 GPIO_ACTIVE_HIGH>; tx-fault-gpios = <&cps_gpio1 26 GPIO_ACTIVE_HIGH>; }; and then the ethernet node has a link to it: ethernet { phy-names = "comphy"; phys = <&cps_comphy5 0>; sfp = <&sfp1>; }; Phylink will then driver the SFP and tell the MAC what to do. Andrew
> On Tue, Aug 13, 2024 at 10:17:03PM +0000, Tristram.Ha@microchip.com wrote: > > > > > > From: Tristram Ha <tristram.ha@microchip.com> > > > > > > > > > > > > The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct > > > > > > connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. > > > > > > > > > > > > SFP is typically used so the default is 1. The driver can detect > > > > > > 10/100/1000 SFP and change the mode to 2. For direct connect this mode > > > > > > has to be explicitly set to 0 as driver cannot detect that > > > > > > configuration. > > > > > > > > > > Could you explain this in more detail. Other SGMII blocks don't need > > > > > this. Why is this block special? > > > > > > > > > > Has this anything to do with in-band signalling? > > > > > > > > There are 2 ways to program the hardware registers so that the SGMII > > > > module can communicate with either 1000Base-T/LX/SX SFP or > > > > 10/100/1000Base-T SFP. When a SFP is plugged in the driver can try to > > > > detect which type and if it thinks 10/100/1000Base-T SFP is used it > > > > changes the mode to 2 and program appropriately. > > > > > > What should happen here is that phylink will read the SFP EEPROM and > > > determine what mode should be used. It will then tell the MAC or PCS > > > how to configure itself, 1000BaseX, or SGMII. Look at the > > > mac_link_up() callback, parameter interface. > > > > I am not sure the module can retrieve SFP EEPROM information. > > The board should be designed such that the I2C bus pins of the SFP > cage are connected to an I2C controller. There are also a few pins > which ideally should be connected to GPIOs, LOS, Tx disable etc. You > can then put a node in DT describing the SFP cage: > > Documentation/devicetree/bindings/net/sff,sfp.yaml > > sfp2: sfp { > compatible = "sff,sfp"; > i2c-bus = <&sfp_i2c>; > los-gpios = <&cps_gpio1 28 GPIO_ACTIVE_HIGH>; > mod-def0-gpios = <&cps_gpio1 27 GPIO_ACTIVE_LOW>; > pinctrl-names = "default"; > pinctrl-0 = <&cps_sfpp0_pins>; > tx-disable-gpios = <&cps_gpio1 29 GPIO_ACTIVE_HIGH>; > tx-fault-gpios = <&cps_gpio1 26 GPIO_ACTIVE_HIGH>; > }; > > and then the ethernet node has a link to it: > > ethernet { > phy-names = "comphy"; > phys = <&cps_comphy5 0>; > sfp = <&sfp1>; > }; > > Phylink will then driver the SFP and tell the MAC what to do. I do not think the KSZ9477 switch design allows I2C access to the SFP EEPROM. The SGMII module provides only 2 registers that work like PHY's MMD operation to access vendor-specific registers. One of such registers controls how the module communicates with the SFP so that it works with either 1000Base-T/SX/LX fiber-like SFP or 10/100/1000Base-T copper SFP. Actually the default setting works with 10/100/1000Base-T copper SFP without any programming. That register needs to be changed when 1000Base-T/SX/LX fiber SFP is used. I will see if those vendor-specific registers contain the SFP EEPROM, but I do not know how the sfp_bus structure allows mapping the register access to probe for those information.
> On 14/08/2024 01:09, Tristram.Ha@microchip.com wrote: > >> On 10/08/2024 01:38, Tristram.Ha@microchip.com wrote: > >>> From: Tristram Ha <tristram.ha@microchip.com> > >>> > >>> The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct > >>> connect, 1 for 1000BaseT SFP, and 2 for 10/100/1000 SFP. > >> > >> Binding should say it, not commit msg. But aren't you duplicating > >> something like phy-connection-type? > > > > The sgmii-mode parameter is just used internally. I am not sure using > > This does not matter. > > > phy-connection-type or phy-mode is appropriate. > > Depends on what this property expressed in terms of hardware. Looks like > you want to say which SGMII mode is being used? The driver can detect whether 10/100/1000Base-T copper SFP is being used. So the main purpose of this device tree parameter is to indicate the SGMII module is directly connected to another one without using any SFP. This is a very rare case. In such case the device tree parameter can be changed to a flag to just indicate SFP is not used.
> > The board should be designed such that the I2C bus pins of the SFP > > cage are connected to an I2C controller. There are also a few pins > > which ideally should be connected to GPIOs, LOS, Tx disable etc. You > > can then put a node in DT describing the SFP cage: > > > > Documentation/devicetree/bindings/net/sff,sfp.yaml > > > > sfp2: sfp { > > compatible = "sff,sfp"; > > i2c-bus = <&sfp_i2c>; > > los-gpios = <&cps_gpio1 28 GPIO_ACTIVE_HIGH>; > > mod-def0-gpios = <&cps_gpio1 27 GPIO_ACTIVE_LOW>; > > pinctrl-names = "default"; > > pinctrl-0 = <&cps_sfpp0_pins>; > > tx-disable-gpios = <&cps_gpio1 29 GPIO_ACTIVE_HIGH>; > > tx-fault-gpios = <&cps_gpio1 26 GPIO_ACTIVE_HIGH>; > > }; > > > > and then the ethernet node has a link to it: > > > > ethernet { > > phy-names = "comphy"; > > phys = <&cps_comphy5 0>; > > sfp = <&sfp1>; > > }; > > > > Phylink will then driver the SFP and tell the MAC what to do. > > I do not think the KSZ9477 switch design allows I2C access to the SFP > EEPROM. This is not a switch design issue, it is a board design issue. Plenty of Marvell switches have a PCR which do SGMII and 1000BaseX. Only the SFP SERDES data lines are connected to the switch. The I2C bus and other lines are connected to the SoC, not the switch. Do you have the schematics for the board you are testing on? Is it open? Can you give us a link? Andrew
> > > The board should be designed such that the I2C bus pins of the SFP > > > cage are connected to an I2C controller. There are also a few pins > > > which ideally should be connected to GPIOs, LOS, Tx disable etc. You > > > can then put a node in DT describing the SFP cage: > > > > > > Documentation/devicetree/bindings/net/sff,sfp.yaml > > > > > > sfp2: sfp { > > > compatible = "sff,sfp"; > > > i2c-bus = <&sfp_i2c>; > > > los-gpios = <&cps_gpio1 28 GPIO_ACTIVE_HIGH>; > > > mod-def0-gpios = <&cps_gpio1 27 GPIO_ACTIVE_LOW>; > > > pinctrl-names = "default"; > > > pinctrl-0 = <&cps_sfpp0_pins>; > > > tx-disable-gpios = <&cps_gpio1 29 GPIO_ACTIVE_HIGH>; > > > tx-fault-gpios = <&cps_gpio1 26 GPIO_ACTIVE_HIGH>; > > > }; > > > > > > and then the ethernet node has a link to it: > > > > > > ethernet { > > > phy-names = "comphy"; > > > phys = <&cps_comphy5 0>; > > > sfp = <&sfp1>; > > > }; > > > > > > Phylink will then driver the SFP and tell the MAC what to do. > > > > I do not think the KSZ9477 switch design allows I2C access to the SFP > > EEPROM. > > This is not a switch design issue, it is a board design issue. Plenty > of Marvell switches have a PCR which do SGMII and 1000BaseX. Only the > SFP SERDES data lines are connected to the switch. The I2C bus and > other lines are connected to the SoC, not the switch. > > Do you have the schematics for the board you are testing on? Is it > open? Can you give us a link? My KSZ9477 board does not have that I2C connection, so I cannot implement the change as suggested. I am getting a new design board that needs verification of this connection. After I make it work I will re-submit the patch. Thanks for your help.
> My KSZ9477 board does not have that I2C connection, so I cannot > implement the change as suggested. I would say any board without the I2C bus connected is fatally broken. Most SFPs are buggy, and don't follow the standard. You need to read the vendor and product ID so you can enable various quirks to make them work correctly. > I am getting a new design board that needs verification of this > connection. After I make it work I will re-submit the patch. O.K, that sounds good. Andrew
diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml index 52acc15ebcbf..b4a9746556bf 100644 --- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml +++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml @@ -71,6 +71,13 @@ properties: enum: [2000, 4000, 8000, 12000, 16000, 20000, 24000, 28000] default: 8000 + microchip,sgmii-mode: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + SGMII mode to use for the SGMII port + enum: [0, 1, 2] + default: 1 + interrupts: maxItems: 1 @@ -137,6 +144,7 @@ examples: compatible = "microchip,ksz9477"; reg = <0>; reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; + sgmii-mode = <1>; spi-max-frequency = <44000000>; @@ -173,6 +181,10 @@ examples: full-duplex; }; }; + port@6 { + reg = <6>; + label = "lan6"; + }; }; };