| Message ID | 20230430112834.11520-2-mail@david-bauer.net (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/2] mt7530: register OF node for internal MDIO bus | expand |
On 30.04.2023 14:28, David Bauer wrote: > Document the ability to add nodes for the MDIO bus connecting the > switch-internal PHYs. This is quite interesting. Currently the PHY muxing feature for the MT7530 switch looks for some fake ethernet-phy definitions on the mdio-bus where the switch is also defined. Looking at the binding here, there will be an mdio node under the switch node. This could be useful to define the ethernet-phys for PHY muxing here instead, so we don't waste the register addresses on the parent mdio-bus for fake things. It looks like this should work right out of the box. I will do some tests. Are there any examples as to what to configure on the switch PHYs with this change? > > Signed-off-by: David Bauer <mail@david-bauer.net> > --- > .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > index e532c6b795f4..50f8f83cc440 100644 > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > @@ -128,6 +128,12 @@ properties: > See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt for > details for the regulator setup on these boards. > > + mdio: > + $ref: /schemas/net/mdio.yaml# > + unevaluatedProperties: false > + description: > + Node for the internal MDIO bus connected to the embedded ethernet-PHYs. Please set this property as false for mediatek,mt7988-switch as it doesn't use MDIO. Arınç
On Sun, Apr 30, 2023 at 03:34:43PM +0300, Arınç ÜNAL wrote: > On 30.04.2023 14:28, David Bauer wrote: > > Document the ability to add nodes for the MDIO bus connecting the > > switch-internal PHYs. > > This is quite interesting. Currently the PHY muxing feature for the MT7530 > switch looks for some fake ethernet-phy definitions on the mdio-bus where > the switch is also defined. > > Looking at the binding here, there will be an mdio node under the switch > node. This could be useful to define the ethernet-phys for PHY muxing here > instead, so we don't waste the register addresses on the parent mdio-bus for > fake things. It looks like this should work right out of the box. I will do > some tests. > > Are there any examples as to what to configure on the switch PHYs with this > change? > > > > > Signed-off-by: David Bauer <mail@david-bauer.net> > > --- > > .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > index e532c6b795f4..50f8f83cc440 100644 > > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > @@ -128,6 +128,12 @@ properties: > > See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt for > > details for the regulator setup on these boards. > > + mdio: > > + $ref: /schemas/net/mdio.yaml# > > + unevaluatedProperties: false > > + description: > > + Node for the internal MDIO bus connected to the embedded ethernet-PHYs. > > Please set this property as false for mediatek,mt7988-switch as it doesn't > use MDIO. Well, quite the opposite is true. This change is **needed** on MT7988 as the built-in 1GE PHYs of the MT7988 are connected to the (internal) MDIO bus of the switch. And they do need calibration data assigned as nvmem via device tree. tl;dr: Despite not being connected via MDIO itself also MT7988 exposes an internal MDIO bus for the switch PHYs.
On 30.04.2023 15:44, Daniel Golle wrote: > On Sun, Apr 30, 2023 at 03:34:43PM +0300, Arınç ÜNAL wrote: >> On 30.04.2023 14:28, David Bauer wrote: >>> Document the ability to add nodes for the MDIO bus connecting the >>> switch-internal PHYs. >> >> This is quite interesting. Currently the PHY muxing feature for the MT7530 >> switch looks for some fake ethernet-phy definitions on the mdio-bus where >> the switch is also defined. >> >> Looking at the binding here, there will be an mdio node under the switch >> node. This could be useful to define the ethernet-phys for PHY muxing here >> instead, so we don't waste the register addresses on the parent mdio-bus for >> fake things. It looks like this should work right out of the box. I will do >> some tests. >> >> Are there any examples as to what to configure on the switch PHYs with this >> change? >> >>> >>> Signed-off-by: David Bauer <mail@david-bauer.net> >>> --- >>> .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> index e532c6b795f4..50f8f83cc440 100644 >>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> @@ -128,6 +128,12 @@ properties: >>> See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt for >>> details for the regulator setup on these boards. >>> + mdio: >>> + $ref: /schemas/net/mdio.yaml# >>> + unevaluatedProperties: false >>> + description: >>> + Node for the internal MDIO bus connected to the embedded ethernet-PHYs. >> >> Please set this property as false for mediatek,mt7988-switch as it doesn't >> use MDIO. > > Well, quite the opposite is true. This change is **needed** on MT7988 as > the built-in 1GE PHYs of the MT7988 are connected to the (internal) MDIO > bus of the switch. And they do need calibration data assigned as nvmem > via device tree. > > tl;dr: Despite not being connected via MDIO itself also MT7988 exposes an > internal MDIO bus for the switch PHYs. Got it, thanks. Arınç
On 30.04.2023 14:28, David Bauer wrote: > Document the ability to add nodes for the MDIO bus connecting the > switch-internal PHYs. > > Signed-off-by: David Bauer <mail@david-bauer.net> > --- > .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > index e532c6b795f4..50f8f83cc440 100644 > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > @@ -128,6 +128,12 @@ properties: > See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt for > details for the regulator setup on these boards. > > + mdio: > + $ref: /schemas/net/mdio.yaml# > + unevaluatedProperties: false > + description: > + Node for the internal MDIO bus connected to the embedded ethernet-PHYs. Maybe saying "connected to the internal switch PHYs" would better explain this. Arınç
On 30.04.2023 15:34, Arınç ÜNAL wrote: > On 30.04.2023 14:28, David Bauer wrote: >> Document the ability to add nodes for the MDIO bus connecting the >> switch-internal PHYs. > > This is quite interesting. Currently the PHY muxing feature for the > MT7530 switch looks for some fake ethernet-phy definitions on the > mdio-bus where the switch is also defined. > > Looking at the binding here, there will be an mdio node under the switch > node. This could be useful to define the ethernet-phys for PHY muxing > here instead, so we don't waste the register addresses on the parent > mdio-bus for fake things. It looks like this should work right out of > the box. I will do some tests. Once I start using the mdio node it forces me to define all the PHYs which were defined as ports. [ 4.159534] mt7530-mdio mdio-bus:1f lan0 (uninitialized): no phy at 1 [ 4.166002] mt7530-mdio mdio-bus:1f lan0 (uninitialized): failed to connect to PHY: -ENODEV [ 4.174421] mt7530-mdio mdio-bus:1f lan0 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 1 [ 4.185236] mt7530-mdio mdio-bus:1f lan1 (uninitialized): no phy at 2 [ 4.191753] mt7530-mdio mdio-bus:1f lan1 (uninitialized): failed to connect to PHY: -ENODEV [ 4.200150] mt7530-mdio mdio-bus:1f lan1 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 2 [ 4.210844] mt7530-mdio mdio-bus:1f lan2 (uninitialized): no phy at 3 [ 4.217361] mt7530-mdio mdio-bus:1f lan2 (uninitialized): failed to connect to PHY: -ENODEV [ 4.225734] mt7530-mdio mdio-bus:1f lan2 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 3 [ 4.236394] mt7530-mdio mdio-bus:1f lan3 (uninitialized): no phy at 4 [ 4.242901] mt7530-mdio mdio-bus:1f lan3 (uninitialized): failed to connect to PHY: -ENODEV [ 4.251297] mt7530-mdio mdio-bus:1f lan3 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 4 We can either force defining the PHYs on the mdio node which would break the ABI, or forget about doing PHY muxing this way. Arınç
On Sun, Apr 30, 2023 at 07:17:10PM +0300, Arınç ÜNAL wrote: > On 30.04.2023 15:34, Arınç ÜNAL wrote: > > On 30.04.2023 14:28, David Bauer wrote: > > > Document the ability to add nodes for the MDIO bus connecting the > > > switch-internal PHYs. > > > > This is quite interesting. Currently the PHY muxing feature for the > > MT7530 switch looks for some fake ethernet-phy definitions on the > > mdio-bus where the switch is also defined. > > > > Looking at the binding here, there will be an mdio node under the switch > > node. This could be useful to define the ethernet-phys for PHY muxing > > here instead, so we don't waste the register addresses on the parent > > mdio-bus for fake things. It looks like this should work right out of > > the box. I will do some tests. > > Once I start using the mdio node it forces me to define all the PHYs which > were defined as ports. Try setting ds->slave_mii_bus to the MDIO bus you register via of_mdiobus_register(). Andrew
On 30.04.2023 20:18, Andrew Lunn wrote: > On Sun, Apr 30, 2023 at 07:17:10PM +0300, Arınç ÜNAL wrote: >> On 30.04.2023 15:34, Arınç ÜNAL wrote: >>> On 30.04.2023 14:28, David Bauer wrote: >>>> Document the ability to add nodes for the MDIO bus connecting the >>>> switch-internal PHYs. >>> >>> This is quite interesting. Currently the PHY muxing feature for the >>> MT7530 switch looks for some fake ethernet-phy definitions on the >>> mdio-bus where the switch is also defined. >>> >>> Looking at the binding here, there will be an mdio node under the switch >>> node. This could be useful to define the ethernet-phys for PHY muxing >>> here instead, so we don't waste the register addresses on the parent >>> mdio-bus for fake things. It looks like this should work right out of >>> the box. I will do some tests. >> >> Once I start using the mdio node it forces me to define all the PHYs which >> were defined as ports. > > Try setting ds->slave_mii_bus to the MDIO bus you register via > of_mdiobus_register(). That seems to be the case already, under mt7530_setup_mdio(): bus = devm_mdiobus_alloc(dev); if (!bus) return -ENOMEM; ds->slave_mii_bus = bus; The bus is registered with devm_of_mdiobus_register(), if that matters. (My current knowledge about OF or OF helpers for MDIO is next to nothing.) The same behaviour is there. Arınç
> > Try setting ds->slave_mii_bus to the MDIO bus you register via > > of_mdiobus_register(). > > That seems to be the case already, under mt7530_setup_mdio(): > > bus = devm_mdiobus_alloc(dev); > if (!bus) > return -ENOMEM; > > ds->slave_mii_bus = bus; > > The bus is registered with devm_of_mdiobus_register(), if that matters. (My > current knowledge about OF or OF helpers for MDIO is next to nothing.) > > The same behaviour is there. Maybe take a look at what is going on in dsa_slave_phy_setup() and dsa_slave_phy_connect(). The way i understand it, is it first looks in DT to see if there is a phy-handle, and if there is, it uses it. If not, it assumes there is a 1:1 mapping between port number and PHY address, and looks to see if a PHY has been found on ds->slave_mii_bus at that address, and uses it. So i don't think you need to list the PHY, the fallback should be used. Andrew
On 30.04.2023 21:48, Andrew Lunn wrote: >>> Try setting ds->slave_mii_bus to the MDIO bus you register via >>> of_mdiobus_register(). >> >> That seems to be the case already, under mt7530_setup_mdio(): >> >> bus = devm_mdiobus_alloc(dev); >> if (!bus) >> return -ENOMEM; >> >> ds->slave_mii_bus = bus; >> >> The bus is registered with devm_of_mdiobus_register(), if that matters. (My >> current knowledge about OF or OF helpers for MDIO is next to nothing.) >> >> The same behaviour is there. > > Maybe take a look at what is going on in dsa_slave_phy_setup() and > dsa_slave_phy_connect(). > > The way i understand it, is it first looks in DT to see if there is a > phy-handle, and if there is, it uses it. If not, it assumes there is a > 1:1 mapping between port number and PHY address, and looks to see if a > PHY has been found on ds->slave_mii_bus at that address, and uses it. > > So i don't think you need to list the PHY, the fallback should be > used. Thanks for pointing me in the right direction Andrew. I applied this diff: diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 389f33a12534..19d0c209e7e9 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -117,8 +117,12 @@ struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr) mdiodev = bus->mdio_map[addr]; - if (!mdiodev) + if (!mdiodev) { + dev_info(&bus->dev, "mdio device doesn't exist\n"); return NULL; + } + + dev_info(&bus->dev, "mdio device exists\n"); if (!(mdiodev->flags & MDIO_DEVICE_FLAG_PHY)) return NULL; diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 165bb2cb8431..0be408e32a76 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -2487,6 +2487,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev) /* We could not connect to a designated PHY or SFP, so try to * use the switch internal MDIO bus instead */ + netdev_err(slave_dev, "using switch's internal MDIO bus\n"); ret = dsa_slave_phy_connect(slave_dev, dp->index, phy_flags); } if (ret) { With or without this patch, the switch's internal MDIO bus is used to set up the PHYs. DT that defines ethphy0 only, without this patch applied: [ 4.660784] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus [ 4.669026] mdio_bus mt7530-0: mdio device exists [ 4.677693] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) [ 4.693238] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus [ 4.701589] mdio_bus mt7530-0: mdio device exists [ 4.707101] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL) [ 4.718550] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus [ 4.726856] mdio_bus mt7530-0: mdio device exists [ 4.732384] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL) [ 4.743822] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus [ 4.752154] mdio_bus mt7530-0: mdio device exists [ 4.757662] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL) [ 4.769099] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus [ 4.781872] mdio_bus mt7530-0: mdio device exists [ 4.787413] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL) Same DT but with this patch applied: [ 4.621547] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode [ 4.631524] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus [ 4.639764] mdio_bus mt7530-0: mdio device exists [ 4.647770] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) [ 4.663898] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus [ 4.672253] mdio_bus mt7530-0: mdio device doesn't exist [ 4.677597] mt7530-mdio mdio-bus:1f lan0 (uninitialized): no phy at 1 [ 4.684053] mt7530-mdio mdio-bus:1f lan0 (uninitialized): failed to connect to PHY: -ENODEV [ 4.692435] mt7530-mdio mdio-bus:1f lan0 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 1 [ 4.703087] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus [ 4.711408] mdio_bus mt7530-0: mdio device doesn't exist [ 4.716731] mt7530-mdio mdio-bus:1f lan1 (uninitialized): no phy at 2 [ 4.723214] mt7530-mdio mdio-bus:1f lan1 (uninitialized): failed to connect to PHY: -ENODEV [ 4.731597] mt7530-mdio mdio-bus:1f lan1 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 2 [ 4.742199] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus [ 4.755431] mdio_bus mt7530-0: mdio device doesn't exist [ 4.760793] mt7530-mdio mdio-bus:1f lan2 (uninitialized): no phy at 3 [ 4.767263] mt7530-mdio mdio-bus:1f lan2 (uninitialized): failed to connect to PHY: -ENODEV [ 4.775632] mt7530-mdio mdio-bus:1f lan2 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 3 [ 4.786270] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus [ 4.794591] mdio_bus mt7530-0: mdio device doesn't exist [ 4.799944] mt7530-mdio mdio-bus:1f lan3 (uninitialized): no phy at 4 [ 4.806397] mt7530-mdio mdio-bus:1f lan3 (uninitialized): failed to connect to PHY: -ENODEV [ 4.814782] mt7530-mdio mdio-bus:1f lan3 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 4 DT without the mdio node defined, with this patch applied: [ 4.650766] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode [ 4.660687] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus [ 4.668937] mdio_bus mt7530-0: mdio device exists [ 4.677787] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) [ 4.693165] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus [ 4.701517] mdio_bus mt7530-0: mdio device exists [ 4.707029] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL) [ 4.718469] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus [ 4.726773] mdio_bus mt7530-0: mdio device exists [ 4.732322] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL) [ 4.743793] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus [ 4.752143] mdio_bus mt7530-0: mdio device exists [ 4.757662] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL) [ 4.769105] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus [ 4.781905] mdio_bus mt7530-0: mdio device exists [ 4.787459] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL) This is how I define it, mind you no phandles. switch@1f { ... mdio { #address-cells = <0x01>; #size-cells = <0x00>; ethernet-phy@0 { reg = <0x00>; }; }; }; Like you said, if the mdio node is not defined, the driver will assume 1:1 mapping. If not, it will need all the PHYs to be defined on the mdio node along with on the ports node. Hence back to my original statement, we can either force defining the PHYs on the mdio node which would break the ABI, or forget about doing PHY muxing this way. There are no MDIO operations needed on the PHYs for the PHY muxing anyway, so I'd rather do this some other way. Thanks for the help. Arınç
On Sun, Apr 30, 2023 at 10:54:50PM +0300, Arınç ÜNAL wrote: > On 30.04.2023 21:48, Andrew Lunn wrote: > > > > Try setting ds->slave_mii_bus to the MDIO bus you register via > > > > of_mdiobus_register(). > > > > > > That seems to be the case already, under mt7530_setup_mdio(): > > > > > > bus = devm_mdiobus_alloc(dev); > > > if (!bus) > > > return -ENOMEM; > > > > > > ds->slave_mii_bus = bus; > > > > > > The bus is registered with devm_of_mdiobus_register(), if that matters. (My > > > current knowledge about OF or OF helpers for MDIO is next to nothing.) > > > > > > The same behaviour is there. > > > > Maybe take a look at what is going on in dsa_slave_phy_setup() and > > dsa_slave_phy_connect(). > > > > The way i understand it, is it first looks in DT to see if there is a > > phy-handle, and if there is, it uses it. If not, it assumes there is a > > 1:1 mapping between port number and PHY address, and looks to see if a > > PHY has been found on ds->slave_mii_bus at that address, and uses it. > > > > So i don't think you need to list the PHY, the fallback should be > > used. > > Thanks for pointing me in the right direction Andrew. > > I applied this diff: > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > index 389f33a12534..19d0c209e7e9 100644 > --- a/drivers/net/phy/mdio_bus.c > +++ b/drivers/net/phy/mdio_bus.c > @@ -117,8 +117,12 @@ struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr) > mdiodev = bus->mdio_map[addr]; > - if (!mdiodev) > + if (!mdiodev) { > + dev_info(&bus->dev, "mdio device doesn't exist\n"); > return NULL; > + } > + > + dev_info(&bus->dev, "mdio device exists\n"); > if (!(mdiodev->flags & MDIO_DEVICE_FLAG_PHY)) > return NULL; > diff --git a/net/dsa/slave.c b/net/dsa/slave.c > index 165bb2cb8431..0be408e32a76 100644 > --- a/net/dsa/slave.c > +++ b/net/dsa/slave.c > @@ -2487,6 +2487,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev) > /* We could not connect to a designated PHY or SFP, so try to > * use the switch internal MDIO bus instead > */ > + netdev_err(slave_dev, "using switch's internal MDIO bus\n"); > ret = dsa_slave_phy_connect(slave_dev, dp->index, phy_flags); > } > if (ret) { > > With or without this patch, the switch's internal MDIO bus is used to set > up the PHYs. > > DT that defines ethphy0 only, without this patch applied: > > [ 4.660784] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus > [ 4.669026] mdio_bus mt7530-0: mdio device exists > [ 4.677693] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.693238] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus > [ 4.701589] mdio_bus mt7530-0: mdio device exists > [ 4.707101] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.718550] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus > [ 4.726856] mdio_bus mt7530-0: mdio device exists > [ 4.732384] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.743822] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus > [ 4.752154] mdio_bus mt7530-0: mdio device exists > [ 4.757662] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.769099] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus > [ 4.781872] mdio_bus mt7530-0: mdio device exists > [ 4.787413] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL) > > Same DT but with this patch applied: > > [ 4.621547] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode > [ 4.631524] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus > [ 4.639764] mdio_bus mt7530-0: mdio device exists > [ 4.647770] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.663898] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus > [ 4.672253] mdio_bus mt7530-0: mdio device doesn't exist > [ 4.677597] mt7530-mdio mdio-bus:1f lan0 (uninitialized): no phy at 1 > [ 4.684053] mt7530-mdio mdio-bus:1f lan0 (uninitialized): failed to connect to PHY: -ENODEV > [ 4.692435] mt7530-mdio mdio-bus:1f lan0 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 1 > [ 4.703087] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus > [ 4.711408] mdio_bus mt7530-0: mdio device doesn't exist > [ 4.716731] mt7530-mdio mdio-bus:1f lan1 (uninitialized): no phy at 2 > [ 4.723214] mt7530-mdio mdio-bus:1f lan1 (uninitialized): failed to connect to PHY: -ENODEV > [ 4.731597] mt7530-mdio mdio-bus:1f lan1 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 2 > [ 4.742199] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus > [ 4.755431] mdio_bus mt7530-0: mdio device doesn't exist > [ 4.760793] mt7530-mdio mdio-bus:1f lan2 (uninitialized): no phy at 3 > [ 4.767263] mt7530-mdio mdio-bus:1f lan2 (uninitialized): failed to connect to PHY: -ENODEV > [ 4.775632] mt7530-mdio mdio-bus:1f lan2 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 3 > [ 4.786270] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus > [ 4.794591] mdio_bus mt7530-0: mdio device doesn't exist > [ 4.799944] mt7530-mdio mdio-bus:1f lan3 (uninitialized): no phy at 4 > [ 4.806397] mt7530-mdio mdio-bus:1f lan3 (uninitialized): failed to connect to PHY: -ENODEV > [ 4.814782] mt7530-mdio mdio-bus:1f lan3 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 4 > > DT without the mdio node defined, with this patch applied: > > [ 4.650766] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode > [ 4.660687] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus > [ 4.668937] mdio_bus mt7530-0: mdio device exists > [ 4.677787] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.693165] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus > [ 4.701517] mdio_bus mt7530-0: mdio device exists > [ 4.707029] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.718469] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus > [ 4.726773] mdio_bus mt7530-0: mdio device exists > [ 4.732322] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.743793] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus > [ 4.752143] mdio_bus mt7530-0: mdio device exists > [ 4.757662] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.769105] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus > [ 4.781905] mdio_bus mt7530-0: mdio device exists > [ 4.787459] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL) > > This is how I define it, mind you no phandles. > > switch@1f { > ... > mdio { > #address-cells = <0x01>; > #size-cells = <0x00>; > > ethernet-phy@0 { > reg = <0x00>; > }; > }; > }; > > Like you said, if the mdio node is not defined, the driver will assume 1:1 > mapping. If not, it will need all the PHYs to be defined on the mdio node > along with on the ports node. Hence back to my original statement, we can > either force defining the PHYs on the mdio node which would break the ABI, > or forget about doing PHY muxing this way. > > There are no MDIO operations needed on the PHYs for the PHY muxing anyway, > so I'd rather do this some other way. The problem is __of_mdiobus_register() :-( If there is a node in DT, the scanning of all addresses is disabled. It looks at just the addresses listed in DT. Plain mdiobus_register() by default looks at all addresses to discover if a PHY is there. Hence when slave.c tries to connect to the PHY, only the PHY you listed in DT has been found. I don't see a way around this, at least not without adding a new variant of of_mdiobus_register which combined DT with a bus scan. Andrew
Hi Arinc, thanks for spotting this issue. On 4/30/23 21:54, Arınç ÜNAL wrote: > On 30.04.2023 21:48, Andrew Lunn wrote: >>>> Try setting ds->slave_mii_bus to the MDIO bus you register via >>>> of_mdiobus_register(). >>> >>> That seems to be the case already, under mt7530_setup_mdio(): >>> >>> bus = devm_mdiobus_alloc(dev); >>> if (!bus) >>> return -ENOMEM; >>> >>> ds->slave_mii_bus = bus; >>> >>> The bus is registered with devm_of_mdiobus_register(), if that matters. (My >>> current knowledge about OF or OF helpers for MDIO is next to nothing.) >>> >>> The same behaviour is there. >> >> Maybe take a look at what is going on in dsa_slave_phy_setup() and >> dsa_slave_phy_connect(). >> >> The way i understand it, is it first looks in DT to see if there is a >> phy-handle, and if there is, it uses it. If not, it assumes there is a >> 1:1 mapping between port number and PHY address, and looks to see if a >> PHY has been found on ds->slave_mii_bus at that address, and uses it. >> >> So i don't think you need to list the PHY, the fallback should be >> used. > > Thanks for pointing me in the right direction Andrew. > > I applied this diff: > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > index 389f33a12534..19d0c209e7e9 100644 > --- a/drivers/net/phy/mdio_bus.c > +++ b/drivers/net/phy/mdio_bus.c > @@ -117,8 +117,12 @@ struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr) > > mdiodev = bus->mdio_map[addr]; > > - if (!mdiodev) > + if (!mdiodev) { > + dev_info(&bus->dev, "mdio device doesn't exist\n"); > return NULL; > + } > + > + dev_info(&bus->dev, "mdio device exists\n"); > > if (!(mdiodev->flags & MDIO_DEVICE_FLAG_PHY)) > return NULL; > diff --git a/net/dsa/slave.c b/net/dsa/slave.c > index 165bb2cb8431..0be408e32a76 100644 > --- a/net/dsa/slave.c > +++ b/net/dsa/slave.c > @@ -2487,6 +2487,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev) > /* We could not connect to a designated PHY or SFP, so try to > * use the switch internal MDIO bus instead > */ > + netdev_err(slave_dev, "using switch's internal MDIO bus\n"); > ret = dsa_slave_phy_connect(slave_dev, dp->index, phy_flags); > } > if (ret) { > > With or without this patch, the switch's internal MDIO bus is used to set > up the PHYs. > > DT that defines ethphy0 only, without this patch applied: > > [ 4.660784] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus > [ 4.669026] mdio_bus mt7530-0: mdio device exists > [ 4.677693] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.693238] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus > [ 4.701589] mdio_bus mt7530-0: mdio device exists > [ 4.707101] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.718550] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus > [ 4.726856] mdio_bus mt7530-0: mdio device exists > [ 4.732384] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.743822] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus > [ 4.752154] mdio_bus mt7530-0: mdio device exists > [ 4.757662] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.769099] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus > [ 4.781872] mdio_bus mt7530-0: mdio device exists > [ 4.787413] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL) > > Same DT but with this patch applied: > > [ 4.621547] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode > [ 4.631524] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus > [ 4.639764] mdio_bus mt7530-0: mdio device exists > [ 4.647770] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.663898] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus > [ 4.672253] mdio_bus mt7530-0: mdio device doesn't exist > [ 4.677597] mt7530-mdio mdio-bus:1f lan0 (uninitialized): no phy at 1 > [ 4.684053] mt7530-mdio mdio-bus:1f lan0 (uninitialized): failed to connect to PHY: -ENODEV > [ 4.692435] mt7530-mdio mdio-bus:1f lan0 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 1 > [ 4.703087] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus > [ 4.711408] mdio_bus mt7530-0: mdio device doesn't exist > [ 4.716731] mt7530-mdio mdio-bus:1f lan1 (uninitialized): no phy at 2 > [ 4.723214] mt7530-mdio mdio-bus:1f lan1 (uninitialized): failed to connect to PHY: -ENODEV > [ 4.731597] mt7530-mdio mdio-bus:1f lan1 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 2 > [ 4.742199] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus > [ 4.755431] mdio_bus mt7530-0: mdio device doesn't exist > [ 4.760793] mt7530-mdio mdio-bus:1f lan2 (uninitialized): no phy at 3 > [ 4.767263] mt7530-mdio mdio-bus:1f lan2 (uninitialized): failed to connect to PHY: -ENODEV > [ 4.775632] mt7530-mdio mdio-bus:1f lan2 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 3 > [ 4.786270] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus > [ 4.794591] mdio_bus mt7530-0: mdio device doesn't exist > [ 4.799944] mt7530-mdio mdio-bus:1f lan3 (uninitialized): no phy at 4 > [ 4.806397] mt7530-mdio mdio-bus:1f lan3 (uninitialized): failed to connect to PHY: -ENODEV > [ 4.814782] mt7530-mdio mdio-bus:1f lan3 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 4 > > DT without the mdio node defined, with this patch applied: > > [ 4.650766] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode > [ 4.660687] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus > [ 4.668937] mdio_bus mt7530-0: mdio device exists > [ 4.677787] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.693165] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus > [ 4.701517] mdio_bus mt7530-0: mdio device exists > [ 4.707029] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.718469] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus > [ 4.726773] mdio_bus mt7530-0: mdio device exists > [ 4.732322] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.743793] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus > [ 4.752143] mdio_bus mt7530-0: mdio device exists > [ 4.757662] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL) > [ 4.769105] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus > [ 4.781905] mdio_bus mt7530-0: mdio device exists > [ 4.787459] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL) > > This is how I define it, mind you no phandles. > > switch@1f { > ... > mdio { > #address-cells = <0x01>; > #size-cells = <0x00>; > > ethernet-phy@0 { > reg = <0x00>; > }; > }; > }; > > Like you said, if the mdio node is not defined, the driver will assume 1:1 > mapping. If not, it will need all the PHYs to be defined on the mdio node > along with on the ports node. Hence back to my original statement, we can > either force defining the PHYs on the mdio node which would break the ABI, > or forget about doing PHY muxing this way. While i was not aware of this side effect, I don't see how this breaks the ABI. Existing device-trees not defining the MDIO node will still continue to work. Wouldn't we just skip the whole issue by documenting the need for defining all PHYs used on the switch when defining the MDIO bus? Best David > > There are no MDIO operations needed on the PHYs for the PHY muxing anyway, > so I'd rather do this some other way. > > Thanks for the help. > > Arınç
On 1.05.2023 12:22, David Bauer wrote: > Hi Arinc, > > thanks for spotting this issue. > > On 4/30/23 21:54, Arınç ÜNAL wrote: >> On 30.04.2023 21:48, Andrew Lunn wrote: >>>>> Try setting ds->slave_mii_bus to the MDIO bus you register via >>>>> of_mdiobus_register(). >>>> >>>> That seems to be the case already, under mt7530_setup_mdio(): >>>> >>>> bus = devm_mdiobus_alloc(dev); >>>> if (!bus) >>>> return -ENOMEM; >>>> >>>> ds->slave_mii_bus = bus; >>>> >>>> The bus is registered with devm_of_mdiobus_register(), if that matters. (My >>>> current knowledge about OF or OF helpers for MDIO is next to nothing.) >>>> >>>> The same behaviour is there. >>> >>> Maybe take a look at what is going on in dsa_slave_phy_setup() and >>> dsa_slave_phy_connect(). >>> >>> The way i understand it, is it first looks in DT to see if there is a >>> phy-handle, and if there is, it uses it. If not, it assumes there is a >>> 1:1 mapping between port number and PHY address, and looks to see if a >>> PHY has been found on ds->slave_mii_bus at that address, and uses it. >>> >>> So i don't think you need to list the PHY, the fallback should be >>> used. >> >> Thanks for pointing me in the right direction Andrew. >> >> I applied this diff: >> >> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c >> index 389f33a12534..19d0c209e7e9 100644 >> --- a/drivers/net/phy/mdio_bus.c >> +++ b/drivers/net/phy/mdio_bus.c >> @@ -117,8 +117,12 @@ struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr) >> >> mdiodev = bus->mdio_map[addr]; >> >> - if (!mdiodev) >> + if (!mdiodev) { >> + dev_info(&bus->dev, "mdio device doesn't exist\n"); >> return NULL; >> + } >> + >> + dev_info(&bus->dev, "mdio device exists\n"); >> >> if (!(mdiodev->flags & MDIO_DEVICE_FLAG_PHY)) >> return NULL; >> diff --git a/net/dsa/slave.c b/net/dsa/slave.c >> index 165bb2cb8431..0be408e32a76 100644 >> --- a/net/dsa/slave.c >> +++ b/net/dsa/slave.c >> @@ -2487,6 +2487,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev) >> /* We could not connect to a designated PHY or SFP, so try to >> * use the switch internal MDIO bus instead >> */ >> + netdev_err(slave_dev, "using switch's internal MDIO bus\n"); >> ret = dsa_slave_phy_connect(slave_dev, dp->index, phy_flags); >> } >> if (ret) { >> >> With or without this patch, the switch's internal MDIO bus is used to set >> up the PHYs. >> >> DT that defines ethphy0 only, without this patch applied: >> >> [ 4.660784] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus >> [ 4.669026] mdio_bus mt7530-0: mdio device exists >> [ 4.677693] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) >> [ 4.693238] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus >> [ 4.701589] mdio_bus mt7530-0: mdio device exists >> [ 4.707101] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL) >> [ 4.718550] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus >> [ 4.726856] mdio_bus mt7530-0: mdio device exists >> [ 4.732384] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL) >> [ 4.743822] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus >> [ 4.752154] mdio_bus mt7530-0: mdio device exists >> [ 4.757662] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL) >> [ 4.769099] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus >> [ 4.781872] mdio_bus mt7530-0: mdio device exists >> [ 4.787413] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL) >> >> Same DT but with this patch applied: >> >> [ 4.621547] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode >> [ 4.631524] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus >> [ 4.639764] mdio_bus mt7530-0: mdio device exists >> [ 4.647770] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) >> [ 4.663898] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus >> [ 4.672253] mdio_bus mt7530-0: mdio device doesn't exist >> [ 4.677597] mt7530-mdio mdio-bus:1f lan0 (uninitialized): no phy at 1 >> [ 4.684053] mt7530-mdio mdio-bus:1f lan0 (uninitialized): failed to connect to PHY: -ENODEV >> [ 4.692435] mt7530-mdio mdio-bus:1f lan0 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 1 >> [ 4.703087] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus >> [ 4.711408] mdio_bus mt7530-0: mdio device doesn't exist >> [ 4.716731] mt7530-mdio mdio-bus:1f lan1 (uninitialized): no phy at 2 >> [ 4.723214] mt7530-mdio mdio-bus:1f lan1 (uninitialized): failed to connect to PHY: -ENODEV >> [ 4.731597] mt7530-mdio mdio-bus:1f lan1 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 2 >> [ 4.742199] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus >> [ 4.755431] mdio_bus mt7530-0: mdio device doesn't exist >> [ 4.760793] mt7530-mdio mdio-bus:1f lan2 (uninitialized): no phy at 3 >> [ 4.767263] mt7530-mdio mdio-bus:1f lan2 (uninitialized): failed to connect to PHY: -ENODEV >> [ 4.775632] mt7530-mdio mdio-bus:1f lan2 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 3 >> [ 4.786270] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus >> [ 4.794591] mdio_bus mt7530-0: mdio device doesn't exist >> [ 4.799944] mt7530-mdio mdio-bus:1f lan3 (uninitialized): no phy at 4 >> [ 4.806397] mt7530-mdio mdio-bus:1f lan3 (uninitialized): failed to connect to PHY: -ENODEV >> [ 4.814782] mt7530-mdio mdio-bus:1f lan3 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 4 >> >> DT without the mdio node defined, with this patch applied: >> >> [ 4.650766] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode >> [ 4.660687] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus >> [ 4.668937] mdio_bus mt7530-0: mdio device exists >> [ 4.677787] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) >> [ 4.693165] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus >> [ 4.701517] mdio_bus mt7530-0: mdio device exists >> [ 4.707029] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL) >> [ 4.718469] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus >> [ 4.726773] mdio_bus mt7530-0: mdio device exists >> [ 4.732322] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL) >> [ 4.743793] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus >> [ 4.752143] mdio_bus mt7530-0: mdio device exists >> [ 4.757662] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL) >> [ 4.769105] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus >> [ 4.781905] mdio_bus mt7530-0: mdio device exists >> [ 4.787459] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL) >> >> This is how I define it, mind you no phandles. >> >> switch@1f { >> ... >> mdio { >> #address-cells = <0x01>; >> #size-cells = <0x00>; >> >> ethernet-phy@0 { >> reg = <0x00>; >> }; >> }; >> }; >> >> Like you said, if the mdio node is not defined, the driver will assume 1:1 >> mapping. If not, it will need all the PHYs to be defined on the mdio node >> along with on the ports node. Hence back to my original statement, we can >> either force defining the PHYs on the mdio node which would break the ABI, >> or forget about doing PHY muxing this way. > > While i was not aware of this side effect, I don't see how this breaks the ABI. Your patch doesn't break it, my then-intention of doing PHY muxing by utilising this would. Your first patch is perfectly fine as is. > > Existing device-trees not defining the MDIO node will still continue to work. Agreed, I also confirmed this with my test above. > > Wouldn't we just skip the whole issue by documenting the need for defining all PHYs > used on the switch when defining the MDIO bus? Good idea, please do that. Arınç
diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml index e532c6b795f4..50f8f83cc440 100644 --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml @@ -128,6 +128,12 @@ properties: See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt for details for the regulator setup on these boards. + mdio: + $ref: /schemas/net/mdio.yaml# + unevaluatedProperties: false + description: + Node for the internal MDIO bus connected to the embedded ethernet-PHYs. + mediatek,mcm: type: boolean description:
Document the ability to add nodes for the MDIO bus connecting the switch-internal PHYs. Signed-off-by: David Bauer <mail@david-bauer.net> --- .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml | 6 ++++++ 1 file changed, 6 insertions(+)