| Message ID | 20120715131618.GF4450@game.jcrosoft.org |
|---|---|
| State | New |
| Headers | show |
Hi Jean-Christophe, Comments below. As I mentioned in the SPEAr pull request just now, we are very close to final 3.5, so only fixes to bugs should go in. Please help me out a bit below. On Sun, Jul 15, 2012 at 6:16 AM, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote: > Jean-Christophe PLAGNIOL-VILLARD (4): > ARM: at91: gpio: implement request > ARM: at91: gpio: implement gpio_free Are the two above regressions, or just general improvement? Looks like the latter to me? > at91: regroup gpio and pinctrl under a simple-bus This looks like a cleanup patch to me. I'm also not sure that this is appropriate use of a simple-bus. The patch seems to have been posted for review the same day as you sent the pull request, and not cc:d to devicetree-discuss. > arm: at91: at91sam9x5: fix gpio number per bank Ok, this one I can believe is a proper fix. So, please let me know if I should just cherry-pick in the last patch and include that in the last 3.5 pull request, or if you want to respin the branch. Also, if any of the other patches are truly 3.5 material, I'd like to hear a bit more motivation as to why. Thanks! -Olof
On 20:54 Tue 17 Jul , Olof Johansson wrote: > Hi Jean-Christophe, > > Comments below. As I mentioned in the SPEAr pull request just now, we > are very close to final 3.5, so only fixes to bugs should go in. > Please help me out a bit below. > > On Sun, Jul 15, 2012 at 6:16 AM, Jean-Christophe PLAGNIOL-VILLARD > <plagnioj@jcrosoft.com> wrote: > > > Jean-Christophe PLAGNIOL-VILLARD (4): > > ARM: at91: gpio: implement request > > ARM: at91: gpio: implement gpio_free > > Are the two above regressions, or just general improvement? Looks like > the latter to me? On non dt I agree but on DT the gpio are not mux in the kernel. Today it work if the bootloader mux it as gpio so the kernel work onther wise BUG and this is the case today > > > at91: regroup gpio and pinctrl under a simple-bus > > This looks like a cleanup patch to me. I'm also not sure that this is > appropriate use of a simple-bus. The patch seems to have been posted > for review the same day as you sent the pull request, and not cc:d to > devicetree-discuss. forget to cc the devicetree but on at91 the pintcrl and gpio use the same registers so you must you the simple-bus As I've a huge patch series that I'm finishing to fix after found this bug on the gpio it really help to do not rebase it Best Regards, J.
On Tue, Jul 17, 2012 at 10:23 PM, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote: > On 20:54 Tue 17 Jul , Olof Johansson wrote: >> Hi Jean-Christophe, >> >> Comments below. As I mentioned in the SPEAr pull request just now, we >> are very close to final 3.5, so only fixes to bugs should go in. >> Please help me out a bit below. >> >> On Sun, Jul 15, 2012 at 6:16 AM, Jean-Christophe PLAGNIOL-VILLARD >> <plagnioj@jcrosoft.com> wrote: >> >> > Jean-Christophe PLAGNIOL-VILLARD (4): >> > ARM: at91: gpio: implement request >> > ARM: at91: gpio: implement gpio_free >> >> Are the two above regressions, or just general improvement? Looks like >> the latter to me? > On non dt I agree but on DT the gpio are not mux in the kernel. > Today it work if the bootloader mux it as gpio so the kernel work onther wise BUG > > and this is the case today Ok, thanks -- that makes sense. Care to roll those two patches into one and document that in the commit message? >> >> > at91: regroup gpio and pinctrl under a simple-bus >> >> This looks like a cleanup patch to me. I'm also not sure that this is >> appropriate use of a simple-bus. The patch seems to have been posted >> for review the same day as you sent the pull request, and not cc:d to >> devicetree-discuss. > forget to cc the devicetree but on at91 the pintcrl and gpio use the same > registers so you must you the simple-bus > > As I've a huge patch series that I'm finishing to fix after found this bug on > the gpio it really help to do not rebase it Defining the same device twice to have two drivers bind to it doesn't seem like the right solution here. That patch needs an acked-by from Rob Herring or Grant Likely before we pick it up. Existing device trees don't include the pinctrl nodes, so this is 3.6 material as far as I can tell. -Olof
On 22:41 Tue 17 Jul , Olof Johansson wrote: > On Tue, Jul 17, 2012 at 10:23 PM, Jean-Christophe PLAGNIOL-VILLARD > <plagnioj@jcrosoft.com> wrote: > > On 20:54 Tue 17 Jul , Olof Johansson wrote: > >> Hi Jean-Christophe, > >> > >> Comments below. As I mentioned in the SPEAr pull request just now, we > >> are very close to final 3.5, so only fixes to bugs should go in. > >> Please help me out a bit below. > >> > >> On Sun, Jul 15, 2012 at 6:16 AM, Jean-Christophe PLAGNIOL-VILLARD > >> <plagnioj@jcrosoft.com> wrote: > >> > >> > Jean-Christophe PLAGNIOL-VILLARD (4): > >> > ARM: at91: gpio: implement request > >> > ARM: at91: gpio: implement gpio_free > >> > >> Are the two above regressions, or just general improvement? Looks like > >> the latter to me? > > On non dt I agree but on DT the gpio are not mux in the kernel. > > Today it work if the bootloader mux it as gpio so the kernel work onther wise BUG > > > > and this is the case today > > Ok, thanks -- that makes sense. Care to roll those two patches into > one and document that in the commit message? no so much honestly as you wish > > >> > >> > at91: regroup gpio and pinctrl under a simple-bus > >> > >> This looks like a cleanup patch to me. I'm also not sure that this is > >> appropriate use of a simple-bus. The patch seems to have been posted > >> for review the same day as you sent the pull request, and not cc:d to > >> devicetree-discuss. > > forget to cc the devicetree but on at91 the pintcrl and gpio use the same > > registers so you must you the simple-bus > > > > As I've a huge patch series that I'm finishing to fix after found this bug on > > the gpio it really help to do not rebase it > > Defining the same device twice to have two drivers bind to it doesn't > seem like the right solution here. That patch needs an acked-by from > Rob Herring or Grant Likely before we pick it up. Existing device > trees don't include the pinctrl nodes, so this is 3.6 material as far > as I can tell. that's why I'm rewriting the pinctrl and gpio as one driver only but we need the simple bus to describe it as done on other SoC Imx as example so Rob can we have the ack to avoid to re-write about 20 patches that touch the dts of all the at91 socs Best Regards, J.
Hi, Two nits below. On Sun, Jul 15, 2012 at 03:40:37PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: > On the serie 5 bank b and d have 19 and 22 pins only. > > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > --- > .../devicetree/bindings/gpio/gpio_atmel.txt | 5 +++ > arch/arm/boot/dts/at91sam9x5.dtsi | 2 ++ > arch/arm/mach-at91/gpio.c | 33 +++++++++++++------- > 3 files changed, 29 insertions(+), 11 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpio/gpio_atmel.txt b/Documentation/devicetree/bindings/gpio/gpio_atmel.txt > index 66efc80..d8dd425 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio_atmel.txt > +++ b/Documentation/devicetree/bindings/gpio/gpio_atmel.txt > @@ -9,6 +9,10 @@ Required properties: > unused). > - gpio-controller: Marks the device node as a GPIO controller. > > +optional properties: > +- gpio-nb: Number of gpio if absent 32. "Number of gpios, if absent default is 32" > + > + > Example: > pioA: gpio@fffff200 { > compatible = "atmel,at91rm9200-gpio"; > @@ -16,5 +20,6 @@ Example: > interrupts = <2 4>; > #gpio-cells = <2>; > gpio-controller; > + gpio-nb = <32>; > }; Whitespace (tabs vs spaces) -Olof
On Wed, Jul 18, 2012 at 3:57 AM, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote: > On 22:41 Tue 17 Jul , Olof Johansson wrote: >> On Tue, Jul 17, 2012 at 10:23 PM, Jean-Christophe PLAGNIOL-VILLARD >> <plagnioj@jcrosoft.com> wrote: >> > On 20:54 Tue 17 Jul , Olof Johansson wrote: >> >> Hi Jean-Christophe, >> >> >> >> Comments below. As I mentioned in the SPEAr pull request just now, we >> >> are very close to final 3.5, so only fixes to bugs should go in. >> >> Please help me out a bit below. >> >> >> >> On Sun, Jul 15, 2012 at 6:16 AM, Jean-Christophe PLAGNIOL-VILLARD >> >> <plagnioj@jcrosoft.com> wrote: >> >> >> >> > Jean-Christophe PLAGNIOL-VILLARD (4): >> >> > ARM: at91: gpio: implement request >> >> > ARM: at91: gpio: implement gpio_free >> >> >> >> Are the two above regressions, or just general improvement? Looks like >> >> the latter to me? >> > On non dt I agree but on DT the gpio are not mux in the kernel. >> > Today it work if the bootloader mux it as gpio so the kernel work onther wise BUG >> > >> > and this is the case today >> >> Ok, thanks -- that makes sense. Care to roll those two patches into >> one and document that in the commit message? > no so much honestly as you wish >> >> >> >> >> > at91: regroup gpio and pinctrl under a simple-bus >> >> >> >> This looks like a cleanup patch to me. I'm also not sure that this is >> >> appropriate use of a simple-bus. The patch seems to have been posted >> >> for review the same day as you sent the pull request, and not cc:d to >> >> devicetree-discuss. >> > forget to cc the devicetree but on at91 the pintcrl and gpio use the same >> > registers so you must you the simple-bus >> > >> > As I've a huge patch series that I'm finishing to fix after found this bug on >> > the gpio it really help to do not rebase it >> >> Defining the same device twice to have two drivers bind to it doesn't >> seem like the right solution here. That patch needs an acked-by from >> Rob Herring or Grant Likely before we pick it up. Existing device >> trees don't include the pinctrl nodes, so this is 3.6 material as far >> as I can tell. > that's why I'm rewriting the pinctrl and gpio as one driver only > > but we need the simple bus to describe it as done on other SoC Imx as example > > so Rob can we have the ack to avoid to re-write about 20 patches that touch > the dts of all the at91 socs I'm sending in the last batch of fixes for 3.5 without this branch in it. It's also getting too late to get them into 3.6 at this time since the merge window is about to open and all code should already be in arm-soc. Jean-Christophe, when you've had a chance to clean up this branch and get it ready for upstream merging, please mark the patches for stable as appropriate to get them into 3.5.x. -Olof
On 07/15/2012 03:40 PM, Jean-Christophe PLAGNIOL-VILLARD : > confire the pin as pio when requested > > Cc: Nicolas Ferre <nicolas.ferre@atmel.com> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > --- > arch/arm/mach-at91/gpio.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c > index 325837a..994ed02 100644 > --- a/arch/arm/mach-at91/gpio.c > +++ b/arch/arm/mach-at91/gpio.c > @@ -44,6 +44,7 @@ struct at91_gpio_chip { > > #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip) > > +static int at91_gpiolib_request(struct gpio_chip *chip, unsigned offset); > static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip); > static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val); > static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset); > @@ -57,6 +58,7 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset); > { \ > .chip = { \ > .label = name, \ > + .request = at91_gpiolib_request, \ > .direction_input = at91_gpiolib_direction_input, \ > .direction_output = at91_gpiolib_direction_output, \ > .get = at91_gpiolib_get, \ > @@ -861,6 +863,18 @@ void __init at91_gpio_irq_setup(void) > } > > /* gpiolib support */ > + > + > +static int at91_gpiolib_request(struct gpio_chip *chip, unsigned offset) > +{ > + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip); > + void __iomem *pio = at91_gpio->regbase; > + unsigned mask = 1 << offset; > + > + __raw_writel(mask, pio + PIO_PER); > + return 0; > +} > + > static int at91_gpiolib_direction_input(struct gpio_chip *chip, > unsigned offset) > { >
On 07/15/2012 03:40 PM, Jean-Christophe PLAGNIOL-VILLARD : > configure a gpio as input when freeing it to reduce power consumption > > Cc: Nicolas Ferre <nicolas.ferre@atmel.com> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > --- > arch/arm/mach-at91/gpio.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c > index 994ed02..3833f82 100644 > --- a/arch/arm/mach-at91/gpio.c > +++ b/arch/arm/mach-at91/gpio.c > @@ -45,6 +45,7 @@ struct at91_gpio_chip { > #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip) > > static int at91_gpiolib_request(struct gpio_chip *chip, unsigned offset); > +static void at91_gpiolib_free(struct gpio_chip *chip, unsigned offset); > static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip); > static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val); > static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset); > @@ -59,6 +60,7 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset); > .chip = { \ > .label = name, \ > .request = at91_gpiolib_request, \ > + .free = at91_gpiolib_free, \ > .direction_input = at91_gpiolib_direction_input, \ > .direction_output = at91_gpiolib_direction_output, \ > .get = at91_gpiolib_get, \ > @@ -863,7 +865,10 @@ void __init at91_gpio_irq_setup(void) > } > > /* gpiolib support */ > - > +static void at91_gpiolib_free(struct gpio_chip *chip, unsigned offset) > +{ > + at91_gpiolib_direction_input(chip, offset); I know we talked about it, but now I am not sure. Maybe a safer solution would be to not touching the gpio configuration when freeing it... > +} > > static int at91_gpiolib_request(struct gpio_chip *chip, unsigned offset) > { >
On 07/15/2012 03:40 PM, Jean-Christophe PLAGNIOL-VILLARD : > Fix also the reg size as we have 512 bytes bank not 256 bytes per gpio/mux > controller Definitively, there is not enough information about the purpose of this patch: please elaborate a bit more the comment. Moreover, it seems strange to change the device tree files without introducing the pinctrl/pinmux driver (and the associated documentation) at the same time... So, no, not these changes now. > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > --- > arch/arm/boot/dts/at91sam9260.dtsi | 56 ++++++++++++--------- > arch/arm/boot/dts/at91sam9263.dtsi | 98 ++++++++++++++++++++---------------- > arch/arm/boot/dts/at91sam9g45.dtsi | 87 ++++++++++++++++++-------------- > arch/arm/boot/dts/at91sam9n12.dtsi | 79 ++++++++++++++++------------- > arch/arm/boot/dts/at91sam9x5.dtsi | 73 +++++++++++++++------------ > 5 files changed, 223 insertions(+), 170 deletions(-) > > diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi > index f449efc..1f6c3cd 100644 > --- a/arch/arm/boot/dts/at91sam9260.dtsi > +++ b/arch/arm/boot/dts/at91sam9260.dtsi > @@ -96,31 +96,41 @@ > interrupts = <26 4 27 4 28 4>; > }; > > - pioA: gpio@fffff400 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff400 0x100>; > - interrupts = <2 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > + pinctrl@fffff400 { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + compatible = "atmel,at91rm9200-pinctrl", "simple-bus"; > + reg = <0xfffff400 0x200 > + 0xfffff600 0x200 > + 0xfffff800 0x200>; > + > + pioA: gpio@80018000 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff400 0x200>; > + interrupts = <2 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > > - pioB: gpio@fffff600 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff600 0x100>; > - interrupts = <3 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > + pioB: gpio@fffff600 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff600 0x200>; > + interrupts = <3 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > > - pioC: gpio@fffff800 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff800 0x100>; > - interrupts = <4 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > + pioC: gpio@fffff800 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff800 0x200>; > + interrupts = <4 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > }; > > dbgu: serial@fffff200 { > diff --git a/arch/arm/boot/dts/at91sam9263.dtsi b/arch/arm/boot/dts/at91sam9263.dtsi > index 0209913..84ac493 100644 > --- a/arch/arm/boot/dts/at91sam9263.dtsi > +++ b/arch/arm/boot/dts/at91sam9263.dtsi > @@ -87,49 +87,61 @@ > reg = <0xfffffd10 0x10>; > }; > > - pioA: gpio@fffff200 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff200 0x100>; > - interrupts = <2 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > - > - pioB: gpio@fffff400 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff400 0x100>; > - interrupts = <3 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > - > - pioC: gpio@fffff600 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff600 0x100>; > - interrupts = <4 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > - > - pioD: gpio@fffff800 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff800 0x100>; > - interrupts = <4 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > - > - pioE: gpio@fffffa00 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffffa00 0x100>; > - interrupts = <4 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > + pinctrl@fffff200 { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + compatible = "atmel,at91rm9200-pinctrl", "simple-bus"; > + reg = <0xfffff200 0x200 > + 0xfffff400 0x200 > + 0xfffff600 0x200 > + 0xfffff800 0x200 > + 0xfffffa00 0x200>; > + > + pioA: gpio@fffff200 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff200 0x200>; > + interrupts = <2 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > + > + pioB: gpio@fffff400 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff400 0x200>; > + interrupts = <3 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > + > + pioC: gpio@fffff600 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff600 0x200>; > + interrupts = <4 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > + > + pioD: gpio@fffff800 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff800 0x200>; > + interrupts = <4 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > + > + pioE: gpio@fffffa00 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffffa00 0x200>; > + interrupts = <4 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > }; > > dbgu: serial@ffffee00 { > diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi > index 7dbccaf..9e03049 100644 > --- a/arch/arm/boot/dts/at91sam9g45.dtsi > +++ b/arch/arm/boot/dts/at91sam9g45.dtsi > @@ -105,49 +105,58 @@ > interrupts = <21 4>; > }; > > - pioA: gpio@fffff200 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff200 0x100>; > - interrupts = <2 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > + pinctrl@fffff200 { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + compatible = "atmel,at91rm9200-pinctrl", "simple-bus"; > + > + reg = <0xfffff200 0xa00>; > + > + pioA: gpio@fffff200 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff200 0x200>; > + interrupts = <2 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > > - pioB: gpio@fffff400 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff400 0x100>; > - interrupts = <3 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > + pioB: gpio@fffff400 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff400 0x200>; > + interrupts = <3 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > > - pioC: gpio@fffff600 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff600 0x100>; > - interrupts = <4 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > + pioC: gpio@fffff600 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff600 0x200>; > + interrupts = <4 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > > - pioD: gpio@fffff800 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffff800 0x100>; > - interrupts = <5 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > + pioD: gpio@fffff800 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffff800 0x200>; > + interrupts = <5 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > > - pioE: gpio@fffffa00 { > - compatible = "atmel,at91rm9200-gpio"; > - reg = <0xfffffa00 0x100>; > - interrupts = <5 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > + pioE: gpio@fffffa00 { > + compatible = "atmel,at91rm9200-gpio"; > + reg = <0xfffffa00 0x200>; > + interrupts = <5 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > }; > > dbgu: serial@ffffee00 { > diff --git a/arch/arm/boot/dts/at91sam9n12.dtsi b/arch/arm/boot/dts/at91sam9n12.dtsi > index cb84de7..7bd81c4 100644 > --- a/arch/arm/boot/dts/at91sam9n12.dtsi > +++ b/arch/arm/boot/dts/at91sam9n12.dtsi > @@ -100,40 +100,51 @@ > interrupts = <20 4>; > }; > > - pioA: gpio@fffff400 { > - compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > - reg = <0xfffff400 0x100>; > - interrupts = <2 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > - > - pioB: gpio@fffff600 { > - compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > - reg = <0xfffff600 0x100>; > - interrupts = <2 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > - > - pioC: gpio@fffff800 { > - compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > - reg = <0xfffff800 0x100>; > - interrupts = <3 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > - > - pioD: gpio@fffffa00 { > - compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > - reg = <0xfffffa00 0x100>; > - interrupts = <3 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > + pinctrl@fffff400 { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + compatible = "atmel,at91rm9200-pinctrl", "simple-bus"; > + reg = <0xfffff400 0x200 > + 0xfffff600 0x200 > + 0xfffff800 0x200 > + 0xfffffa00 0x200>; > + > + pioA: gpio@fffff400 { > + compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > + reg = <0xfffff400 0x200>; > + interrupts = <2 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > + > + pioB: gpio@fffff600 { > + compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > + reg = <0xfffff600 0x200>; > + interrupts = <2 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > + > + pioC: gpio@fffff800 { > + compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > + reg = <0xfffff800 0x200>; > + interrupts = <3 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > + > + pioD: gpio@fffffa00 { > + compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > + reg = <0xfffffa00 0x200>; > + interrupts = <3 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > }; > > dbgu: serial@fffff200 { > diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi > index 6b3ef43..e25c8d0 100644 > --- a/arch/arm/boot/dts/at91sam9x5.dtsi > +++ b/arch/arm/boot/dts/at91sam9x5.dtsi > @@ -107,40 +107,51 @@ > interrupts = <21 4>; > }; > > - pioA: gpio@fffff400 { > - compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > - reg = <0xfffff400 0x100>; > - interrupts = <2 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > + pinctrl@fffff200 { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + compatible = "atmel,at91rm9200-pinctrl", "simple-bus"; > + reg = <0xfffff400 0x200 > + 0xfffff600 0x200 > + 0xfffff800 0x200 > + 0xfffffa00 0x200>; > + > + pioA: gpio@fffff400 { > + compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > + reg = <0xfffff400 0x200>; > + interrupts = <2 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > > - pioB: gpio@fffff600 { > - compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > - reg = <0xfffff600 0x100>; > - interrupts = <2 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > + pioB: gpio@fffff600 { > + compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > + reg = <0xfffff600 0x200>; > + interrupts = <2 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > > - pioC: gpio@fffff800 { > - compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > - reg = <0xfffff800 0x100>; > - interrupts = <3 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > - }; > + pioC: gpio@fffff800 { > + compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > + reg = <0xfffff800 0x200>; > + interrupts = <3 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > > - pioD: gpio@fffffa00 { > - compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > - reg = <0xfffffa00 0x100>; > - interrupts = <3 4>; > - #gpio-cells = <2>; > - gpio-controller; > - interrupt-controller; > + pioD: gpio@fffffa00 { > + compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > + reg = <0xfffffa00 0x200>; > + interrupts = <3 4>; > + #gpio-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > }; > > dbgu: serial@fffff200 { >
On 07/15/2012 03:40 PM, Jean-Christophe PLAGNIOL-VILLARD : > On the serie 5 bank b and d have 19 and 22 pins only. No, it is not understandable. > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > --- > .../devicetree/bindings/gpio/gpio_atmel.txt | 5 +++ > arch/arm/boot/dts/at91sam9x5.dtsi | 2 ++ > arch/arm/mach-at91/gpio.c | 33 +++++++++++++------- > 3 files changed, 29 insertions(+), 11 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpio/gpio_atmel.txt b/Documentation/devicetree/bindings/gpio/gpio_atmel.txt > index 66efc80..d8dd425 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio_atmel.txt > +++ b/Documentation/devicetree/bindings/gpio/gpio_atmel.txt > @@ -9,6 +9,10 @@ Required properties: > unused). > - gpio-controller: Marks the device node as a GPIO controller. > > +optional properties: > +- gpio-nb: Number of gpio if absent 32. Cf. Olof's comments. > + > + > Example: > pioA: gpio@fffff200 { > compatible = "atmel,at91rm9200-gpio"; > @@ -16,5 +20,6 @@ Example: > interrupts = <2 4>; > #gpio-cells = <2>; > gpio-controller; > + gpio-nb = <32>; No need: 32 is the default. > }; > > diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi > index e25c8d0..e4d785b 100644 > --- a/arch/arm/boot/dts/at91sam9x5.dtsi > +++ b/arch/arm/boot/dts/at91sam9x5.dtsi > @@ -132,6 +132,7 @@ > interrupts = <2 4>; > #gpio-cells = <2>; > gpio-controller; > + gpio-nb = <19>; > interrupt-controller; > }; > > @@ -150,6 +151,7 @@ > interrupts = <3 4>; > #gpio-cells = <2>; > gpio-controller; > + gpio-nb = <22>; > interrupt-controller; > }; > }; > diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c > index 3833f82..1ab06e8 100644 > --- a/arch/arm/mach-at91/gpio.c > +++ b/arch/arm/mach-at91/gpio.c > @@ -31,6 +31,8 @@ > > #include "generic.h" > > +#define MAX_NB_GPIO_PER_BANK 32 > + > struct at91_gpio_chip { > struct gpio_chip chip; > struct at91_gpio_chip *next; /* Bank sharing same clock */ > @@ -55,7 +57,7 @@ static int at91_gpiolib_direction_input(struct gpio_chip *chip, > unsigned offset); > static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset); > > -#define AT91_GPIO_CHIP(name, nr_gpio) \ > +#define AT91_GPIO_CHIP(name) \ > { \ > .chip = { \ > .label = name, \ > @@ -67,16 +69,16 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset); > .set = at91_gpiolib_set, \ > .dbg_show = at91_gpiolib_dbg_show, \ > .to_irq = at91_gpiolib_to_irq, \ > - .ngpio = nr_gpio, \ > + .ngpio = MAX_NB_GPIO_PER_BANK, \ > }, \ > } > > static struct at91_gpio_chip gpio_chip[] = { > - AT91_GPIO_CHIP("pioA", 32), > - AT91_GPIO_CHIP("pioB", 32), > - AT91_GPIO_CHIP("pioC", 32), > - AT91_GPIO_CHIP("pioD", 32), > - AT91_GPIO_CHIP("pioE", 32), > + AT91_GPIO_CHIP("pioA"), > + AT91_GPIO_CHIP("pioB"), > + AT91_GPIO_CHIP("pioC"), > + AT91_GPIO_CHIP("pioD"), > + AT91_GPIO_CHIP("pioE"), > }; > > static int gpio_banks; > @@ -91,7 +93,7 @@ static unsigned long at91_gpio_caps; > > static inline void __iomem *pin_to_controller(unsigned pin) > { > - pin /= 32; > + pin /= MAX_NB_GPIO_PER_BANK; > if (likely(pin < gpio_banks)) > return gpio_chip[pin].regbase; > > @@ -100,7 +102,7 @@ static inline void __iomem *pin_to_controller(unsigned pin) > > static inline unsigned pin_to_mask(unsigned pin) > { > - return 1 << (pin % 32); > + return 1 << (pin % MAX_NB_GPIO_PER_BANK); > } > > > @@ -998,6 +1000,7 @@ static void __init of_at91_gpio_init_one(struct device_node *np) > { > int alias_idx; > struct at91_gpio_chip *at91_gpio; > + uint32_t ngpio; > > if (!np) > return; > @@ -1010,7 +1013,7 @@ static void __init of_at91_gpio_init_one(struct device_node *np) > } > > at91_gpio = &gpio_chip[alias_idx]; > - at91_gpio->chip.base = alias_idx * at91_gpio->chip.ngpio; > + at91_gpio->chip.base = alias_idx * MAX_NB_GPIO_PER_BANK; > > at91_gpio->regbase = of_iomap(np, 0); > if (!at91_gpio->regbase) { > @@ -1030,6 +1033,14 @@ static void __init of_at91_gpio_init_one(struct device_node *np) > if (of_device_is_compatible(np, "atmel,at91sam9x5-gpio")) > at91_gpio_caps |= AT91_GPIO_CAP_PIO3; > > + if (!of_property_read_u32(np, "gpio-nb", &ngpio)) { > + if (ngpio >= MAX_NB_GPIO_PER_BANK) > + pr_err("at91_gpio.%d, gpio-nb >= %d failback to %d\n", > + alias_idx, MAX_NB_GPIO_PER_BANK, MAX_NB_GPIO_PER_BANK); > + else > + at91_gpio->chip.ngpio = ngpio; > + } > + > /* Setup clock */ > if (at91_gpio_setup_clk(alias_idx)) > goto ioremap_err; > @@ -1067,7 +1078,7 @@ static void __init at91_gpio_init_one(int idx, u32 regbase, int pioc_hwirq) > { > struct at91_gpio_chip *at91_gpio = &gpio_chip[idx]; > > - at91_gpio->chip.base = idx * at91_gpio->chip.ngpio; > + at91_gpio->chip.base = idx * MAX_NB_GPIO_PER_BANK; > at91_gpio->pioc_hwirq = pioc_hwirq; > at91_gpio->pioc_idx = idx; Otherwise, I like it: so please resend it with corrections. Bye,
Hi, This patch series fix 2 issue on at91 first the gpio are not mux to gpio shen requested and free second on the sam9x5 series we ma not have 32 pins routed on each bank The following changes since commit 84a1caf1453c3d44050bd22db958af4a7f99315c: Linux 3.5-rc7 (2012-07-14 15:40:28 -0700) are available in the git repository at: git://github.com/at91linux/linux-at91.git tags/at91-for-3.5-gpio-fix for you to fetch changes up to 8b8d117749961ec387e7a3b1ecc29eb24c73ca9c: arm: at91: at91sam9x5: fix gpio number per bank (2012-07-15 20:46:06 +0800) ---------------------------------------------------------------- arm: at91: gpio fixes This patch series fix 2 issue on at91 first the gpio are not mux to gpio shen requested and free second on the sam9x5 series we ma not have 32 pins routed on each bank Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> ---------------------------------------------------------------- Jean-Christophe PLAGNIOL-VILLARD (4): ARM: at91: gpio: implement request ARM: at91: gpio: implement gpio_free at91: regroup gpio and pinctrl under a simple-bus arm: at91: at91sam9x5: fix gpio number per bank Documentation/devicetree/bindings/gpio/gpio_atmel.txt | 5 +++++ arch/arm/boot/dts/at91sam9260.dtsi | 56 +++++++++++++++++++++++++++++++++----------------------- arch/arm/boot/dts/at91sam9263.dtsi | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------- arch/arm/boot/dts/at91sam9g45.dtsi | 87 ++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------- arch/arm/boot/dts/at91sam9n12.dtsi | 79 +++++++++++++++++++++++++++++++++++++++++++++---------------------------------- arch/arm/boot/dts/at91sam9x5.dtsi | 75 ++++++++++++++++++++++++++++++++++++++++++++------------------------------- arch/arm/mach-at91/gpio.c | 52 +++++++++++++++++++++++++++++++++++++++++----------- 7 files changed, 271 insertions(+), 181 deletions(-) Best Regards, J.