* [PATCH 1/2] gpio: dwapb: Use "stride" rather than "size" for register distance
@ 2018-02-12 10:11 Linus Walleij
2018-02-12 10:11 ` [PATCH 2/2] gpio: dwapb: Call directly into the gpiochip to read value Linus Walleij
2018-02-14 1:40 ` [PATCH 1/2] gpio: dwapb: Use "stride" rather than "size" for register distance Hoan Tran
0 siblings, 2 replies; 4+ messages in thread
From: Linus Walleij @ 2018-02-12 10:11 UTC (permalink / raw)
To: linux-gpio, Hoan Tran; +Cc: Linus Walleij
This terminology is more precise. Also cut the stride calculation
in the preprocessor, it confuses more than it helps when reading
the driver.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpio-dwapb.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 6730c6642ce3..677988f21369 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -53,9 +53,9 @@
#define GPIO_EXT_PORTD 0x5c
#define DWAPB_MAX_PORTS 4
-#define GPIO_EXT_PORT_SIZE (GPIO_EXT_PORTB - GPIO_EXT_PORTA)
-#define GPIO_SWPORT_DR_SIZE (GPIO_SWPORTB_DR - GPIO_SWPORTA_DR)
-#define GPIO_SWPORT_DDR_SIZE (GPIO_SWPORTB_DDR - GPIO_SWPORTA_DDR)
+#define GPIO_EXT_PORT_STRIDE 0x04 /* register stride 32 bits */
+#define GPIO_SWPORT_DR_STRIDE 0x0c /* register stride 3*32 bits */
+#define GPIO_SWPORT_DDR_STRIDE 0x0c /* register stride 3*32 bits */
#define GPIO_REG_OFFSET_V2 1
@@ -476,10 +476,10 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
return -ENOMEM;
#endif
- dat = gpio->regs + GPIO_EXT_PORTA + (pp->idx * GPIO_EXT_PORT_SIZE);
- set = gpio->regs + GPIO_SWPORTA_DR + (pp->idx * GPIO_SWPORT_DR_SIZE);
+ dat = gpio->regs + GPIO_EXT_PORTA + (pp->idx * GPIO_EXT_PORT_STRIDE);
+ set = gpio->regs + GPIO_SWPORTA_DR + (pp->idx * GPIO_SWPORT_DR_STRIDE);
dirout = gpio->regs + GPIO_SWPORTA_DDR +
- (pp->idx * GPIO_SWPORT_DDR_SIZE);
+ (pp->idx * GPIO_SWPORT_DDR_STRIDE);
err = bgpio_init(&port->gc, gpio->dev, 4, dat, set, NULL, dirout,
NULL, 0);
@@ -710,13 +710,13 @@ static int dwapb_gpio_suspend(struct device *dev)
BUG_ON(!ctx);
- offset = GPIO_SWPORTA_DDR + idx * GPIO_SWPORT_DDR_SIZE;
+ offset = GPIO_SWPORTA_DDR + idx * GPIO_SWPORT_DDR_STRIDE;
ctx->dir = dwapb_read(gpio, offset);
- offset = GPIO_SWPORTA_DR + idx * GPIO_SWPORT_DR_SIZE;
+ offset = GPIO_SWPORTA_DR + idx * GPIO_SWPORT_DR_STRIDE;
ctx->data = dwapb_read(gpio, offset);
- offset = GPIO_EXT_PORTA + idx * GPIO_EXT_PORT_SIZE;
+ offset = GPIO_EXT_PORTA + idx * GPIO_EXT_PORT_STRIDE;
ctx->ext = dwapb_read(gpio, offset);
/* Only port A can provide interrupts */
@@ -753,13 +753,13 @@ static int dwapb_gpio_resume(struct device *dev)
BUG_ON(!ctx);
- offset = GPIO_SWPORTA_DR + idx * GPIO_SWPORT_DR_SIZE;
+ offset = GPIO_SWPORTA_DR + idx * GPIO_SWPORT_DR_STRIDE;
dwapb_write(gpio, offset, ctx->data);
- offset = GPIO_SWPORTA_DDR + idx * GPIO_SWPORT_DDR_SIZE;
+ offset = GPIO_SWPORTA_DDR + idx * GPIO_SWPORT_DDR_STRIDE;
dwapb_write(gpio, offset, ctx->dir);
- offset = GPIO_EXT_PORTA + idx * GPIO_EXT_PORT_SIZE;
+ offset = GPIO_EXT_PORTA + idx * GPIO_EXT_PORT_STRIDE;
dwapb_write(gpio, offset, ctx->ext);
/* Only port A can provide interrupts */
--
2.14.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] gpio: dwapb: Call directly into the gpiochip to read value
2018-02-12 10:11 [PATCH 1/2] gpio: dwapb: Use "stride" rather than "size" for register distance Linus Walleij
@ 2018-02-12 10:11 ` Linus Walleij
2018-02-14 1:41 ` Hoan Tran
2018-02-14 1:40 ` [PATCH 1/2] gpio: dwapb: Use "stride" rather than "size" for register distance Hoan Tran
1 sibling, 1 reply; 4+ messages in thread
From: Linus Walleij @ 2018-02-12 10:11 UTC (permalink / raw)
To: linux-gpio, Hoan Tran; +Cc: Linus Walleij
We were going out through the (legacy) gpio API to read the value
of a line to set up polarity inversion. This is abusive. Do something
less abusive by looking up the actual struct gpio_chip *
instance and calling .get() directly on it.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpio-dwapb.c | 37 ++++++++++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 677988f21369..b0704a883513 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -9,8 +9,6 @@
*/
#include <linux/acpi.h>
#include <linux/gpio/driver.h>
-/* FIXME: for gpio_get_value(), replace this with direct register read */
-#include <linux/gpio.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/interrupt.h>
@@ -153,16 +151,40 @@ static int dwapb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
return irq_find_mapping(gpio->domain, offset);
}
+static struct dwapb_gpio_port *dwapb_offs_to_port(struct dwapb_gpio *gpio, unsigned int offs)
+{
+ struct dwapb_gpio_port *port;
+ int i;
+
+ for (i = 0; i < gpio->nr_ports; i++) {
+ port = &gpio->ports[i];
+ if (port->idx == offs / 32)
+ return port;
+ }
+
+ return NULL;
+}
+
static void dwapb_toggle_trigger(struct dwapb_gpio *gpio, unsigned int offs)
{
- u32 v = dwapb_read(gpio, GPIO_INT_POLARITY);
+ struct dwapb_gpio_port *port = dwapb_offs_to_port(gpio, offs);
+ struct gpio_chip *gc;
+ u32 pol;
+ int val;
+
+ if (!port)
+ return;
+ gc = &port->gc;
- if (gpio_get_value(gpio->ports[0].gc.base + offs))
- v &= ~BIT(offs);
+ pol = dwapb_read(gpio, GPIO_INT_POLARITY);
+ /* Just read the current value right out of the data register */
+ val = gc->get(gc, offs % 32);
+ if (val)
+ pol &= ~BIT(offs);
else
- v |= BIT(offs);
+ pol |= BIT(offs);
- dwapb_write(gpio, GPIO_INT_POLARITY, v);
+ dwapb_write(gpio, GPIO_INT_POLARITY, pol);
}
static u32 dwapb_do_irq(struct dwapb_gpio *gpio)
@@ -481,6 +503,7 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
dirout = gpio->regs + GPIO_SWPORTA_DDR +
(pp->idx * GPIO_SWPORT_DDR_STRIDE);
+ /* This registers 32 GPIO lines per port */
err = bgpio_init(&port->gc, gpio->dev, 4, dat, set, NULL, dirout,
NULL, 0);
if (err) {
--
2.14.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 2/2] gpio: dwapb: Call directly into the gpiochip to read value
2018-02-12 10:11 ` [PATCH 2/2] gpio: dwapb: Call directly into the gpiochip to read value Linus Walleij
@ 2018-02-14 1:41 ` Hoan Tran
0 siblings, 0 replies; 4+ messages in thread
From: Hoan Tran @ 2018-02-14 1:41 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio
Hi Linus,
On Mon, Feb 12, 2018 at 2:11 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> We were going out through the (legacy) gpio API to read the value
> of a line to set up polarity inversion. This is abusive. Do something
> less abusive by looking up the actual struct gpio_chip *
> instance and calling .get() directly on it.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/gpio/gpio-dwapb.c | 37 ++++++++++++++++++++++++++++++-------
> 1 file changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 677988f21369..b0704a883513 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -9,8 +9,6 @@
> */
> #include <linux/acpi.h>
> #include <linux/gpio/driver.h>
> -/* FIXME: for gpio_get_value(), replace this with direct register read */
> -#include <linux/gpio.h>
> #include <linux/err.h>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> @@ -153,16 +151,40 @@ static int dwapb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
> return irq_find_mapping(gpio->domain, offset);
> }
>
> +static struct dwapb_gpio_port *dwapb_offs_to_port(struct dwapb_gpio *gpio, unsigned int offs)
> +{
> + struct dwapb_gpio_port *port;
> + int i;
> +
> + for (i = 0; i < gpio->nr_ports; i++) {
> + port = &gpio->ports[i];
> + if (port->idx == offs / 32)
> + return port;
> + }
> +
> + return NULL;
> +}
> +
> static void dwapb_toggle_trigger(struct dwapb_gpio *gpio, unsigned int offs)
> {
> - u32 v = dwapb_read(gpio, GPIO_INT_POLARITY);
> + struct dwapb_gpio_port *port = dwapb_offs_to_port(gpio, offs);
> + struct gpio_chip *gc;
> + u32 pol;
> + int val;
> +
> + if (!port)
> + return;
> + gc = &port->gc;
>
> - if (gpio_get_value(gpio->ports[0].gc.base + offs))
> - v &= ~BIT(offs);
> + pol = dwapb_read(gpio, GPIO_INT_POLARITY);
> + /* Just read the current value right out of the data register */
> + val = gc->get(gc, offs % 32);
> + if (val)
> + pol &= ~BIT(offs);
> else
> - v |= BIT(offs);
> + pol |= BIT(offs);
>
> - dwapb_write(gpio, GPIO_INT_POLARITY, v);
> + dwapb_write(gpio, GPIO_INT_POLARITY, pol);
> }
>
> static u32 dwapb_do_irq(struct dwapb_gpio *gpio)
> @@ -481,6 +503,7 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
> dirout = gpio->regs + GPIO_SWPORTA_DDR +
> (pp->idx * GPIO_SWPORT_DDR_STRIDE);
>
> + /* This registers 32 GPIO lines per port */
> err = bgpio_init(&port->gc, gpio->dev, 4, dat, set, NULL, dirout,
> NULL, 0);
> if (err) {
> --
> 2.14.3
>
Acked-by: Hoan Tran <hotran@apm.com>
Thanks
Hoan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] gpio: dwapb: Use "stride" rather than "size" for register distance
2018-02-12 10:11 [PATCH 1/2] gpio: dwapb: Use "stride" rather than "size" for register distance Linus Walleij
2018-02-12 10:11 ` [PATCH 2/2] gpio: dwapb: Call directly into the gpiochip to read value Linus Walleij
@ 2018-02-14 1:40 ` Hoan Tran
1 sibling, 0 replies; 4+ messages in thread
From: Hoan Tran @ 2018-02-14 1:40 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio
Hi Linus,
On Mon, Feb 12, 2018 at 2:11 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> This terminology is more precise. Also cut the stride calculation
> in the preprocessor, it confuses more than it helps when reading
> the driver.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/gpio/gpio-dwapb.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 6730c6642ce3..677988f21369 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -53,9 +53,9 @@
> #define GPIO_EXT_PORTD 0x5c
>
> #define DWAPB_MAX_PORTS 4
> -#define GPIO_EXT_PORT_SIZE (GPIO_EXT_PORTB - GPIO_EXT_PORTA)
> -#define GPIO_SWPORT_DR_SIZE (GPIO_SWPORTB_DR - GPIO_SWPORTA_DR)
> -#define GPIO_SWPORT_DDR_SIZE (GPIO_SWPORTB_DDR - GPIO_SWPORTA_DDR)
> +#define GPIO_EXT_PORT_STRIDE 0x04 /* register stride 32 bits */
> +#define GPIO_SWPORT_DR_STRIDE 0x0c /* register stride 3*32 bits */
> +#define GPIO_SWPORT_DDR_STRIDE 0x0c /* register stride 3*32 bits */
>
> #define GPIO_REG_OFFSET_V2 1
>
> @@ -476,10 +476,10 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
> return -ENOMEM;
> #endif
>
> - dat = gpio->regs + GPIO_EXT_PORTA + (pp->idx * GPIO_EXT_PORT_SIZE);
> - set = gpio->regs + GPIO_SWPORTA_DR + (pp->idx * GPIO_SWPORT_DR_SIZE);
> + dat = gpio->regs + GPIO_EXT_PORTA + (pp->idx * GPIO_EXT_PORT_STRIDE);
> + set = gpio->regs + GPIO_SWPORTA_DR + (pp->idx * GPIO_SWPORT_DR_STRIDE);
> dirout = gpio->regs + GPIO_SWPORTA_DDR +
> - (pp->idx * GPIO_SWPORT_DDR_SIZE);
> + (pp->idx * GPIO_SWPORT_DDR_STRIDE);
>
> err = bgpio_init(&port->gc, gpio->dev, 4, dat, set, NULL, dirout,
> NULL, 0);
> @@ -710,13 +710,13 @@ static int dwapb_gpio_suspend(struct device *dev)
>
> BUG_ON(!ctx);
>
> - offset = GPIO_SWPORTA_DDR + idx * GPIO_SWPORT_DDR_SIZE;
> + offset = GPIO_SWPORTA_DDR + idx * GPIO_SWPORT_DDR_STRIDE;
> ctx->dir = dwapb_read(gpio, offset);
>
> - offset = GPIO_SWPORTA_DR + idx * GPIO_SWPORT_DR_SIZE;
> + offset = GPIO_SWPORTA_DR + idx * GPIO_SWPORT_DR_STRIDE;
> ctx->data = dwapb_read(gpio, offset);
>
> - offset = GPIO_EXT_PORTA + idx * GPIO_EXT_PORT_SIZE;
> + offset = GPIO_EXT_PORTA + idx * GPIO_EXT_PORT_STRIDE;
> ctx->ext = dwapb_read(gpio, offset);
>
> /* Only port A can provide interrupts */
> @@ -753,13 +753,13 @@ static int dwapb_gpio_resume(struct device *dev)
>
> BUG_ON(!ctx);
>
> - offset = GPIO_SWPORTA_DR + idx * GPIO_SWPORT_DR_SIZE;
> + offset = GPIO_SWPORTA_DR + idx * GPIO_SWPORT_DR_STRIDE;
> dwapb_write(gpio, offset, ctx->data);
>
> - offset = GPIO_SWPORTA_DDR + idx * GPIO_SWPORT_DDR_SIZE;
> + offset = GPIO_SWPORTA_DDR + idx * GPIO_SWPORT_DDR_STRIDE;
> dwapb_write(gpio, offset, ctx->dir);
>
> - offset = GPIO_EXT_PORTA + idx * GPIO_EXT_PORT_SIZE;
> + offset = GPIO_EXT_PORTA + idx * GPIO_EXT_PORT_STRIDE;
> dwapb_write(gpio, offset, ctx->ext);
>
> /* Only port A can provide interrupts */
> --
> 2.14.3
>
Acked-by: Hoan Tran <hotran@apm.com>
Regards
Hoan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-14 1:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-12 10:11 [PATCH 1/2] gpio: dwapb: Use "stride" rather than "size" for register distance Linus Walleij
2018-02-12 10:11 ` [PATCH 2/2] gpio: dwapb: Call directly into the gpiochip to read value Linus Walleij
2018-02-14 1:41 ` Hoan Tran
2018-02-14 1:40 ` [PATCH 1/2] gpio: dwapb: Use "stride" rather than "size" for register distance Hoan Tran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).