linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree
@ 2014-04-22  0:54 Tony Lindgren
  2014-04-22  0:54 ` [PATCH 1/2] ARM: OMAP2+: Fix oops for GPMC free Tony Lindgren
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Tony Lindgren @ 2014-04-22  0:54 UTC (permalink / raw)
  To: linux-arm-kernel

Hi all,

Here are two fixes to GPMC issues I've seen. It seems that we have
few more issues left to solve:

1. The remap of a device with gpmc_cs_remap seems to fail for
   a device if it's address specified in the .dts file is
   different from the address used by the bootloader

2. There seems to be some timing issues with smc911x where
   rsync of larger files and apt-get dist-upgrade can produce 
   strange errors. This seems to work reliably when booted in
   legacy mode.

3. The DT mappings of GPMC devices are wrong for most devices
   where the ranges property should contain the GPMC partition
   size (16, 32, 128 or 256 MB), and the reg property for the
   device should only contain the device IO range. So only NOR
   should use large values for ranges and IO range, the rest
   should use the minimum 16 MB range for GPMC partition, and
   0x4 - 0x20000 for the IO range. And while patching these
   it makes sense to also unify the GPMC ranges being used for
   devices.

Pekon, can you take a look at 1 and 2 above?

Then for 3 above, it seems that we cannot safely change
ranges before 1 and 2 are working reliably. Or else we have
patch things twice, once to fix the range sizes, then to
unify the mappings for the range address...

Regards,

Tony

Tony Lindgren (2):
  ARM: OMAP2+: Fix oops for GPMC free
  ARM: OMAP2+: Fix GPMC remap for devices using an offset

 arch/arm/mach-omap2/gpmc.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

-- 
1.8.1.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] ARM: OMAP2+: Fix oops for GPMC free
  2014-04-22  0:54 [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree Tony Lindgren
@ 2014-04-22  0:54 ` Tony Lindgren
  2014-04-22  0:54 ` [PATCH 2/2] ARM: OMAP2+: Fix GPMC remap for devices using an offset Tony Lindgren
  2014-04-22  6:55 ` [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree Javier Martinez Canillas
  2 siblings, 0 replies; 8+ messages in thread
From: Tony Lindgren @ 2014-04-22  0:54 UTC (permalink / raw)
  To: linux-arm-kernel

If gpmc_cs_remap() fails we will get an error because we are calling
release_resource() on an uninitialized resource. Let's fix that by
checking the resource flags. And while at it, let's also make
gpmc_cs_delete_mem() use the res pointer that we already have to
avoid confusion.

Without this patch we can get the following error:

omap-gpmc 6e000000.gpmc: cannot remap GPMC CS 1 to 0x01000300
Unable to handle kernel NULL pointer dereference at virtual address 00000018
...
(gpmc_cs_free+0x94/0xc8)
(gpmc_probe_generic_child+0x178/0x1ec)
(gpmc_probe_dt+0x1bc/0x2cc)
(gpmc_probe+0x250/0x44c)
(platform_drv_probe+0x3c/0x6c)
(really_probe+0x74/0x208)
(driver_probe_device+0x34/0x50)
(bus_for_each_drv+0x60/0x8c)
(device_attach+0x80/0xa4)
(bus_probe_device+0x88/0xb0)
(device_add+0x320/0x450)
(of_platform_device_create_pdata+0x80/0x9c)
(of_platform_bus_create+0xd0/0x170)
(of_platform_bus_create+0x12c/0x170)
(of_platform_populate+0x60/0x98)
(pdata_quirks_init+0x30/0x48)
(customize_machine+0x20/0x48)
(do_one_initcall+0x2c/0x14c)
(do_basic_setup+0x98/0xd8)
(kernel_init_freeable+0x12c/0x1e0)
(kernel_init+0x8/0xf0)
(ret_from_fork+0x14/0x2c)
Code: e1a04000 e59f0070 eb195136 e5942010 (e5923018)

Cc: Pekon Gupta <pekon@ti.com>
Signed-off-by: tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/gpmc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index ab43755..84e57e6 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -501,7 +501,7 @@ static int gpmc_cs_delete_mem(int cs)
 	int r;
 
 	spin_lock(&gpmc_mem_lock);
-	r = release_resource(&gpmc_cs_mem[cs]);
+	r = release_resource(res);
 	res->start = 0;
 	res->end = 0;
 	spin_unlock(&gpmc_mem_lock);
@@ -586,6 +586,8 @@ EXPORT_SYMBOL(gpmc_cs_request);
 
 void gpmc_cs_free(int cs)
 {
+	struct resource	*res = &gpmc_cs_mem[cs];
+
 	spin_lock(&gpmc_mem_lock);
 	if (cs >= gpmc_cs_num || cs < 0 || !gpmc_cs_reserved(cs)) {
 		printk(KERN_ERR "Trying to free non-reserved GPMC CS%d\n", cs);
@@ -594,7 +596,8 @@ void gpmc_cs_free(int cs)
 		return;
 	}
 	gpmc_cs_disable_mem(cs);
-	release_resource(&gpmc_cs_mem[cs]);
+	if (res->flags)
+		release_resource(res);
 	gpmc_cs_set_reserved(cs, 0);
 	spin_unlock(&gpmc_mem_lock);
 }
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] ARM: OMAP2+: Fix GPMC remap for devices using an offset
  2014-04-22  0:54 [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree Tony Lindgren
  2014-04-22  0:54 ` [PATCH 1/2] ARM: OMAP2+: Fix oops for GPMC free Tony Lindgren
@ 2014-04-22  0:54 ` Tony Lindgren
  2014-04-22  6:55 ` [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree Javier Martinez Canillas
  2 siblings, 0 replies; 8+ messages in thread
From: Tony Lindgren @ 2014-04-22  0:54 UTC (permalink / raw)
  To: linux-arm-kernel

At least the smc91x driver expects the device to be at 0x300
offset from bus base address. This does not work currently
for GPMC when booted in device tree mode as it attempts to
remap the the allocated GPMC partition to the address
configured by the device tree plus the device offset.

Note that this works just fine when booted with legacy mode.

Let's fix the issue by just ignoring any device specific
offset while remapping. And let's make sure the remap
address confirms to the GPMC 16MB minimum granularity
as listed in the TRM for GPMC_CONFIG7 BASEADDRESS bits.

Otherwise we can get something like this:

omap-gpmc 6e000000.gpmc: cannot remap GPMC CS 1 to 0x01000300

Cc: Pekon Gupta <pekon@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/gpmc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 84e57e6..9fe8c94 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -527,6 +527,14 @@ static int gpmc_cs_remap(int cs, u32 base)
 		pr_err("%s: requested chip-select is disabled\n", __func__);
 		return -ENODEV;
 	}
+
+	/*
+	 * Make sure we ignore any device offsets from the GPMC partition
+	 * allocated for the chip select and that the new base confirms
+	 * to the GPMC 16MB minimum granularity.
+	 */ 
+	base &= ~(SZ_16M - 1);
+
 	gpmc_cs_get_memconf(cs, &old_base, &size);
 	if (base == old_base)
 		return 0;
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree
  2014-04-22  0:54 [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree Tony Lindgren
  2014-04-22  0:54 ` [PATCH 1/2] ARM: OMAP2+: Fix oops for GPMC free Tony Lindgren
  2014-04-22  0:54 ` [PATCH 2/2] ARM: OMAP2+: Fix GPMC remap for devices using an offset Tony Lindgren
@ 2014-04-22  6:55 ` Javier Martinez Canillas
  2014-04-22 15:23   ` Tony Lindgren
  2 siblings, 1 reply; 8+ messages in thread
From: Javier Martinez Canillas @ 2014-04-22  6:55 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Tony,

On Tue, Apr 22, 2014 at 2:54 AM, Tony Lindgren <tony@atomide.com> wrote:
> Hi all,
>
> Here are two fixes to GPMC issues I've seen. It seems that we have
> few more issues left to solve:
>
> 1. The remap of a device with gpmc_cs_remap seems to fail for
>    a device if it's address specified in the .dts file is
>    different from the address used by the bootloader
>
> 2. There seems to be some timing issues with smc911x where
>    rsync of larger files and apt-get dist-upgrade can produce
>    strange errors. This seems to work reliably when booted in
>    legacy mode.
>

In what board are you having this issue? The smsc911x driver supports
both SMSC's LAN911x and LAN921x families and I see that we have two
.dtsi files with different timings
(arm/boot/dts/omap-gpmc-smsc{911x,9221}.dtsi).

This is only a wild guess, but maybe your board has a smsc LAN921x
chip but is including omap-gpmc-smsc911x.dtsi on its DTS?

> 3. The DT mappings of GPMC devices are wrong for most devices
>    where the ranges property should contain the GPMC partition
>    size (16, 32, 128 or 256 MB), and the reg property for the
>    device should only contain the device IO range. So only NOR
>    should use large values for ranges and IO range, the rest
>    should use the minimum 16 MB range for GPMC partition, and
>    0x4 - 0x20000 for the IO range. And while patching these
>    it makes sense to also unify the GPMC ranges being used for
>    devices.
>
> Pekon, can you take a look at 1 and 2 above?
>
> Then for 3 above, it seems that we cannot safely change
> ranges before 1 and 2 are working reliably. Or else we have
> patch things twice, once to fix the range sizes, then to
> unify the mappings for the range address...
>
> Regards,
>
> Tony
>
> Tony Lindgren (2):
>   ARM: OMAP2+: Fix oops for GPMC free
>   ARM: OMAP2+: Fix GPMC remap for devices using an offset
>

These fixes look good to me.

Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org>

>  arch/arm/mach-omap2/gpmc.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> --
> 1.8.1.1
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree
  2014-04-22  6:55 ` [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree Javier Martinez Canillas
@ 2014-04-22 15:23   ` Tony Lindgren
  2014-04-23  0:00     ` Tony Lindgren
  0 siblings, 1 reply; 8+ messages in thread
From: Tony Lindgren @ 2014-04-22 15:23 UTC (permalink / raw)
  To: linux-arm-kernel

* Javier Martinez Canillas <javier@dowhile0.org> [140421 23:55]:
> On Tue, Apr 22, 2014 at 2:54 AM, Tony Lindgren <tony@atomide.com> wrote:
> >
> > 2. There seems to be some timing issues with smc911x where
> >    rsync of larger files and apt-get dist-upgrade can produce
> >    strange errors. This seems to work reliably when booted in
> >    legacy mode.
> >
> 
> In what board are you having this issue? The smsc911x driver supports
> both SMSC's LAN911x and LAN921x families and I see that we have two
> .dtsi files with different timings
> (arm/boot/dts/omap-gpmc-smsc{911x,9221}.dtsi).
> 
> This is only a wild guess, but maybe your board has a smsc LAN921x
> chip but is including omap-gpmc-smsc911x.dtsi on its DTS?

Yes it seems to have two LAN9220s, so this could be the reason.
I don't think we had the omap-gpmc-smsc9221.dtsi when I added the
timings initially.

This is on a sbc-t3730 that I'm using as a gateway that was behaving
reliably before I upgraded it to DT based booting. It's currently
at v3.13-rc3 something, but I don't think we've much GPMC changes
since then.

I'll try upgrading the kernel today and running some tests with
rsync. Looks like we can also remove quite a bit of duplicate
timing data by using omap-gpmc-smsc9221.dtsi, I'll try something
like the patch below.

In any case, I suggest others run some tests on their GPMC Ethernet
too.

Regards,

Tony

8< ----------------------
--- a/arch/arm/boot/dts/omap3-cm-t3x30.dtsi
+++ b/arch/arm/boot/dts/omap3-cm-t3x30.dtsi
@@ -10,18 +10,6 @@
 			cpu0-supply = <&vcc>;
 		};
 	};
-
-	vddvario: regulator-vddvario {
-		compatible = "regulator-fixed";
-		regulator-name = "vddvario";
-		regulator-always-on;
-	};
-
-	vdd33a: regulator-vdd33a {
-		compatible = "regulator-fixed";
-		regulator-name = "vdd33a";
-		regulator-always-on;
-	};
 };
 
 &omap3_pmx_core {
@@ -51,42 +39,18 @@
 	};
 };
 
+#include "omap-gpmc-smsc9221.dtsi"
+
 &gpmc {
 	ranges = <5 0 0x2c000000 0x01000000>;
 
-	smsc1: ethernet at 5,0 {
+	smsc1: ethernet at gpmc {
 		compatible = "smsc,lan9221", "smsc,lan9115";
 		pinctrl-names = "default";
 		pinctrl-0 = <&smsc1_pins>;
 		interrupt-parent = <&gpio6>;
 		interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
 		reg = <5 0 0xff>;
-		bank-width = <2>;
-		gpmc,mux-add-data;
-		gpmc,cs-on-ns = <0>;
-		gpmc,cs-rd-off-ns = <186>;
-		gpmc,cs-wr-off-ns = <186>;
-		gpmc,adv-on-ns = <12>;
-		gpmc,adv-rd-off-ns = <48>;
-		gpmc,adv-wr-off-ns = <48>;
-		gpmc,oe-on-ns = <54>;
-		gpmc,oe-off-ns = <168>;
-		gpmc,we-on-ns = <54>;
-		gpmc,we-off-ns = <168>;
-		gpmc,rd-cycle-ns = <186>;
-		gpmc,wr-cycle-ns = <186>;
-		gpmc,access-ns = <114>;
-		gpmc,page-burst-access-ns = <6>;
-		gpmc,bus-turnaround-ns = <12>;
-		gpmc,cycle2cycle-delay-ns = <18>;
-		gpmc,wr-data-mux-bus-ns = <90>;
-		gpmc,wr-access-ns = <186>;
-		gpmc,cycle2cycle-samecsen;
-		gpmc,cycle2cycle-diffcsen;
-		vddvario-supply = <&vddvario>;
-		vdd33a-supply = <&vdd33a>;
-		reg-io-width = <4>;
-		smsc,save-mac-address;
 	};
 };
 
--- a/arch/arm/boot/dts/omap3-sb-t35.dtsi
+++ b/arch/arm/boot/dts/omap3-sb-t35.dtsi
@@ -2,20 +2,6 @@
  * Common support for CompuLab SB-T35 used on SBC-T3530, SBC-T3517 and SBC-T3730
  */
 
-/ {
-	vddvario_sb_t35: regulator-vddvario-sb-t35 {
-		compatible = "regulator-fixed";
-		regulator-name = "vddvario";
-		regulator-always-on;
-	};
-
-	vdd33a_sb_t35: regulator-vdd33a-sb-t35 {
-		compatible = "regulator-fixed";
-		regulator-name = "vdd33a";
-		regulator-always-on;
-	};
-};
-
 &omap3_pmx_core {
 	smsc2_pins: pinmux_smsc2_pins {
 		pinctrl-single,pins = <
@@ -38,27 +24,28 @@
 		bank-width = <2>;
 		gpmc,mux-add-data;
 		gpmc,cs-on-ns = <0>;
-		gpmc,cs-rd-off-ns = <186>;
-		gpmc,cs-wr-off-ns = <186>;
-		gpmc,adv-on-ns = <12>;
-		gpmc,adv-rd-off-ns = <48>;
-		gpmc,adv-wr-off-ns = <48>;
-		gpmc,oe-on-ns = <54>;
-		gpmc,oe-off-ns = <168>;
-		gpmc,we-on-ns = <54>;
-		gpmc,we-off-ns = <168>;
-		gpmc,rd-cycle-ns = <186>;
-		gpmc,wr-cycle-ns = <186>;
-		gpmc,access-ns = <114>;
-		gpmc,page-burst-access-ns = <6>;
-		gpmc,bus-turnaround-ns = <12>;
-		gpmc,cycle2cycle-delay-ns = <18>;
-		gpmc,wr-data-mux-bus-ns = <90>;
-		gpmc,wr-access-ns = <186>;
+		gpmc,cs-rd-off-ns = <42>;
+		gpmc,cs-wr-off-ns = <36>;
+		gpmc,adv-on-ns = <6>;
+		gpmc,adv-rd-off-ns = <12>;
+		gpmc,adv-wr-off-ns = <12>;
+		gpmc,oe-on-ns = <0>;
+		gpmc,oe-off-ns = <42>;
+		gpmc,we-on-ns = <0>;
+		gpmc,we-off-ns = <36>;
+		gpmc,rd-cycle-ns = <60>;
+		gpmc,wr-cycle-ns = <54>;
+		gpmc,access-ns = <36>;
+		gpmc,page-burst-access-ns = <0>;
+		gpmc,bus-turnaround-ns = <0>;
+		gpmc,cycle2cycle-delay-ns = <0>;
+		gpmc,wr-data-mux-bus-ns = <18>;
+		gpmc,wr-access-ns = <42>;
 		gpmc,cycle2cycle-samecsen;
 		gpmc,cycle2cycle-diffcsen;
-		vddvario-supply = <&vddvario_sb_t35>;
-		vdd33a-supply = <&vdd33a_sb_t35>;
+
+		vddvario-supply = <&vddvario>;
+		vdd33a-supply = <&vdd33a>;
 		reg-io-width = <4>;
 		smsc,save-mac-address;
 	};
--- a/arch/arm/boot/dts/omap3-sbc-t3517.dts
+++ b/arch/arm/boot/dts/omap3-sbc-t3517.dts
@@ -8,6 +8,19 @@
 / {
 	model = "CompuLab SBC-T3517 with CM-T3517";
 	compatible = "compulab,omap3-sbc-t3517", "compulab,omap3-cm-t3517", "ti,am3517", "ti,omap3";
+
+	/* Only one GPMC smsc9220 on SBC-T3517, CM-T3517 uses am35x Ethernet */
+	vddvario: regulator-vddvario-sb-t35 {
+		compatible = "regulator-fixed";
+		regulator-name = "vddvario";
+		regulator-always-on;
+	};
+
+	vdd33a: regulator-vdd33a-sb-t35 {
+		compatible = "regulator-fixed";
+		regulator-name = "vdd33a";
+		regulator-always-on;
+	};
 };
 
 &omap3_pmx_core {

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree
  2014-04-22 15:23   ` Tony Lindgren
@ 2014-04-23  0:00     ` Tony Lindgren
  2014-04-23 18:08       ` Tony Lindgren
  0 siblings, 1 reply; 8+ messages in thread
From: Tony Lindgren @ 2014-04-23  0:00 UTC (permalink / raw)
  To: linux-arm-kernel

* Tony Lindgren <tony@atomide.com> [140422 08:24]:
> * Javier Martinez Canillas <javier@dowhile0.org> [140421 23:55]:
> > On Tue, Apr 22, 2014 at 2:54 AM, Tony Lindgren <tony@atomide.com> wrote:
> > >
> > > 2. There seems to be some timing issues with smc911x where
> > >    rsync of larger files and apt-get dist-upgrade can produce
> > >    strange errors. This seems to work reliably when booted in
> > >    legacy mode.
> > >
> > 
> > In what board are you having this issue? The smsc911x driver supports
> > both SMSC's LAN911x and LAN921x families and I see that we have two
> > .dtsi files with different timings
> > (arm/boot/dts/omap-gpmc-smsc{911x,9221}.dtsi).
> > 
> > This is only a wild guess, but maybe your board has a smsc LAN921x
> > chip but is including omap-gpmc-smsc911x.dtsi on its DTS?
> 
> Yes it seems to have two LAN9220s, so this could be the reason.
> I don't think we had the omap-gpmc-smsc9221.dtsi when I added the
> timings initially.
> 
> This is on a sbc-t3730 that I'm using as a gateway that was behaving
> reliably before I upgraded it to DT based booting. It's currently
> at v3.13-rc3 something, but I don't think we've much GPMC changes
> since then.
> 
> I'll try upgrading the kernel today and running some tests with
> rsync. Looks like we can also remove quite a bit of duplicate
> timing data by using omap-gpmc-smsc9221.dtsi, I'll try something
> like the patch below.
> 
> In any case, I suggest others run some tests on their GPMC Ethernet
> too.
>  
> +#include "omap-gpmc-smsc9221.dtsi"
> +

The 9221 timings won't work at all on 9220, it requires a 9221.
I'll post a better clean-up patch to use the 911x timings.

Upgraded the kernel and the occasional corruption is still
there. I guess I need to test also the same kernel in legacy
mode to try to narrow it down.

Regards,

Tony

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree
  2014-04-23  0:00     ` Tony Lindgren
@ 2014-04-23 18:08       ` Tony Lindgren
  2014-04-23 18:42         ` Javier Martinez Canillas
  0 siblings, 1 reply; 8+ messages in thread
From: Tony Lindgren @ 2014-04-23 18:08 UTC (permalink / raw)
  To: linux-arm-kernel

* Tony Lindgren <tony@atomide.com> [140422 17:01]:
> * Tony Lindgren <tony@atomide.com> [140422 08:24]:
> > * Javier Martinez Canillas <javier@dowhile0.org> [140421 23:55]:
> > > On Tue, Apr 22, 2014 at 2:54 AM, Tony Lindgren <tony@atomide.com> wrote:
> > > >
> > > > 2. There seems to be some timing issues with smc911x where
> > > >    rsync of larger files and apt-get dist-upgrade can produce
> > > >    strange errors. This seems to work reliably when booted in
> > > >    legacy mode.
> > > >
> > > 
> > > In what board are you having this issue? The smsc911x driver supports
> > > both SMSC's LAN911x and LAN921x families and I see that we have two
> > > .dtsi files with different timings
> > > (arm/boot/dts/omap-gpmc-smsc{911x,9221}.dtsi).
> > > 
> > > This is only a wild guess, but maybe your board has a smsc LAN921x
> > > chip but is including omap-gpmc-smsc911x.dtsi on its DTS?
> > 
> > Yes it seems to have two LAN9220s, so this could be the reason.
> > I don't think we had the omap-gpmc-smsc9221.dtsi when I added the
> > timings initially.
> > 
> > This is on a sbc-t3730 that I'm using as a gateway that was behaving
> > reliably before I upgraded it to DT based booting. It's currently
> > at v3.13-rc3 something, but I don't think we've much GPMC changes
> > since then.
> > 
> > I'll try upgrading the kernel today and running some tests with
> > rsync. Looks like we can also remove quite a bit of duplicate
> > timing data by using omap-gpmc-smsc9221.dtsi, I'll try something
> > like the patch below.
> > 
> > In any case, I suggest others run some tests on their GPMC Ethernet
> > too.
> >  
> > +#include "omap-gpmc-smsc9221.dtsi"
> > +
> 
> The 9221 timings won't work at all on 9220, it requires a 9221.
> I'll post a better clean-up patch to use the 911x timings.
> 
> Upgraded the kernel and the occasional corruption is still
> there. I guess I need to test also the same kernel in legacy
> mode to try to narrow it down.

OK hopefully got it figured out now. For legacy booting we were
always using just the bootloader timings. With device tree, we
started using the timings with your commit d72b4415 (ARM: dts:
omap3-igep0020: Add SMSC911x LAN chip support) that was probably
actually tested on a LAN9221 instead of LAN9220?

In any case, I've patched omap-gpmc-smsc911x.dtsi so the values
match the u-boot values, and so far that seems to be working just
fine. Will post few patches shortly.

Regards,

Tony

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree
  2014-04-23 18:08       ` Tony Lindgren
@ 2014-04-23 18:42         ` Javier Martinez Canillas
  0 siblings, 0 replies; 8+ messages in thread
From: Javier Martinez Canillas @ 2014-04-23 18:42 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Tony,

On Wed, Apr 23, 2014 at 8:08 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Tony Lindgren <tony@atomide.com> [140422 17:01]:
>> * Tony Lindgren <tony@atomide.com> [140422 08:24]:
>> > * Javier Martinez Canillas <javier@dowhile0.org> [140421 23:55]:
>> > > On Tue, Apr 22, 2014 at 2:54 AM, Tony Lindgren <tony@atomide.com> wrote:
>> > > >
>> > > > 2. There seems to be some timing issues with smc911x where
>> > > >    rsync of larger files and apt-get dist-upgrade can produce
>> > > >    strange errors. This seems to work reliably when booted in
>> > > >    legacy mode.
>> > > >
>> > >
>> > > In what board are you having this issue? The smsc911x driver supports
>> > > both SMSC's LAN911x and LAN921x families and I see that we have two
>> > > .dtsi files with different timings
>> > > (arm/boot/dts/omap-gpmc-smsc{911x,9221}.dtsi).
>> > >
>> > > This is only a wild guess, but maybe your board has a smsc LAN921x
>> > > chip but is including omap-gpmc-smsc911x.dtsi on its DTS?
>> >
>> > Yes it seems to have two LAN9220s, so this could be the reason.
>> > I don't think we had the omap-gpmc-smsc9221.dtsi when I added the
>> > timings initially.
>> >
>> > This is on a sbc-t3730 that I'm using as a gateway that was behaving
>> > reliably before I upgraded it to DT based booting. It's currently
>> > at v3.13-rc3 something, but I don't think we've much GPMC changes
>> > since then.
>> >
>> > I'll try upgrading the kernel today and running some tests with
>> > rsync. Looks like we can also remove quite a bit of duplicate
>> > timing data by using omap-gpmc-smsc9221.dtsi, I'll try something
>> > like the patch below.
>> >
>> > In any case, I suggest others run some tests on their GPMC Ethernet
>> > too.
>> >
>> > +#include "omap-gpmc-smsc9221.dtsi"
>> > +
>>
>> The 9221 timings won't work at all on 9220, it requires a 9221.
>> I'll post a better clean-up patch to use the 911x timings.
>>
>> Upgraded the kernel and the occasional corruption is still
>> there. I guess I need to test also the same kernel in legacy
>> mode to try to narrow it down.
>
> OK hopefully got it figured out now. For legacy booting we were
> always using just the bootloader timings. With device tree, we
> started using the timings with your commit d72b4415 (ARM: dts:
> omap3-igep0020: Add SMSC911x LAN chip support) that was probably
> actually tested on a LAN9221 instead of LAN9220?
>

Right, since the same driver (drivers/net/ethernet/smsc/smsc911x.c) is
used for both SMSC families I assumed that the same timings could be
used by both chips.

So I took the timings from IGEP board support in u-boot but then you
did the refactoring and later Florian added another .dtsi for SMSC
9221 in commit aac9aa3 ("ARM: dts: omap: Add common file for
SMSC9221").

I didn't notice about this new .dtsi file until you reported your
issue and the IGEPv2 does indeed have a SMSC LAN9221i ethernet chip so
it is wrongly including omap-gpmc-smsc911x.dtsi and should include
omap-gpmc-smsc9221.dtsi instead.

> In any case, I've patched omap-gpmc-smsc911x.dtsi so the values
> match the u-boot values, and so far that seems to be working just
> fine. Will post few patches shortly.
>

Great, I'll patch the IGEPv2 DTS file too to use
omap-gpmc-smsc9221.dtsi, do some testing and post a patch.

> Regards,
>
> Tony
>

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-04-23 18:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-22  0:54 [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree Tony Lindgren
2014-04-22  0:54 ` [PATCH 1/2] ARM: OMAP2+: Fix oops for GPMC free Tony Lindgren
2014-04-22  0:54 ` [PATCH 2/2] ARM: OMAP2+: Fix GPMC remap for devices using an offset Tony Lindgren
2014-04-22  6:55 ` [PATCH 0/2] Fix few omap gpmc regressions when booted with device tree Javier Martinez Canillas
2014-04-22 15:23   ` Tony Lindgren
2014-04-23  0:00     ` Tony Lindgren
2014-04-23 18:08       ` Tony Lindgren
2014-04-23 18:42         ` Javier Martinez Canillas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).