Re: [PATCH v2 1/6] davinci: add support for aemif timing configuration

2010-08-06 Thread Kevin Hilman
"Nori, Sekhar"  writes:

> On Thu, Aug 05, 2010 at 23:50:13, Kevin Hilman wrote:
>> Sekhar Nori  writes:
>>
>> > This patch adds support to configure the AEMIF interface
>> > with supplied timing values.
>> >
>> > Since this capability is useful both from NOR and NAND
>> > flashes, it is provided as a new interface and in a file
>> > of its own.
>> >
>> > AEMIF timing configuration is required in cases:
>> >
>> > 1) Where the AEMIF clock rate can change at runtime (a side
>> >affect of cpu frequency change).
>> >
>> > 2) Where U-Boot does not support NAND/NOR but supports other
>> >media like SPI Flash or MMC/SD and thus does not care about
>> >setting up the AEMIF timing for kernel to use.
>> >
>> > 3) Where U-Boot just hasn't configured the timing values and
>> >cannot be upgraded because the box is already in the field.
>> >
>> > Since there is now a header file for AEMIF interface, the
>> > common (non-NAND specific) defines for AEMIF registers have
>> > been moved from nand.h into the newly created aemif.h
>> >
>> > Signed-off-by: Sekhar Nori 
>>
>> This series looks great, and is a very nice cleanup.  Also, those
>> performance improvments are pretty impressive.
>>
>> Apart from the minor comment below (which I can fixup myself if you
>> approve), I'll apply this whole series.
>
> Thanks Kevin!
>
> Patch 2/6 touches mtd nand driver. Do you want to take that patch through
> davinci as well? I guess mtd list would have to be involved someway. Shall
> I post the series to mtd list with your sign-off/ack to see what the mtd
> maintainer wants to do?

Yes please.  You can add my ack.

>>
>> [...]
>>
>> > +
>> > +#define TIMING_MASK(TA(TA_MAX) | \
>> > +   RHOLD(RHOLD_MAX) | \
>> > +   RSTROBE(RSTROBE_MAX) |  \
>> > +   RSETUP(RSETUP_MAX) | \
>> > +   WHOLD(WHOLD_MAX) | \
>> > +   WSTROBE(WSTROBE_MAX) | \
>> > +   WSETUP(WSETUP_MAX))
>> > +
>> > +#define NS_IN_KHZ  100
>>
>> Minor nit:  already nas NSEC_PER_MSEC, can use that instead.
>
> If I am going to have to re-post this, I can make this change while
> posting.

OK, thanks.

Kevin
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


RE: [PATCH v2 1/6] davinci: add support for aemif timing configuration

2010-08-06 Thread Nori, Sekhar
On Thu, Aug 05, 2010 at 23:50:13, Kevin Hilman wrote:
> Sekhar Nori  writes:
>
> > This patch adds support to configure the AEMIF interface
> > with supplied timing values.
> >
> > Since this capability is useful both from NOR and NAND
> > flashes, it is provided as a new interface and in a file
> > of its own.
> >
> > AEMIF timing configuration is required in cases:
> >
> > 1) Where the AEMIF clock rate can change at runtime (a side
> >affect of cpu frequency change).
> >
> > 2) Where U-Boot does not support NAND/NOR but supports other
> >media like SPI Flash or MMC/SD and thus does not care about
> >setting up the AEMIF timing for kernel to use.
> >
> > 3) Where U-Boot just hasn't configured the timing values and
> >cannot be upgraded because the box is already in the field.
> >
> > Since there is now a header file for AEMIF interface, the
> > common (non-NAND specific) defines for AEMIF registers have
> > been moved from nand.h into the newly created aemif.h
> >
> > Signed-off-by: Sekhar Nori 
>
> This series looks great, and is a very nice cleanup.  Also, those
> performance improvments are pretty impressive.
>
> Apart from the minor comment below (which I can fixup myself if you
> approve), I'll apply this whole series.

Thanks Kevin!

Patch 2/6 touches mtd nand driver. Do you want to take that patch through
davinci as well? I guess mtd list would have to be involved someway. Shall
I post the series to mtd list with your sign-off/ack to see what the mtd
maintainer wants to do?

>
> [...]
>
> > +
> > +#define TIMING_MASK(TA(TA_MAX) | \
> > +   RHOLD(RHOLD_MAX) | \
> > +   RSTROBE(RSTROBE_MAX) |  \
> > +   RSETUP(RSETUP_MAX) | \
> > +   WHOLD(WHOLD_MAX) | \
> > +   WSTROBE(WSTROBE_MAX) | \
> > +   WSETUP(WSETUP_MAX))
> > +
> > +#define NS_IN_KHZ  100
>
> Minor nit:  already nas NSEC_PER_MSEC, can use that instead.

If I am going to have to re-post this, I can make this change while
posting.

Thanks,
Sekhar

___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PATCH v2 1/6] davinci: add support for aemif timing configuration

2010-08-05 Thread Kevin Hilman
Sekhar Nori  writes:

> This patch adds support to configure the AEMIF interface
> with supplied timing values.
>
> Since this capability is useful both from NOR and NAND
> flashes, it is provided as a new interface and in a file
> of its own.
>
> AEMIF timing configuration is required in cases:
>
> 1) Where the AEMIF clock rate can change at runtime (a side
>affect of cpu frequency change).
>
> 2) Where U-Boot does not support NAND/NOR but supports other
>media like SPI Flash or MMC/SD and thus does not care about
>setting up the AEMIF timing for kernel to use.
>
> 3) Where U-Boot just hasn't configured the timing values and
>cannot be upgraded because the box is already in the field.
>
> Since there is now a header file for AEMIF interface, the
> common (non-NAND specific) defines for AEMIF registers have
> been moved from nand.h into the newly created aemif.h
>
> Signed-off-by: Sekhar Nori 

This series looks great, and is a very nice cleanup.  Also, those
performance improvments are pretty impressive.

Apart from the minor comment below (which I can fixup myself if you
approve), I'll apply this whole series.

[...]

> +
> +#define TIMING_MASK  (TA(TA_MAX) | \
> + RHOLD(RHOLD_MAX) | \
> + RSTROBE(RSTROBE_MAX) |  \
> + RSETUP(RSETUP_MAX) | \
> + WHOLD(WHOLD_MAX) | \
> + WSTROBE(WSTROBE_MAX) | \
> + WSETUP(WSETUP_MAX))
> +
> +#define NS_IN_KHZ100

Minor nit:  already nas NSEC_PER_MSEC, can use that instead.

Kevin
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


[PATCH v2 1/6] davinci: add support for aemif timing configuration

2010-07-06 Thread Sekhar Nori
This patch adds support to configure the AEMIF interface
with supplied timing values.

Since this capability is useful both from NOR and NAND
flashes, it is provided as a new interface and in a file
of its own.

AEMIF timing configuration is required in cases:

1) Where the AEMIF clock rate can change at runtime (a side
   affect of cpu frequency change).

2) Where U-Boot does not support NAND/NOR but supports other
   media like SPI Flash or MMC/SD and thus does not care about
   setting up the AEMIF timing for kernel to use.

3) Where U-Boot just hasn't configured the timing values and
   cannot be upgraded because the box is already in the field.

Since there is now a header file for AEMIF interface, the
common (non-NAND specific) defines for AEMIF registers have
been moved from nand.h into the newly created aemif.h

Signed-off-by: Sekhar Nori 
---
v2:
1) return error from aemif_clk_rate() in case the timing values obtained
   exceed the max allowed
2) removed common aemif register definitions from nand.h

 arch/arm/mach-davinci/Makefile |2 +-
 arch/arm/mach-davinci/aemif.c  |  134 
 arch/arm/mach-davinci/include/mach/aemif.h |   36 
 arch/arm/mach-davinci/include/mach/nand.h  |3 -
 drivers/mtd/nand/davinci_nand.c|1 +
 5 files changed, 172 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/mach-davinci/aemif.c
 create mode 100644 arch/arm/mach-davinci/include/mach/aemif.h

diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile
index eab4c0f..77a0f71 100644
--- a/arch/arm/mach-davinci/Makefile
+++ b/arch/arm/mach-davinci/Makefile
@@ -5,7 +5,7 @@
 
 # Common objects
 obj-y  := time.o clock.o serial.o io.o psc.o \
-  gpio.o dma.o usb.o common.o sram.o
+  gpio.o dma.o usb.o common.o sram.o aemif.o
 
 obj-$(CONFIG_DAVINCI_MUX)  += mux.o
 
diff --git a/arch/arm/mach-davinci/aemif.c b/arch/arm/mach-davinci/aemif.c
new file mode 100644
index 000..12b712f
--- /dev/null
+++ b/arch/arm/mach-davinci/aemif.c
@@ -0,0 +1,134 @@
+/*
+ * AEMIF support for DaVinci SoCs
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated. http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/* Timing value configuration */
+
+#define TA(x)  ((x) << 2)
+#define RHOLD(x)   ((x) << 4)
+#define RSTROBE(x) ((x) << 7)
+#define RSETUP(x)  ((x) << 13)
+#define WHOLD(x)   ((x) << 17)
+#define WSTROBE(x) ((x) << 20)
+#define WSETUP(x)  ((x) << 26)
+
+#define TA_MAX 0x3
+#define RHOLD_MAX  0x7
+#define RSTROBE_MAX0x3f
+#define RSETUP_MAX 0xf
+#define WHOLD_MAX  0x7
+#define WSTROBE_MAX0x3f
+#define WSETUP_MAX 0xf
+
+#define TIMING_MASK(TA(TA_MAX) | \
+   RHOLD(RHOLD_MAX) | \
+   RSTROBE(RSTROBE_MAX) |  \
+   RSETUP(RSETUP_MAX) | \
+   WHOLD(WHOLD_MAX) | \
+   WSTROBE(WSTROBE_MAX) | \
+   WSETUP(WSETUP_MAX))
+
+#define NS_IN_KHZ  100
+
+/*
+ * aemif_calc_rate - calculate timing data.
+ * @wanted: The cycle time needed in nanoseconds.
+ * @clk: The input clock rate in kHz.
+ * @max: The maximum divider value that can be programmed.
+ *
+ * On success, returns the calculated timing value minus 1 for easy
+ * programming into AEMIF timing registers, else negative errno.
+ */
+static int aemif_calc_rate(int wanted, unsigned long clk, int max)
+{
+   int result;
+
+   result = DIV_ROUND_UP((wanted * clk), NS_IN_KHZ) - 1;
+
+   pr_debug("%s: result %d from %ld, %d\n", __func__, result, clk, wanted);
+
+   /* It is generally OK to have a more relaxed timing than requested... */
+   if (result < 0)
+   result = 0;
+
+   /* ... But configuring tighter timings is not an option. */
+   else if (result > max)
+   result = -EINVAL;
+
+   return result;
+}
+
+/**
+ * davinci_aemif_setup_timing - setup timing values for a given AEMIF interface
+ * @t: timing values to be progammed
+ * @base: The virtual base address of the AEMIF interface
+ * @cs: chip-select to program the timing values for
+ *
+ * This function programs the given timing values (in real clock) into the
+ * AEMIF registers taking the AEMIF clock into account.
+ *
+ * This function does not use any locking while programming the AEMIF
+ * because it is expected that there is only one user of a given
+ * chip-select.
+ *
+ * Returns 0 on success, else negative errno.
+ */
+int davinci_aemif_setup_timing(struct davinci_aemif_timing *t,
+   voi