Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Kyungmin Park]

Hi,

It's should be sent to MTD list. and we also fix the NOR similar ways.
It's already posted but not committed.

Thank you,
Kyungmin Park

On Tue, Jul 1, 2008 at 11:32 PM, Kamat, Nishant <nskamat@ti.com> wrote:
>
> From: Nishant Kamat <nskamat@ti.com>
>
> MTD: OMAP2-NAND: Fix partition reading from board info
>
> The parse_mtd_partitions() function no longer returns
> a negative error in case cmdline is not passed.
> See commit: b0d06afb607
>
> Signed-off-by: Nishant Kamat <nskamat@ti.com>
> ---
>  drivers/mtd/nand/omap2.c |    2 +-
>  1 files changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-omap-ti.ldp/drivers/mtd/nand/omap2.c
> ===================================================================
> --- linux-omap-ti.ldp.orig/drivers/mtd/nand/omap2.c     2008-06-30 22:01:50.000000000 +0530
> +++ linux-omap-ti.ldp/drivers/mtd/nand/omap2.c  2008-06-30 22:03:34.446471469 +0530
> @@ -699,7 +699,7 @@
>        err = parse_mtd_partitions(&info->mtd, part_probes, &info->parts, 0);
>        if (err > 0)
>                add_mtd_partitions(&info->mtd, info->parts, err);
> -       else if (err < 0 && pdata->parts)
> +       else if (pdata->parts)
>                add_mtd_partitions(&info->mtd, pdata->parts, pdata->nr_parts);
>        else
>  #endif
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

RE: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Kamat, Nishant]

Hi,


> Kyungmin Park on July 02, 2008 4:57 AM, wrote:
>
> Hi,
>
> It's should be sent to MTD list. and we also fix the NOR similar ways.
> It's already posted but not committed.


Posting the patch on MTD list. Please merge.

---< Cut here >---

From: Nishant Kamat <nskamat@ti.com>

This patch fixes the omap2 nand driver code that allows
reading of partition data from board info.

The parse_mtd_partitions() function no longer returns a
negative error in case cmdline is not passed.
See commit: b0d06afb607

Signed-off-by: Nishant Kamat <nskamat@ti.com>
---
 drivers/mtd/nand/omap2.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: linux-omap-ti.ldp/drivers/mtd/nand/omap2.c
===================================================================
--- linux-omap-ti.ldp.orig/drivers/mtd/nand/omap2.c     2008-06-30 22:01:50.000000000 +0530
+++ linux-omap-ti.ldp/drivers/mtd/nand/omap2.c  2008-06-30 22:03:34.446471469 +0530
@@ -699,7 +699,7 @@
        err = parse_mtd_partitions(&info->mtd, part_probes, &info->parts, 0);
        if (err > 0)
                add_mtd_partitions(&info->mtd, info->parts, err);
-       else if (err < 0 && pdata->parts)
+       else if (pdata->parts)
                add_mtd_partitions(&info->mtd, pdata->parts, pdata->nr_parts);
        else
 #endif

Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Tony Lindgren]

* Kamat, Nishant <nskamat@ti.com> [080703 14:55]:
> Hi,
> 
> 
> > Kyungmin Park on July 02, 2008 4:57 AM, wrote:
> >
> > Hi,
> >
> > It's should be sent to MTD list. and we also fix the NOR similar ways.
> > It's already posted but not committed.
> 
> 
> Posting the patch on MTD list. Please merge.

Looks like this driver does not exist outside linux-omap tree, care to
send the whole driver to MTD list?

Pushing this fix to linux-omap tree meanwhile.

Tony


> ---< Cut here >---
> 
> From: Nishant Kamat <nskamat@ti.com>
> 
> This patch fixes the omap2 nand driver code that allows
> reading of partition data from board info.
> 
> The parse_mtd_partitions() function no longer returns a
> negative error in case cmdline is not passed.
> See commit: b0d06afb607
> 
> Signed-off-by: Nishant Kamat <nskamat@ti.com>
> ---
>  drivers/mtd/nand/omap2.c |    2 +-
>  1 files changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-omap-ti.ldp/drivers/mtd/nand/omap2.c
> ===================================================================
> --- linux-omap-ti.ldp.orig/drivers/mtd/nand/omap2.c     2008-06-30 22:01:50.000000000 +0530
> +++ linux-omap-ti.ldp/drivers/mtd/nand/omap2.c  2008-06-30 22:03:34.446471469 +0530
> @@ -699,7 +699,7 @@
>         err = parse_mtd_partitions(&info->mtd, part_probes, &info->parts, 0);
>         if (err > 0)
>                 add_mtd_partitions(&info->mtd, info->parts, err);
> -       else if (err < 0 && pdata->parts)
> +       else if (pdata->parts)
>                 add_mtd_partitions(&info->mtd, pdata->parts, pdata->nr_parts);
>         else
>  #endif

Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[David Brownell]

On Monday 04 August 2008, Tony Lindgren wrote:
> Looks like this driver does not exist outside linux-omap tree,
> care to send the whole driver to MTD list?

Unless someone created a big-endian OMAP, the cpu_to_le16()
calls in the read/write buffer calls can be removed ... and
then the read buffer code can use __raw_readsw(...) to get
what's been a significant speedup on some other ARMs.  (The
atmel_nand.c code should be usable as-is.)

If there are big-endian OMAPs, it's still unclear why that
would be needed (maybe the hardware ECC code would need to
become byteorder-aware?) ... little point in byteswapping
on the i/o paths, except maybe to slow things down.

It'd make sense to merge the driver version which handles
both OMAP2 and OMAP3, too.  With beagleboard.org pushing
out those puppies, the omap3 support matters more.  :)

- Dave

Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Lennert Buytenhek]

On Mon, Aug 04, 2008 at 01:02:46PM -0700, David Brownell wrote:

> > Looks like this driver does not exist outside linux-omap tree,
> > care to send the whole driver to MTD list?
> 
> Unless someone created a big-endian OMAP,

Don't they use standard ARM ARM cores, which can all be used in both
LE and BE modes?

Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[David Brownell]

On Monday 04 August 2008, Lennert Buytenhek wrote:
> On Mon, Aug 04, 2008 at 01:02:46PM -0700, David Brownell wrote:
> 
> > > Looks like this driver does not exist outside linux-omap tree,
> > > care to send the whole driver to MTD list?
> > 
> > Unless someone created a big-endian OMAP,
> 
> Don't they use standard ARM ARM cores, which can all be used in both
> LE and BE modes?

And when those cores turn into silicon, the choice is usually
hard-wired.  Regardless, all OMAP cores I've worked with are
set up for LE mode ... and then there's still the point that
byteswapping to/from the NAND chip seems to add no value.

Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Lennert Buytenhek]

On Mon, Aug 04, 2008 at 01:26:38PM -0700, David Brownell wrote:

> > > > Looks like this driver does not exist outside linux-omap tree,
> > > > care to send the whole driver to MTD list?
> > > 
> > > Unless someone created a big-endian OMAP,
> > 
> > Don't they use standard ARM ARM cores, which can all be used in both
> > LE and BE modes?
> 
> And when those cores turn into silicon, the choice is usually
> hard-wired.

I've never seen CPU endianity being hardwired in any ARM system ever
-- but maybe OMAP is different.

(Out of all the ARM subarchs supported in the kernel right now, I've had
ep93xx'es, iop13xx'es, iop32x'es, iop33x'es, ixp2000s, ixp23xx'es,
ixp4xx'es, kirkwoods, mv78xx0's, and orion5x'es all running in BE mode
at some point.)

Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[David Brownell]

On Monday 04 August 2008, Lennert Buytenhek wrote:
> 
> I've never seen CPU endianity being hardwired in any ARM system ever
> -- but maybe OMAP is different.

I'll let TI answer that one, since I'm not going to look at docs for
all the ARM's I've ever used.

My observation stands *REGARDLESS* of whether endianness was fixed in
hardware, bootloader, or kernel ... and in any case, with very few
exceptions (not including OMAP), Linux uses ARMs in LE mode:

  ~/kernel/linux-2.6/arch/arm/configs$ grep ENDIAN * | egrep -v '#' |egrep -v OHCI
  ixp2000_defconfig:CONFIG_ARCH_SUPPORTS_BIG_ENDIAN=y
  ixp2000_defconfig:CONFIG_CPU_BIG_ENDIAN=y
  ixp23xx_defconfig:CONFIG_ARCH_SUPPORTS_BIG_ENDIAN=y
  ixp23xx_defconfig:CONFIG_CPU_BIG_ENDIAN=y
  ixp4xx_defconfig:CONFIG_ARCH_SUPPORTS_BIG_ENDIAN=y
  ixp4xx_defconfig:CONFIG_CPU_BIG_ENDIAN=y
  ~/kernel/linux-2.6/arch/arm/configs$  ls | wc -l
  105
  ~/kernel/linux-2.6/arch/arm/configs$ 

To repeat:  there's no point in having the words byteswapped when
writing, then again when reading, like this driver does.  All that
does is ensure slow I/O paths.  Were you disagreeing with that main
point?  Or just quibbling about where any unusual big-endianness
might come from?

- Dave

Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Lennert Buytenhek]

On Mon, Aug 04, 2008 at 02:37:34PM -0700, David Brownell wrote:

> > I've never seen CPU endianity being hardwired in any ARM system ever
> > -- but maybe OMAP is different.
> 
> I'll let TI answer that one, since I'm not going to look at docs for
> all the ARM's I've ever used.
> 
> My observation stands *REGARDLESS* of whether endianness was fixed in
> hardware, bootloader, or kernel ... and in any case, with very few
> exceptions (not including OMAP), Linux uses ARMs in LE mode:
> 
>   ~/kernel/linux-2.6/arch/arm/configs$ grep ENDIAN * | egrep -v '#' |egrep -v OHCI
>   ixp2000_defconfig:CONFIG_ARCH_SUPPORTS_BIG_ENDIAN=y
>   ixp2000_defconfig:CONFIG_CPU_BIG_ENDIAN=y
>   ixp23xx_defconfig:CONFIG_ARCH_SUPPORTS_BIG_ENDIAN=y
>   ixp23xx_defconfig:CONFIG_CPU_BIG_ENDIAN=y
>   ixp4xx_defconfig:CONFIG_ARCH_SUPPORTS_BIG_ENDIAN=y
>   ixp4xx_defconfig:CONFIG_CPU_BIG_ENDIAN=y
>   ~/kernel/linux-2.6/arch/arm/configs$  ls | wc -l
>   105
>   ~/kernel/linux-2.6/arch/arm/configs$ 

That's just because not many people ask for BE or use BE -- not
because the LE'ness would be hardwired in hardware as you asserted
earlier.


> To repeat:  there's no point in having the words byteswapped when
> writing, then again when reading, like this driver does.  All that
> does is ensure slow I/O paths.  Were you disagreeing with that main
> point?  Or just quibbling about where any unusual big-endianness
> might come from?

I was responding to your statement that ARM CPU endianity is
hardwired when ARM cores are turned into silicon.

RE: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Woodruff, Richard]

> On Monday 04 August 2008, Lennert Buytenhek wrote:
> > On Mon, Aug 04, 2008 at 01:02:46PM -0700, David Brownell wrote:
> >
> > > > Looks like this driver does not exist outside linux-omap tree,
> > > > care to send the whole driver to MTD list?
> > >
> > > Unless someone created a big-endian OMAP,
> >
> > Don't they use standard ARM ARM cores, which can all be used in both
> > LE and BE modes?
>
> And when those cores turn into silicon, the choice is usually
> hard-wired.  Regardless, all OMAP cores I've worked with are
> set up for LE mode ... and then there's still the point that
> byteswapping to/from the NAND chip seems to add no value.

Yes all OMAP-ARM's have been LE.

I've only used a couple ARMs in the past where it was run time configurable.  Most hardware hard wire it as Dave asserts.

As trivia you might find a BE-DSP in the OMAP1 SOC and LE-ARM.  So, internal bridge chips usually allow hardware swapping.  OMAP3-DSP is LE.

The most visible BE-ARM seems to be Intel's IXP network processors.  Matching network byte order apparently saves some cycles.  Earlier BE ones I recall were trying to pick up Mot-68K-BE sockets which had BE friendly peripherals.

Regards,
Richard W.

Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Lennert Buytenhek]

On Mon, Aug 04, 2008 at 04:23:35PM -0500, Woodruff, Richard wrote:

> The most visible BE-ARM seems to be Intel's IXP network processors.

The IXPs are one of the few cases where the vendors _ships an all-BE
software development environment by default_ -- but that doesn't mean
that BE doesn't work on ARM CPUs where the vendor ships a LE software
development environment by default (BE has certainly worked on all
ARM CPUs I've ever tried it on).

RE: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Woodruff, Richard]

> > The most visible BE-ARM seems to be Intel's IXP network processors.
>
> The IXPs are one of the few cases where the vendors _ships an all-BE
> software development environment by default_ -- but that doesn't mean
> that BE doesn't work on ARM CPUs where the vendor ships a LE software
> development environment by default (BE has certainly worked on all
> ARM CPUs I've ever tried it on).

I wonder how many shipping systems use LE.  A grep of current ARM Linux Kconfig's only shows 3 IXP systems setting CONFIG_CPU_BIG_ENDIAN. Perhaps you are correct about ability but few seem to use it. Seems like a trivia question for ARM Ltd.

At any rate all OMAPs have been hardwired.

Regards,
Richard W.

Re: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Lennert Buytenhek]

On Mon, Aug 04, 2008 at 04:45:19PM -0500, Woodruff, Richard wrote:

> > > The most visible BE-ARM seems to be Intel's IXP network processors.
> >
> > The IXPs are one of the few cases where the vendors _ships an all-BE
> > software development environment by default_ -- but that doesn't mean
> > that BE doesn't work on ARM CPUs where the vendor ships a LE software
> > development environment by default (BE has certainly worked on all
> > ARM CPUs I've ever tried it on).
> 
> I wonder how many shipping systems use LE.  A grep of current ARM Linux Kconfig's only shows 3 IXP systems setting CONFIG_CPU_BIG_ENDIAN. Perhaps you are correct about ability but few seem to use it. Seems like a trivia question for ARM Ltd.
> 
> At any rate all OMAPs have been hardwired.

Hmmm.  So bit [7] of the system control register is ignored entirely,
and if you write a 1 to it, nothing at all happens and the system
boots as usual?  (To test, add a line "OBJS += big-endian.o" to the
top of arch/arm/boot/compressed/Makefile.)

Does it also mean that the ARMv6 based OMAPs have the SETEND
instruction behave as NOPs?

RE: [PATCH] MTD: OMAP2-NAND: Fix partition reading from board info

[Woodruff, Richard]

> Hmmm.  So bit [7] of the system control register is ignored entirely,
> and if you write a 1 to it, nothing at all happens and the system
> boots as usual?  (To test, add a line "OBJS += big-endian.o" to the
> top of arch/arm/boot/compressed/Makefile.)
>
> Does it also mean that the ARMv6 based OMAPs have the SETEND
> instruction behave as NOPs?

Last time I explicitly tried was back on OMAP1.  I then dug into doc's it was said that the bit was hardwired at the integration point.  I don't recall signal name.

Now, Peeking into OMAP2 (armv6) MPU integration docs I see that BIGENDINIT is pulled low.  This forces the behavior as I indicated.  OMAP3 (armv7) does something similar at cpu-to-bus bridge.

Doing a quick desktop search in documents I see the armv6 behavior is defined as part of BIGENDINIT in ARM TRMs.  I'll guess SETEND instruction follows what ever that dictates.  I don't know as I've never tried.

Regards,
Richard W.