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:

...

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: stable@vger.kernel.org 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

You should probably prod Andrew Lunn, he suggested that I move the fixed link code from macb_mii_init() to _probe(). Here, you're at least partially directly undoing that.

(ref: https://www.mail-archive.com/netdev@vger.kernel.org/msg221018.html)

...

Signed-off-by: Ahmad Fatoum a.fatoum@pengutronix.de

drivers/net/ethernet/cadence/macb_main.c | 26 +++++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index a6c911bb5ce2..d202a03c42ed 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -481,11 +481,6 @@ static int macb_mii_probe(struct net_device *dev) if (np) { if (of_phy_is_fixed_link(np)) {

• 	if (of_phy_register_fixed_link(np) < 0) {
  

• 		dev_err(&bp->pdev->dev,
  

• 			"broken fixed-link specification\n");
  

• 		return -ENODEV;
  

• 	}
  bp->phy_node = of_node_get(np);
  

  } else { bp->phy_node = of_parse_phandle(np, "phy-handle", 0);

@@ -568,7 +563,7 @@ static int macb_mii_init(struct macb *bp) { struct macb_platform_data *pdata; struct device_node *np;

• int err;

• int err = -ENXIO;

/* Enable management port */ macb_writel(bp, NCR, MACB_BIT(MPE)); @@ -591,10 +586,21 @@ static int macb_mii_init(struct macb *bp) dev_set_drvdata(&bp->dev->dev, bp->mii_bus); np = bp->pdev->dev.of_node;

• if (pdata)

• bp->mii_bus->phy_mask = pdata->phy_mask;
  

• if (np && of_phy_is_fixed_link(np)) {

• if (of_phy_register_fixed_link(np) < 0) {
  

• 	dev_err(&bp->pdev->dev,
  

• 			"broken fixed-link specification\n");
  

• 	goto err_out_free_mdiobus;
  

• }
  

• err = mdiobus_register(bp->mii_bus);
  

• } else {

• if (pdata)
  

• 	bp->mii_bus->phy_mask = pdata->phy_mask;
  

• err = of_mdiobus_register(bp->mii_bus, np);
  

• }

• err = of_mdiobus_register(bp->mii_bus, np); if (err) goto err_out_free_mdiobus;

@@ -606,9 +612,9 @@ static int macb_mii_init(struct macb *bp) err_out_unregister_bus: mdiobus_unregister(bp->mii_bus); +err_out_free_mdiobus: if (np && of_phy_is_fixed_link(np)) of_phy_deregister_fixed_link(np); -err_out_free_mdiobus: of_node_put(bp->phy_node); mdiobus_free(bp->mii_bus); err_out: -- 2.18.0

-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://urldefense.proofpoint.com/v2/url?u=http-3A__www.pengutronix.de_&... |

Hi,

On Wed, Aug 15, 2018 at 3:35 AM Andrew Lunn andrew@lunn.ch wrote:

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:

...

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]

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c index 3e84107..197baa6 100644 --- a/drivers/net/ethernet/ti/davinci_mdio.c +++ b/drivers/net/ethernet/ti/davinci_mdio.c @@ -245,6 +245,13 @@ static int davinci_mdio_read(struct mii_bus *bus, int phy_id, int phy_reg) u32 reg; int ret;

+ if (phy_id == 2) + return 0; + if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK) return -EINVAL;

@@ -289,6 +296,13 @@ static int davinci_mdio_write(struct mii_bus *bus, int phy_id, u32 reg; int ret;

+ if (phy_id == 2) + return 0; + if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK) return -EINVAL;

Cheers, --Prabhakar Lad

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]

======================================= diff --git a/arch/arm/boot/dts/at91-sama5d3_xplained_ung8087.dts b/arch/arm/boot/dts/at91-sama5d3_xplained_ung8087.dts new file mode 100644 index 000000000000..164b6a47d0bf --- /dev/null +++ b/arch/arm/boot/dts/at91-sama5d3_xplained_ung8087.dts @@ -0,0 +1,393 @@ +/* + * at91-sama5d3_xplained_ung8087.dts - Device Tree file for the EVB-KSZ9477 board + * + * Copyright (C) 2014 Atmel, + * 2014 Nicolas Ferre nicolas.ferre@atmel.com + * 2018 Ahmad Fatoum a.fatoum@pengutronix.de + * + * Licensed under GPLv2 or later. + */ +/dts-v1/; +#include "sama5d36.dtsi" + +/ { + model = "SAMA5D3 Xplained"; + compatible = "atmel,sama5d3-xplained", "atmel,sama5d3", "atmel,sama5"; + + chosen { + bootargs = "console=ttyS0,115200"; + }; + + memory { + reg = <0x20000000 0x10000000>; + }; + + 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 = <&pioE 0 GPIO_ACTIVE_LOW>; + }; + }; + + spi0: spi@f0004000 { + cs-gpios = <&pioD 13 0>, <0>, <0>, <&pioD 16 0>; + 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 = <&pioB 28 GPIO_ACTIVE_LOW>; + reset-gpios = <&pioC 31 GPIO_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 = <&pioE 1 GPIO_ACTIVE_HIGH>; + }; + }; + + spi1: spi@f8008000 { + pinctrl-0 = <&pinctrl_spi_ksz>; + cs-gpios = <&pioC 25 0>; + 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 = <&pioB 28 GPIO_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_PIOA 30 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>, + <AT91_PIOA 31 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>; + }; + + pinctrl_i2c2_pu: i2c2_pu { + atmel,pins = + <AT91_PIOA 18 AT91_PERIPH_B AT91_PINCTRL_PULL_UP>, + <AT91_PIOA 19 AT91_PERIPH_B AT91_PINCTRL_PULL_UP>; + }; + + pinctrl_mmc0_cd: mmc0_cd { + atmel,pins = + <AT91_PIOE 0 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP_DEGLITCH>; + }; + + pinctrl_mmc1_cd: mmc1_cd { + atmel,pins = + <AT91_PIOE 1 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP_DEGLITCH>; + }; + + pinctrl_usba_vbus: usba_vbus { + atmel,pins = + <AT91_PIOE 9 AT91_PERIPH_GPIO AT91_PINCTRL_DEGLITCH>; /* PE9, conflicts with A9 */ + }; + + pinctrl_spi_ksz: spi_ksz { + atmel,pins = + < + AT91_PIOB 28 AT91_PERIPH_GPIO AT91_PINCTRL_DEGLITCH + AT91_PIOC 31 AT91_PERIPH_GPIO AT91_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 = <0x3 0x0 0x2>; + 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 = <0x0 0x40000>; + }; + + bootloader@40000 { + label = "bootloader"; + reg = <0x40000 0x80000>; + }; + + bootloaderenv@c0000 { + label = "bootloader env"; + reg = <0xc0000 0xc0000>; + }; + + dtb@180000 { + label = "device tree"; + reg = <0x180000 0x80000>; + }; + + kernel@200000 { + label = "kernel"; + reg = <0x200000 0x600000>; + }; + + rootfs@800000 { + label = "rootfs"; + reg = <0x800000 0x0f800000>; + }; + }; + }; + }; + }; + + usb0: gadget@500000 { + atmel,vbus-gpio = <&pioE 9 GPIO_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 + &pioE 3 GPIO_ACTIVE_LOW + &pioE 4 GPIO_ACTIVE_LOW + >; + status = "okay"; + }; + + usb2: ehci@700000 { + status = "okay"; + }; + }; + + gpio_keys { + compatible = "gpio-keys"; + + bp3 { + label = "PB_USER"; + gpios = <&pioE 29 GPIO_ACTIVE_LOW>; + linux,code = <0x104>; + gpio-key,wakeup; + }; + }; + + leds { + compatible = "gpio-leds"; + + d2 { + label = "d2"; + gpios = <&pioE 23 GPIO_ACTIVE_LOW>; /* PE23, conflicts with A23, CTS2 */ + linux,default-trigger = "heartbeat"; + }; + + d3 { + label = "d3"; + gpios = <&pioE 24 GPIO_ACTIVE_HIGH>; + }; + }; +};