arm,davinci: configure davinci aemif chipselects through OF
PATCH 1/1 arm,davinci: configure davinci aemif chipselects through OF
--- .../devicetree/bindings/arm/davinci/aemif.txt | 85 ++++++++++++++++ arch/arm/mach-davinci/aemif.c | 105 +++++++++++++++++++- arch/arm/mach-davinci/include/mach/aemif.h | 1 + 3 files changed, 190 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/davinci/aemif.txt
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +85 -0
@@ -0,0 +1,85 @@ +* Texas Instruments Davinci AEMIF + +This file provides information, what the device node for the +davinci aemifa interface contain. + +Required properties: +- compatible: "ti,davinci-emifa"; +- #address-cells : Should be either two or three. The first cell is the + chipselect number, and the remaining cells are the + offset into the chipselect. +- #size-cells : Either one or two, depending on how large each chipselect + can be. +- ranges : Each range corresponds to a single chipselect, and cover + the entire access window as configured. + +Optional properties: +- none + +Optional subnodes: +- Chipselect setup: + - Required properties: + - compatible: "ti,davinci-cs"; + - #address-cells = <1>; + - #size-cells = <1>; + + Timing setup, all timings in nanoseconds + - cs: chipselect (value 2,3,4 or 5) + - asize: Asynchronous Data Bus Width. + value: + 0: 8 bit + 1: 16 bit + - ta: Minimum Turn-Around time. + - rhold: Read hold width + - rstrobe: Read strobe width + - rsetup: Read setup width + - whold: Write hold width + - wstrobe: Write strobe width + - wsetup: Write setup width + - ew: Extend Wait bit + value: + 0: Extended wait cycles disabled. + 1: Extended wait cycles enabled. + -ss: Select Strobe bit. + value: + 0: Normal Mode enabled. + 1: Select Strobe Mode enabled. +- CFI driver: + see: Documentation/devicetree/bindings/mtd/mtd-physmap.txt + +Example (enbw_cmc board): + aemif at 60000000 { + compatible = "ti,davinci-emifa"; + #address-cells = <2>; + #size-cells = <1>; + reg = <0x68000000 0x80000>; + ranges = <2 0 0x60000000 0x02000000 + 3 0 0x62000000 0x02000000 + 4 0 0x64000000 0x02000000 + 5 0 0x66000000 0x02000000>; + cs2 at 0x60000000 { + compatible = "ti,davinci-cs"; + #address-cells = <1>; + #size-cells = <1>; + /* all timings in nanoseconds */ + cs = <2>; + asize = <1>; + ta = <0>; + rhold = <7>; + rstrobe = <42>; + rsetup = <14>; + whold = <7>; + wstrobe = <42>; + wsetup = <14>; + ew = <0>; + ss = <0>; + }; + flash at 2,0 { + compatible = "cfi-flash"; + reg = <2 0x0 0x400000>; + #address-cells = <1>; + #size-cells = <1>; + bank-width = <2>; + device-width = <2>; + }; + };
arch/arm/mach-davinci/aemif.c +104 -1
132 lines not on this page · raw message
arch/arm/mach-davinci/include/mach/aemif.h +1 -0
@@ -33,4 +33,5 @@ struct davinci_aemif_timing {int davinci_aemif_setup_timing (struct davinci_aemif_timing *t,void __iomem *base,unsigned cs); +int davinci_aemif_setup_timing_of (void );#endif
Signature · 1 line
1.7.6.4
PATCH 1/1 arm, davinci: configure davinci aemif chipselects through OF
Hello.
Quoted 13 lines · Heiko Schocher
Signed-off-by Heiko Schocher<[email protected]>Cc davinci-linux-open-source at linux.davincidsp.comCc devicetree-discuss at lists.ozlabs.orgCc linux-arm-kernel at lists.infradead.orgCc grant.likely at secretlab.caCc Sekhar Nori<[email protected]>Cc Kevin Hilman<[email protected]>Cc Wolfgang Denk<[email protected]>arch/arm/mach-davinci/aemif.c +0
-0
[...]
Quoted 4 lines
arch/arm/mach-davinci/aemif.c +0
-0@@ -131,3 +138,99 @@ int davinci_aemif_setup_timing(struct davinci_aemif_timing *t,return 0 ; }EXPORT_SYMBOL (davinci_aemif_setup_timing);
arch/arm/mach-davinci/aemif.c int davinci_aemif_setup_timing(struct davinci_aemif_timing *t, · line 150
Quoted 10 lines
arch/arm/mach-davinci/aemif.c +10
-0+ +#if defined(CONFIG_OF ) +static int dv_get_value (struct device_node *np,const char *name) +{ +const u32 *data; +int len; + + data =of_get_property (np, name,&len); +if (data) +return be32_to_cpu (readl (data));
Why readl() here?! Device tree is not a peripheral device...
arch/arm/mach-davinci/aemif.c int davinci_aemif_setup_timing(struct davinci_aemif_timing *t, · line 153
Quoted 3 lines
arch/arm/mach-davinci/aemif.c +3
-0+ +return -EINVAL ; +}
Isn't there already a standard helper for that, of_property_read_u32()?
WBR, Sergei
PATCH 1/1 arm, davinci: configure davinci aemif chipselects through OF
Hello Sergei,
Quoted 37 lines · Sergei Shtylyov
Hello.
Quoted 14 lines · Heiko Schocher
Signed-off-by Heiko Schocher<[email protected]>Cc davinci-linux-open-source at linux.davincidsp.comCc devicetree-discuss at lists.ozlabs.orgCc linux-arm-kernel at lists.infradead.orgCc grant.likely at secretlab.caCc Sekhar Nori<[email protected]>Cc Kevin Hilman<[email protected]>Cc Wolfgang Denk<[email protected]>arch/arm/mach-davinci/aemif.c +0
-0[...]
Quoted 5 lines
arch/arm/mach-davinci/aemif.c +0
-0@@ -131,3 +138,99 @@ int davinci_aemif_setup_timing(structdavinci_aemif_timing *t, return 0; }EXPORT_SYMBOL (davinci_aemif_setup_timing);arch/arm/mach-davinci/aemif.c int davinci_aemif_setup_timing(struct · line 150
Quoted 10 lines
arch/arm/mach-davinci/aemif.c +10
-0+ +#if defined(CONFIG_OF ) +static int dv_get_value (struct device_node *np,const char *name) +{ +const u32 *data; +int len; + + data =of_get_property (np, name,&len); +if (data) +return be32_to_cpu (readl (data));Why
readl()here?! Device tree is not a peripheral device...
Quoted 5 lines
arch/arm/mach-davinci/aemif.c int davinci_aemif_setup_timing(struct davinci_aemif_timing *t, · line 153
Quoted 3 lines
arch/arm/mach-davinci/aemif.c +3
-0+ +return -EINVAL ; +}Isn't there already a standard helper for that,
of_property_read_u32()?
Yep, fixed that.
Thanks!
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 arm, davinci: configure davinci aemif chipselects through OF
In reply to Heiko SchocherHello.
Quoted 8 lines · Heiko Schocher
Signed-off-by Heiko Schocher <[email protected]>Cc davinci-linux-open-source at linux.davincidsp.comCc devicetree-discuss at lists.ozlabs.orgCc linux-arm-kernel at lists.infradead.orgCc grant.likely at secretlab.caCc Sekhar Nori <[email protected]>Cc Kevin Hilman <[email protected]>Cc Wolfgang Denk <[email protected]>
[...]
Quoted 10 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +4
-0@@ -0,0 +1,85 @@ +* Texas Instruments Davinci AEMIF + +This file provides information, what the device node for the +davinci aemifa interface contain.
[...]
Documentation/devicetree/bindings/arm/davinci/aemif.txt line 60
Quoted 11 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +11
-0+Example (enbw_cmc board): + aemif at 60000000 { + compatible = "ti,davinci-emifa"; + #address-cells = <2>; + #size-cells = <1>; + reg = <0x68000000 0x80000>; + ranges = <2 0 0x60000000 0x02000000 + 3 0 0x62000000 0x02000000 + 4 0 0x64000000 0x02000000 + 5 0 0x66000000 0x02000000>; + cs2 at 0x60000000 {
0x shouldn't be included.
Quoted 7 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +7
-0+ compatible = "ti,davinci-cs"; + #address-cells = <1>; + #size-cells = <1>; + /* all timings in nanoseconds */ + cs = <2>; + asize = <1>; + ta = <0>;
Documentation/devicetree/bindings/arm/davinci/aemif.txt line 77
Quoted 10 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +10
-0+ rhold = <7>; + rstrobe = <42>; + rsetup = <14>; + whold = <7>; + wstrobe = <42>; + wsetup = <14>; + ew = <0>; + ss = <0>; + }; + flash at 2,0 {
Why you have one kind of address for cs2 at 60000000 node and other kind for
this node?
WBR, Sergei
PATCH 1/1 arm, davinci: configure davinci aemif chipselects through OF
Hello Sergei,
Quoted 38 lines · Sergei Shtylyov
Hello.
Quoted 8 lines · Heiko Schocher
Signed-off-by Heiko Schocher <[email protected]>Cc davinci-linux-open-source at linux.davincidsp.comCc devicetree-discuss at lists.ozlabs.orgCc linux-arm-kernel at lists.infradead.orgCc grant.likely at secretlab.caCc Sekhar Nori <[email protected]>Cc Kevin Hilman <[email protected]>Cc Wolfgang Denk <[email protected]>[...]
Quoted 11 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +4
-0@@ -0,0 +1,85 @@ +* Texas Instruments Davinci AEMIF + +This file provides information, what the device node for the +davinci aemifa interface contain.[...]
Documentation/devicetree/bindings/arm/davinci/aemif.txt line 60
Quoted 11 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +11
-0+Example (enbw_cmc board): + aemif at 60000000 { + compatible = "ti,davinci-emifa"; + #address-cells = <2>; + #size-cells = <1>; + reg = <0x68000000 0x80000>; + ranges = <2 0 0x60000000 0x02000000 + 3 0 0x62000000 0x02000000 + 4 0 0x64000000 0x02000000 + 5 0 0x66000000 0x02000000>; + cs2 at 0x60000000 {
Quoted 1 line
0x shouldn't be included.
fixed.
Quoted 18 lines
Quoted 7 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +7
-0+ compatible = "ti,davinci-cs"; + #address-cells = <1>; + #size-cells = <1>; + /* all timings in nanoseconds */ + cs = <2>; + asize = <1>; + ta = <0>;Documentation/devicetree/bindings/arm/davinci/aemif.txt line 77
Quoted 10 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +10
-0+ rhold = <7>; + rstrobe = <42>; + rsetup = <14>; + whold = <7>; + wstrobe = <42>; + wsetup = <14>; + ew = <0>; + ss = <0>; + }; + flash at 2,0 {
Quoted 2 lines
Why you have one kind of address for cs2 at 60000000 node and other kind
for this node?
Typo it must be
cs2 at 68000010 {
[...]
}
As the chipselect registers start @0x68000000 maybe
I should add in ranges a "x 0 0x68000000 0x00080000" line? But which value has x, as this range has no chipselect? So I could add subnodes "cs2 at 680000yy" with a reg entry "reg = <x yy 4>;" with yy = offset from 0x68000000 and drop the "cs" property?
Or are the "ti,davinci-cs" nodes no subnodes from "ti,davinci-emifa", so I easy can use "reg = <0x68000010 4>;"?
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 arm,davinci: configure davinci aemif chipselects through OF
In reply to Heiko SchocherHi Heiko,
On Sun, Dec 04, 2011 at 15:11:19, Heiko Schocher wrote:
Please provide a patch description. Nice to see device tree support being added for DaVinci devices.
Quoted 21 lines
Signed-off-by Heiko Schocher <[email protected]>Cc davinci-linux-open-source at linux.davincidsp.comCc devicetree-discuss at lists.ozlabs.orgCc linux-arm-kernel at lists.infradead.orgCc grant.likely at secretlab.caCc Sekhar Nori <[email protected]>Cc Kevin Hilman <[email protected]>Cc Wolfgang Denk <[email protected]>--- .../devicetree/bindings/arm/davinci/aemif.txt | 85 ++++++++++++++++ arch/arm/mach-davinci/aemif.c | 105 +++++++++++++++++++- arch/arm/mach-davinci/include/mach/aemif.h | 1 + 3 files changed, 190 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/davinci/aemif.txtDocumentation/devicetree/bindings/arm/davinci/aemif.txt new file +0
-0
Documentation/devicetree/bindings/arm/davinci/aemif.txt line 4
Quoted 4 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +4
-0@@ -0,0 +1,85 @@ +* Texas Instruments Davinci AEMIF + +This file provides information, what the device node for the +davinci aemifa interface contain.
^^^^^^
aemifQuoted 102 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +81
-0+ +Required properties: +- compatible: "ti,davinci-emifa"; +- #address-cells : Should be either two or three. The first cell is the + chipselect number, and the remaining cells are the + offset into the chipselect. +- #size-cells : Either one or two, depending on how large each chipselect + can be. +- ranges : Each range corresponds to a single chipselect, and cover + the entire access window as configured. + +Optional properties: +- none + +Optional subnodes: +- Chipselect setup: + - Required properties: + - compatible: "ti,davinci-cs"; + - #address-cells = <1>; + - #size-cells = <1>; + + Timing setup, all timings in nanoseconds + - cs: chipselect (value 2,3,4 or 5) + - asize: Asynchronous Data Bus Width. + value: + 0: 8 bit + 1: 16 bit + - ta: Minimum Turn-Around time. + - rhold: Read hold width + - rstrobe: Read strobe width + - rsetup: Read setup width + - whold: Write hold width + - wstrobe: Write strobe width + - wsetup: Write setup width + - ew: Extend Wait bit + value: + 0: Extended wait cycles disabled. + 1: Extended wait cycles enabled. + -ss: Select Strobe bit. + value: + 0: Normal Mode enabled. + 1: Select Strobe Mode enabled. +- CFI driver: + see: Documentation/devicetree/bindings/mtd/mtd-physmap.txt + +Example (enbw_cmc board): + aemif at 60000000 { + compatible = "ti,davinci-emifa"; + #address-cells = <2>; + #size-cells = <1>; + reg = <0x68000000 0x80000>; + ranges = <2 0 0x60000000 0x02000000 + 3 0 0x62000000 0x02000000 + 4 0 0x64000000 0x02000000 + 5 0 0x66000000 0x02000000>; + cs2 at 0x60000000 { + compatible = "ti,davinci-cs"; + #address-cells = <1>; + #size-cells = <1>; + /* all timings in nanoseconds */ + cs = <2>; + asize = <1>; + ta = <0>; + rhold = <7>; + rstrobe = <42>; + rsetup = <14>; + whold = <7>; + wstrobe = <42>; + wsetup = <14>; + ew = <0>; + ss = <0>; + }; + flash at 2,0 { + compatible = "cfi-flash"; + reg = <2 0x0 0x400000>; + #address-cells = <1>; + #size-cells = <1>; + bank-width = <2>; + device-width = <2>; + }; + };arch/arm/mach-davinci/aemif.c +3
-1@@ -13,12 +13,14 @@#include <linux/err.h> #include <linux/clk.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> #include <linux/time.h> #include <mach/aemif.h> /* Timing value configuration */ - +#define ASIZE (x) (x)#define TA (x) ((x) << 2)#define RHOLD (x) ((x) << 4)#define RSTROBE (x) ((x) << 7)
arch/arm/mach-davinci/aemif.c line 32
Quoted 5 lines
arch/arm/mach-davinci/aemif.c +2
-0@@ -26,7 +28,10 @@#define WHOLD (x) ((x) << 17)#define WSTROBE (x) ((x) << 20)#define WSETUP (x) ((x) << 26) +#define EW (x) ((x) << 30) +#define SS (x) ((x) << 31)
You are adding support for additional configuration parameters which should be done in a separate patch.
Quoted 78 lines
arch/arm/mach-davinci/aemif.c +63
-0+#define ASIZE_MAX 0x1#define TA_MAX 0x3#define RHOLD_MAX 0x7#define RSTROBE_MAX 0x3f@@ -34,6 +39,8 @@#define WHOLD_MAX 0x7#define WSTROBE_MAX 0x3f#define WSETUP_MAX 0xf +#define EW_MAX 0x1 +#define SS_MAX 0x1#define TIMING_MASK (TA(TA_MAX) | \ RHOLD(RHOLD_MAX) | \@@ -131,3 +138,99 @@ int davinci_aemif_setup_timing(struct davinci_aemif_timing *t,return 0 ; }EXPORT_SYMBOL (davinci_aemif_setup_timing); + +#if defined(CONFIG_OF ) +static int dv_get_value (struct device_node *np,const char *name) +{ +const u32 *data; +int len; + + data =of_get_property (np, name, &len); +if (data) +return be32_to_cpu (readl (data)); + +return -EINVAL ; +} + +static int davinci_aemif_setup_timing_of_one (struct device_node *np, +void __iomem *base) +{ +unsigned val; +int asize, ta, rhold, rstrobe, rsetup, whold, wstrobe, wsetup; +int ew, ss; +int cs; +unsigned offset; +struct clk *aemif_clk; +unsigned long clkrate; + + aemif_clk =clk_get (NULL ,"aemif" ); +if (IS_ERR (aemif_clk)) +return PTR_ERR (aemif_clk); + + clkrate =clk_get_rate (aemif_clk); + + clkrate /=1000 ;/* turn clock into kHz for ease of use */ + + cs =dv_get_value (np,"cs" ); +if (cs <2 ) +return -EINVAL ; + + offset =A1CR_OFFSET + (cs -2 ) *4 ; + asize =dv_get_value (np,"asize" ); + ta =aemif_calc_rate (dv_get_value (np,"ta" ), clkrate,TA_MAX ); + rhold =aemif_calc_rate (dv_get_value (np,"rhold" ), clkrate, +RHOLD_MAX ); + rstrobe =aemif_calc_rate (dv_get_value (np,"rstrobe" ), clkrate, +RSTROBE_MAX ); + rsetup =aemif_calc_rate (dv_get_value (np,"rsetup" ), clkrate, +RSETUP_MAX ); + whold =aemif_calc_rate (dv_get_value (np,"whold" ), clkrate, +WHOLD_MAX ); + wstrobe =aemif_calc_rate (dv_get_value (np,"wstrobe" ), clkrate, +WSTROBE_MAX ); + wsetup =aemif_calc_rate (dv_get_value (np,"wsetup" ), clkrate, +WSETUP_MAX ); + ew =dv_get_value (np,"ew" ); + ss =dv_get_value (np,"ss" ); + +if (asize <0 || ta <0 || rhold <0 || rstrobe <0 || rsetup <0 || + whold <0 || wstrobe <0 || wsetup <0 || ew <0 || + ss <0 ) { +pr_err ("%s: cannot get suitable timings\n" , __func__); +return -EINVAL ;
arch/arm/mach-davinci/aemif.c int davinci_aemif_setup_timing(struct davinci_aemif_timing *t, · line 210
Quoted 10 lines
arch/arm/mach-davinci/aemif.c +10
-0+ } + + val =ASIZE (asize) |TA (ta) |RHOLD (rhold) |RSTROBE (rstrobe) | +RSETUP (rsetup) |WHOLD (whold) |WSTROBE (wstrobe) | +WSETUP (wsetup) |EW (ew) |SS (ss); + +__raw_writel (val, base + offset); + +return 0 ; +}
This shares a large amount of code with davinci_aemif_setup_timing(). Can you try writing this as a OF wrapper to the existing function?
Thanks,
Sekhar
PATCH 1/1 arm, davinci: configure davinci aemif chipselects through OF
Hello Nori,
Quoted 6 lines · Sekhar
Hi Heiko,
On Sun, Dec 04, 2011 at 15:11:19, Heiko Schocher wrote:
Please provide a patch description. Nice to see device tree support being added for DaVinci devices.
fixed.
Quoted 15 lines
Quoted 15 lines
Signed-off-by Heiko Schocher <[email protected]>Cc davinci-linux-open-source at linux.davincidsp.comCc devicetree-discuss at lists.ozlabs.orgCc linux-arm-kernel at lists.infradead.orgCc grant.likely at secretlab.caCc Sekhar Nori <[email protected]>Cc Kevin Hilman <[email protected]>Cc Wolfgang Denk <[email protected]>--- .../devicetree/bindings/arm/davinci/aemif.txt | 85 ++++++++++++++++ arch/arm/mach-davinci/aemif.c | 105 +++++++++++++++++++- arch/arm/mach-davinci/include/mach/aemif.h | 1 + 3 files changed, 190 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/davinci/aemif.txt
Quoted 12 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt line 4
Quoted 10 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +4
-0@@ -0,0 +1,85 @@ +* Texas Instruments Davinci AEMIF + +This file provides information, what the device node for the +davinci aemifa interface contain.^^^^^^ aemif
fixed, thanks.
Quoted 3 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt line 7
Quoted 3 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +3
-0+ +Required properties: +- compatible: "ti,davinci-emifa";
Shouldn't this also be "ti,davinci-aemif" ?
[...]
Quoted 14 lines
Quoted 14 lines
arch/arm/mach-davinci/aemif.c +2
-0@@ -13,12 +13,14 @@#include <linux/err.h> #include <linux/clk.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> #include <linux/time.h> #include <mach/aemif.h>
Quoted 15 lines
arch/arm/mach-davinci/aemif.c line 32
Quoted 12 lines
arch/arm/mach-davinci/aemif.c +3
-1/* Timing value configuration */ - +#define ASIZE (x) (x)#define TA (x) ((x) << 2)#define RHOLD (x) ((x) << 4)#define RSTROBE (x) ((x) << 7)@@ -26,7 +28,10 @@#define WHOLD (x) ((x) << 17)#define WSTROBE (x) ((x) << 20)#define WSETUP (x) ((x) << 26) +#define EW (x) ((x) << 30) +#define SS (x) ((x) << 31)You are adding support for additional configuration parameters which should be done in a separate patch.
Hmm.. they are only used in the OF case ... is this split really needed?
[...]
Quoted 3 lines
Quoted 3 lines
arch/arm/mach-davinci/aemif.c +3
-0+static int davinci_aemif_setup_timing_of_one (struct device_node *np, +void __iomem *base) +{
[...]
Quoted 11 lines
arch/arm/mach-davinci/aemif.c int davinci_aemif_setup_timing(struct davinci_aemif_timing *t, · line 210
Quoted 8 lines
arch/arm/mach-davinci/aemif.c +8
-0+ val =ASIZE (asize) |TA (ta) |RHOLD (rhold) |RSTROBE (rstrobe) | +RSETUP (rsetup) |WHOLD (whold) |WSTROBE (wstrobe) | +WSETUP (wsetup) |EW (ew) |SS (ss); + +__raw_writel (val, base + offset); + +return 0 ; +}This shares a large amount of code with
davinci_aemif_setup_timing(). Can you try writing this as a OF wrapper to the existing function?
Done.
Waiting to your response to my 2 questions above, after that sending the v2.
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 arm,davinci: configure davinci aemif chipselects through OF
Quoted 13 lines · Heiko Schocher
Quoted 4 lines
Quoted 4 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +0
-1--- /dev/nullQuoted 8 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt line 4
Quoted 6 lines
+++
b/Documentation/devicetree/bindings/arm/davinci/aemif.txtDocumentation/devicetree/bindings/arm/davinci/aemif.txt new file +4
-0@@ -0,0 +1,85 @@ +* Texas Instruments Davinci AEMIF + +This file provides information, what the device node for the +davinci aemifa interface contain.^^^^^^ aemif
Quoted 7 lines
fixed, thanks.
Quoted 3 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt line 7
Quoted 3 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +3
-0+ +Required properties: +- compatible: "ti,davinci-emifa";Shouldn't this also be "ti,davinci-aemif" ?
Yes, makes sense.
Quoted 32 lines
[...]
Quoted 14 lines
Quoted 14 lines
arch/arm/mach-davinci/aemif.c +2
-0@@ -13,12 +13,14 @@#include <linux/err.h> #include <linux/clk.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> #include <linux/time.h> #include <mach/aemif.h> Quoted 15 lines
arch/arm/mach-davinci/aemif.c line 32
Quoted 12 lines
arch/arm/mach-davinci/aemif.c +3
-1/* Timing value configuration */ - +#define ASIZE (x) (x)#define TA (x) ((x) << 2)#define RHOLD (x) ((x) << 4)#define RSTROBE (x) ((x) << 7)@@ -26,7 +28,10 @@#define WHOLD (x) ((x) << 17)#define WSTROBE (x) ((x) << 20)#define WSETUP (x) ((x) << 26) +#define EW (x) ((x) << 30) +#define SS (x) ((x) << 31)You are adding support for additional configuration parameters which should be done in a separate patch.
Quoted 2 lines
Hmm.. they are only used in the OF case ... is this split really needed?
But they should also be useful in the non-OF case, no? Why restrict their usage to the OF case?
Thanks,
Sekhar
PATCH 1/1 arm, davinci: configure davinci aemif chipselects through OF
Hello Nori,
Quoted 14 lines · Sekhar
Quoted 12 lines · Heiko Schocher
Quoted 12 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt line 4
Quoted 10 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +4
-0@@ -0,0 +1,85 @@ +* Texas Instruments Davinci AEMIF + +This file provides information, what the device node for the +davinci aemifa interface contain.^^^^^^ aemif
Quoted 8 lines
Quoted 6 lines
fixed, thanks.
Quoted 3 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt line 7
Quoted 3 lines
Documentation/devicetree/bindings/arm/davinci/aemif.txt new file +3
-0+ +Required properties: +- compatible: "ti,davinci-emifa";Shouldn't this also be "ti,davinci-aemif" ?
Yes, makes sense.
Ok, change this.
Quoted 29 lines
Quoted 29 lines
[...]
Quoted 28 lines
Quoted 21 lines
arch/arm/mach-davinci/aemif.c +3
-1@@ -13,12 +13,14 @@#include <linux/err.h> #include <linux/clk.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> #include <linux/time.h> #include <mach/aemif.h> /* Timing value configuration */ - +#define ASIZE (x) (x)#define TA (x) ((x) << 2)#define RHOLD (x) ((x) << 4)#define RSTROBE (x) ((x) << 7)arch/arm/mach-davinci/aemif.c line 32
Quoted 5 lines
arch/arm/mach-davinci/aemif.c +2
-0@@ -26,7 +28,10 @@#define WHOLD (x) ((x) << 17)#define WSTROBE (x) ((x) << 20)#define WSETUP (x) ((x) << 26) +#define EW (x) ((x) << 30) +#define SS (x) ((x) << 31)You are adding support for additional configuration parameters which should be done in a separate patch.
Quoted 5 lines
Quoted 2 lines
Hmm.. they are only used in the OF case ... is this split really needed?
But they should also be useful in the non-OF case, no? Why restrict their usage to the OF case?
I thought there is no need, because nobody needed this yet ... Ok, I change this too, so I have to fix this files:
$ grep -lr davinci_aemif_timing . ./arch/arm/mach-davinci/board-dm644x-evm.c ./arch/arm/mach-davinci/board-dm646x-evm.c ./arch/arm/mach-davinci/board-da850-evm.c ./arch/arm/mach-davinci/include/mach/aemif.h ./arch/arm/mach-davinci/include/mach/nand.h ./arch/arm/mach-davinci/board-da830-evm.c ./drivers/mtd/nand/davinci_nand.c
I do not know the values for this boards for the new parameters. How to proceed? Do you know this values? Or is it Ok, if I add in arch/arm/mach-davinci/include/mach/aemif.h
a "#define AEMIF_VALUE_NOT USED 0xff" and if the new value is set to this I don't change this setting in the register?
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 arm,davinci: configure davinci aemif chipselects through OF
Hi Heiko,
Quoted 28 lines · Heiko Schocher
Quoted 28 lines
Quoted 28 lines
Quoted 28 lines
Quoted 21 lines
arch/arm/mach-davinci/aemif.c +3
-1@@ -13,12 +13,14 @@#include <linux/err.h> #include <linux/clk.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> #include <linux/time.h> #include <mach/aemif.h> /* Timing value configuration */ - +#define ASIZE (x) (x)#define TA (x) ((x) << 2)#define RHOLD (x) ((x) << 4)#define RSTROBE (x) ((x) << 7)arch/arm/mach-davinci/aemif.c line 32
Quoted 5 lines
arch/arm/mach-davinci/aemif.c +2
-0@@ -26,7 +28,10 @@#define WHOLD (x) ((x) << 17)#define WSTROBE (x) ((x) << 20)#define WSETUP (x) ((x) << 26) +#define EW (x) ((x) << 30) +#define SS (x) ((x) << 31)You are adding support for additional configuration parameters which should be done in a separate patch.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Hmm.. they are only used in the OF case ... is this split really needed?
But they should also be useful in the non-OF case, no? Why restrict their usage to the OF case?
I thought there is no need, because nobody needed this yet ... Ok, I change this too, so I have to fix this files:
True, no one configured these so far (relied on the defaults).
Also, they probably did not get added into the timing API since they are not really timing parameters. Its better to add a separate configuration API for these.
It will be better to move the driver outside of arch/arm/ before adding new features. I don't know if there is a place defined yet for "memory interface" drivers, so, drivers/misc/ may be the place.
I have added Greg and Arnd for their opinion in this.
Greg, Arnd,
Brief background:
DaVinci AEMIF is an async memory interface peripheral implemented in arch/arm/mach-davinci/aemif.c. It helps interface to NAND, NOR and other asynchronous memories. Currently it just provides an API for timing value configuration. The API is invoked by the MTD NAND driver.
Specification here: http://www.ti.com/lit/ug/sprue20c/sprue20c.pdf
We are looking at a place for this outside of architecture code.
Quoted 10 lines
$ grep -lr davinci_aemif_timing . ./arch/arm/mach-davinci/board-dm644x-evm.c ./arch/arm/mach-davinci/board-dm646x-evm.c ./arch/arm/mach-davinci/board-da850-evm.c ./arch/arm/mach-davinci/include/mach/aemif.h ./arch/arm/mach-davinci/include/mach/nand.h ./arch/arm/mach-davinci/board-da830-evm.c ./drivers/mtd/nand/davinci_nand.c
Quoted 2 lines
I do not know the values for this boards for the new parameters. How to proceed? Do you know this values? Or is it Ok, if I add in
If we add a new API, there won't be a need to touch existing board code.
Quoted 6 lines
arch/arm/mach-davinci/include/mach/aemif.h
a "#defineAEMIF_VALUE_NOTUSED0xff" and if the new value is set to this I don't change this setting in the register?bye,
Heiko
Thanks,
Sekhar
PATCH 1/1 arm, davinci: configure davinci aemif chipselects through OF
Quoted 9 lines · Sekhar
DaVinci AEMIF is an async memory interface peripheral implemented in
arch/arm/mach-davinci/aemif.c. It helps interface to NAND, NOR and other asynchronous memories. Currently it just provides an API for timing value configuration. The API is invoked by the MTD NAND driver.Specification here: http://www.ti.com/lit/ug/sprue20c/sprue20c.pdf
We are looking at a place for this outside of architecture code.
I think the best place depends on how you plan to use the sram interface. If all memory behind the aemif is handled by mtd, I would put the entire driver somewhere in the mtd infrastructure.
If you want it to provide endpoint devices that are handled by distinct subsystems in Linux, I would make it an mfd multifunction device and make the common code a driver that scans the connected memories in order to register its child devices for each of the subsystems.
Arnd
PATCH 1/1 arm,davinci: configure davinci aemif chipselects through OF
Hi Arnd,
Quoted 7 lines · Arnd Bergmann
Quoted 6 lines · Sekhar
DaVinci AEMIF is an async memory interface peripheral implemented in
arch/arm/mach-davinci/aemif.c. It helps interface to NAND, NOR and other asynchronous memories. Currently it just provides an API for timing value configuration. The API is invoked by the MTD NAND driver.
Quoted 7 lines
Quoted 3 lines
Specification here: http://www.ti.com/lit/ug/sprue20c/sprue20c.pdf
We are looking at a place for this outside of architecture code.
I think the best place depends on how you plan to use the sram interface. If all memory behind the aemif is handled by mtd, I would put the entire driver somewhere in the mtd infrastructure.
All the boards and use cases I have come across for AEMIF so far (5-6 years) have either used NAND or NOR memory.
Quoted 5 lines
If you want it to provide endpoint devices that are handled by distinct subsystems in Linux, I would make it an mfd multifunction device and make the common code a driver that scans the connected memories in order to register its child devices for each of the subsystems.
Okay. Thanks for the explanation. Since the users of AEMIF at this point are mtd devices, I propose moving it to drivers/mtd/davinci-aemif.c (of course, mtd folks need to approve).
Thanks,
Sekhar
PATCH 1/1 arm, davinci: configure davinci aemif chipselects through OF
Hello Arnd and Sekhar,
Quoted 9 lines · Sekhar
[...]
Quoted 6 lines · Arnd Bergmann
[...]
If you want it to provide endpoint devices that are handled by distinct subsystems in Linux, I would make it an mfd multifunction device and make the common code a driver that scans the connected memories in order to register its child devices for each of the subsystems.
Quoted 3 lines
Okay. Thanks for the explanation. Since the users of AEMIF at this point are mtd devices, I propose moving it to
drivers/mtd/davinci-aemif.c(of course, mtd folks need to approve).
We have a vested interest in the davinci AEMIF setup facilities in-kernel; I'm electing to pipe-up now rather than later. Sadly our board is not yet in mainline so my opinions may be redirected to the bit-bucket as you see fit. We are planning to post a patch series for our complete board support -- but we can't do it right now.
The AEMIF is useful for interfacing to other asynchronous devices too; our newest board uses it for accessing memory mapped FPGA functional blocks via UIO and for permanent storage to a compact flash using pata_platform. In both cases the timings and mode for the chip-select are manually configured before the devices are registered. In both cases the performance of the endpoints could be better preserved across CPU freq transitions if the hooks for cpufreq transitions recently proposed by Sudhakar [1] were part of an mfd device and hence applicable to devices other than mtd.
Again, I apologize for requesting features for boards that are not yet in mainline.
Best Regards,
Ben Gardiner
[1] http://article.gmane.org/gmane.linux.drivers.mtd/36876/
---
Nanometrics Inc.
http://www.nanometrics.ca
PATCH 1/1 arm,davinci: configure davinci aemif chipselects through OF
Hi Ben,
Quoted 32 lines · Ben Gardiner
Hello Arnd and Sekhar,
Quoted 9 lines · Sekhar
[...]
Quoted 6 lines · Arnd Bergmann
[...]
If you want it to provide endpoint devices that are handled by distinct subsystems in Linux, I would make it an mfd multifunction device and make the common code a driver that scans the connected memories in order to register its child devices for each of the subsystems.Quoted 3 lines
Okay. Thanks for the explanation. Since the users of AEMIF at this point are mtd devices, I propose moving it to
drivers/mtd/davinci-aemif.c(of course, mtd folks need to approve).We have a vested interest in the davinci AEMIF setup facilities in-kernel; I'm electing to pipe-up now rather than later. Sadly our board is not yet in mainline so my opinions may be redirected to the bit-bucket as you see fit. We are planning to post a patch series for our complete board support -- but we can't do it right now.
The AEMIF is useful for interfacing to other asynchronous devices too; our newest board uses it for accessing memory mapped FPGA functional blocks via UIO and for permanent storage to a compact flash using
pata_platform. In both cases the timings and mode for the chip-select are manually configured before the devices are registered. In both cases the performance of the endpoints could be better preserved across CPU freq transitions if the hooks for cpufreq transitions recently proposed by Sudhakar [1] were part of an mfd device and hence applicable to devices other than mtd.
Quoted 2 lines
Again, I apologize for requesting features for boards that are not yet in mainline.
No problem. Thanks for showing a use case I didn't know existed. Now we just need to find someone to work on moving aemif to mfd multifunction device :)
Thanks,
Sekhar