Re: [PATCH] ARM: dts: Revert disabling of smc91x for n900

Re: [PATCH] ARM: dts: Revert disabling of smc91x for n900

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 3177 bytes --]

On Tuesday 06 January 2015 00:02:29 Tony Lindgren wrote:
> Revert "ARM: dts: Disable smc91x on n900 until bootloader
> dependency is removed". We've now fixed the issues that
> caused problems with uninitialized hardware depending on
> the bootloader version. Mostly things got fixed with
> the following commits:
> 
> 9a894953a97b ("ARM: dts: Fix bootloader version dependencies
> by muxing n900 smc91x pins") 7d2911c43815 ("net: smc91x: Fix
> gpios for device tree based booting")
> 
> Note that this only affects the early development boards
> with Ethernet that we still have in a few automated boot
> test systems.
> 
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -702,9 +702,6 @@
> 
>  	ethernet@gpmc {
>  		compatible = "smsc,lan91c94";
> -
> -		status = "disabled";
> -
>  		interrupt-parent = <&gpio2>;
>  		interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;	/* gpio54 */
>  		reg = <1 0x300 0xf>;		/* 16 byte IO range at offset 0x300
> */

Hello Tony,

to make smc ethernet working in n900 qemu I needed to apply this patch:

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index ff36fbe..d96eeb8 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -770,7 +770,8 @@
 		compatible = "smsc,lan91c94";
 		interrupt-parent = <&gpio2>;
 		interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;	/* gpio54 */
-		reg = <1 0x300 0xf>;		/* 16 byte IO range at offset 0x300 */
+//		reg = <1 0x300 0xf>;		/* 16 byte IO range at offset 0x300 */
+		reg = <1 0x0 0xf>;		/* 16 byte IO range at offset 0x300 */
 		bank-width = <2>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&ethernet_pins>;

With this patch I see in dmesg:

[   20.577911] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc91x: smc_probe
[   20.580535] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc91x: bank signature probe 
returned 0x3300
[   20.585327] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc91x: revision = 0x3391
[   20.590087] smc91x.c: v1.1, sep 22 2004 by Nicolas Pitre <nico@fluxnic.net>
[   20.593627] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc_reset
[   20.596832] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc_phy_detect
[   20.611938] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc91x: smc_shutdown
[   20.615875] smc91x 2000000.ethernet eth0: SMC91C11xFD (rev 1) at d08be000 IRQ 166
[   20.618682] 
[   20.621124] smc91x 2000000.ethernet eth0: Ethernet addr: 52:54:00:12:34:56
[   20.624938] smc91x 2000000.ethernet eth0: No PHY found

(and eth0 exists in ifconfig)

If I do not apply my patch I got this error message:

[   22.134704] smc91x 2000300.ethernet (unnamed net_device) (uninitialized): smc91x: bank signature probe 
returned 0x0000
[   22.140014] smc91x: not found (-19).

and no ethernet device was registered.

With 2.6.28 kernel with N900 patches (but smc91x is unmodified!) ethernet device is working fine.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] ARM: dts: Revert disabling of smc91x for n900

[Tony Lindgren]

* Pali Rohár <pali.rohar@gmail.com> [150218 07:23]:
> On Tuesday 06 January 2015 00:02:29 Tony Lindgren wrote:
> > Revert "ARM: dts: Disable smc91x on n900 until bootloader
> > dependency is removed". We've now fixed the issues that
> > caused problems with uninitialized hardware depending on
> > the bootloader version. Mostly things got fixed with
> > the following commits:
> > 
> > 9a894953a97b ("ARM: dts: Fix bootloader version dependencies
> > by muxing n900 smc91x pins") 7d2911c43815 ("net: smc91x: Fix
> > gpios for device tree based booting")
> > 
> > Note that this only affects the early development boards
> > with Ethernet that we still have in a few automated boot
> > test systems.
> > 
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > 
> > --- a/arch/arm/boot/dts/omap3-n900.dts
> > +++ b/arch/arm/boot/dts/omap3-n900.dts
> > @@ -702,9 +702,6 @@
> > 
> >  	ethernet@gpmc {
> >  		compatible = "smsc,lan91c94";
> > -
> > -		status = "disabled";
> > -
> >  		interrupt-parent = <&gpio2>;
> >  		interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;	/* gpio54 */
> >  		reg = <1 0x300 0xf>;		/* 16 byte IO range at offset 0x300
> > */
> 
> Hello Tony,
> 
> to make smc ethernet working in n900 qemu I needed to apply this patch:
> 
> diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
> index ff36fbe..d96eeb8 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -770,7 +770,8 @@
>  		compatible = "smsc,lan91c94";
>  		interrupt-parent = <&gpio2>;
>  		interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;	/* gpio54 */
> -		reg = <1 0x300 0xf>;		/* 16 byte IO range at offset 0x300 */
> +//		reg = <1 0x300 0xf>;		/* 16 byte IO range at offset 0x300 */
> +		reg = <1 0x0 0xf>;		/* 16 byte IO range at offset 0x300 */
>  		bank-width = <2>;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&ethernet_pins>;

Oh cool, the 0x300 offset is there mostly to suppress warnings about
non-standard location.
 
> With this patch I see in dmesg:
> 
> [   20.577911] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc91x: smc_probe
> [   20.580535] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc91x: bank signature probe 
> returned 0x3300
> [   20.585327] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc91x: revision = 0x3391
> [   20.590087] smc91x.c: v1.1, sep 22 2004 by Nicolas Pitre <nico@fluxnic.net>
> [   20.593627] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc_reset
> [   20.596832] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc_phy_detect
> [   20.611938] smc91x 2000000.ethernet (unnamed net_device) (uninitialized): smc91x: smc_shutdown
> [   20.615875] smc91x 2000000.ethernet eth0: SMC91C11xFD (rev 1) at d08be000 IRQ 166
> [   20.618682] 
> [   20.621124] smc91x 2000000.ethernet eth0: Ethernet addr: 52:54:00:12:34:56
> [   20.624938] smc91x 2000000.ethernet eth0: No PHY found
> 
> (and eth0 exists in ifconfig)
> 
> If I do not apply my patch I got this error message:
> 
> [   22.134704] smc91x 2000300.ethernet (unnamed net_device) (uninitialized): smc91x: bank signature probe 
> returned 0x0000
> [   22.140014] smc91x: not found (-19).
> 
> and no ethernet device was registered.
> 
> With 2.6.28 kernel with N900 patches (but smc91x is unmodified!) ethernet device is working fine.

OK that's good news. Care to do a patch to set the offset 0x0 with added
comment that qemu needs it? I'll test to make sure it works on the
real hardware as well.

Regards,

Tony

Re: [PATCH] ARM: dts: Revert disabling of smc91x for n900

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 4135 bytes --]

On Wednesday 18 February 2015 17:33:53 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar@gmail.com> [150218 07:23]:
> > On Tuesday 06 January 2015 00:02:29 Tony Lindgren wrote:
> > > Revert "ARM: dts: Disable smc91x on n900 until bootloader
> > > dependency is removed". We've now fixed the issues that
> > > caused problems with uninitialized hardware depending on
> > > the bootloader version. Mostly things got fixed with
> > > the following commits:
> > > 
> > > 9a894953a97b ("ARM: dts: Fix bootloader version
> > > dependencies by muxing n900 smc91x pins") 7d2911c43815
> > > ("net: smc91x: Fix gpios for device tree based booting")
> > > 
> > > Note that this only affects the early development boards
> > > with Ethernet that we still have in a few automated boot
> > > test systems.
> > > 
> > > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > > 
> > > --- a/arch/arm/boot/dts/omap3-n900.dts
> > > +++ b/arch/arm/boot/dts/omap3-n900.dts
> > > @@ -702,9 +702,6 @@
> > > 
> > >  	ethernet@gpmc {
> > >  	
> > >  		compatible = "smsc,lan91c94";
> > > 
> > > -
> > > -		status = "disabled";
> > > -
> > > 
> > >  		interrupt-parent = <&gpio2>;
> > >  		interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;	/* gpio54 
*/
> > >  		reg = <1 0x300 0xf>;		/* 16 byte IO range at 
offset
> > >  		0x300
> > > 
> > > */
> > 
> > Hello Tony,
> > 
> > to make smc ethernet working in n900 qemu I needed to apply
> > this patch:
> > 
> > diff --git a/arch/arm/boot/dts/omap3-n900.dts
> > b/arch/arm/boot/dts/omap3-n900.dts index ff36fbe..d96eeb8
> > 100644
> > --- a/arch/arm/boot/dts/omap3-n900.dts
> > +++ b/arch/arm/boot/dts/omap3-n900.dts
> > @@ -770,7 +770,8 @@
> > 
> >  		compatible = "smsc,lan91c94";
> >  		interrupt-parent = <&gpio2>;
> >  		interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;	/* gpio54 */
> > 
> > -		reg = <1 0x300 0xf>;		/* 16 byte IO range at offset 
0x300
> > */ +//		reg = <1 0x300 0xf>;		/* 16 byte IO range at 
offset
> > 0x300 */ +		reg = <1 0x0 0xf>;		/* 16 byte IO range 
at
> > offset 0x300 */
> > 
> >  		bank-width = <2>;
> >  		pinctrl-names = "default";
> >  		pinctrl-0 = <&ethernet_pins>;
> 
> Oh cool, the 0x300 offset is there mostly to suppress warnings
> about non-standard location.
> 
> > With this patch I see in dmesg:
> > 
> > [   20.577911] smc91x 2000000.ethernet (unnamed net_device)
> > (uninitialized): smc91x: smc_probe [   20.580535] smc91x
> > 2000000.ethernet (unnamed net_device) (uninitialized):
> > smc91x: bank signature probe returned 0x3300
> > [   20.585327] smc91x 2000000.ethernet (unnamed net_device)
> > (uninitialized): smc91x: revision = 0x3391 [   20.590087]
> > smc91x.c: v1.1, sep 22 2004 by Nicolas Pitre
> > <nico@fluxnic.net> [   20.593627] smc91x 2000000.ethernet
> > (unnamed net_device) (uninitialized): smc_reset [  
> > 20.596832] smc91x 2000000.ethernet (unnamed net_device)
> > (uninitialized): smc_phy_detect [   20.611938] smc91x
> > 2000000.ethernet (unnamed net_device) (uninitialized):
> > smc91x: smc_shutdown [   20.615875] smc91x 2000000.ethernet
> > eth0: SMC91C11xFD (rev 1) at d08be000 IRQ 166 [  
> > 20.618682]
> > [   20.621124] smc91x 2000000.ethernet eth0: Ethernet addr:
> > 52:54:00:12:34:56 [   20.624938] smc91x 2000000.ethernet
> > eth0: No PHY found
> > 
> > (and eth0 exists in ifconfig)
> > 
> > If I do not apply my patch I got this error message:
> > 
> > [   22.134704] smc91x 2000300.ethernet (unnamed net_device)
> > (uninitialized): smc91x: bank signature probe returned
> > 0x0000
> > [   22.140014] smc91x: not found (-19).
> > 
> > and no ethernet device was registered.
> > 
> > With 2.6.28 kernel with N900 patches (but smc91x is
> > unmodified!) ethernet device is working fine.
> 
> OK that's good news. Care to do a patch to set the offset 0x0
> with added comment that qemu needs it? I'll test to make sure
> it works on the real hardware as well.
> 
> Regards,
> 
> Tony

Yes, I can send proper git format-patch, but first let me know if 
that change does not break your HW...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] ARM: dts: Revert disabling of smc91x for n900

[Tony Lindgren]

* Pali Rohár <pali.rohar@gmail.com> [150218 11:07]:
> On Wednesday 18 February 2015 17:33:53 Tony Lindgren wrote:
> > > */ +//		reg = <1 0x300 0xf>;		/* 16 byte IO range at 
> offset
> > > 0x300 */ +		reg = <1 0x0 0xf>;		/* 16 byte IO range 
> at
> > > offset 0x300 */
> > > 
> > >  		bank-width = <2>;
> > >  		pinctrl-names = "default";
> > >  		pinctrl-0 = <&ethernet_pins>;
> > 
> > Oh cool, the 0x300 offset is there mostly to suppress warnings
> > about non-standard location.
...

> > OK that's good news. Care to do a patch to set the offset 0x0
> > with added comment that qemu needs it? I'll test to make sure
> > it works on the real hardware as well.
> 
> Yes, I can send proper git format-patch, but first let me know if 
> that change does not break your HW...

Yes using reg = <1 0 0xf> works, it just adds this extra warning:

smc91x 2000000.ethernet (unnamed net_device) (uninitialized):
smc91x: IOADDR d09d6000 doesn't match configuration (300).

And I'm pretty sure that can be fixed by setting the EEPROM
offset to 0 instead of the default 0x300. People with smc91x
most likely want to write at least the MAC address to the
EEPROM, so might as well set the offset to zero then too.

Of course it's always possible to do do a omap3-n900-qemu.dts
if larger changes are needed :)

Regards,

Tony

Re: [PATCH] ARM: dts: Revert disabling of smc91x for n900

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1781 bytes --]

On Wednesday 18 February 2015 23:42:06 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar@gmail.com> [150218 11:07]:
> > On Wednesday 18 February 2015 17:33:53 Tony Lindgren wrote:
> > > > */ +//		reg = <1 0x300 0xf>;		/* 16 byte IO range 
at
> > 
> > offset
> > 
> > > > 0x300 */ +		reg = <1 0x0 0xf>;		/* 16 byte IO 
range
> > 
> > at
> > 
> > > > offset 0x300 */
> > > > 
> > > >  		bank-width = <2>;
> > > >  		pinctrl-names = "default";
> > > >  		pinctrl-0 = <&ethernet_pins>;
> > > 
> > > Oh cool, the 0x300 offset is there mostly to suppress
> > > warnings about non-standard location.
> 
> ...
> 
> > > OK that's good news. Care to do a patch to set the offset
> > > 0x0 with added comment that qemu needs it? I'll test to
> > > make sure it works on the real hardware as well.
> > 
> > Yes, I can send proper git format-patch, but first let me
> > know if that change does not break your HW...
> 
> Yes using reg = <1 0 0xf> works, it just adds this extra
> warning:
> 
> smc91x 2000000.ethernet (unnamed net_device) (uninitialized):
> smc91x: IOADDR d09d6000 doesn't match configuration (300).
> 
> And I'm pretty sure that can be fixed by setting the EEPROM
> offset to 0 instead of the default 0x300. People with smc91x
> most likely want to write at least the MAC address to the
> EEPROM, so might as well set the offset to zero then too.
> 
> Of course it's always possible to do do a omap3-n900-qemu.dts
> if larger changes are needed :)
> 
> Regards,
> 
> Tony

I would like to avoid using separate DTS for qemu. When we have 
only one DTS file (for both qemu and real HW), we can test for 
regression in qemu and we are sure that we have same software 
configuration...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] ARM: dts: Revert disabling of smc91x for n900

[Tony Lindgren]

* Pali Rohár <pali.rohar@gmail.com> [150218 15:58]:
> On Wednesday 18 February 2015 23:42:06 Tony Lindgren wrote:
> > Of course it's always possible to do do a omap3-n900-qemu.dts
> > if larger changes are needed :)
>
> I would like to avoid using separate DTS for qemu. When we have 
> only one DTS file (for both qemu and real HW), we can test for 
> regression in qemu and we are sure that we have same software 
> configuration...

Agreed, and I'd rather take the extra warning on rare to find
hardware to avoid an etra .dts file.

Are there any reasons whey the n900 qemu support could not
be in the mainline qemu btw?

Regards,

Tony

Re: [PATCH] ARM: dts: Revert disabling of smc91x for n900

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 894 bytes --]

On Thursday 19 February 2015 05:50:48 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar@gmail.com> [150218 15:58]:
> > On Wednesday 18 February 2015 23:42:06 Tony Lindgren wrote:
> > > Of course it's always possible to do do a
> > > omap3-n900-qemu.dts if larger changes are needed :)
> > 
> > I would like to avoid using separate DTS for qemu. When we
> > have only one DTS file (for both qemu and real HW), we can
> > test for regression in qemu and we are sure that we have
> > same software configuration...
> 
> Agreed, and I'd rather take the extra warning on rare to find
> hardware to avoid an etra .dts file.
> 
> Are there any reasons whey the n900 qemu support could not
> be in the mainline qemu btw?
> 
> Regards,
> 
> Tony

Missing lot of omap stuff (in mainline qemu) which linaro have 
not sent to mainline yet...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] ARM: dts: Revert disabling of smc91x for n900

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1982 bytes --]

On Wednesday 18 February 2015 23:42:06 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar@gmail.com> [150218 11:07]:
> > On Wednesday 18 February 2015 17:33:53 Tony Lindgren wrote:
> > > > */ +//		reg = <1 0x300 0xf>;		/* 16 byte IO range at
> > 
> > offset
> > 
> > > > 0x300 */ +		reg = <1 0x0 0xf>;		/* 16 byte IO range
> > 
> > at
> > 
> > > > offset 0x300 */
> > > > 
> > > >  		bank-width = <2>;
> > > >  		pinctrl-names = "default";
> > > >  		pinctrl-0 = <&ethernet_pins>;
> > > 
> > > Oh cool, the 0x300 offset is there mostly to suppress
> > > warnings about non-standard location.
> 
> ...
> 
> > > OK that's good news. Care to do a patch to set the offset
> > > 0x0 with added comment that qemu needs it? I'll test to
> > > make sure it works on the real hardware as well.
> > 
> > Yes, I can send proper git format-patch, but first let me
> > know if that change does not break your HW...
> 
> Yes using reg = <1 0 0xf> works, it just adds this extra
> warning:
> 
> smc91x 2000000.ethernet (unnamed net_device) (uninitialized):
> smc91x: IOADDR d09d6000 doesn't match configuration (300).
> 
> And I'm pretty sure that can be fixed by setting the EEPROM
> offset to 0 instead of the default 0x300. People with smc91x
> most likely want to write at least the MAC address to the
> EEPROM, so might as well set the offset to zero then too.
> 
> Of course it's always possible to do do a omap3-n900-qemu.dts
> if larger changes are needed :)
> 
> Regards,
> 
> Tony

Anyway, here are original Nokia board data (2.6.28) for smc91x ethernet:

https://gitorious.org/linux-n900/linux-n900/source/629fc5ab00cafb31272c478efa2c2b35fabd4c70:arch/arm/mach-omap2/board-rx51-peripherals.c#L42

https://gitorious.org/linux-n900/linux-n900/source/629fc5ab00cafb31272c478efa2c2b35fabd4c70:arch/arm/mach-omap2/board-rx51-peripherals.c#L274

Can you check if it match with our data in DT file?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]