From: Ahmad Fatoum <a.fatoum@pengutronix.de> Date: 2018-08-14 14:12:40
The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
There, of_mdiobus_register was called even for the fixed-link representing
the SPI-connected switch PHY, with the result that the driver attempts to
enumerate PHYs on a non-existent MDIO bus:
libphy: MACB_mii_bus: probed
mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
[snip]
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
macb f0028000.ethernet: broken fixed-link specification
Cc: <redacted>
Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
drivers/net/ethernet/cadence/macb_main.c | 26 +++++++++++++++---------
1 file changed, 16 insertions(+), 10 deletions(-)
Hello Ahmad,
On Tue, Aug 14, 2018 at 04:12:40PM +0200, Ahmad Fatoum wrote:
The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
There, of_mdiobus_register was called even for the fixed-link representing
the SPI-connected switch PHY, with the result that the driver attempts to
enumerate PHYs on a non-existent MDIO bus:
libphy: MACB_mii_bus: probed
mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
[snip]
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
macb f0028000.ethernet: broken fixed-link specification
Cc: <redacted>
Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
I added the people involved in 739de9a1563a to Cc. Maybe they want to
comment. So not stripping the remaining part of the original mail.
Best regards
Uwe
Hello Ahmed, Uwe,
On Tue, Aug 14, 2018 at 05:58:12PM +0200, Uwe Kleine-König wrote:
Hello Ahmad,
On Tue, Aug 14, 2018 at 04:12:40PM +0200, Ahmad Fatoum wrote:
quoted
The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
There, of_mdiobus_register was called even for the fixed-link representing
the SPI-connected switch PHY, with the result that the driver attempts to
enumerate PHYs on a non-existent MDIO bus:
libphy: MACB_mii_bus: probed
mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
[snip]
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
macb f0028000.ethernet: broken fixed-link specification
Cc: <redacted>
Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
I added the people involved in 739de9a1563a to Cc. Maybe they want to
comment. So not stripping the remaining part of the original mail.
Best regards
Uwe
From: Andrew Lunn <andrew@lunn.ch> Date: 2018-08-15 05:22:47
On Tue, Aug 14, 2018 at 05:58:12PM +0200, Uwe Kleine-König wrote:
Hello Ahmad,
On Tue, Aug 14, 2018 at 04:12:40PM +0200, Ahmad Fatoum wrote:
quoted
The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
There, of_mdiobus_register was called even for the fixed-link representing
the SPI-connected switch PHY, with the result that the driver attempts to
enumerate PHYs on a non-existent MDIO bus:
libphy: MACB_mii_bus: probed
mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
[snip]
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
macb f0028000.ethernet: broken fixed-link specification
Cc: <redacted>
Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
I added the people involved in 739de9a1563a to Cc. Maybe they want to
comment. So not stripping the remaining part of the original mail.
Thanks Uwe for Cc: in my.
Ahmed, where is the device tree for the EVB-KSZ9477?
Thanks
Andrew
Hi,
On Wed, Aug 15, 2018 at 3:35 AM Andrew Lunn [off-list ref] wrote:
On Tue, Aug 14, 2018 at 05:58:12PM +0200, Uwe Kleine-König wrote:
quoted
Hello Ahmad,
On Tue, Aug 14, 2018 at 04:12:40PM +0200, Ahmad Fatoum wrote:
quoted
The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
There, of_mdiobus_register was called even for the fixed-link representing
the SPI-connected switch PHY, with the result that the driver attempts to
enumerate PHYs on a non-existent MDIO bus:
I ran into a similar problem on v14.4 for davinci_mdio I had to patch
it with [1].
The cpsw has 2 phys one phy is connected to KSZ9031 and other to
ksz9897 Ethernet switch
which is treated as a fixed phy with no mdio lines because of which
mdio_read/write failed.
This didn’t happen in v4.9.x something in core has changed ?
[1]
From: Ahmad Fatoum <a.fatoum@pengutronix.de> Date: 2018-08-16 09:51:06
On 08/15/2018 04:32 AM, Andrew Lunn wrote:
Ahmed, where is the device tree for the EVB-KSZ9477?
I've attached it [1]. It's still work-in-progress (DSA doesn't work yet for example), but Ethernet is usable with Linux v4.18 and my patch applied.
Cheers
Ahmad
[1]
=======================================
@@ -0,0 +1,393 @@+/*+*at91-sama5d3_xplained_ung8087.dts-DeviceTreefilefortheEVB-KSZ9477board+*+*Copyright(C)2014Atmel,+*2014NicolasFerre<nicolas.ferre@atmel.com>+*2018AhmadFatoum<a.fatoum@pengutronix.de>+*+*LicensedunderGPLv2orlater.+*/+/dts-v1/;+#include"sama5d36.dtsi"++/{+model="SAMA5D3 Xplained";+compatible="atmel,sama5d3-xplained","atmel,sama5d3","atmel,sama5";++chosen{+bootargs="console=ttyS0,115200";+};++memory{+reg=<0x200000000x10000000>;+};++clocks{+slow_xtal{+clock-frequency=<32768>;+};++main_xtal{+clock-frequency=<12000000>;+};+};++ahb{+apb{+mmc0:mmc@f0000000{+pinctrl-0=<&pinctrl_mmc0_clk_cmd_dat0&pinctrl_mmc0_dat1_3&pinctrl_mmc0_dat4_7&pinctrl_mmc0_cd>;+status="okay";+slot@0{+reg=<0>;+bus-width=<8>;+cd-gpios=<&pioE0GPIO_ACTIVE_LOW>;+};+};++spi0:spi@f0004000{+cs-gpios=<&pioD130>,<0>,<0>,<&pioD160>;+id=<0>;+status="okay";+};++can0:can@f000c000{+status="okay";+};++i2c0:i2c@f0014000{+pinctrl-0=<&pinctrl_i2c0_pu>;+status="okay";+};++i2c1:i2c@f0018000{+status="okay";++pmic:act8865@5b{+compatible="active-semi,act8865";+reg=<0x5b>;+status="okay";++regulators{+vcc_1v8_reg:DCDC_REG1{+regulator-name="VCC_1V8";+regulator-min-microvolt=<1800000>;+regulator-max-microvolt=<1800000>;+regulator-always-on;+};++vcc_1v2_reg:DCDC_REG2{+regulator-name="VCC_1V2";+regulator-min-microvolt=<1200000>;+regulator-max-microvolt=<1200000>;+regulator-always-on;+};++vcc_3v3_reg:DCDC_REG3{+regulator-name="VCC_3V3";+regulator-min-microvolt=<3300000>;+regulator-max-microvolt=<3300000>;+regulator-always-on;+};++vddfuse_reg:LDO_REG1{+regulator-name="FUSE_2V5";+regulator-min-microvolt=<2500000>;+regulator-max-microvolt=<2500000>;+};++vddana_reg:LDO_REG2{+regulator-name="VDDANA";+regulator-min-microvolt=<3300000>;+regulator-max-microvolt=<3300000>;+regulator-always-on;+};+};+};+};++macb0:ethernet@f0028000{+phy-mode="rgmii";+gpios=<&pioB28GPIO_ACTIVE_LOW>;+reset-gpios=<&pioC31GPIO_ACTIVE_LOW>;+status="okay";++fixed-link{+speed=<1000>;+full-duplex;+};+};++pwm0:pwm@f002c000{+pinctrl-names="default";+pinctrl-0=<&pinctrl_pwm0_pwmh0_0&pinctrl_pwm0_pwmh1_0>;+status="okay";+};++usart0:serial@f001c000{+status="okay";+};++usart1:serial@f0020000{+pinctrl-0=<&pinctrl_usart1&pinctrl_usart1_rts_cts>;+status="okay";+};++uart0:serial@f0024000{+status="okay";+};++mmc1:mmc@f8000000{+pinctrl-0=<&pinctrl_mmc1_clk_cmd_dat0&pinctrl_mmc1_dat1_3&pinctrl_mmc1_cd>;+status="okay";+slot@0{+reg=<0>;+bus-width=<4>;+cd-gpios=<&pioE1GPIO_ACTIVE_HIGH>;+};+};++spi1:spi@f8008000{+pinctrl-0=<&pinctrl_spi_ksz>;+cs-gpios=<&pioC250>;+id=<1>;+status="okay";++ksz9477:ksz9477@0{+compatible="microchip,ksz9477","microchip,ksz9893";+reg=<0>;++/* Bus clock is 132 MHz. */+spi-max-frequency=<44000000>;+spi-cpha;+spi-cpol;+gpios=<&pioB28GPIO_ACTIVE_LOW>;+status="okay";++ports{+#address-cells=<1>;+#size-cells=<0>;++port@0{+reg=<0>;+label="lan0";+};++port@1{+reg=<1>;+label="lan1";+};++port@2{+reg=<2>;+label="lan2";+};++port@3{+reg=<3>;+label="lan3";+};++port@4{+reg=<4>;+label="lan4";+};++port@5{+reg=<5>;+label="cpu";+ethernet=<&macb0>;+phy-mode="rgmii-id";++fixed-link{+speed=<0x3e8>;+full-duplex;+};+};++/* port 6 is connected to eth0 */+};+};+};++adc0:adc@f8018000{+pinctrl-0=<+&pinctrl_adc0_adtrg+&pinctrl_adc0_ad0+&pinctrl_adc0_ad1+&pinctrl_adc0_ad2+&pinctrl_adc0_ad3+&pinctrl_adc0_ad4+&pinctrl_adc0_ad5+&pinctrl_adc0_ad6+&pinctrl_adc0_ad7+&pinctrl_adc0_ad8+&pinctrl_adc0_ad9+>;+status="okay";+};++i2c2:i2c@f801c000{+dmas=<0>,<0>;/* Do not use DMA for i2c2 */+pinctrl-0=<&pinctrl_i2c2_pu>;+status="okay";+};++macb1:ethernet@f802c000{+phy-mode="rmii";+status="disabled";+};++dbgu:serial@ffffee00{+status="okay";+};++pinctrl@fffff200{+board{+pinctrl_i2c0_pu:i2c0_pu{+atmel,pins=+<AT91_PIOA30AT91_PERIPH_AAT91_PINCTRL_PULL_UP>,+<AT91_PIOA31AT91_PERIPH_AAT91_PINCTRL_PULL_UP>;+};++pinctrl_i2c2_pu:i2c2_pu{+atmel,pins=+<AT91_PIOA18AT91_PERIPH_BAT91_PINCTRL_PULL_UP>,+<AT91_PIOA19AT91_PERIPH_BAT91_PINCTRL_PULL_UP>;+};++pinctrl_mmc0_cd:mmc0_cd{+atmel,pins=+<AT91_PIOE0AT91_PERIPH_GPIOAT91_PINCTRL_PULL_UP_DEGLITCH>;+};++pinctrl_mmc1_cd:mmc1_cd{+atmel,pins=+<AT91_PIOE1AT91_PERIPH_GPIOAT91_PINCTRL_PULL_UP_DEGLITCH>;+};++pinctrl_usba_vbus:usba_vbus{+atmel,pins=+<AT91_PIOE9AT91_PERIPH_GPIOAT91_PINCTRL_DEGLITCH>;/* PE9, conflicts with A9 */+};++pinctrl_spi_ksz:spi_ksz{+atmel,pins=+<+AT91_PIOB28AT91_PERIPH_GPIOAT91_PINCTRL_DEGLITCH+AT91_PIOC31AT91_PERIPH_GPIOAT91_PINCTRL_DEGLITCH+>;+};+};+};++pmc:pmc@fffffc00{+main:mainck{+clock-frequency=<12000000>;+};+};+};++ebi:ebi@10000000{+pinctrl-0=<&pinctrl_ebi_nand_addr>;+pinctrl-names="default";+status="okay";++nand_controller:nand-controller{+status="okay";++nand@3{+reg=<0x30x00x2>;+atmel,rb=<0>;+nand-bus-width=<8>;+nand-ecc-mode="hw";+nand-ecc-strength=<4>;+nand-ecc-step-size=<512>;+nand-on-flash-bbt;+label="atmel_nand";++partitions{+compatible="fixed-partitions";+#address-cells=<1>;+#size-cells=<1>;++at91bootstrap@0{+label="at91bootstrap";+reg=<0x00x40000>;+};++bootloader@40000{+label="bootloader";+reg=<0x400000x80000>;+};++bootloaderenv@c0000{+label="bootloader env";+reg=<0xc00000xc0000>;+};++dtb@180000{+label="device tree";+reg=<0x1800000x80000>;+};++kernel@200000{+label="kernel";+reg=<0x2000000x600000>;+};++rootfs@800000{+label="rootfs";+reg=<0x8000000x0f800000>;+};+};+};+};+};++usb0:gadget@500000{+atmel,vbus-gpio=<&pioE9GPIO_ACTIVE_HIGH>;/* PE9, conflicts with A9 */+pinctrl-names="default";+pinctrl-0=<&pinctrl_usba_vbus>;+status="okay";+};++usb1:ohci@600000{+num-ports=<3>;+atmel,vbus-gpio=<0+&pioE3GPIO_ACTIVE_LOW+&pioE4GPIO_ACTIVE_LOW+>;+status="okay";+};++usb2:ehci@700000{+status="okay";+};+};++gpio_keys{+compatible="gpio-keys";++bp3{+label="PB_USER";+gpios=<&pioE29GPIO_ACTIVE_LOW>;+linux,code=<0x104>;+gpio-key,wakeup;+};+};++leds{+compatible="gpio-leds";++d2{+label="d2";+gpios=<&pioE23GPIO_ACTIVE_LOW>;/* PE23, conflicts with A23, CTS2 */+linux,default-trigger="heartbeat";+};++d3{+label="d3";+gpios=<&pioE24GPIO_ACTIVE_HIGH>;+};+};+};
From: Andrew Lunn <andrew@lunn.ch> Date: 2018-08-16 17:23:47
On Thu, Aug 16, 2018 at 08:54:40AM +0200, Ahmad Fatoum wrote:
On 08/15/2018 04:32 AM, Andrew Lunn wrote:
quoted
Ahmed, where is the device tree for the EVB-KSZ9477?
I've attached it [1]. It's still work-in-progress (DSA doesn't work yet for example), but Ethernet is usable with Linux v4.18 and my patch applied.
Thanks.
So the problem is, macb does not put phy DT nodes inside an mdio
subnode. It places them directly in the MAC node. So
of_mdiobus_register() is being called with the MAC
np. of_mdiobus_register() then looks for children of the MAC node,
assuming they are phys. But when you have a fixed phy node, it is not
a phy, it does not have a reg property, and you get these warnings.
There are cases when you need both fixed-phy and a mdio bus. e.g. a
DSA switch hanging off MDIO.
So we have a few things here...
1) A regression. We should find a fix for that. Maybe we should
special case a child node called 'fixed-link' in
of_mdiobus_register(). I would suggest adding a single warning if
such node is found.
2) Missing functionality. Add support for an mdio container node.
node = of_get_child_by_name(np, "mdio");
if (node)
err = of_mdiobus_register(bp->mii_bus, node);
else
err = of_mdiobus_register(bp->mii_bus, np);
3) Modify the existing dts files to make use of this container.
Because of backwards compatibility, we cannot force the use of it,
but we can encourage it.
Andrew
From: Ahmad Fatoum <a.fatoum@pengutronix.de> Date: 2018-08-20 15:33:00
On 08/16/2018 04:24 PM, Andrew Lunn wrote:
1) A regression. We should find a fix for that. Maybe we should
special case a child node called 'fixed-link' in
of_mdiobus_register(). I would suggest adding a single warning if
such node is found.
I just sent out a v2, which warns if fixed-link is encountered
in of_mdiobus_register(). The actual fixed-link handling remains in the
macb driver, because I think it's would be out of scope for a regression
fix to change where fixed-links are handled for all using drivers...
2) Missing functionality. Add support for an mdio container node.
node = of_get_child_by_name(np, "mdio");
if (node)
err = of_mdiobus_register(bp->mii_bus, node);
else
err = of_mdiobus_register(bp->mii_bus, np);
Done.
3) Modify the existing dts files to make use of this container.
Because of backwards compatibility, we cannot force the use of it,
but we can encourage it.