I've updated the series with all of Anton's comments (thanks Anton) and
retested on the Synopsys controller with multiple banks.
Thanks,
Jamie Iles (7):
basic_mmio_gpio: remove runtime width/endianness evaluation
basic_mmio_gpio: convert to platform_{get,set}_drvdata()
basic_mmio_gpio: allow overriding number of gpio
basic_mmio_gpio: request register regions
basic_mmio_gpio: detect output method at probe time
basic_mmio_gpio: support different input/output registers
basic_mmio_gpio: support direction registers
drivers/gpio/basic_mmio_gpio.c | 383 ++++++++++++++++++++++++++++++---------
include/linux/basic_mmio_gpio.h | 1 +
2 files changed, 302 insertions(+), 82 deletions(-)
--
1.7.4.2
Remove endianness/width calculations at runtime by installing function
pointers for bit-to-mask conversion and register accessors.
Reported-by: Thomas Gleixner <[email protected]>
Signed-off-by: Jamie Iles <[email protected]>
Acked-by: Anton Vorontsov <[email protected]>
Cc: Grant Likely <[email protected]>
---
drivers/gpio/basic_mmio_gpio.c | 136 +++++++++++++++++++++++++++-------------
1 files changed, 92 insertions(+), 44 deletions(-)
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
index 3addea6..e935a24 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -63,6 +63,10 @@ o ` ~~~~\___/~~~~ ` controller in FPGA is ,.`
struct bgpio_chip {
struct gpio_chip gc;
+
+ unsigned long (*read_reg)(void __iomem *reg);
+ void (*write_reg)(void __iomem *reg, unsigned long data);
+
void __iomem *reg_dat;
void __iomem *reg_set;
void __iomem *reg_clr;
@@ -74,7 +78,7 @@ struct bgpio_chip {
* Some GPIO controllers work with the big-endian bits notation,
* e.g. in a 8-bits register, GPIO7 is the least significant bit.
*/
- int big_endian_bits;
+ unsigned long (*pin2mask)(struct bgpio_chip *bgc, unsigned int pin);
/*
* Used to lock bgpio_chip->data. Also, this is needed to keep
@@ -91,70 +95,77 @@ static struct bgpio_chip *to_bgpio_chip(struct gpio_chip *gc)
return container_of(gc, struct bgpio_chip, gc);
}
-static unsigned long bgpio_in(struct bgpio_chip *bgc)
+static void bgpio_write8(void __iomem *reg, unsigned long data)
{
- switch (bgc->bits) {
- case 8:
- return __raw_readb(bgc->reg_dat);
- case 16:
- return __raw_readw(bgc->reg_dat);
- case 32:
- return __raw_readl(bgc->reg_dat);
-#if BITS_PER_LONG >= 64
- case 64:
- return __raw_readq(bgc->reg_dat);
-#endif
- }
- return -EINVAL;
+ __raw_writeb(data, reg);
}
-static void bgpio_out(struct bgpio_chip *bgc, void __iomem *reg,
- unsigned long data)
+static unsigned long bgpio_read8(void __iomem *reg)
{
- switch (bgc->bits) {
- case 8:
- __raw_writeb(data, reg);
- return;
- case 16:
- __raw_writew(data, reg);
- return;
- case 32:
- __raw_writel(data, reg);
- return;
+ return __raw_readb(reg);
+}
+
+static void bgpio_write16(void __iomem *reg, unsigned long data)
+{
+ __raw_writew(data, reg);
+}
+
+static unsigned long bgpio_read16(void __iomem *reg)
+{
+ return __raw_readw(reg);
+}
+
+static void bgpio_write32(void __iomem *reg, unsigned long data)
+{
+ __raw_writel(data, reg);
+}
+
+static unsigned long bgpio_read32(void __iomem *reg)
+{
+ return __raw_readl(reg);
+}
+
#if BITS_PER_LONG >= 64
- case 64:
- __raw_writeq(data, reg);
- return;
-#endif
- }
+static void bgpio_write64(void __iomem *reg, unsigned long data)
+{
+ __raw_writeq(data, reg);
}
+static unsigned long bgpio_read64(void __iomem *reg)
+{
+ return __raw_readq(reg);
+}
+#endif /* BITS_PER_LONG >= 64 */
+
static unsigned long bgpio_pin2mask(struct bgpio_chip *bgc, unsigned int pin)
{
- if (bgc->big_endian_bits)
- return 1 << (bgc->bits - 1 - pin);
- else
- return 1 << pin;
+ return 1 << pin;
+}
+
+static unsigned long bgpio_pin2mask_be(struct bgpio_chip *bgc,
+ unsigned int pin)
+{
+ return 1 << (bgc->bits - 1 - pin);
}
static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
{
struct bgpio_chip *bgc = to_bgpio_chip(gc);
- return bgpio_in(bgc) & bgpio_pin2mask(bgc, gpio);
+ return bgc->read_reg(bgc->reg_dat) & bgc->pin2mask(bgc, gpio);
}
static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
{
struct bgpio_chip *bgc = to_bgpio_chip(gc);
- unsigned long mask = bgpio_pin2mask(bgc, gpio);
+ unsigned long mask = bgc->pin2mask(bgc, gpio);
unsigned long flags;
if (bgc->reg_set) {
if (val)
- bgpio_out(bgc, bgc->reg_set, mask);
+ bgc->write_reg(bgc->reg_set, mask);
else
- bgpio_out(bgc, bgc->reg_clr, mask);
+ bgc->write_reg(bgc->reg_clr, mask);
return;
}
@@ -165,7 +176,7 @@ static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
else
bgc->data &= ~mask;
- bgpio_out(bgc, bgc->reg_dat, bgc->data);
+ bgc->write_reg(bgc->reg_dat, bgc->data);
spin_unlock_irqrestore(&bgc->lock, flags);
}
@@ -181,9 +192,44 @@ static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
return 0;
}
-static int __devinit bgpio_probe(struct platform_device *pdev)
+static int bgpio_setup_accessors(struct platform_device *pdev,
+ struct bgpio_chip *bgc)
{
const struct platform_device_id *platid = platform_get_device_id(pdev);
+
+ switch (bgc->bits) {
+ case 8:
+ bgc->read_reg = bgpio_read8;
+ bgc->write_reg = bgpio_write8;
+ break;
+ case 16:
+ bgc->read_reg = bgpio_read16;
+ bgc->write_reg = bgpio_write16;
+ break;
+ case 32:
+ bgc->read_reg = bgpio_read32;
+ bgc->write_reg = bgpio_write32;
+ break;
+#if BITS_PER_LONG >= 64
+ case 64:
+ bgc->read_reg = bgpio_read64;
+ bgc->write_reg = bgpio_write64;
+ break;
+#endif /* BITS_PER_LONG >= 64 */
+ default:
+ dev_err(&pdev->dev, "unsupported data width %u bits\n",
+ bgc->bits);
+ return -EINVAL;
+ }
+
+ bgc->pin2mask = strcmp(platid->name, "basic-mmio-gpio-be") ?
+ bgpio_pin2mask : bgpio_pin2mask_be;
+
+ return 0;
+}
+
+static int __devinit bgpio_probe(struct platform_device *pdev)
+{
struct device *dev = &pdev->dev;
struct bgpio_pdata *pdata = dev_get_platdata(dev);
struct bgpio_chip *bgc;
@@ -232,9 +278,11 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
spin_lock_init(&bgc->lock);
bgc->bits = bits;
- bgc->big_endian_bits = !strcmp(platid->name, "basic-mmio-gpio-be");
- bgc->data = bgpio_in(bgc);
+ ret = bgpio_setup_accessors(pdev, bgc);
+ if (ret)
+ return ret;
+ bgc->data = bgc->read_reg(bgc->reg_dat);
bgc->gc.ngpio = bits;
bgc->gc.direction_input = bgpio_dir_in;
bgc->gc.direction_output = bgpio_dir_out;
--
1.7.4.2
Use the platform drvdata helpers rather than working on the struct
device itself.
Signed-off-by: Jamie Iles <[email protected]>
Acked-by: Anton Vorontsov <[email protected]>
Cc: Grant Likely <[email protected]>
---
drivers/gpio/basic_mmio_gpio.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
index e935a24..5db5de4 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -296,7 +296,7 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
else
bgc->gc.base = -1;
- dev_set_drvdata(dev, bgc);
+ platform_set_drvdata(pdev, bgc);
ret = gpiochip_add(&bgc->gc);
if (ret)
@@ -307,7 +307,7 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
static int __devexit bgpio_remove(struct platform_device *pdev)
{
- struct bgpio_chip *bgc = dev_get_drvdata(&pdev->dev);
+ struct bgpio_chip *bgc = platform_get_drvdata(pdev);
return gpiochip_remove(&bgc->gc);
}
--
1.7.4.2
Make sure that we get the register regions with request_mem_region()
before ioremap() to make sure we have exclusive access.
Signed-off-by: Jamie Iles <[email protected]>
Acked-by: Anton Vorontsov <[email protected]>
Cc: Grant Likely <[email protected]>
---
drivers/gpio/basic_mmio_gpio.c | 16 +++++++++++++---
1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
index 2b2d384..90f7f89 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -192,6 +192,16 @@ static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
return 0;
}
+static void __iomem *bgpio_request_and_map(struct device *dev,
+ struct resource *res)
+{
+ if (!devm_request_mem_region(dev, res->start, resource_size(res),
+ res->name ?: "mmio_gpio"))
+ return NULL;
+
+ return devm_ioremap(dev, res->start, resource_size(res));
+}
+
static int bgpio_setup_accessors(struct platform_device *pdev,
struct bgpio_chip *bgc)
{
@@ -258,7 +268,7 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
if (!bgc)
return -ENOMEM;
- bgc->reg_dat = devm_ioremap(dev, res_dat->start, dat_sz);
+ bgc->reg_dat = bgpio_request_and_map(dev, res_dat);
if (!bgc->reg_dat)
return -ENOMEM;
@@ -269,8 +279,8 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
resource_size(res_set) != dat_sz)
return -EINVAL;
- bgc->reg_set = devm_ioremap(dev, res_set->start, dat_sz);
- bgc->reg_clr = devm_ioremap(dev, res_clr->start, dat_sz);
+ bgc->reg_set = bgpio_request_and_map(dev, res_set);
+ bgc->reg_clr = bgpio_request_and_map(dev, res_clr);
if (!bgc->reg_set || !bgc->reg_clr)
return -ENOMEM;
} else if (res_set || res_clr) {
--
1.7.4.2
Rather than detecting the output method each time in the .set()
callback, do it at probe time and set the appropriate callback.
Signed-off-by: Jamie Iles <[email protected]>
Acked-by: Anton Vorontsov <[email protected]>
Cc: Grant Likely <[email protected]>
---
drivers/gpio/basic_mmio_gpio.c | 88 +++++++++++++++++++++++++++-------------
1 files changed, 59 insertions(+), 29 deletions(-)
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
index 90f7f89..9be1867 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -161,14 +161,6 @@ static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
unsigned long mask = bgc->pin2mask(bgc, gpio);
unsigned long flags;
- if (bgc->reg_set) {
- if (val)
- bgc->write_reg(bgc->reg_set, mask);
- else
- bgc->write_reg(bgc->reg_clr, mask);
- return;
- }
-
spin_lock_irqsave(&bgc->lock, flags);
if (val)
@@ -181,6 +173,18 @@ static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
spin_unlock_irqrestore(&bgc->lock, flags);
}
+static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
+ int val)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ unsigned long mask = bgc->pin2mask(bgc, gpio);
+
+ if (val)
+ bgc->write_reg(bgc->reg_set, mask);
+ else
+ bgc->write_reg(bgc->reg_clr, mask);
+}
+
static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
{
return 0;
@@ -188,7 +192,8 @@ static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
{
- bgpio_set(gc, gpio, val);
+ gc->set(gc, gpio, val);
+
return 0;
}
@@ -238,14 +243,52 @@ static int bgpio_setup_accessors(struct platform_device *pdev,
return 0;
}
+/*
+ * Create the device and allocate the resources. For setting GPIO's there are
+ * two supported configurations:
+ *
+ * - single output register resource (named "dat").
+ * - set/clear pair (named "set" and "clr").
+ *
+ * For the single output register, this drives a 1 by setting a bit and a zero
+ * by clearing a bit. For the set clr pair, this drives a 1 by setting a bit
+ * in the set register and clears it by setting a bit in the clear register.
+ * The configuration is detected by which resources are present.
+ */
+static int bgpio_setup_io(struct platform_device *pdev,
+ struct bgpio_chip *bgc)
+{
+ struct resource *res_set;
+ struct resource *res_clr;
+
+ res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
+ res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
+ if (res_set && res_clr) {
+ if (resource_size(res_set) != resource_size(res_clr) ||
+ resource_size(res_set) != bgc->bits)
+ return -EINVAL;
+
+ bgc->reg_set = bgpio_request_and_map(&pdev->dev, res_set);
+ bgc->reg_clr = bgpio_request_and_map(&pdev->dev, res_clr);
+ if (!bgc->reg_set || !bgc->reg_clr)
+ return -ENOMEM;
+
+ bgc->gc.set = bgpio_set_with_clear;
+ } else if (res_set || res_clr) {
+ return -EINVAL;
+ } else {
+ bgc->gc.set = bgpio_set;
+ }
+
+ return 0;
+}
+
static int __devinit bgpio_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct bgpio_pdata *pdata = dev_get_platdata(dev);
struct bgpio_chip *bgc;
struct resource *res_dat;
- struct resource *res_set;
- struct resource *res_clr;
resource_size_t dat_sz;
int bits;
int ret;
@@ -272,23 +315,6 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
if (!bgc->reg_dat)
return -ENOMEM;
- res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
- res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
- if (res_set && res_clr) {
- if (resource_size(res_set) != resource_size(res_clr) ||
- resource_size(res_set) != dat_sz)
- return -EINVAL;
-
- bgc->reg_set = bgpio_request_and_map(dev, res_set);
- bgc->reg_clr = bgpio_request_and_map(dev, res_clr);
- if (!bgc->reg_set || !bgc->reg_clr)
- return -ENOMEM;
- } else if (res_set || res_clr) {
- return -EINVAL;
- }
-
- spin_lock_init(&bgc->lock);
-
if (pdata) {
bgc->gc.base = pdata->base;
if (pdata->ngpio > 0)
@@ -302,13 +328,17 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
if (ret)
return ret;
+ ret = bgpio_setup_io(pdev, bgc);
+ if (ret)
+ return ret;
+
+ spin_lock_init(&bgc->lock);
bgc->data = bgc->read_reg(bgc->reg_dat);
bgc->gc.ngpio = ngpio;
bgc->gc.direction_input = bgpio_dir_in;
bgc->gc.direction_output = bgpio_dir_out;
bgc->gc.get = bgpio_get;
- bgc->gc.set = bgpio_set;
bgc->gc.dev = dev;
bgc->gc.label = dev_name(dev);
--
1.7.4.2
Some controllers have separate input and output registers. For these
controllers, allow a register named "in" to be used for reading the
value of a GPIO pin.
Signed-off-by: Jamie Iles <[email protected]>
Acked-by: Anton Vorontsov <[email protected]>
Cc: Grant Likely <[email protected]>
---
drivers/gpio/basic_mmio_gpio.c | 31 +++++++++++++++++++++++++++++--
1 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
index 9be1867..ed4b0c6 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -70,6 +70,7 @@ struct bgpio_chip {
void __iomem *reg_dat;
void __iomem *reg_set;
void __iomem *reg_clr;
+ void __iomem *reg_in;
/* Number of bits (GPIOs): <register width> * 8. */
int bits;
@@ -148,13 +149,20 @@ static unsigned long bgpio_pin2mask_be(struct bgpio_chip *bgc,
return 1 << (bgc->bits - 1 - pin);
}
-static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
+static int bgpio_get_dat(struct gpio_chip *gc, unsigned int gpio)
{
struct bgpio_chip *bgc = to_bgpio_chip(gc);
return bgc->read_reg(bgc->reg_dat) & bgc->pin2mask(bgc, gpio);
}
+static int bgpio_get_in(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+
+ return bgc->read_reg(bgc->reg_in) & bgc->pin2mask(bgc, gpio);
+}
+
static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
{
struct bgpio_chip *bgc = to_bgpio_chip(gc);
@@ -254,12 +262,19 @@ static int bgpio_setup_accessors(struct platform_device *pdev,
* by clearing a bit. For the set clr pair, this drives a 1 by setting a bit
* in the set register and clears it by setting a bit in the clear register.
* The configuration is detected by which resources are present.
+ *
+ * For getting GPIO values, there are two supported configurations:
+ *
+ * - single output/input register resource (named "dat").
+ * - separate output/input register resources (named "dat" and "in").
+ *
*/
static int bgpio_setup_io(struct platform_device *pdev,
struct bgpio_chip *bgc)
{
struct resource *res_set;
struct resource *res_clr;
+ struct resource *res_in;
res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
@@ -280,6 +295,19 @@ static int bgpio_setup_io(struct platform_device *pdev,
bgc->gc.set = bgpio_set;
}
+ res_in = platform_get_resource_byname(pdev, IORESOURCE_MEM, "in");
+ if (res_in) {
+ bgc->reg_in = bgpio_request_and_map(&pdev->dev, res_in);
+ if (!bgc->reg_in)
+ return -ENOMEM;
+ bgc->gc.get = bgpio_get_in;
+ } else {
+ bgc->gc.get = bgpio_get_dat;
+ }
+
+ return 0;
+}
+
return 0;
}
@@ -338,7 +366,6 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
bgc->gc.ngpio = ngpio;
bgc->gc.direction_input = bgpio_dir_in;
bgc->gc.direction_output = bgpio_dir_out;
- bgc->gc.get = bgpio_get;
bgc->gc.dev = dev;
bgc->gc.label = dev_name(dev);
--
1.7.4.2
Most controllers require the direction of a GPIO to be set by writing to
a direction register. Add support for either an input direction
register or an output direction register.
Signed-off-by: Jamie Iles <[email protected]>
Acked-by: Anton Vorontsov <[email protected]>
Cc: Grant Likely <[email protected]>
---
drivers/gpio/basic_mmio_gpio.c | 102 +++++++++++++++++++++++++++++++++++++++-
1 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
index ed4b0c6..9e0157a 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -71,6 +71,7 @@ struct bgpio_chip {
void __iomem *reg_set;
void __iomem *reg_clr;
void __iomem *reg_in;
+ void __iomem *reg_dir;
/* Number of bits (GPIOs): <register width> * 8. */
int bits;
@@ -89,6 +90,9 @@ struct bgpio_chip {
/* Shadowed data register to clear/set bits safely. */
unsigned long data;
+
+ /* Shadowed direction registers to clear/set direction safely. */
+ unsigned long dir;
};
static struct bgpio_chip *to_bgpio_chip(struct gpio_chip *gc)
@@ -193,15 +197,72 @@ static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
bgc->write_reg(bgc->reg_clr, mask);
}
+static int bgpio_simple_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+ return 0;
+}
+
+static int bgpio_simple_dir_out(struct gpio_chip *gc, unsigned int gpio,
+ int val)
+{
+ gc->set(gc, gpio, val);
+
+ return 0;
+}
+
static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ unsigned long flags;
+
+ spin_lock_irqsave(&bgc->lock, flags);
+ bgc->dir &= ~bgc->pin2mask(bgc, gpio);
+ bgc->write_reg(bgc->reg_dir, bgc->dir);
+ spin_unlock_irqrestore(&bgc->lock, flags);
+
return 0;
}
static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ unsigned long flags;
+
+ gc->set(gc, gpio, val);
+
+ spin_lock_irqsave(&bgc->lock, flags);
+ bgc->dir |= bgc->pin2mask(bgc, gpio);
+ bgc->write_reg(bgc->reg_dir, bgc->dir);
+ spin_unlock_irqrestore(&bgc->lock, flags);
+
+ return 0;
+}
+
+static int bgpio_dir_in_inv(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ unsigned long flags;
+
+ spin_lock_irqsave(&bgc->lock, flags);
+ bgc->dir |= bgc->pin2mask(bgc, gpio);
+ bgc->write_reg(bgc->reg_dir, bgc->dir);
+ spin_unlock_irqrestore(&bgc->lock, flags);
+
+ return 0;
+}
+
+static int bgpio_dir_out_inv(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ unsigned long flags;
+
gc->set(gc, gpio, val);
+ spin_lock_irqsave(&bgc->lock, flags);
+ bgc->dir &= ~bgc->pin2mask(bgc, gpio);
+ bgc->write_reg(bgc->reg_dir, bgc->dir);
+ spin_unlock_irqrestore(&bgc->lock, flags);
+
return 0;
}
@@ -268,6 +329,13 @@ static int bgpio_setup_accessors(struct platform_device *pdev,
* - single output/input register resource (named "dat").
* - separate output/input register resources (named "dat" and "in").
*
+ * For setting the GPIO direction, there are three supported configurations:
+ *
+ * - simple bidirection GPIO that requires no configuration.
+ * - an output direction register (named "dirout") where a 1 bit
+ * indicates the GPIO is an output.
+ * - an input direction register (named "dirin") where a 1 bit indicates
+ * the GPIO is an input.
*/
static int bgpio_setup_io(struct platform_device *pdev,
struct bgpio_chip *bgc)
@@ -308,6 +376,34 @@ static int bgpio_setup_io(struct platform_device *pdev,
return 0;
}
+static int bgpio_setup_direction(struct platform_device *pdev,
+ struct bgpio_chip *bgc)
+{
+ struct resource *res_dirout;
+ struct resource *res_dirin;
+
+ res_dirout = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ "dirout");
+ res_dirin = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirin");
+ if (res_dirout && res_dirin) {
+ return -EINVAL;
+ } else if (res_dirout) {
+ bgc->reg_dir = bgpio_request_and_map(&pdev->dev, res_dirout);
+ if (!bgc->reg_dir)
+ return -ENOMEM;
+ bgc->gc.direction_output = bgpio_dir_out;
+ bgc->gc.direction_input = bgpio_dir_in;
+ } else if (res_dirin) {
+ bgc->reg_dir = bgpio_request_and_map(&pdev->dev, res_dirin);
+ if (!bgc->reg_dir)
+ return -ENOMEM;
+ bgc->gc.direction_output = bgpio_dir_out_inv;
+ bgc->gc.direction_input = bgpio_dir_in_inv;
+ } else {
+ bgc->gc.direction_output = bgpio_simple_dir_out;
+ bgc->gc.direction_input = bgpio_simple_dir_in;
+ }
+
return 0;
}
@@ -361,11 +457,13 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
return ret;
spin_lock_init(&bgc->lock);
+ ret = bgpio_setup_direction(pdev, bgc);
+ if (ret)
+ return ret;
+
bgc->data = bgc->read_reg(bgc->reg_dat);
bgc->gc.ngpio = ngpio;
- bgc->gc.direction_input = bgpio_dir_in;
- bgc->gc.direction_output = bgpio_dir_out;
bgc->gc.dev = dev;
bgc->gc.label = dev_name(dev);
--
1.7.4.2
Some platforms may have a number of GPIO that is less than the register
width of the peripheral.
Signed-off-by: Jamie Iles <[email protected]>
Acked-by: Anton Vorontsov <[email protected]>
Cc: Grant Likely <[email protected]>
---
drivers/gpio/basic_mmio_gpio.c | 18 ++++++++++++------
include/linux/basic_mmio_gpio.h | 1 +
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
index 5db5de4..2b2d384 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -239,6 +239,7 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
resource_size_t dat_sz;
int bits;
int ret;
+ int ngpio;
res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
if (!res_dat)
@@ -249,6 +250,7 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
return -EINVAL;
bits = dat_sz * 8;
+ ngpio = bits;
if (bits > BITS_PER_LONG)
return -EINVAL;
@@ -277,13 +279,22 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
spin_lock_init(&bgc->lock);
+ if (pdata) {
+ bgc->gc.base = pdata->base;
+ if (pdata->ngpio > 0)
+ ngpio = pdata->ngpio;
+ } else {
+ bgc->gc.base = -1;
+ }
+
bgc->bits = bits;
ret = bgpio_setup_accessors(pdev, bgc);
if (ret)
return ret;
bgc->data = bgc->read_reg(bgc->reg_dat);
- bgc->gc.ngpio = bits;
+
+ bgc->gc.ngpio = ngpio;
bgc->gc.direction_input = bgpio_dir_in;
bgc->gc.direction_output = bgpio_dir_out;
bgc->gc.get = bgpio_get;
@@ -291,11 +302,6 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
bgc->gc.dev = dev;
bgc->gc.label = dev_name(dev);
- if (pdata)
- bgc->gc.base = pdata->base;
- else
- bgc->gc.base = -1;
-
platform_set_drvdata(pdev, bgc);
ret = gpiochip_add(&bgc->gc);
diff --git a/include/linux/basic_mmio_gpio.h b/include/linux/basic_mmio_gpio.h
index 198087a..f23ec73 100644
--- a/include/linux/basic_mmio_gpio.h
+++ b/include/linux/basic_mmio_gpio.h
@@ -15,6 +15,7 @@
struct bgpio_pdata {
int base;
+ int ngpio;
};
#endif /* __BASIC_MMIO_GPIO_H */
--
1.7.4.2
On Fri, Apr 08, 2011 at 03:16:50PM +0100, Jamie Iles wrote:
[...]
> + res_in = platform_get_resource_byname(pdev, IORESOURCE_MEM, "in");
> + if (res_in) {
> + bgc->reg_in = bgpio_request_and_map(&pdev->dev, res_in);
> + if (!bgc->reg_in)
> + return -ENOMEM;
> + bgc->gc.get = bgpio_get_in;
> + } else {
> + bgc->gc.get = bgpio_get_dat;
> + }
> +
> + return 0;
> +}
> +
> return 0;
> }
Hm.. will this compile?
--
Anton Vorontsov
Email: [email protected]
On Fri, Apr 08, 2011 at 03:16:51PM +0100, Jamie Iles wrote:
[...]
> static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> {
> + struct bgpio_chip *bgc = to_bgpio_chip(gc);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&bgc->lock, flags);
For readability, I would still add an empty line here.
> + bgc->dir &= ~bgc->pin2mask(bgc, gpio);
> + bgc->write_reg(bgc->reg_dir, bgc->dir);
And here.
> + spin_unlock_irqrestore(&bgc->lock, flags);
> +
> return 0;
> }
>
> static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> {
> + struct bgpio_chip *bgc = to_bgpio_chip(gc);
> + unsigned long flags;
> +
> + gc->set(gc, gpio, val);
> +
> + spin_lock_irqsave(&bgc->lock, flags);
Ditto.
> + bgc->dir |= bgc->pin2mask(bgc, gpio);
> + bgc->write_reg(bgc->reg_dir, bgc->dir);
Ditto.
> + spin_unlock_irqrestore(&bgc->lock, flags);
> +
> + return 0;
> +}
> +
> +static int bgpio_dir_in_inv(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct bgpio_chip *bgc = to_bgpio_chip(gc);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&bgc->lock, flags);
etc...
Thanks,
--
Anton Vorontsov
Email: [email protected]
On Fri, Apr 08, 2011 at 06:36:41PM +0400, Anton Vorontsov wrote:
> On Fri, Apr 08, 2011 at 03:16:50PM +0100, Jamie Iles wrote:
> [...]
> > + res_in = platform_get_resource_byname(pdev, IORESOURCE_MEM, "in");
> > + if (res_in) {
> > + bgc->reg_in = bgpio_request_and_map(&pdev->dev, res_in);
> > + if (!bgc->reg_in)
> > + return -ENOMEM;
> > + bgc->gc.get = bgpio_get_in;
> > + } else {
> > + bgc->gc.get = bgpio_get_dat;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > return 0;
> > }
>
> Hm.. will this compile?
No, I accidentally squashed patches 6 and 7 together then obviously
didn't split them properly. I'll fix this (and the spacing around the
locking and repost).
Sorry!
Jamie
On Fri, Apr 08, 2011 at 03:16:50PM +0100, Jamie Iles wrote:
> Some controllers have separate input and output registers. For these
> controllers, allow a register named "in" to be used for reading the
> value of a GPIO pin.
>
> Signed-off-by: Jamie Iles <[email protected]>
> Acked-by: Anton Vorontsov <[email protected]>
> Cc: Grant Likely <[email protected]>
> ---
> drivers/gpio/basic_mmio_gpio.c | 31 +++++++++++++++++++++++++++++--
> 1 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
> index 9be1867..ed4b0c6 100644
> --- a/drivers/gpio/basic_mmio_gpio.c
> +++ b/drivers/gpio/basic_mmio_gpio.c
> @@ -70,6 +70,7 @@ struct bgpio_chip {
> void __iomem *reg_dat;
> void __iomem *reg_set;
> void __iomem *reg_clr;
> + void __iomem *reg_in;
Btw, I wonder if it makes sense (and more logical) for separate output/
input registers case to use "reg_dat" for 'input' and 'reg_set' for
output?
I.e. no need for 'reg_in', but instead modify some logic in
bgpio_setup_io(), and implement bgpio_set_set:
if (res_dat && res_set && !res_clr) {
bgc->gc.get = bgpio_get_dat;
bgc->gc.set = bgpio_set_set;
} else {
bgc->gc.get = bgpio_get_dat;
bgc->gc.set = bgpio_set_dat;
}
Thanks,
--
Anton Vorontsov
Email: [email protected]
On Fri, Apr 08, 2011 at 06:46:28PM +0400, Anton Vorontsov wrote:
> On Fri, Apr 08, 2011 at 03:16:50PM +0100, Jamie Iles wrote:
> > Some controllers have separate input and output registers. For these
> > controllers, allow a register named "in" to be used for reading the
> > value of a GPIO pin.
> >
> > Signed-off-by: Jamie Iles <[email protected]>
> > Acked-by: Anton Vorontsov <[email protected]>
> > Cc: Grant Likely <[email protected]>
> > ---
> > drivers/gpio/basic_mmio_gpio.c | 31 +++++++++++++++++++++++++++++--
> > 1 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
> > index 9be1867..ed4b0c6 100644
> > --- a/drivers/gpio/basic_mmio_gpio.c
> > +++ b/drivers/gpio/basic_mmio_gpio.c
> > @@ -70,6 +70,7 @@ struct bgpio_chip {
> > void __iomem *reg_dat;
> > void __iomem *reg_set;
> > void __iomem *reg_clr;
> > + void __iomem *reg_in;
>
> Btw, I wonder if it makes sense (and more logical) for separate output/
> input registers case to use "reg_dat" for 'input' and 'reg_set' for
> output?
Sounds like a good idea. Something like this?
Jamie
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
index 054fbee..fc3826a 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -185,6 +185,24 @@ static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
bgc->write_reg(bgc->reg_clr, mask);
}
+static void bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ unsigned long mask = bgc->pin2mask(bgc, gpio);
+ unsigned long flags;
+
+ spin_lock_irqsave(&bgc->lock, flags);
+
+ if (val)
+ bgc->data |= mask;
+ else
+ bgc->data &= ~mask;
+
+ bgc->write_reg(bgc->reg_set, bgc->data);
+
+ spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
{
return 0;
@@ -245,10 +263,12 @@ static int bgpio_setup_accessors(struct platform_device *pdev,
/*
* Create the device and allocate the resources. For setting GPIO's there are
- * two supported configurations:
+ * three supported configurations:
*
- * - single output register resource (named "dat").
+ * - single input/output register resource (named "dat").
* - set/clear pair (named "set" and "clr").
+ * - single output register resource and single input resource ("set" and
+ * dat").
*
* For the single output register, this drives a 1 by setting a bit and a zero
* by clearing a bit. For the set clr pair, this drives a 1 by setting a bit
@@ -260,6 +280,15 @@ static int bgpio_setup_io(struct platform_device *pdev,
{
struct resource *res_set;
struct resource *res_clr;
+ struct resource *res_dat;
+
+ res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
+ if (!res_dat)
+ return -EINVAL;
+
+ bgc->reg_dat = bgpio_request_and_map(&pdev->dev, res_dat);
+ if (!bgc->reg_dat)
+ return -ENOMEM;
res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
@@ -274,12 +303,22 @@ static int bgpio_setup_io(struct platform_device *pdev,
return -ENOMEM;
bgc->gc.set = bgpio_set_with_clear;
- } else if (res_set || res_clr) {
- return -EINVAL;
+ } else if (res_set && !res_clr) {
+ if (resource_size(res_set) != resource_size(res_dat) ||
+ resource_size(res_set) != bgc->bits / 8)
+ return -EINVAL;
+
+ bgc->reg_set = bgpio_request_and_map(&pdev->dev, res_set);
+ if (!bgc->reg_set)
+ return -ENOMEM;
+
+ bgc->gc.set = bgpio_set_set;
} else {
bgc->gc.set = bgpio_set;
}
+ bgc->gc.get = bgpio_get;
+
return 0;
}
@@ -311,10 +350,6 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
if (!bgc)
return -ENOMEM;
- bgc->reg_dat = bgpio_request_and_map(dev, res_dat);
- if (!bgc->reg_dat)
- return -ENOMEM;
-
if (pdata) {
bgc->gc.base = pdata->base;
if (pdata->ngpio > 0)
@@ -338,7 +373,6 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
bgc->gc.ngpio = ngpio;
bgc->gc.direction_input = bgpio_dir_in;
bgc->gc.direction_output = bgpio_dir_out;
- bgc->gc.get = bgpio_get;
bgc->gc.dev = dev;
bgc->gc.label = dev_name(dev);
--
1.7.4.2
On Fri, Apr 08, 2011 at 05:11:13PM +0100, Jamie Iles wrote:
[...]
> > > +++ b/drivers/gpio/basic_mmio_gpio.c
> > > @@ -70,6 +70,7 @@ struct bgpio_chip {
> > > void __iomem *reg_dat;
> > > void __iomem *reg_set;
> > > void __iomem *reg_clr;
> > > + void __iomem *reg_in;
> >
> > Btw, I wonder if it makes sense (and more logical) for separate output/
> > input registers case to use "reg_dat" for 'input' and 'reg_set' for
> > output?
>
> Sounds like a good idea. Something like this?
Yep, exactly.
[...]
> + } else if (res_set && !res_clr) {
> + if (resource_size(res_set) != resource_size(res_dat) ||
> + resource_size(res_set) != bgc->bits / 8)
We derive bgc->bits from resource_size(res_dat), so '!= bgc->bits / 8'
condition is uneeded.
Thanks,
--
Anton Vorontsov
Email: [email protected]
On Fri, Apr 08, 2011 at 08:26:30PM +0400, Anton Vorontsov wrote:
> On Fri, Apr 08, 2011 at 05:11:13PM +0100, Jamie Iles wrote:
> [...]
> > > > +++ b/drivers/gpio/basic_mmio_gpio.c
> > > > @@ -70,6 +70,7 @@ struct bgpio_chip {
> > > > void __iomem *reg_dat;
> > > > void __iomem *reg_set;
> > > > void __iomem *reg_clr;
> > > > + void __iomem *reg_in;
> > >
> > > Btw, I wonder if it makes sense (and more logical) for separate output/
> > > input registers case to use "reg_dat" for 'input' and 'reg_set' for
> > > output?
> >
> > Sounds like a good idea. Something like this?
>
> Yep, exactly.
>
> [...]
> > + } else if (res_set && !res_clr) {
> > + if (resource_size(res_set) != resource_size(res_dat) ||
> > + resource_size(res_set) != bgc->bits / 8)
>
> We derive bgc->bits from resource_size(res_dat), so '!= bgc->bits / 8'
> condition is uneeded.
Doh! Not sure how I missed that! I'll fix these up and repost them all
on Monday.
Thanks,
Jamie