[PATCH 1/2] ARM: dts: imx6qdl-sabresd: Pass 'phy-reset-duration' property

[PATCH 1/2] ARM: dts: imx6qdl-sabresd: Pass 'phy-reset-duration' property

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@nxp.com>

As per the AR8031 datasheet:

"For a reliable power on reset, suggest to keep asserting the reset
low long enough (10ms) to ensure the clock is stable and clock-to-reset
1ms requirement is satisfied."

So do as suggested and keep the reset low for 10ms.

Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index a6d445c..aebe1b2 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -169,6 +169,7 @@
 	pinctrl-0 = <&pinctrl_enet>;
 	phy-mode = "rgmii";
 	phy-reset-gpios = <&gpio1 25 0>;
+	phy-reset-duration = <10>;
 	status = "okay";
 };
 
-- 
1.9.1

[PATCH 2/2] ARM: dts: imx6qdl-microsom-ar8035: Adjust Ethernet PHY reset duration

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@nxp.com>

As per the AR8035 datasheet:

"For a reliable power on reset, suggest to keep asserting the reset
low long enough (10ms) to ensure the clock is stable and clock-to-reset
1ms requirement is satisfied."

So do as suggested and keep the reset low for 10ms.

Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi b/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
index 469ef58..d187177 100644
--- a/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
@@ -45,7 +45,7 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_microsom_enet_ar8035>;
 	phy-mode = "rgmii";
-	phy-reset-duration = <2>;
+	phy-reset-duration = <10>;
 	phy-reset-gpios = <&gpio4 15 0>;
 	status = "okay";
 };
-- 
1.9.1

[PATCH 2/2] ARM: dts: imx6qdl-microsom-ar8035: Adjust Ethernet PHY reset duration

[Russell King - ARM Linux]

On Mon, Jan 04, 2016 at 05:11:40PM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
> 
> As per the AR8035 datasheet:
> 
> "For a reliable power on reset, suggest to keep asserting the reset
> low long enough (10ms) to ensure the clock is stable and clock-to-reset
> 1ms requirement is satisfied."

This is questionable.  The quote you indicate above is "For a reliable
POWER ON RESET".  It depends what use this DT is being put to.  Given
that the only boot loader which is trustworthy on SoldRun hardware is
their uboot versions, and these don't use DT, I would say that the
only time that this is used is when the kernel is booting.

That reset is not a power on reset, the phy has been powered for a
comparitively long time by the time the kernel gets to use it.  So, I
think on balance I'm going to NAK this change until there is a reason
for it to be made - iow, when there _is_ a boot loader where the
requirement for this parameter to be used at power on is required.

However, I think that should be discussed, in particular whether
there should be a separate property for the power on reset duration.

The final argument against it is that most "power on reset" requirements
are to keep the reset signal asserted while the _power_ _and_ _clocks_
both stabilise.  By the time we get to running any code, the power must
have stabilised (otherwise the SoC won't be operating.)  Moreover, as
the SoC is responsible for providing the AR8035 with its clock, the
SoC must be sufficiently out of reset for its own clocks to have
stabilised internally.  The clock manager software (whatever it is)
is responsible for ensuring that when a clock output is enabled, the
user knows when the clock has stabilised.  So, a boot loader enabling
the clock output which drives the AR8035 should be aware of the point
at which the clock has stabilised, and at that time should start timing
out the reset delay specified in the _existing_ DT file, which is
sufficiently long to satisfy the clock-to-reset delay of 1ms.

So, on that second point, it's also a NAK.  That's two good reasons
why the existing DT specification here is actually correct.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 2/2] ARM: dts: imx6qdl-microsom-ar8035: Adjust Ethernet PHY reset duration

[Fabio Estevam]

On Mon, Jan 4, 2016 at 8:42 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jan 04, 2016 at 05:11:40PM -0200, Fabio Estevam wrote:
>> From: Fabio Estevam <fabio.estevam@nxp.com>
>>
>> As per the AR8035 datasheet:
>>
>> "For a reliable power on reset, suggest to keep asserting the reset
>> low long enough (10ms) to ensure the clock is stable and clock-to-reset
>> 1ms requirement is satisfied."
>
> This is questionable.  The quote you indicate above is "For a reliable
> POWER ON RESET".  It depends what use this DT is being put to.  Given
> that the only boot loader which is trustworthy on SoldRun hardware is
> their uboot versions, and these don't use DT, I would say that the
> only time that this is used is when the kernel is booting.
>
> That reset is not a power on reset, the phy has been powered for a
> comparitively long time by the time the kernel gets to use it.  So, I
> think on balance I'm going to NAK this change until there is a reason
> for it to be made - iow, when there _is_ a boot loader where the
> requirement for this parameter to be used at power on is required.

Ok, understood. I was working on a fix for U-boot where the 2ms reset
was not enough, so I thought about extending it to the kernel. I see
your point and agree that for the kernel we could keep the existing
2ms reset delay.