[PATCH v2 0/2] ARM: at91: remove phy fixup for sama5d3xek boards

Hello,

This patch removes a board specific hook for sama5d3xek boards from the
sama5d3 generic DT board file.

This hook (which register a phy fixup configuring board specific delays
in the ksz9021 ethernet phy) is now replaced by the appropriate DT
properties definitions in the sama5d3xcm.dtsi file.

Best Regards,

Boris

Changes since v1:
 - fix txc-skew-ps and rxc-skew-ps delays
 - remove phy address info to handle Ronetix and Embest HW designs

Boris BREZILLON (2):
  ARM: at91/dt: describe rgmii ethernet phy connected to sama5d3xek
    boards
  ARM: at91: remove phy fixup for sama5d3xek boards

 arch/arm/boot/dts/sama5d3xcm.dtsi   | 15 +++++++++++++++
 arch/arm/mach-at91/board-dt-sama5.c | 22 ----------------------
 2 files changed, 15 insertions(+), 22 deletions(-)

-- 
1.8.3.2

Add ethernet-phy node and specify phy interrupt (connected to pin PB25).

The PHY address is not specified here because atmel have 2 different
designs
for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
(Embest design) and the other one is connection PHYAD0 to a pull up
resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
As a result, Ronetix design will have its PHY available at address 0x1 and
Embest design at 0x7.
Let the net PHY core automatically detect the PHY address by scanning the
MDIO bus.

Define board specific delays to apply to RGMII signals.

Signed-off-by: Boris BREZILLON <redacted>
---
 arch/arm/boot/dts/sama5d3xcm.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d3xcm.dtsi b/arch/arm/boot/dts/sama5d3xcm.dtsi
index b0b1331..fc68bae 100644
--- a/arch/arm/boot/dts/sama5d3xcm.dtsi
+++ b/arch/arm/boot/dts/sama5d3xcm.dtsi

@@ -34,6 +34,21 @@
 
 			macb0: ethernet@f0028000 {
 				phy-mode = "rgmii";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				ethernet-phy {
+					interrupt-parent = <&pioB>;
+					interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+					txen-skew-ps = <800>;
+					txc-skew-ps = <3000>;
+					rxdv-skew-ps = <400>;
+					rxc-skew-ps = <3000>;
+					rxd0-skew-ps = <400>;
+					rxd1-skew-ps = <400>;
+					rxd2-skew-ps = <400>;
+					rxd3-skew-ps = <400>;
+				};
 			};
 
 			pmc: pmc@fffffc00 {

-- 
1.8.3.2

These board specific delays are now configured through micrel's specific
DT bindings (see Documentation/devicetree/bindings/net/micrel-ksz9021.txt).

Remove this phy fixup registration from sama5 DT machine file to keep it
as generic as possible.

Signed-off-by: Boris BREZILLON <redacted>
---
 arch/arm/mach-at91/board-dt-sama5.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
index d6fe04b..8c5814f 100644
--- a/arch/arm/mach-at91/board-dt-sama5.c
+++ b/arch/arm/mach-at91/board-dt-sama5.c

@@ -35,30 +35,8 @@ static void __init sama5_dt_timer_init(void)
 	at91sam926x_pit_init();
 }
 
-static int ksz9021rn_phy_fixup(struct phy_device *phy)
-{
-	int value;
-
-	/* Set delay values */
-	value = MICREL_KSZ9021_RGMII_CLK_CTRL_PAD_SCEW | 0x8000;
-	phy_write(phy, MICREL_KSZ9021_EXTREG_CTRL, value);
-	value = 0xF2F4;
-	phy_write(phy, MICREL_KSZ9021_EXTREG_DATA_WRITE, value);
-	value = MICREL_KSZ9021_RGMII_RX_DATA_PAD_SCEW | 0x8000;
-	phy_write(phy, MICREL_KSZ9021_EXTREG_CTRL, value);
-	value = 0x2222;
-	phy_write(phy, MICREL_KSZ9021_EXTREG_DATA_WRITE, value);
-
-	return 0;
-}
-
 static void __init sama5_dt_device_init(void)
 {
-	if (of_machine_is_compatible("atmel,sama5d3xcm") &&
-	    IS_ENABLED(CONFIG_PHYLIB))
-		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
-			ksz9021rn_phy_fixup);
-
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
 

-- 
1.8.3.2

Hi Boris,

2014-06-26 3:13 GMT-07:00 Boris BREZILLON [off-list ref]:

Add ethernet-phy node and specify phy interrupt (connected to pin PB25).

The PHY address is not specified here because atmel have 2 different
designs
for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
(Embest design) and the other one is connection PHYAD0 to a pull up
resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
As a result, Ronetix design will have its PHY available at address 0x1 and
Embest design at 0x7.
Let the net PHY core automatically detect the PHY address by scanning the
MDIO bus.

I though the compatible string was listed as a required property, but
it is not. The 'reg' property however is listed as required, although
the of_miodbus_register() works just fine without it, although that is
a Linux-specific implementation detail.

Define board specific delays to apply to RGMII signals.

Signed-off-by: Boris BREZILLON <redacted>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

-- 
Florian

Hi Florian,

On 26/06/2014 20:15, Florian Fainelli wrote:

Hi Boris,

2014-06-26 3:13 GMT-07:00 Boris BREZILLON [off-list ref]:

quoted

Add ethernet-phy node and specify phy interrupt (connected to pin PB25).

The PHY address is not specified here because atmel have 2 different
designs
for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
(Embest design) and the other one is connection PHYAD0 to a pull up
resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
As a result, Ronetix design will have its PHY available at address 0x1 and
Embest design at 0x7.
Let the net PHY core automatically detect the PHY address by scanning the
MDIO bus.

I though the compatible string was listed as a required property, but
it is not. The 'reg' property however is listed as required, although
the of_miodbus_register() works just fine without it, although that is
a Linux-specific implementation detail.

Indeed, it's listed in the required property list of the DT binding doc,
but the code implement auto detection if reg is missing.
However this line [1] clearly shows that specifying the reg property is
the preferred way of doing things.

I could define 2 different sama5d3xcm.dtsi (sama5d3xcm-ronetix.dtsi and
sama5d3xcm-embest.dtsi) to avoid this dirty hack,
but then we would have 2 more dtb and the user would have to determine
which CPU module he owns to choose the appropriate dtb.
If at91, arm-soc and DT maintainers agree with this approach I can
definitely propose something.

quoted

Define board specific delays to apply to RGMII signals.

Signed-off-by: Boris BREZILLON <redacted>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks for your review.

Best Regards,

Boris

[1] http://lxr.free-electrons.com/source/drivers/of/of_mdio.c#L187

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

On 26/06/2014 22:01, Boris BREZILLON :

Hi Florian,

On 26/06/2014 20:15, Florian Fainelli wrote:

quoted

Hi Boris,

2014-06-26 3:13 GMT-07:00 Boris BREZILLON [off-list ref]:

quoted

Add ethernet-phy node and specify phy interrupt (connected to pin PB25).

The PHY address is not specified here because atmel have 2 different
designs
for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
(Embest design) and the other one is connection PHYAD0 to a pull up
resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
As a result, Ronetix design will have its PHY available at address 0x1 and
Embest design at 0x7.
Let the net PHY core automatically detect the PHY address by scanning the
MDIO bus.

I though the compatible string was listed as a required property, but
it is not. The 'reg' property however is listed as required, although
the of_miodbus_register() works just fine without it, although that is
a Linux-specific implementation detail.

Indeed, it's listed in the required property list of the DT binding doc,
but the code implement auto detection if reg is missing.
However this line [1] clearly shows that specifying the reg property is
the preferred way of doing things.

I could define 2 different sama5d3xcm.dtsi (sama5d3xcm-ronetix.dtsi and
sama5d3xcm-embest.dtsi) to avoid this dirty hack,
but then we would have 2 more dtb and the user would have to determine
which CPU module he owns to choose the appropriate dtb.
If at91, arm-soc and DT maintainers agree with this approach I can
definitely propose something.

Yes Boris, I definitively prefer not to add another .dtsi file for this
series if we can avoid it.

So, I would push for this "reg-less" solution. If it is chosen, you can
add my:
Acked-by: Nicolas Ferre <redacted>

Thanks, bye.

quoted

quoted

Define board specific delays to apply to RGMII signals.

Signed-off-by: Boris BREZILLON <redacted>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks for your review.

Best Regards,

Boris

[1] http://lxr.free-electrons.com/source/drivers/of/of_mdio.c#L187

-- 
Nicolas Ferre

From: Boris BREZILLON <redacted>
Date: Thu, 26 Jun 2014 12:13:33 +0200

This patch removes a board specific hook for sama5d3xek boards from the
sama5d3 generic DT board file.

This hook (which register a phy fixup configuring board specific delays
in the ksz9021 ethernet phy) is now replaced by the appropriate DT
properties definitions in the sama5d3xcm.dtsi file.

Changes since v1:
 - fix txc-skew-ps and rxc-skew-ps delays
 - remove phy address info to handle Ronetix and Embest HW designs

These patches do not apply cleanly to 'net' nor 'net-next', in fact
you did not even say which tree these changes are targetting.

Please respin these patches and explicitly say what tree you wish
them to be applied to.

Thanks.

Hello David,

On Tue, 01 Jul 2014 15:38:06 -0700 (PDT)
David Miller [off-list ref] wrote:

From: Boris BREZILLON <redacted>
Date: Thu, 26 Jun 2014 12:13:33 +0200

quoted

This patch removes a board specific hook for sama5d3xek boards from
the sama5d3 generic DT board file.

This hook (which register a phy fixup configuring board specific
delays in the ksz9021 ethernet phy) is now replaced by the
appropriate DT properties definitions in the sama5d3xcm.dtsi file.

Changes since v1:
 - fix txc-skew-ps and rxc-skew-ps delays
 - remove phy address info to handle Ronetix and Embest HW designs

These patches do not apply cleanly to 'net' nor 'net-next', in fact
you did not even say which tree these changes are targetting.

Actually, these patches were intended to be taken through at91 (and then
arm-soc) tree.

The reason I added netdev in Cc is because I wanted to get feedback on
the DT phy node definition (which I got from Florian).

I guess patch 2 does not apply cleanly because I based this work on top
of other changes which are not merged yet ([1]).

Nicolas, I'll take care to rebase this series on top of linus/master
branch (or whatever branch you want me to base it onto).

Thanks,

Boris

[1]http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265216.html

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Hello Bo,

I know you're quite busy, but if you have some time could you test this
series on both CPU Modules (Embest and Ronetix): I only own the Embest
one.

Best Regards,

Boris

On Thu, 26 Jun 2014 12:13:33 +0200
Boris BREZILLON [off-list ref] wrote:

Hello,

This patch removes a board specific hook for sama5d3xek boards from the
sama5d3 generic DT board file.

This hook (which register a phy fixup configuring board specific delays
in the ksz9021 ethernet phy) is now replaced by the appropriate DT
properties definitions in the sama5d3xcm.dtsi file.

Best Regards,

Boris

Changes since v1:
 - fix txc-skew-ps and rxc-skew-ps delays
 - remove phy address info to handle Ronetix and Embest HW designs

Boris BREZILLON (2):
  ARM: at91/dt: describe rgmii ethernet phy connected to sama5d3xek
    boards
  ARM: at91: remove phy fixup for sama5d3xek boards

 arch/arm/boot/dts/sama5d3xcm.dtsi   | 15 +++++++++++++++
 arch/arm/mach-at91/board-dt-sama5.c | 22 ----------------------
 2 files changed, 15 insertions(+), 22 deletions(-)

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Hi Boris,

On 07/10/2014 12:34 AM, Boris BREZILLON wrote:

Hello Bo,

I know you're quite busy, but if you have some time could you test this
series on both CPU Modules (Embest and Ronetix): I only own the Embest
one.

Best Regards,

Boris

On Thu, 26 Jun 2014 12:13:33 +0200
Boris BREZILLON [off-list ref] wrote:

quoted

Hello,

This patch removes a board specific hook for sama5d3xek boards from the
sama5d3 generic DT board file.

This hook (which register a phy fixup configuring board specific delays
in the ksz9021 ethernet phy) is now replaced by the appropriate DT
properties definitions in the sama5d3xcm.dtsi file.

Best Regards,

Boris

Changes since v1:
  - fix txc-skew-ps and rxc-skew-ps delays
  - remove phy address info to handle Ronetix and Embest HW designs

Boris BREZILLON (2):
   ARM: at91/dt: describe rgmii ethernet phy connected to sama5d3xek
     boards
   ARM: at91: remove phy fixup for sama5d3xek boards

  arch/arm/boot/dts/sama5d3xcm.dtsi   | 15 +++++++++++++++
  arch/arm/mach-at91/board-dt-sama5.c | 22 ----------------------
  2 files changed, 15 insertions(+), 22 deletions(-)

For this series, test OK on sama5d33ek (Ronetix) and sama5d34ek (Embest).

Tested-by: Bo Shen <redacted>

Best Regards,
Bo Shen