[U-Boot] [PATCH] ppc4xx: Don't fixup NOR flash node size on Ebony

[U-Boot] [PATCH] ppc4xx: Don't fixup NOR flash node size on Ebony

[Stefan Roese]

Don't update size in "reg" property of NOR FLASH device tree nodes.
This leads to a problem with this NOR flash setup, where
chip-select 0 is connected to 512KiB SRAM *and* 512KiB NOR flash.
And with this fixup-code, the size of the NOR flash node is changed
from 512KiB to 1MiB which leads to a crash in the Linux MTD driver.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
---
 include/configs/ebony.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/configs/ebony.h b/include/configs/ebony.h
index 10f425d..eb58240 100644
--- a/include/configs/ebony.h
+++ b/include/configs/ebony.h
@@ -117,6 +117,14 @@
 #define CONFIG_SYS_FLASH_ADDR0         0x5555
 #define CONFIG_SYS_FLASH_ADDR1         0x2aaa
 #define CONFIG_SYS_FLASH_WORD_SIZE     unsigned char
+/*
+ * Don't update size in "reg" property of NOR FLASH device tree nodes.
+ * This leads to a problem with this NOR flash setup, where
+ * chip-select 0 is connected to 512KiB SRAM *and* 512KiB NOR flash.
+ * And with this fixup-code, the size of the NOR flash node is changed
+ * from 512KiB to 1MiB which leads to a crash in the Linux MTD driver.
+ */
+#undef CONFIG_FDT_FIXUP_NOR_FLASH_SIZE
 
 #ifdef CONFIG_ENV_IS_IN_FLASH
 #define CONFIG_ENV_SECT_SIZE	0x10000	/* size of one complete sector		*/
-- 
1.7.2.3

[U-Boot] [PATCH] ppc4xx: Don't fixup NOR flash node size on Ebony

[Wolfgang Denk]

Dear Stefan Roese,

In message <1284555484-27269-1-git-send-email-sr@denx.de> you wrote:
> Don't update size in "reg" property of NOR FLASH device tree nodes.
> This leads to a problem with this NOR flash setup, where
> chip-select 0 is connected to 512KiB SRAM *and* 512KiB NOR flash.
> And with this fixup-code, the size of the NOR flash node is changed
> from 512KiB to 1MiB which leads to a crash in the Linux MTD driver.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> ---
>  include/configs/ebony.h |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/include/configs/ebony.h b/include/configs/ebony.h
> index 10f425d..eb58240 100644
> --- a/include/configs/ebony.h
> +++ b/include/configs/ebony.h
> @@ -117,6 +117,14 @@
>  #define CONFIG_SYS_FLASH_ADDR0         0x5555
>  #define CONFIG_SYS_FLASH_ADDR1         0x2aaa
>  #define CONFIG_SYS_FLASH_WORD_SIZE     unsigned char
> +/*
> + * Don't update size in "reg" property of NOR FLASH device tree nodes.
> + * This leads to a problem with this NOR flash setup, where
> + * chip-select 0 is connected to 512KiB SRAM *and* 512KiB NOR flash.
> + * And with this fixup-code, the size of the NOR flash node is changed
> + * from 512KiB to 1MiB which leads to a crash in the Linux MTD driver.
> + */
> +#undef CONFIG_FDT_FIXUP_NOR_FLASH_SIZE

Sorry, but NAK.


Even if connected to the same chip select, NOR flash and SRAM are
different devices, which are described by different properties in the
device tree and which have separate addresses and sizes.

The "reg" property of NOR FLASH device tree nodes *should* be updated
with the correct size of the NOR flash as determined by the CFI
driver; on this board, this should be 512 KiB.

Both the bd_info command and the device tree should report a NOR flash
size of 512 KiB.

The SRAM size is completely independent of that. As we don't provide
auto-sizing for the sram, the board config file should contain (read:
needs to get fixed to contain) the correct CONFIG_SYS_SRAM_BASE and
CONFIG_SYS_SRAM_SIZE settings (with CONFIG_SYS_SRAM_SIZE = 512 KiB).

Both the bd_info command and the device tree should report a SRAM size
of 512 KiB.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Any sufficiently advanced technology is indistinguishable from magic.
                                                   - Arthur C. Clarke

[U-Boot] [PATCH] ppc4xx: Don't fixup NOR flash node size on Ebony

[Stefan Roese]

Hi Wolfgang,

On Wednesday 15 September 2010 15:14:03 Wolfgang Denk wrote:
> > +/*
> > + * Don't update size in "reg" property of NOR FLASH device tree nodes.
> > + * This leads to a problem with this NOR flash setup, where
> > + * chip-select 0 is connected to 512KiB SRAM *and* 512KiB NOR flash.
> > + * And with this fixup-code, the size of the NOR flash node is changed
> > + * from 512KiB to 1MiB which leads to a crash in the Linux MTD driver.
> > + */
> > +#undef CONFIG_FDT_FIXUP_NOR_FLASH_SIZE
> 
> Sorry, but NAK.
> 
> 
> Even if connected to the same chip select, NOR flash and SRAM are
> different devices, which are described by different properties in the
> device tree and which have separate addresses and sizes.

Correct.
 
> The "reg" property of NOR FLASH device tree nodes *should* be updated
> with the correct size of the NOR flash as determined by the CFI
> driver; on this board, this should be 512 KiB.

This value of 512KiB is already present in the NOR flash device tree node.
 
> Both the bd_info command and the device tree should report a NOR flash
> size of 512 KiB.

bdinfo shows 4.5MiB of flashsize, since Ebony has additional flash devices at 
different chip-selects. There are additional device tree nodes to describe 
these NOR flash devices as well. The total of all NOR flash device tree nodes 
will be 4.5MiB as well, using this patch. With out this patch it is 5MiB.

> The SRAM size is completely independent of that. As we don't provide
> auto-sizing for the sram, the board config file should contain (read:
> needs to get fixed to contain) the correct CONFIG_SYS_SRAM_BASE and
> CONFIG_SYS_SRAM_SIZE settings (with CONFIG_SYS_SRAM_SIZE = 512 KiB).
> 
> Both the bd_info command and the device tree should report a SRAM size
> of 512 KiB.

My patch doesn't touch any SRAM lated stuff. It fixes a real issue, where 
Ebony currently crashes upon Linux booting trying to map/detect NOR flash 
devices at an inaccessible memory region.

Perhaps I described the change best. I'll try again:

This patch disables the PPC4xx specific device-tree-blob fixup code, which is 
enabled per default for all AMCC/APM eval boards, that writes the chip-select 
size into the size of the "reg" property of the NOR flash device tree nodes. 
This fixup code is intended for boards with multiple flash configurations, 
passing the detected flash size in the device tree node to the Linux kernel. 
On Ebony this code doesn't work correctly, since it has an 512KiB SRAM and an 
512KiB NOR flash connected to one chip-select. This code updates incorrectly 
1MiB of chip-select size into the NOR flash device tree node.

I hope this is clearer now.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

[U-Boot] [PATCH] ppc4xx: Don't fixup NOR flash node size on Ebony

[Wolfgang Denk]

Dear Stefan Roese,

In message <201009151543.02284.sr@denx.de> you wrote:
> 
> > The "reg" property of NOR FLASH device tree nodes *should* be updated
> > with the correct size of the NOR flash as determined by the CFI
> > driver; on this board, this should be 512 KiB.
> 
> This value of 512KiB is already present in the NOR flash device tree node.

It should be updated in any case using the values as determined by the
CFI flash driver.

> > Both the bd_info command and the device tree should report a NOR flash
> > size of 512 KiB.
> 
> bdinfo shows 4.5MiB of flashsize, since Ebony has additional flash devices at 

OK.

> different chip-selects. There are additional device tree nodes to describe 
> these NOR flash devices as well. The total of all NOR flash device tree nodes 
> will be 4.5MiB as well, using this patch. With out this patch it is 5MiB.

Then there is a bug somewhere, because you seem to add 4 + 1 MiB
flash sizes when the real sizes are only 4 MiB + 512 KiB.

> > The SRAM size is completely independent of that. As we don't provide
> > auto-sizing for the sram, the board config file should contain (read:
> > needs to get fixed to contain) the correct CONFIG_SYS_SRAM_BASE and
> > CONFIG_SYS_SRAM_SIZE settings (with CONFIG_SYS_SRAM_SIZE = 512 KiB).
> > 
> > Both the bd_info command and the device tree should report a SRAM size
> > of 512 KiB.
> 
> My patch doesn't touch any SRAM lated stuff. It fixes a real issue, where 
> Ebony currently crashes upon Linux booting trying to map/detect NOR flash 
> devices at an inaccessible memory region.

I wanted to point out that the SRAM settings are missing and need
fixing, too.

> This patch disables the PPC4xx specific device-tree-blob fixup code, which is 
> enabled per default for all AMCC/APM eval boards, that writes the chip-select 
> size into the size of the "reg" property of the NOR flash device tree nodes. 

Ah!  That's the bug, then. You must not write any map size, but only
the real flash device size as determined by the CFI driver.

> This fixup code is intended for boards with multiple flash configurations, 
> passing the detected flash size in the device tree node to the Linux kernel. 
> On Ebony this code doesn't work correctly, since it has an 512KiB SRAM and an 
> 512KiB NOR flash connected to one chip-select. This code updates incorrectly 
> 1MiB of chip-select size into the NOR flash device tree node.

It does not work correctly because you are incorrectly using the size
of the chip select mapping when you should be using the flash device
size instead.  Please fix this problem.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Nearly everyone is in favor of going  to  heaven  but  too  many  are
hoping  they'll  live  long  enough  to see an easing of the entrance
requirements. Never appeal to a man's "better nature." he  might  not
have one.