diff mbox

[1/6] ARM: dts: microsom-ar8035: MDIO pad must be set open drain

Message ID E1XL7M1-0002yD-K3@rmk-PC.arm.linux.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Russell King Aug. 23, 2014, 9:11 a.m. UTC
From: Rabeeh Khoury <rabeeh@solid-run.com>
To: Shawn Guo <shawn.guo@freescale.com>

MDIO pad must be set open drain.

Signed-off-by: Rabeeh Khoury <rabeeh@solid-run.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Iain Paton Aug. 24, 2014, 9:44 a.m. UTC | #1
On 23/08/14 10:11, Russell King wrote:
> From: Rabeeh Khoury <rabeeh@solid-run.com>
> To: Shawn Guo <shawn.guo@freescale.com>
> 
> MDIO pad must be set open drain.
> 
> Signed-off-by: Rabeeh Khoury <rabeeh@solid-run.com>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  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 d16066608e21..db9f45b2c573 100644
> --- a/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
> @@ -17,7 +17,7 @@
>  	enet {
>  		pinctrl_microsom_enet_ar8035: microsom-enet-ar8035 {
>  			fsl,pins = <
> -				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
> +				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b8b0
>  				MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
>  				/* AR8035 reset */
>  				MX6QDL_PAD_KEY_ROW4__GPIO4_IO15		0x130b0
> 

Can you elaborate some more on the reasons for this?  

I'd like to understand if it's something specific to the hardware on that 
board, or if other i.MX6 boards using the ar8035 are doing it wrong as well.

The datasheet strongly suggests this is the correct thing to do as MDIO 
should electrically be open drain. However that suggests many more instances 
of this incorrect configuration exist which also need changed, and not just 
for ar8035.

While I'm reluctant to attribute something like this to the fec/interrupt 
problems some people are seeing, I'd also like to be able to rule it out.
Russell King - ARM Linux Aug. 24, 2014, 9:58 a.m. UTC | #2
On Sun, Aug 24, 2014 at 10:44:54AM +0100, Iain Paton wrote:
> On 23/08/14 10:11, Russell King wrote:
> > From: Rabeeh Khoury <rabeeh@solid-run.com>
> > To: Shawn Guo <shawn.guo@freescale.com>
> > 
> > MDIO pad must be set open drain.
> > 
> > Signed-off-by: Rabeeh Khoury <rabeeh@solid-run.com>
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > ---
> >  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 d16066608e21..db9f45b2c573 100644
> > --- a/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
> > @@ -17,7 +17,7 @@
> >  	enet {
> >  		pinctrl_microsom_enet_ar8035: microsom-enet-ar8035 {
> >  			fsl,pins = <
> > -				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
> > +				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b8b0
> >  				MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
> >  				/* AR8035 reset */
> >  				MX6QDL_PAD_KEY_ROW4__GPIO4_IO15		0x130b0
> > 
> 
> Can you elaborate some more on the reasons for this?  
> 
> I'd like to understand if it's something specific to the hardware on that 
> board, or if other i.MX6 boards using the ar8035 are doing it wrong as well.
> 
> The datasheet strongly suggests this is the correct thing to do as MDIO 
> should electrically be open drain. However that suggests many more instances 
> of this incorrect configuration exist which also need changed, and not just 
> for ar8035.
> 
> While I'm reluctant to attribute something like this to the fec/interrupt 
> problems some people are seeing, I'd also like to be able to rule it out.

I'm merely passing the patch along; I've forwarded your question to
Rabeeh via IRC, and may have an answer soon.

FYI, I've run for a long time (close to a year now) on various iMX6
SolidRun platforms without the above patch and haven't seen a problem.
I've now also running with it on Solo and Quad and haven't seen any
ill effects there, despite the machines running root-NFS, doing
regular compiles and git activity.

For me, the problem device right now is that placing the Vivante 2D
GPU under stress (using gtkperf) causes the pixel engines to lock up.
The Solo is extremely unstable, locking up in less than a second on
the first or second test, whereas the Quad is much better, normally
taking two runs before locking up in the circle drawing.  The Solo
also locks up the 2D pixel engines if it needs ot resize the display
(due to HDMI being connected.)
Eric Bénard Aug. 25, 2014, 6:53 a.m. UTC | #3
Hi Russell,

Le Sun, 24 Aug 2014 10:58:55 +0100,
Russell King - ARM Linux <linux@arm.linux.org.uk> a écrit :
> For me, the problem device right now is that placing the Vivante 2D
> GPU under stress (using gtkperf) causes the pixel engines to lock up.
> The Solo is extremely unstable, locking up in less than a second on
> the first or second test, whereas the Quad is much better, normally
> taking two runs before locking up in the circle drawing.  The Solo
> also locks up the 2D pixel engines if it needs ot resize the display
> (due to HDMI being connected.)
> 
Maybe the lock you get is related to what this patch is supposed to
fix in Freescale's kernel ?
http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/?h=imx_3.10.17_1.0.1_ga&id=ba5139e1daa3f5834b061a099bcec8e85575a2c0

Best regards,
Eric
Shawn Guo Aug. 26, 2014, 10:15 a.m. UTC | #4
On Sun, Aug 24, 2014 at 10:44:54AM +0100, Iain Paton wrote:
> On 23/08/14 10:11, Russell King wrote:
> > From: Rabeeh Khoury <rabeeh@solid-run.com>
> > To: Shawn Guo <shawn.guo@freescale.com>
> > 
> > MDIO pad must be set open drain.
> > 
> > Signed-off-by: Rabeeh Khoury <rabeeh@solid-run.com>
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > ---
> >  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 d16066608e21..db9f45b2c573 100644
> > --- a/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
> > @@ -17,7 +17,7 @@
> >  	enet {
> >  		pinctrl_microsom_enet_ar8035: microsom-enet-ar8035 {
> >  			fsl,pins = <
> > -				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
> > +				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b8b0
> >  				MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
> >  				/* AR8035 reset */
> >  				MX6QDL_PAD_KEY_ROW4__GPIO4_IO15		0x130b0
> > 
> 
> Can you elaborate some more on the reasons for this?  

I just got the following text from Rabeeh explaining the change.

"This patch is important for the MicroSOM implementation due to the
following details -

1. VIH of the Atheros phy is 1.7V.
2. NVCC_ENET which is the power domain of the MDIO pad is driven by the
   PHY's LDO (i.e. either 1.8v or 2.5v).
3. The MicroSOM implements an onbouard 1.6kohm pull up to 3.3v (R3000).

In the case the PHY's LDO was 1.8v then there would be only a 100mV
margin for the signal to be acknowledged as high (1.8v-1.7v).
Due to that setting the pad as an open drain will let the 1.6kohm pull
that signal high to 3.3 that assures enough margins to the PHY to be
acked as '1' logic.

Notice that this change is not required to the SabreSD boards since that
hardware drives the NVCC_EIM power island from a 3.3v power rail."

> 
> I'd like to understand if it's something specific to the hardware on that 
> board, or if other i.MX6 boards using the ar8035 are doing it wrong as well.

So from text above, it's a MicroSOM board specific change, I think.

Shawn

> 
> The datasheet strongly suggests this is the correct thing to do as MDIO 
> should electrically be open drain. However that suggests many more instances 
> of this incorrect configuration exist which also need changed, and not just 
> for ar8035.
> 
> While I'm reluctant to attribute something like this to the fec/interrupt 
> problems some people are seeing, I'd also like to be able to rule it out.
> 
>
Iain Paton Aug. 26, 2014, 12:08 p.m. UTC | #5
On 24/08/14 10:58, Russell King - ARM Linux wrote:
> On Sun, Aug 24, 2014 at 10:44:54AM +0100, Iain Paton wrote:
>> On 23/08/14 10:11, Russell King wrote:
>>> From: Rabeeh Khoury <rabeeh@solid-run.com>
>>> To: Shawn Guo <shawn.guo@freescale.com>
>>>
>>> MDIO pad must be set open drain.
>>>
>>> Signed-off-by: Rabeeh Khoury <rabeeh@solid-run.com>
>>> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
>>> ---
>>>  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 d16066608e21..db9f45b2c573 100644
>>> --- a/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
>>> +++ b/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
>>> @@ -17,7 +17,7 @@
>>>  	enet {
>>>  		pinctrl_microsom_enet_ar8035: microsom-enet-ar8035 {
>>>  			fsl,pins = <
>>> -				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
>>> +				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b8b0
>>>  				MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
>>>  				/* AR8035 reset */
>>>  				MX6QDL_PAD_KEY_ROW4__GPIO4_IO15		0x130b0
>>>
>>
>> Can you elaborate some more on the reasons for this?  
>>
>> I'd like to understand if it's something specific to the hardware on that 
>> board, or if other i.MX6 boards using the ar8035 are doing it wrong as well.
>>
>> The datasheet strongly suggests this is the correct thing to do as MDIO 
>> should electrically be open drain. However that suggests many more instances 
>> of this incorrect configuration exist which also need changed, and not just 
>> for ar8035.
>>
>> While I'm reluctant to attribute something like this to the fec/interrupt 
>> problems some people are seeing, I'd also like to be able to rule it out.
> 
> I'm merely passing the patch along; I've forwarded your question to
> Rabeeh via IRC, and may have an answer soon.
> 
> FYI, I've run for a long time (close to a year now) on various iMX6
> SolidRun platforms without the above patch and haven't seen a problem.
> I've now also running with it on Solo and Quad and haven't seen any
> ill effects there, despite the machines running root-NFS, doing
> regular compiles and git activity.

I've not seen any problems either.  Digging through IEEE 802.3 clause 22
only provides references to MDIO being implemented using a tri-state 
driver. Then in Clause 45 (10gig) there's a one liner saying it 'may' be 
implemented using open drain.

The i.MX6 manual specifically states support for Clause 22.

However, as 802.3 requires the pull-up be present, it seems unlikely there 
will be any practical difference between an implementation using open drain 
and tri-state signalling. Indeed, open drain would seem to be the logical
way to implement it electrically.

The AR8035 datasheet specifically marks MDIO and INT as being open drain,
however the KSZ9021 (as used on sabre-lite) datasheet doesn't, instead it 
marks them as Ipu/O (for Input with internal pull-up/Output).

I'd suggest that this is a question of correctness more than a practical 
problem and I have no objection to the patch being applied.
Iain Paton Aug. 26, 2014, 12:16 p.m. UTC | #6
On 26/08/14 11:15, Shawn Guo wrote:
> On Sun, Aug 24, 2014 at 10:44:54AM +0100, Iain Paton wrote:
>> On 23/08/14 10:11, Russell King wrote:
>>> From: Rabeeh Khoury <rabeeh@solid-run.com>
>>> To: Shawn Guo <shawn.guo@freescale.com>
>>>
>>> MDIO pad must be set open drain.
>>>
>>> Signed-off-by: Rabeeh Khoury <rabeeh@solid-run.com>
>>> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
>>> ---
>>>  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 d16066608e21..db9f45b2c573 100644
>>> --- a/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
>>> +++ b/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
>>> @@ -17,7 +17,7 @@
>>>  	enet {
>>>  		pinctrl_microsom_enet_ar8035: microsom-enet-ar8035 {
>>>  			fsl,pins = <
>>> -				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
>>> +				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b8b0
>>>  				MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
>>>  				/* AR8035 reset */
>>>  				MX6QDL_PAD_KEY_ROW4__GPIO4_IO15		0x130b0
>>>
>>
>> Can you elaborate some more on the reasons for this?  
> 
> I just got the following text from Rabeeh explaining the change.
> 
> "This patch is important for the MicroSOM implementation due to the
> following details -
> 
> 1. VIH of the Atheros phy is 1.7V.
> 2. NVCC_ENET which is the power domain of the MDIO pad is driven by the
>    PHY's LDO (i.e. either 1.8v or 2.5v).
> 3. The MicroSOM implements an onbouard 1.6kohm pull up to 3.3v (R3000).
> 
> In the case the PHY's LDO was 1.8v then there would be only a 100mV
> margin for the signal to be acknowledged as high (1.8v-1.7v).
> Due to that setting the pad as an open drain will let the 1.6kohm pull
> that signal high to 3.3 that assures enough margins to the PHY to be
> acked as '1' logic.
> 
> Notice that this change is not required to the SabreSD boards since that
> hardware drives the NVCC_EIM power island from a 3.3v power rail."
> 
>>
>> I'd like to understand if it's something specific to the hardware on that 
>> board, or if other i.MX6 boards using the ar8035 are doing it wrong as well.
> 
> So from text above, it's a MicroSOM board specific change, I think.

Ok, that makes a lot of sense. Practical implementation reasons wins any 
argument.  
No objections from me to this, I think we understand the reasons now.

Iain
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi b/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
index d16066608e21..db9f45b2c573 100644
--- a/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-microsom-ar8035.dtsi
@@ -17,7 +17,7 @@ 
 	enet {
 		pinctrl_microsom_enet_ar8035: microsom-enet-ar8035 {
 			fsl,pins = <
-				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
+				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b8b0
 				MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
 				/* AR8035 reset */
 				MX6QDL_PAD_KEY_ROW4__GPIO4_IO15		0x130b0