Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 07:58 Tue 18 Dec , Wolfgang Grandegger wrote:
> On 12/18/2012 06:55 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 20:47 Mon 17 Dec , Wolfgang Grandegger wrote:
> >> On 12/17/2012 07:02 PM, Roland Stigge wrote:
> >>> On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
> /* Do synchronous data output with a single write access */
> __raw_writel(~mask, pio + PIO_OWDR);
> __raw_writel(mask, pio + PIO_OWER);
> __raw_writel(val, pio + PIO_ODSR);
>
> For caching we would need a storage. Not sure if it's worth compared to
> a context switch into the kernel.
> >>>
> >>> Block GPIO is not only for you in userspace. ;-) You can also implement
> >>> efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
> >>> always context switches involved.
> >>
> >> OK, what do you think about the following untested patch:
> >>
> >> From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
> >> From: Wolfgang Grandegger
> >> Date: Mon, 3 Dec 2012 08:31:55 +0100
> >> Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
> >>
> >> Signed-off-by: Wolfgang Grandegger
> >> ---
> >> arch/arm/mach-at91/gpio.c | 29 +
> >> 1 file changed, 29 insertions(+)
> >>
> >> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
> >> index be42cf0..cf6bd45 100644
> >> --- a/arch/arm/mach-at91/gpio.c
> >> +++ b/arch/arm/mach-at91/gpio.c
> >> @@ -42,13 +42,16 @@ struct at91_gpio_chip {
> >>void __iomem*regbase; /* PIO bank virtual address */
> >>struct clk *clock; /* associated clock */
> >>struct irq_domain *domain;/* associated irq domain */
> >> + unsigned long mask_shadow;/* synchronous data output */
> >> };
> >>
> >> #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
> >>
> >> 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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
> >> mask, unsigned long val);
> >> static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
> >> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip,
> >> unsigned long mask);
> >> static int at91_gpiolib_direction_output(struct gpio_chip *chip,
> >> unsigned offset, int val);
> >> static int at91_gpiolib_direction_input(struct gpio_chip *chip,
> >> @@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
> >> unsigned offset);
> >>.direction_input = at91_gpiolib_direction_input, \
> >>.direction_output = at91_gpiolib_direction_output, \
> >>.get = at91_gpiolib_get, \
> >> + .get_block= at91_gpiolib_get_block, \
> >>.set = at91_gpiolib_set, \
> >> + .set_block= at91_gpiolib_set_block, \
> >>.dbg_show = at91_gpiolib_dbg_show, \
> >>.to_irq = at91_gpiolib_to_irq,\
> >>.ngpio= nr_gpio,\
> >> @@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
> >> unsigned offset)
> >>return (pdsr & mask) != 0;
> >> }
> >>
> >> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip,
> >> unsigned long mask)
> >> +{
> >> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> >> + void __iomem *pio = at91_gpio->regbase;
> >> + u32 pdsr;
> >> +
> >> + pdsr = __raw_readl(pio + PIO_PDSR);
> >> + return pdsr & mask;
> >> +}
> >> +
> >> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int
> >> val)
> >> {
> >>struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> >> @@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
> >> unsigned offset, int val)
> >>__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
> >> }
> >>
> >> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
> >> mask, unsigned long val)
> >> +{
> >> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> >> + void __iomem *pio = at91_gpio->regbase;
> >> +
> >> + /* Do synchronous data output with a single write access */
> >> + if (mask != at91_gpio->mask_shadow) {
> >> + at91_gpio->mask_shadow = mask;
> >> + __raw_writel(~mask, pio + PIO_OWDR);
> >> + __raw_writel(mask, pio + PIO_OWER);
> >> + }
> >> + __raw_writel(val, pio + PIO_ODSR);
> >> +}
> > this driver is only for old at91 platfrom if you touch at91 you need to
> > update
> > the pinctrl too
>
> Well, the patch is for the hardware I have at hand and I can test. There
> are many other GPIO hardware
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 07:58 Tue 18 Dec , Wolfgang Grandegger wrote:
On 12/18/2012 06:55 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 20:47 Mon 17 Dec , Wolfgang Grandegger wrote:
On 12/17/2012 07:02 PM, Roland Stigge wrote:
On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
/* Do synchronous data output with a single write access */
__raw_writel(~mask, pio + PIO_OWDR);
__raw_writel(mask, pio + PIO_OWER);
__raw_writel(val, pio + PIO_ODSR);
For caching we would need a storage. Not sure if it's worth compared to
a context switch into the kernel.
Block GPIO is not only for you in userspace. ;-) You can also implement
efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
always context switches involved.
OK, what do you think about the following untested patch:
From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger w...@grandegger.com
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger w...@grandegger.com
---
arch/arm/mach-at91/gpio.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..cf6bd45 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -42,13 +42,16 @@ struct at91_gpio_chip {
void __iomem*regbase; /* PIO bank virtual address */
struct clk *clock; /* associated clock */
struct irq_domain *domain;/* associated irq domain */
+ unsigned long mask_shadow;/* synchronous data output */
};
#define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip,
unsigned long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block= at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block= at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq,\
.ngpio= nr_gpio,\
@@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
unsigned offset)
return (pdsr mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip,
unsigned long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int
val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ /* Do synchronous data output with a single write access */
+ if (mask != at91_gpio-mask_shadow) {
+ at91_gpio-mask_shadow = mask;
+ __raw_writel(~mask, pio + PIO_OWDR);
+ __raw_writel(mask, pio + PIO_OWER);
+ }
+ __raw_writel(val, pio + PIO_ODSR);
+}
this driver is only for old at91 platfrom if you touch at91 you need to
update
the pinctrl too
Well, the patch is for the hardware I have at hand and I can test. There
are many other GPIO hardware interfaces which could be enhanced with
block gpio. Roland only did it for the interfaces in driver/gpio. Also,
I think, an ACK for this patch series would be nice before we continue.
no sorry the support of the pinctrl is mandatory
check drivers/pinctrl/pinctrl-at91.c
as this new drivers include both
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/18/2012 06:55 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 20:47 Mon 17 Dec , Wolfgang Grandegger wrote:
>> On 12/17/2012 07:02 PM, Roland Stigge wrote:
>>> On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
/* Do synchronous data output with a single write access */
__raw_writel(~mask, pio + PIO_OWDR);
__raw_writel(mask, pio + PIO_OWER);
__raw_writel(val, pio + PIO_ODSR);
For caching we would need a storage. Not sure if it's worth compared to
a context switch into the kernel.
>>>
>>> Block GPIO is not only for you in userspace. ;-) You can also implement
>>> efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
>>> always context switches involved.
>>
>> OK, what do you think about the following untested patch:
>>
>> From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
>> From: Wolfgang Grandegger
>> Date: Mon, 3 Dec 2012 08:31:55 +0100
>> Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
>>
>> Signed-off-by: Wolfgang Grandegger
>> ---
>> arch/arm/mach-at91/gpio.c | 29 +
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
>> index be42cf0..cf6bd45 100644
>> --- a/arch/arm/mach-at91/gpio.c
>> +++ b/arch/arm/mach-at91/gpio.c
>> @@ -42,13 +42,16 @@ struct at91_gpio_chip {
>> void __iomem*regbase; /* PIO bank virtual address */
>> struct clk *clock; /* associated clock */
>> struct irq_domain *domain;/* associated irq domain */
>> +unsigned long mask_shadow;/* synchronous data output */
>> };
>>
>> #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
>>
>> 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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
>> mask, unsigned long val);
>> static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
>> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip,
>> unsigned long mask);
>> static int at91_gpiolib_direction_output(struct gpio_chip *chip,
>> unsigned offset, int val);
>> static int at91_gpiolib_direction_input(struct gpio_chip *chip,
>> @@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
>> unsigned offset);
>> .direction_input = at91_gpiolib_direction_input, \
>> .direction_output = at91_gpiolib_direction_output, \
>> .get = at91_gpiolib_get, \
>> +.get_block= at91_gpiolib_get_block, \
>> .set = at91_gpiolib_set, \
>> +.set_block= at91_gpiolib_set_block, \
>> .dbg_show = at91_gpiolib_dbg_show, \
>> .to_irq = at91_gpiolib_to_irq,\
>> .ngpio= nr_gpio,\
>> @@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
>> unsigned offset)
>> return (pdsr & mask) != 0;
>> }
>>
>> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip,
>> unsigned long mask)
>> +{
>> +struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> +void __iomem *pio = at91_gpio->regbase;
>> +u32 pdsr;
>> +
>> +pdsr = __raw_readl(pio + PIO_PDSR);
>> +return pdsr & mask;
>> +}
>> +
>> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int
>> val)
>> {
>> struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> @@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
>> unsigned offset, int val)
>> __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
>> }
>>
>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
>> mask, unsigned long val)
>> +{
>> +struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> +void __iomem *pio = at91_gpio->regbase;
>> +
>> +/* Do synchronous data output with a single write access */
>> +if (mask != at91_gpio->mask_shadow) {
>> +at91_gpio->mask_shadow = mask;
>> +__raw_writel(~mask, pio + PIO_OWDR);
>> +__raw_writel(mask, pio + PIO_OWER);
>> +}
>> +__raw_writel(val, pio + PIO_ODSR);
>> +}
> this driver is only for old at91 platfrom if you touch at91 you need to update
> the pinctrl too
Well, the patch is for the hardware I have at hand and I can test. There
are many other GPIO hardware interfaces which could be enhanced with
block gpio. Roland only did it for the interfaces in driver/gpio. Also,
I think, an ACK for this patch series would be nice before we continue.
Wolfgang.
--
To
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 10:33 PM, Roland Stigge wrote: > On 17/12/12 20:47, Wolfgang Grandegger wrote: >> On 12/17/2012 07:02 PM, Roland Stigge wrote: >>> On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote: /* Do synchronous data output with a single write access */ __raw_writel(~mask, pio + PIO_OWDR); __raw_writel(mask, pio + PIO_OWER); __raw_writel(val, pio + PIO_ODSR); For caching we would need a storage. Not sure if it's worth compared to a context switch into the kernel. >>> >>> Block GPIO is not only for you in userspace. ;-) You can also implement >>> efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not >>> always context switches involved. >> >> OK, what do you think about the following untested patch: > > Looks good! > > Why "untested"? ;-) Because I didn't have a chance to test it yet. Will do tomorrow. Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 20:47 Mon 17 Dec , Wolfgang Grandegger wrote:
> On 12/17/2012 07:02 PM, Roland Stigge wrote:
> > On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
> >>/* Do synchronous data output with a single write access */
> >>__raw_writel(~mask, pio + PIO_OWDR);
> >>__raw_writel(mask, pio + PIO_OWER);
> >>__raw_writel(val, pio + PIO_ODSR);
> >>
> >> For caching we would need a storage. Not sure if it's worth compared to
> >> a context switch into the kernel.
> >
> > Block GPIO is not only for you in userspace. ;-) You can also implement
> > efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
> > always context switches involved.
>
> OK, what do you think about the following untested patch:
>
> From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
> From: Wolfgang Grandegger
> Date: Mon, 3 Dec 2012 08:31:55 +0100
> Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
>
> Signed-off-by: Wolfgang Grandegger
> ---
> arch/arm/mach-at91/gpio.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
> index be42cf0..cf6bd45 100644
> --- a/arch/arm/mach-at91/gpio.c
> +++ b/arch/arm/mach-at91/gpio.c
> @@ -42,13 +42,16 @@ struct at91_gpio_chip {
> void __iomem*regbase; /* PIO bank virtual address */
> struct clk *clock; /* associated clock */
> struct irq_domain *domain;/* associated irq domain */
> + unsigned long mask_shadow;/* synchronous data output */
> };
>
> #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
>
> 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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
> mask, unsigned long val);
> static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
> long mask);
> static int at91_gpiolib_direction_output(struct gpio_chip *chip,
>unsigned offset, int val);
> static int at91_gpiolib_direction_input(struct gpio_chip *chip,
> @@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
> unsigned offset);
> .direction_input = at91_gpiolib_direction_input, \
> .direction_output = at91_gpiolib_direction_output, \
> .get = at91_gpiolib_get, \
> + .get_block= at91_gpiolib_get_block, \
> .set = at91_gpiolib_set, \
> + .set_block= at91_gpiolib_set_block, \
> .dbg_show = at91_gpiolib_dbg_show, \
> .to_irq = at91_gpiolib_to_irq,\
> .ngpio= nr_gpio,\
> @@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
> unsigned offset)
> return (pdsr & mask) != 0;
> }
>
> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
> long mask)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> + u32 pdsr;
> +
> + pdsr = __raw_readl(pio + PIO_PDSR);
> + return pdsr & mask;
> +}
> +
> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int
> val)
> {
> struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> @@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
> unsigned offset, int val)
> __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
> }
>
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
> mask, unsigned long val)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> +
> + /* Do synchronous data output with a single write access */
> + if (mask != at91_gpio->mask_shadow) {
> + at91_gpio->mask_shadow = mask;
> + __raw_writel(~mask, pio + PIO_OWDR);
> + __raw_writel(mask, pio + PIO_OWER);
> + }
> + __raw_writel(val, pio + PIO_ODSR);
> +}
this driver is only for old at91 platfrom if you touch at91 you need to update
the pinctrl too
Best Regards,
J.
> +
> static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> {
> int i;
> --
> 1.7.9.5
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 17/12/12 20:47, Wolfgang Grandegger wrote:
> On 12/17/2012 07:02 PM, Roland Stigge wrote:
>> On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
>>> /* Do synchronous data output with a single write access */
>>> __raw_writel(~mask, pio + PIO_OWDR);
>>> __raw_writel(mask, pio + PIO_OWER);
>>> __raw_writel(val, pio + PIO_ODSR);
>>>
>>> For caching we would need a storage. Not sure if it's worth compared to
>>> a context switch into the kernel.
>>
>> Block GPIO is not only for you in userspace. ;-) You can also implement
>> efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
>> always context switches involved.
>
> OK, what do you think about the following untested patch:
Looks good!
Why "untested"? ;-)
Thanks,
Roland
>>From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
> From: Wolfgang Grandegger
> Date: Mon, 3 Dec 2012 08:31:55 +0100
> Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
>
> Signed-off-by: Wolfgang Grandegger
> ---
> arch/arm/mach-at91/gpio.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
> index be42cf0..cf6bd45 100644
> --- a/arch/arm/mach-at91/gpio.c
> +++ b/arch/arm/mach-at91/gpio.c
> @@ -42,13 +42,16 @@ struct at91_gpio_chip {
> void __iomem*regbase; /* PIO bank virtual address */
> struct clk *clock; /* associated clock */
> struct irq_domain *domain;/* associated irq domain */
> + unsigned long mask_shadow;/* synchronous data output */
> };
>
> #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
>
> 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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
> mask, unsigned long val);
> static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
> long mask);
> static int at91_gpiolib_direction_output(struct gpio_chip *chip,
>unsigned offset, int val);
> static int at91_gpiolib_direction_input(struct gpio_chip *chip,
> @@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
> unsigned offset);
> .direction_input = at91_gpiolib_direction_input, \
> .direction_output = at91_gpiolib_direction_output, \
> .get = at91_gpiolib_get, \
> + .get_block= at91_gpiolib_get_block, \
> .set = at91_gpiolib_set, \
> + .set_block= at91_gpiolib_set_block, \
> .dbg_show = at91_gpiolib_dbg_show, \
> .to_irq = at91_gpiolib_to_irq,\
> .ngpio= nr_gpio,\
> @@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
> unsigned offset)
> return (pdsr & mask) != 0;
> }
>
> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
> long mask)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> + u32 pdsr;
> +
> + pdsr = __raw_readl(pio + PIO_PDSR);
> + return pdsr & mask;
> +}
> +
> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int
> val)
> {
> struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> @@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
> unsigned offset, int val)
> __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
> }
>
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
> mask, unsigned long val)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> +
> + /* Do synchronous data output with a single write access */
> + if (mask != at91_gpio->mask_shadow) {
> + at91_gpio->mask_shadow = mask;
> + __raw_writel(~mask, pio + PIO_OWDR);
> + __raw_writel(mask, pio + PIO_OWER);
> + }
> + __raw_writel(val, pio + PIO_ODSR);
> +}
> +
> static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> {
> int i;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 07:02 PM, Roland Stigge wrote:
> On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
>> /* Do synchronous data output with a single write access */
>> __raw_writel(~mask, pio + PIO_OWDR);
>> __raw_writel(mask, pio + PIO_OWER);
>> __raw_writel(val, pio + PIO_ODSR);
>>
>> For caching we would need a storage. Not sure if it's worth compared to
>> a context switch into the kernel.
>
> Block GPIO is not only for you in userspace. ;-) You can also implement
> efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
> always context switches involved.
OK, what do you think about the following untested patch:
>From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger
---
arch/arm/mach-at91/gpio.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..cf6bd45 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -42,13 +42,16 @@ struct at91_gpio_chip {
void __iomem*regbase; /* PIO bank virtual address */
struct clk *clock; /* associated clock */
struct irq_domain *domain;/* associated irq domain */
+ unsigned long mask_shadow;/* synchronous data output */
};
#define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block= at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block= at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq,\
.ngpio= nr_gpio,\
@@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
unsigned offset)
return (pdsr & mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr & mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+
+ /* Do synchronous data output with a single write access */
+ if (mask != at91_gpio->mask_shadow) {
+ at91_gpio->mask_shadow = mask;
+ __raw_writel(~mask, pio + PIO_OWDR);
+ __raw_writel(mask, pio + PIO_OWER);
+ }
+ __raw_writel(val, pio + PIO_ODSR);
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote: > /* Do synchronous data output with a single write access */ > __raw_writel(~mask, pio + PIO_OWDR); > __raw_writel(mask, pio + PIO_OWER); > __raw_writel(val, pio + PIO_ODSR); > > For caching we would need a storage. Not sure if it's worth compared to > a context switch into the kernel. Block GPIO is not only for you in userspace. ;-) You can also implement efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not always context switches involved. Roland -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 06:15 PM, Roland Stigge wrote:
> On 12/17/2012 05:28 PM, Wolfgang Grandegger wrote:
>> On 12/17/2012 02:51 PM, Roland Stigge wrote:
>>> Hi Wolfgang,
And I guess Russell is right: If possible, we should write outputs
simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate
set/clear.
I wonder if we need to save/restore the state of OWSR at every write
operation or if we need/can cache it. Assuming that block GPIO are the
only code in the kernel that manipulates ODSR.
>>>
>>> Can you please test the following:
>>>
>>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
>>> mask, unsigned long val)
>>> +{
>>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>>> + void __iomem *pio = at91_gpio->regbase;
>>> +
>>> + __raw_writel(~mask, pio + PIO_OWDR);
>>
>> This would also disable normal GPIOs configured for output! From the
>> manual I understand that if the pin is configured for output, we could
>> either use PIO_SODR/PIO_CODR to set/clear the bits individually or
>> PIO_ODSR for synchronous data output. But than we need to care about the
>> non-block GPIO outputs as well... requiring a read-modify-write cycle :(.
>
>>From the manual, I read about OWER: "Enables writing PIO_ODSR for the
> I/O line" (analogous for OWDR). Would interpret this as affecting ODSR
> (for block GPIO) but not SODR/CODR (as currently with single GPIOs).
>
> Have you tried? ;-)
Grrr, I mixed OER with OWER, sorry for the noise. Back to your approach,
which works.
/* Do synchronous data output with a single write access */
__raw_writel(~mask, pio + PIO_OWDR);
__raw_writel(mask, pio + PIO_OWER);
__raw_writel(val, pio + PIO_ODSR);
For caching we would need a storage. Not sure if it's worth compared to
a context switch into the kernel.
Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 05:28 PM, Wolfgang Grandegger wrote:
> On 12/17/2012 02:51 PM, Roland Stigge wrote:
>> Hi Wolfgang,
>>> And I guess Russell is right: If possible, we should write outputs
>>> simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
>>>
>>> I wonder if we need to save/restore the state of OWSR at every write
>>> operation or if we need/can cache it. Assuming that block GPIO are the
>>> only code in the kernel that manipulates ODSR.
>>
>> Can you please test the following:
>>
>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
>> mask, unsigned long val)
>> +{
>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> + void __iomem *pio = at91_gpio->regbase;
>> +
>> + __raw_writel(~mask, pio + PIO_OWDR);
>
> This would also disable normal GPIOs configured for output! From the
> manual I understand that if the pin is configured for output, we could
> either use PIO_SODR/PIO_CODR to set/clear the bits individually or
> PIO_ODSR for synchronous data output. But than we need to care about the
> non-block GPIO outputs as well... requiring a read-modify-write cycle :(.
>From the manual, I read about OWER: "Enables writing PIO_ODSR for the
I/O line" (analogous for OWDR). Would interpret this as affecting ODSR
(for block GPIO) but not SODR/CODR (as currently with single GPIOs).
Have you tried? ;-)
Thanks in advance,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 02:51 PM, Roland Stigge wrote:
> Hi Wolfgang,
>
> On 12/17/2012 02:32 PM, Roland Stigge wrote:
>> And I guess Russell is right: If possible, we should write outputs
>> simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
>>
>> I wonder if we need to save/restore the state of OWSR at every write
>> operation or if we need/can cache it. Assuming that block GPIO are the
>> only code in the kernel that manipulates ODSR.
>
> Can you please test the following:
>
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
> mask, unsigned long val)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> +
> + __raw_writel(~mask, pio + PIO_OWDR);
This would also disable normal GPIOs configured for output! From the
manual I understand that if the pin is configured for output, we could
either use PIO_SODR/PIO_CODR to set/clear the bits individually or
PIO_ODSR for synchronous data output. But than we need to care about the
non-block GPIO outputs as well... requiring a read-modify-write cycle :(.
Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 01:10 PM, Russell King - ARM Linux wrote:
> On Mon, Dec 17, 2012 at 12:51:32PM +0100, Wolfgang Grandegger wrote:
>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
>> mask, unsigned long val)
>> +{
>> +struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> +void __iomem *pio = at91_gpio->regbase;
>> +u32 set_bits = val & mask;
>> +u32 clr_bits = ~val & mask;
>> +
>> +/* GPIO outputs can only be set at once or cleared at once */
>> +if (set_bits)
>> +__raw_writel(set_bits, pio + PIO_SODR);
>> +if (clr_bits)
>> +__raw_writel(clr_bits, pio + PIO_CODR);
>> +}
>
> You obviously didn't see my email...
Well, I did read you mail but it was not obvious to me how to maintain
compatibility with the existing "set" method... at a first glance.
But the synchronous data output is important and it's even simpler than
I thought. The block-set callback can directly write to ODSR because
"at91_set_gpio_output()" already has set OWER. Will give that a try
later today.
Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
Hi Wolfgang,
On 12/17/2012 02:32 PM, Roland Stigge wrote:
> And I guess Russell is right: If possible, we should write outputs
> simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
>
> I wonder if we need to save/restore the state of OWSR at every write
> operation or if we need/can cache it. Assuming that block GPIO are the
> only code in the kernel that manipulates ODSR.
Can you please test the following:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+
+ __raw_writel(~mask, pio + PIO_OWDR);
+ __raw_writel(mask, pio + PIO_OWER);
+ __raw_writel(val, pio + PIO_ODSR);
+}
Would caching OWSR be a significant speedup here?
Thanks in advance,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 12:51 PM, Wolfgang Grandegger wrote: >>> Without having an AT91 available right now, I guess the hardware >>> interface of this GPIO chip is different from the GPIO block API. While >>> the hardware has clear and set registers, the val parameter of >>> at91_gpiolib_set_block() should be interpreted as the actual output >>> values. See lpc32xx_gpo_set_block() for an example for handling set and >>> clear registers like this: First, set_bits and clear_bits words are >>> calculated from mask and val parameters, and finally written to the >>> respective hardware registers. >>> >>> Note that one .set_block() can result in writing both the set and clear >>> registers of the hardware when val contains both 0s and 1s in >>> respectively masked positions. >> >> Oops, I obviously did not test GPIO block write. The patch below does >> work now. Feel free to add it to the next version of your series. > > The patch lacks an important fix, sorry. Please consider the updated > patch below. Thanks! And I guess Russell is right: If possible, we should write outputs simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear. I wonder if we need to save/restore the state of OWSR at every write operation or if we need/can cache it. Assuming that block GPIO are the only code in the kernel that manipulates ODSR. Further: Can we include this patch for arch/arm/mach-at91 via the gpio subsystem or does it need to go separately via arm-soc.git? Thanks, Roland -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On Mon, Dec 17, 2012 at 12:51:32PM +0100, Wolfgang Grandegger wrote:
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
> mask, unsigned long val)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> + u32 set_bits = val & mask;
> + u32 clr_bits = ~val & mask;
> +
> + /* GPIO outputs can only be set at once or cleared at once */
> + if (set_bits)
> + __raw_writel(set_bits, pio + PIO_SODR);
> + if (clr_bits)
> + __raw_writel(clr_bits, pio + PIO_CODR);
> +}
You obviously didn't see my email...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 12:37 PM, Wolfgang Grandegger wrote:
> Hi Roland,
>
> On 12/15/2012 12:49 AM, Roland Stigge wrote:
>> Hi Wolfgang,
>>
>> thank you for the patch!
>>
>> On 14/12/12 18:58, Wolfgang Grandegger wrote:
>>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
>>> mask, unsigned long val)
>>> +{
>>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>>> + void __iomem *pio = at91_gpio->regbase;
>>> +
>>> + __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
>>> +}
>>> +
>>
>> Without having an AT91 available right now, I guess the hardware
>> interface of this GPIO chip is different from the GPIO block API. While
>> the hardware has clear and set registers, the val parameter of
>> at91_gpiolib_set_block() should be interpreted as the actual output
>> values. See lpc32xx_gpo_set_block() for an example for handling set and
>> clear registers like this: First, set_bits and clear_bits words are
>> calculated from mask and val parameters, and finally written to the
>> respective hardware registers.
>>
>> Note that one .set_block() can result in writing both the set and clear
>> registers of the hardware when val contains both 0s and 1s in
>> respectively masked positions.
>
> Oops, I obviously did not test GPIO block write. The patch below does
> work now. Feel free to add it to the next version of your series.
The patch lacks an important fix, sorry. Please consider the updated
patch below.
Wolfgang.
>From a1f93ddea9c6c9c6a80d7a02d3c8d9902823fe47 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger
---
arch/arm/mach-at91/gpio.c | 28
1 file changed, 28 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..77c6f91 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -48,7 +48,9 @@ struct at91_gpio_chip {
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +64,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block= at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block= at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq,\
.ngpio= nr_gpio,\
@@ -896,6 +900,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
unsigned offset)
return (pdsr & mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr & mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +919,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 set_bits = val & mask;
+ u32 clr_bits = ~val & mask;
+
+ /* GPIO outputs can only be set at once or cleared at once */
+ if (set_bits)
+ __raw_writel(set_bits, pio + PIO_SODR);
+ if (clr_bits)
+ __raw_writel(clr_bits, pio + PIO_CODR);
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
Hi Roland,
On 12/15/2012 12:49 AM, Roland Stigge wrote:
> Hi Wolfgang,
>
> thank you for the patch!
>
> On 14/12/12 18:58, Wolfgang Grandegger wrote:
>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
>> mask, unsigned long val)
>> +{
>> +struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> +void __iomem *pio = at91_gpio->regbase;
>> +
>> +__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
>> +}
>> +
>
> Without having an AT91 available right now, I guess the hardware
> interface of this GPIO chip is different from the GPIO block API. While
> the hardware has clear and set registers, the val parameter of
> at91_gpiolib_set_block() should be interpreted as the actual output
> values. See lpc32xx_gpo_set_block() for an example for handling set and
> clear registers like this: First, set_bits and clear_bits words are
> calculated from mask and val parameters, and finally written to the
> respective hardware registers.
>
> Note that one .set_block() can result in writing both the set and clear
> registers of the hardware when val contains both 0s and 1s in
> respectively masked positions.
Oops, I obviously did not test GPIO block write. The patch below does
work now. Feel free to add it to the next version of your series.
I tested with a block having both, inputs and outputs. The handling
of such a mixed block is clumsy because both, read and write do depend
on "block->cur_mask". It needs to be re-set when switching from read to
write or vice versa. Defining two blocks, one for input and the other
for output seems to be the better solution.
Hope this block gpio support will show up in mainline soon.
Thanks.
Wolfgang.
>From 6249995d129b290704cacb2c0114782414abeba7 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger
---
arch/arm/mach-at91/gpio.c | 28
1 file changed, 28 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..0998854 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -48,7 +48,9 @@ struct at91_gpio_chip {
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +64,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block= at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block= at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq,\
.ngpio= nr_gpio,\
@@ -896,6 +900,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
unsigned offset)
return (pdsr & mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr & mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +919,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 set_bits = values & mask;
+ u32 clr_bits = ~values & mask;
+
+ /* GPIO outputs can only be set at once or cleared at once */
+ if (set_bits)
+ __raw_writel(set_bits, pio + PIO_SODR);
+ if (clr_bits)
+ __raw_writel(clr_bits, pio + PIO_CODR);
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 05:28 PM, Wolfgang Grandegger wrote:
On 12/17/2012 02:51 PM, Roland Stigge wrote:
Hi Wolfgang,
And I guess Russell is right: If possible, we should write outputs
simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
I wonder if we need to save/restore the state of OWSR at every write
operation or if we need/can cache it. Assuming that block GPIO are the
only code in the kernel that manipulates ODSR.
Can you please test the following:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ __raw_writel(~mask, pio + PIO_OWDR);
This would also disable normal GPIOs configured for output! From the
manual I understand that if the pin is configured for output, we could
either use PIO_SODR/PIO_CODR to set/clear the bits individually or
PIO_ODSR for synchronous data output. But than we need to care about the
non-block GPIO outputs as well... requiring a read-modify-write cycle :(.
From the manual, I read about OWER: Enables writing PIO_ODSR for the
I/O line (analogous for OWDR). Would interpret this as affecting ODSR
(for block GPIO) but not SODR/CODR (as currently with single GPIOs).
Have you tried? ;-)
Thanks in advance,
Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 06:15 PM, Roland Stigge wrote:
On 12/17/2012 05:28 PM, Wolfgang Grandegger wrote:
On 12/17/2012 02:51 PM, Roland Stigge wrote:
Hi Wolfgang,
And I guess Russell is right: If possible, we should write outputs
simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate
set/clear.
I wonder if we need to save/restore the state of OWSR at every write
operation or if we need/can cache it. Assuming that block GPIO are the
only code in the kernel that manipulates ODSR.
Can you please test the following:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ __raw_writel(~mask, pio + PIO_OWDR);
This would also disable normal GPIOs configured for output! From the
manual I understand that if the pin is configured for output, we could
either use PIO_SODR/PIO_CODR to set/clear the bits individually or
PIO_ODSR for synchronous data output. But than we need to care about the
non-block GPIO outputs as well... requiring a read-modify-write cycle :(.
From the manual, I read about OWER: Enables writing PIO_ODSR for the
I/O line (analogous for OWDR). Would interpret this as affecting ODSR
(for block GPIO) but not SODR/CODR (as currently with single GPIOs).
Have you tried? ;-)
Grrr, I mixed OER with OWER, sorry for the noise. Back to your approach,
which works.
/* Do synchronous data output with a single write access */
__raw_writel(~mask, pio + PIO_OWDR);
__raw_writel(mask, pio + PIO_OWER);
__raw_writel(val, pio + PIO_ODSR);
For caching we would need a storage. Not sure if it's worth compared to
a context switch into the kernel.
Wolfgang.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote: /* Do synchronous data output with a single write access */ __raw_writel(~mask, pio + PIO_OWDR); __raw_writel(mask, pio + PIO_OWER); __raw_writel(val, pio + PIO_ODSR); For caching we would need a storage. Not sure if it's worth compared to a context switch into the kernel. Block GPIO is not only for you in userspace. ;-) You can also implement efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not always context switches involved. Roland -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 07:02 PM, Roland Stigge wrote:
On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
/* Do synchronous data output with a single write access */
__raw_writel(~mask, pio + PIO_OWDR);
__raw_writel(mask, pio + PIO_OWER);
__raw_writel(val, pio + PIO_ODSR);
For caching we would need a storage. Not sure if it's worth compared to
a context switch into the kernel.
Block GPIO is not only for you in userspace. ;-) You can also implement
efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
always context switches involved.
OK, what do you think about the following untested patch:
From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger w...@grandegger.com
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger w...@grandegger.com
---
arch/arm/mach-at91/gpio.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..cf6bd45 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -42,13 +42,16 @@ struct at91_gpio_chip {
void __iomem*regbase; /* PIO bank virtual address */
struct clk *clock; /* associated clock */
struct irq_domain *domain;/* associated irq domain */
+ unsigned long mask_shadow;/* synchronous data output */
};
#define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block= at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block= at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq,\
.ngpio= nr_gpio,\
@@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
unsigned offset)
return (pdsr mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ /* Do synchronous data output with a single write access */
+ if (mask != at91_gpio-mask_shadow) {
+ at91_gpio-mask_shadow = mask;
+ __raw_writel(~mask, pio + PIO_OWDR);
+ __raw_writel(mask, pio + PIO_OWER);
+ }
+ __raw_writel(val, pio + PIO_ODSR);
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 17/12/12 20:47, Wolfgang Grandegger wrote:
On 12/17/2012 07:02 PM, Roland Stigge wrote:
On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
/* Do synchronous data output with a single write access */
__raw_writel(~mask, pio + PIO_OWDR);
__raw_writel(mask, pio + PIO_OWER);
__raw_writel(val, pio + PIO_ODSR);
For caching we would need a storage. Not sure if it's worth compared to
a context switch into the kernel.
Block GPIO is not only for you in userspace. ;-) You can also implement
efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
always context switches involved.
OK, what do you think about the following untested patch:
Looks good!
Why untested? ;-)
Thanks,
Roland
From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger w...@grandegger.com
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger w...@grandegger.com
---
arch/arm/mach-at91/gpio.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..cf6bd45 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -42,13 +42,16 @@ struct at91_gpio_chip {
void __iomem*regbase; /* PIO bank virtual address */
struct clk *clock; /* associated clock */
struct irq_domain *domain;/* associated irq domain */
+ unsigned long mask_shadow;/* synchronous data output */
};
#define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block= at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block= at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq,\
.ngpio= nr_gpio,\
@@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
unsigned offset)
return (pdsr mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int
val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ /* Do synchronous data output with a single write access */
+ if (mask != at91_gpio-mask_shadow) {
+ at91_gpio-mask_shadow = mask;
+ __raw_writel(~mask, pio + PIO_OWDR);
+ __raw_writel(mask, pio + PIO_OWER);
+ }
+ __raw_writel(val, pio + PIO_ODSR);
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 20:47 Mon 17 Dec , Wolfgang Grandegger wrote:
On 12/17/2012 07:02 PM, Roland Stigge wrote:
On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
/* Do synchronous data output with a single write access */
__raw_writel(~mask, pio + PIO_OWDR);
__raw_writel(mask, pio + PIO_OWER);
__raw_writel(val, pio + PIO_ODSR);
For caching we would need a storage. Not sure if it's worth compared to
a context switch into the kernel.
Block GPIO is not only for you in userspace. ;-) You can also implement
efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
always context switches involved.
OK, what do you think about the following untested patch:
From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger w...@grandegger.com
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger w...@grandegger.com
---
arch/arm/mach-at91/gpio.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..cf6bd45 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -42,13 +42,16 @@ struct at91_gpio_chip {
void __iomem*regbase; /* PIO bank virtual address */
struct clk *clock; /* associated clock */
struct irq_domain *domain;/* associated irq domain */
+ unsigned long mask_shadow;/* synchronous data output */
};
#define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block= at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block= at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq,\
.ngpio= nr_gpio,\
@@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
unsigned offset)
return (pdsr mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int
val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ /* Do synchronous data output with a single write access */
+ if (mask != at91_gpio-mask_shadow) {
+ at91_gpio-mask_shadow = mask;
+ __raw_writel(~mask, pio + PIO_OWDR);
+ __raw_writel(mask, pio + PIO_OWER);
+ }
+ __raw_writel(val, pio + PIO_ODSR);
+}
this driver is only for old at91 platfrom if you touch at91 you need to update
the pinctrl too
Best Regards,
J.
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 10:33 PM, Roland Stigge wrote: On 17/12/12 20:47, Wolfgang Grandegger wrote: On 12/17/2012 07:02 PM, Roland Stigge wrote: On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote: /* Do synchronous data output with a single write access */ __raw_writel(~mask, pio + PIO_OWDR); __raw_writel(mask, pio + PIO_OWER); __raw_writel(val, pio + PIO_ODSR); For caching we would need a storage. Not sure if it's worth compared to a context switch into the kernel. Block GPIO is not only for you in userspace. ;-) You can also implement efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not always context switches involved. OK, what do you think about the following untested patch: Looks good! Why untested? ;-) Because I didn't have a chance to test it yet. Will do tomorrow. Wolfgang. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/18/2012 06:55 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 20:47 Mon 17 Dec , Wolfgang Grandegger wrote:
On 12/17/2012 07:02 PM, Roland Stigge wrote:
On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
/* Do synchronous data output with a single write access */
__raw_writel(~mask, pio + PIO_OWDR);
__raw_writel(mask, pio + PIO_OWER);
__raw_writel(val, pio + PIO_ODSR);
For caching we would need a storage. Not sure if it's worth compared to
a context switch into the kernel.
Block GPIO is not only for you in userspace. ;-) You can also implement
efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
always context switches involved.
OK, what do you think about the following untested patch:
From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger w...@grandegger.com
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger w...@grandegger.com
---
arch/arm/mach-at91/gpio.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..cf6bd45 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -42,13 +42,16 @@ struct at91_gpio_chip {
void __iomem*regbase; /* PIO bank virtual address */
struct clk *clock; /* associated clock */
struct irq_domain *domain;/* associated irq domain */
+unsigned long mask_shadow;/* synchronous data output */
};
#define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip,
unsigned long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+.get_block= at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+.set_block= at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq,\
.ngpio= nr_gpio,\
@@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
unsigned offset)
return (pdsr mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip,
unsigned long mask)
+{
+struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+void __iomem *pio = at91_gpio-regbase;
+u32 pdsr;
+
+pdsr = __raw_readl(pio + PIO_PDSR);
+return pdsr mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int
val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+void __iomem *pio = at91_gpio-regbase;
+
+/* Do synchronous data output with a single write access */
+if (mask != at91_gpio-mask_shadow) {
+at91_gpio-mask_shadow = mask;
+__raw_writel(~mask, pio + PIO_OWDR);
+__raw_writel(mask, pio + PIO_OWER);
+}
+__raw_writel(val, pio + PIO_ODSR);
+}
this driver is only for old at91 platfrom if you touch at91 you need to update
the pinctrl too
Well, the patch is for the hardware I have at hand and I can test. There
are many other GPIO hardware interfaces which could be enhanced with
block gpio. Roland only did it for the interfaces in driver/gpio. Also,
I think, an ACK for this patch series would be nice before we continue.
Wolfgang.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
Hi Roland,
On 12/15/2012 12:49 AM, Roland Stigge wrote:
Hi Wolfgang,
thank you for the patch!
On 14/12/12 18:58, Wolfgang Grandegger wrote:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+void __iomem *pio = at91_gpio-regbase;
+
+__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
+}
+
Without having an AT91 available right now, I guess the hardware
interface of this GPIO chip is different from the GPIO block API. While
the hardware has clear and set registers, the val parameter of
at91_gpiolib_set_block() should be interpreted as the actual output
values. See lpc32xx_gpo_set_block() for an example for handling set and
clear registers like this: First, set_bits and clear_bits words are
calculated from mask and val parameters, and finally written to the
respective hardware registers.
Note that one .set_block() can result in writing both the set and clear
registers of the hardware when val contains both 0s and 1s in
respectively masked positions.
Oops, I obviously did not test GPIO block write. The patch below does
work now. Feel free to add it to the next version of your series.
I tested with a block having both, inputs and outputs. The handling
of such a mixed block is clumsy because both, read and write do depend
on block-cur_mask. It needs to be re-set when switching from read to
write or vice versa. Defining two blocks, one for input and the other
for output seems to be the better solution.
Hope this block gpio support will show up in mainline soon.
Thanks.
Wolfgang.
From 6249995d129b290704cacb2c0114782414abeba7 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger w...@grandegger.com
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger w...@grandegger.com
---
arch/arm/mach-at91/gpio.c | 28
1 file changed, 28 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..0998854 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -48,7 +48,9 @@ struct at91_gpio_chip {
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +64,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block= at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block= at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq,\
.ngpio= nr_gpio,\
@@ -896,6 +900,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
unsigned offset)
return (pdsr mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +919,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+ u32 set_bits = values mask;
+ u32 clr_bits = ~values mask;
+
+ /* GPIO outputs can only be set at once or cleared at once */
+ if (set_bits)
+ __raw_writel(set_bits, pio + PIO_SODR);
+ if (clr_bits)
+ __raw_writel(clr_bits, pio + PIO_CODR);
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 12:37 PM, Wolfgang Grandegger wrote:
Hi Roland,
On 12/15/2012 12:49 AM, Roland Stigge wrote:
Hi Wolfgang,
thank you for the patch!
On 14/12/12 18:58, Wolfgang Grandegger wrote:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
+}
+
Without having an AT91 available right now, I guess the hardware
interface of this GPIO chip is different from the GPIO block API. While
the hardware has clear and set registers, the val parameter of
at91_gpiolib_set_block() should be interpreted as the actual output
values. See lpc32xx_gpo_set_block() for an example for handling set and
clear registers like this: First, set_bits and clear_bits words are
calculated from mask and val parameters, and finally written to the
respective hardware registers.
Note that one .set_block() can result in writing both the set and clear
registers of the hardware when val contains both 0s and 1s in
respectively masked positions.
Oops, I obviously did not test GPIO block write. The patch below does
work now. Feel free to add it to the next version of your series.
The patch lacks an important fix, sorry. Please consider the updated
patch below.
Wolfgang.
From a1f93ddea9c6c9c6a80d7a02d3c8d9902823fe47 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger w...@grandegger.com
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger w...@grandegger.com
---
arch/arm/mach-at91/gpio.c | 28
1 file changed, 28 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..77c6f91 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -48,7 +48,9 @@ struct at91_gpio_chip {
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +64,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip,
unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block= at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block= at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq,\
.ngpio= nr_gpio,\
@@ -896,6 +900,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip,
unsigned offset)
return (pdsr mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned
long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +919,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip,
unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+ u32 set_bits = val mask;
+ u32 clr_bits = ~val mask;
+
+ /* GPIO outputs can only be set at once or cleared at once */
+ if (set_bits)
+ __raw_writel(set_bits, pio + PIO_SODR);
+ if (clr_bits)
+ __raw_writel(clr_bits, pio + PIO_CODR);
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On Mon, Dec 17, 2012 at 12:51:32PM +0100, Wolfgang Grandegger wrote:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+ u32 set_bits = val mask;
+ u32 clr_bits = ~val mask;
+
+ /* GPIO outputs can only be set at once or cleared at once */
+ if (set_bits)
+ __raw_writel(set_bits, pio + PIO_SODR);
+ if (clr_bits)
+ __raw_writel(clr_bits, pio + PIO_CODR);
+}
You obviously didn't see my email...
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 12:51 PM, Wolfgang Grandegger wrote: Without having an AT91 available right now, I guess the hardware interface of this GPIO chip is different from the GPIO block API. While the hardware has clear and set registers, the val parameter of at91_gpiolib_set_block() should be interpreted as the actual output values. See lpc32xx_gpo_set_block() for an example for handling set and clear registers like this: First, set_bits and clear_bits words are calculated from mask and val parameters, and finally written to the respective hardware registers. Note that one .set_block() can result in writing both the set and clear registers of the hardware when val contains both 0s and 1s in respectively masked positions. Oops, I obviously did not test GPIO block write. The patch below does work now. Feel free to add it to the next version of your series. The patch lacks an important fix, sorry. Please consider the updated patch below. Thanks! And I guess Russell is right: If possible, we should write outputs simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear. I wonder if we need to save/restore the state of OWSR at every write operation or if we need/can cache it. Assuming that block GPIO are the only code in the kernel that manipulates ODSR. Further: Can we include this patch for arch/arm/mach-at91 via the gpio subsystem or does it need to go separately via arm-soc.git? Thanks, Roland -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
Hi Wolfgang,
On 12/17/2012 02:32 PM, Roland Stigge wrote:
And I guess Russell is right: If possible, we should write outputs
simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
I wonder if we need to save/restore the state of OWSR at every write
operation or if we need/can cache it. Assuming that block GPIO are the
only code in the kernel that manipulates ODSR.
Can you please test the following:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask,
unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ __raw_writel(~mask, pio + PIO_OWDR);
+ __raw_writel(mask, pio + PIO_OWER);
+ __raw_writel(val, pio + PIO_ODSR);
+}
Would caching OWSR be a significant speedup here?
Thanks in advance,
Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 01:10 PM, Russell King - ARM Linux wrote:
On Mon, Dec 17, 2012 at 12:51:32PM +0100, Wolfgang Grandegger wrote:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+void __iomem *pio = at91_gpio-regbase;
+u32 set_bits = val mask;
+u32 clr_bits = ~val mask;
+
+/* GPIO outputs can only be set at once or cleared at once */
+if (set_bits)
+__raw_writel(set_bits, pio + PIO_SODR);
+if (clr_bits)
+__raw_writel(clr_bits, pio + PIO_CODR);
+}
You obviously didn't see my email...
Well, I did read you mail but it was not obvious to me how to maintain
compatibility with the existing set method... at a first glance.
But the synchronous data output is important and it's even simpler than
I thought. The block-set callback can directly write to ODSR because
at91_set_gpio_output() already has set OWER. Will give that a try
later today.
Wolfgang.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/17/2012 02:51 PM, Roland Stigge wrote:
Hi Wolfgang,
On 12/17/2012 02:32 PM, Roland Stigge wrote:
And I guess Russell is right: If possible, we should write outputs
simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
I wonder if we need to save/restore the state of OWSR at every write
operation or if we need/can cache it. Assuming that block GPIO are the
only code in the kernel that manipulates ODSR.
Can you please test the following:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ __raw_writel(~mask, pio + PIO_OWDR);
This would also disable normal GPIOs configured for output! From the
manual I understand that if the pin is configured for output, we could
either use PIO_SODR/PIO_CODR to set/clear the bits individually or
PIO_ODSR for synchronous data output. But than we need to care about the
non-block GPIO outputs as well... requiring a read-modify-write cycle :(.
Wolfgang.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On Sat, Dec 15, 2012 at 12:49:57AM +0100, Roland Stigge wrote: > Without having an AT91 available right now, I guess the hardware > interface of this GPIO chip is different from the GPIO block API. While > the hardware has clear and set registers, the val parameter of > at91_gpiolib_set_block() should be interpreted as the actual output > values. See lpc32xx_gpo_set_block() for an example for handling set and > clear registers like this: First, set_bits and clear_bits words are > calculated from mask and val parameters, and finally written to the > respective hardware registers. > > Note that one .set_block() can result in writing both the set and clear > registers of the hardware when val contains both 0s and 1s in > respectively masked positions. Note also that if this is the same IP as found in SAM3N devices, that it's possible to write the bit values directly through the OWER/OWDR (output write enable register/disable register) plus the ODSR (output data status register) which will synchronously change the state of all the write-enabled output pins. That may be important for some applications. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On Sat, Dec 15, 2012 at 12:49:57AM +0100, Roland Stigge wrote: Without having an AT91 available right now, I guess the hardware interface of this GPIO chip is different from the GPIO block API. While the hardware has clear and set registers, the val parameter of at91_gpiolib_set_block() should be interpreted as the actual output values. See lpc32xx_gpo_set_block() for an example for handling set and clear registers like this: First, set_bits and clear_bits words are calculated from mask and val parameters, and finally written to the respective hardware registers. Note that one .set_block() can result in writing both the set and clear registers of the hardware when val contains both 0s and 1s in respectively masked positions. Note also that if this is the same IP as found in SAM3N devices, that it's possible to write the bit values directly through the OWER/OWDR (output write enable register/disable register) plus the ODSR (output data status register) which will synchronously change the state of all the write-enabled output pins. That may be important for some applications. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
Hi Wolfgang,
thank you for the patch!
On 14/12/12 18:58, Wolfgang Grandegger wrote:
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
> mask, unsigned long val)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> +
> + __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
> +}
> +
Without having an AT91 available right now, I guess the hardware
interface of this GPIO chip is different from the GPIO block API. While
the hardware has clear and set registers, the val parameter of
at91_gpiolib_set_block() should be interpreted as the actual output
values. See lpc32xx_gpo_set_block() for an example for handling set and
clear registers like this: First, set_bits and clear_bits words are
calculated from mask and val parameters, and finally written to the
respective hardware registers.
Note that one .set_block() can result in writing both the set and clear
registers of the hardware when val contains both 0s and 1s in
respectively masked positions.
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/14/2012 03:26 PM, Roland Stigge wrote:
> This set of patches adds:
>
> * Block GPIO API to gpiolib
> * Sysfs support for GPIO API, to provide userland access
> * Device interface for userland access (alternative to sysfs)
> * Devicetree support to instantiate GPIO blocks via DT
> * Example implementations in several gpio drivers since they need
> special accessor functions for block wise GPIO access
> * Fix for race condition in gpiolib on device creation
>
> Signed-off-by: Roland Stigge
> Tested by: Wolfgang Grandegger
I'm going to re-test this version next week. Attached you will find my
GPIO block adoptions for the AT91.
Wolfgang
>From 16c0e4b933dcdb3859184190b62655f4ac90949f Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger
---
arch/arm/mach-at91/gpio.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..dd0853d 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -48,7 +48,9 @@ struct at91_gpio_chip {
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +64,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block = at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block = at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq, \
.ngpio = nr_gpio, \
@@ -896,6 +900,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset)
return (pdsr & mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr & mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +919,14 @@ static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+
+ __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
[PATCH RESEND 0/6 v10] gpio: Add block GPIO
This set of patches adds: * Block GPIO API to gpiolib * Sysfs support for GPIO API, to provide userland access * Device interface for userland access (alternative to sysfs) * Devicetree support to instantiate GPIO blocks via DT * Example implementations in several gpio drivers since they need special accessor functions for block wise GPIO access * Fix for race condition in gpiolib on device creation Signed-off-by: Roland Stigge Tested by: Wolfgang Grandegger -- Changed since v9: * Changed IRQ flags for device interface's poll() from IRQF_TRIGGER_FALLING to IRQF_SHARED * Use block name on gpio_request * Added spinlock for ISR manipulated data (dev interface / poll()) Changed since v8: * Added poll() function to dev interface for IRQ handling * Fixed gpio_block_register() in case of missing CONFIG_GPIO_SYSFS * Fixed mask handling in sysfs interface * Fixed get_user/put_user pointer (cast) Changes since v7: * Fixed error in API documentation (name arg missing in prototype) * Fixed compile error if sysfs is missing: Moved gpio_block_is_output() out of sysfs-ifdef * Added error handling to gpio_block_register() * Added mask attribute to sysfs (although dev interface exists now) Changes since v6: * Changed realloc to list handling * Added mask to kernel API * Added device interface as userspace API * Changed sizeof() to BITS_PER_LONG * Fixed multiline comment style * Documented direction setting by the user * Documented order of handled chips in case of multiple chips in one block Changes since v5: * Documented sysfs: elaborated on "exported" and "values" attributes * Documented sysfs: gpiochip is a separate class now * Aggregated driver support patches for block gpio into one single patch * Added gpio block driver support for twl6040 and pch Changes since v4: * Documented word width * Bugfix: export/unexport on register/unregister * Using default dev_attrs for gpio_block_class * Fix gpiolib: race condition on device creation * Added driver support for ucb14500, vt8500, xilinx Changes since v3: * Added driver support for pca953x, em, pl061, max732x, pcf857x * Coding style improvements * Fixed krealloc memory leak in error case * sysfs: values in hex * Register blocks in a list * Narrowing lock scope * Use S_IWUSR and S_IRUGO instead of direct octal values * Use for_each_set_bit() * Change from unsigned to unsigned long for masks and values Changes since v2: * Added sysfs support * Added devicetree support * Added support for lpc32xx, generic * Added functions for GPIO block registration * Added more error checking * Bit remapping bugfix Changes since v1: * API change to 32/64 bit word, bit masks Thanks to Ryan Mallon, Linus Walleij, Stijn Devriendt, Jean-Christophe Plagniol-Villard, Mark Brown, Greg Kroah-Hartman, Grant Likely, Stefan Roese and Wolfgang Grandegger for reviewing! Roland Stigge (6): gpio: Add a block GPIO API to gpiolib gpio: Add sysfs support to block GPIO API gpio: Add userland device interface to block GPIO gpiolib: Fix default attributes for class gpio: Add device tree support to block GPIO API gpio: Add block gpio to several gpio drivers Documentation/ABI/testing/dev-gpioblock | 34 Documentation/ABI/testing/sysfs-gpio | 31 Documentation/devicetree/bindings/gpio/gpio-block.txt | 36 Documentation/gpio.txt| 58 + drivers/gpio/Makefile |1 drivers/gpio/gpio-em.c| 23 drivers/gpio/gpio-generic.c | 56 + drivers/gpio/gpio-lpc32xx.c | 82 + drivers/gpio/gpio-max730x.c | 61 + drivers/gpio/gpio-max732x.c | 59 + drivers/gpio/gpio-pca953x.c | 64 + drivers/gpio/gpio-pcf857x.c | 24 drivers/gpio/gpio-pch.c | 27 drivers/gpio/gpio-pl061.c | 17 drivers/gpio/gpio-twl6040.c | 32 drivers/gpio/gpio-ucb1400.c | 23 drivers/gpio/gpio-vt8500.c| 24 drivers/gpio/gpio-xilinx.c| 44 + drivers/gpio/gpioblock-of.c | 84 + drivers/gpio/gpiolib.c| 780 -- include/asm-generic/gpio.h| 28 include/linux/gpio.h | 125 ++ 22 files changed, 1666 insertions(+), 47 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RESEND 0/6 v10] gpio: Add block GPIO
This set of patches adds: * Block GPIO API to gpiolib * Sysfs support for GPIO API, to provide userland access * Device interface for userland access (alternative to sysfs) * Devicetree support to instantiate GPIO blocks via DT * Example implementations in several gpio drivers since they need special accessor functions for block wise GPIO access * Fix for race condition in gpiolib on device creation Signed-off-by: Roland Stigge sti...@antcom.de Tested by: Wolfgang Grandegger w...@grandegger.com -- Changed since v9: * Changed IRQ flags for device interface's poll() from IRQF_TRIGGER_FALLING to IRQF_SHARED * Use block name on gpio_request * Added spinlock for ISR manipulated data (dev interface / poll()) Changed since v8: * Added poll() function to dev interface for IRQ handling * Fixed gpio_block_register() in case of missing CONFIG_GPIO_SYSFS * Fixed mask handling in sysfs interface * Fixed get_user/put_user pointer (cast) Changes since v7: * Fixed error in API documentation (name arg missing in prototype) * Fixed compile error if sysfs is missing: Moved gpio_block_is_output() out of sysfs-ifdef * Added error handling to gpio_block_register() * Added mask attribute to sysfs (although dev interface exists now) Changes since v6: * Changed realloc to list handling * Added mask to kernel API * Added device interface as userspace API * Changed sizeof() to BITS_PER_LONG * Fixed multiline comment style * Documented direction setting by the user * Documented order of handled chips in case of multiple chips in one block Changes since v5: * Documented sysfs: elaborated on exported and values attributes * Documented sysfs: gpiochip is a separate class now * Aggregated driver support patches for block gpio into one single patch * Added gpio block driver support for twl6040 and pch Changes since v4: * Documented word width * Bugfix: export/unexport on register/unregister * Using default dev_attrs for gpio_block_class * Fix gpiolib: race condition on device creation * Added driver support for ucb14500, vt8500, xilinx Changes since v3: * Added driver support for pca953x, em, pl061, max732x, pcf857x * Coding style improvements * Fixed krealloc memory leak in error case * sysfs: values in hex * Register blocks in a list * Narrowing lock scope * Use S_IWUSR and S_IRUGO instead of direct octal values * Use for_each_set_bit() * Change from unsigned to unsigned long for masks and values Changes since v2: * Added sysfs support * Added devicetree support * Added support for lpc32xx, generic * Added functions for GPIO block registration * Added more error checking * Bit remapping bugfix Changes since v1: * API change to 32/64 bit word, bit masks Thanks to Ryan Mallon, Linus Walleij, Stijn Devriendt, Jean-Christophe Plagniol-Villard, Mark Brown, Greg Kroah-Hartman, Grant Likely, Stefan Roese and Wolfgang Grandegger for reviewing! Roland Stigge (6): gpio: Add a block GPIO API to gpiolib gpio: Add sysfs support to block GPIO API gpio: Add userland device interface to block GPIO gpiolib: Fix default attributes for class gpio: Add device tree support to block GPIO API gpio: Add block gpio to several gpio drivers Documentation/ABI/testing/dev-gpioblock | 34 Documentation/ABI/testing/sysfs-gpio | 31 Documentation/devicetree/bindings/gpio/gpio-block.txt | 36 Documentation/gpio.txt| 58 + drivers/gpio/Makefile |1 drivers/gpio/gpio-em.c| 23 drivers/gpio/gpio-generic.c | 56 + drivers/gpio/gpio-lpc32xx.c | 82 + drivers/gpio/gpio-max730x.c | 61 + drivers/gpio/gpio-max732x.c | 59 + drivers/gpio/gpio-pca953x.c | 64 + drivers/gpio/gpio-pcf857x.c | 24 drivers/gpio/gpio-pch.c | 27 drivers/gpio/gpio-pl061.c | 17 drivers/gpio/gpio-twl6040.c | 32 drivers/gpio/gpio-ucb1400.c | 23 drivers/gpio/gpio-vt8500.c| 24 drivers/gpio/gpio-xilinx.c| 44 + drivers/gpio/gpioblock-of.c | 84 + drivers/gpio/gpiolib.c| 780 -- include/asm-generic/gpio.h| 28 include/linux/gpio.h | 125 ++ 22 files changed, 1666 insertions(+), 47 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
On 12/14/2012 03:26 PM, Roland Stigge wrote:
This set of patches adds:
* Block GPIO API to gpiolib
* Sysfs support for GPIO API, to provide userland access
* Device interface for userland access (alternative to sysfs)
* Devicetree support to instantiate GPIO blocks via DT
* Example implementations in several gpio drivers since they need
special accessor functions for block wise GPIO access
* Fix for race condition in gpiolib on device creation
Signed-off-by: Roland Stigge sti...@antcom.de
Tested by: Wolfgang Grandegger w...@grandegger.com
I'm going to re-test this version next week. Attached you will find my
GPIO block adoptions for the AT91.
Wolfgang
From 16c0e4b933dcdb3859184190b62655f4ac90949f Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger w...@grandegger.com
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger w...@grandegger.com
---
arch/arm/mach-at91/gpio.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..dd0853d 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -48,7 +48,9 @@ struct at91_gpio_chip {
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 void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +64,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block = at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block = at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq, \
.ngpio = nr_gpio, \
@@ -896,6 +900,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset)
return (pdsr mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +919,14 @@ static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
Hi Wolfgang,
thank you for the patch!
On 14/12/12 18:58, Wolfgang Grandegger wrote:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long
mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio-regbase;
+
+ __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
+}
+
Without having an AT91 available right now, I guess the hardware
interface of this GPIO chip is different from the GPIO block API. While
the hardware has clear and set registers, the val parameter of
at91_gpiolib_set_block() should be interpreted as the actual output
values. See lpc32xx_gpo_set_block() for an example for handling set and
clear registers like this: First, set_bits and clear_bits words are
calculated from mask and val parameters, and finally written to the
respective hardware registers.
Note that one .set_block() can result in writing both the set and clear
registers of the hardware when val contains both 0s and 1s in
respectively masked positions.
Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
