Hi Alexandre,
On 4 December 2014 at 14:47, Alexandre Courbot <[email protected]> wrote:
> On Mon, Dec 1, 2014 at 9:09 PM, <[email protected]> wrote:
>> From: Kamlakant Patel <[email protected]>
>>
>> This patch converts MOXART GPIO driver to use basic_mmio_gpio
>> generic library.
>>
>> Signed-off-by: Kamlakant Patel <[email protected]>
>> Tested-by: Jonas Jensen <[email protected]>
>> ---
>> drivers/gpio/Kconfig | 1 +
>> drivers/gpio/gpio-moxart.c | 101
>> ++++++++++++++-------------------------------
>> 2 files changed, 32 insertions(+), 70 deletions(-)
>>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index 0959ca9..3bd4d63 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -184,6 +184,7 @@ config GPIO_F7188X
>> config GPIO_MOXART
>> bool "MOXART GPIO support"
>> depends on ARCH_MOXART
>> + select GPIO_GENERIC
>> help
>> Select this option to enable GPIO driver for
>> MOXA ART SoC devices.
>> diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
>> index 4661e18..122958f 100644
>> --- a/drivers/gpio/gpio-moxart.c
>> +++ b/drivers/gpio/gpio-moxart.c
>> @@ -23,21 +23,12 @@
>> #include <linux/delay.h>
>> #include <linux/timer.h>
>> #include <linux/bitops.h>
>> +#include <linux/basic_mmio_gpio.h>
>>
>> #define GPIO_DATA_OUT 0x00
>> #define GPIO_DATA_IN 0x04
>> #define GPIO_PIN_DIRECTION 0x08
>>
>> -struct moxart_gpio_chip {
>> - struct gpio_chip gpio;
>> - void __iomem *base;
>> -};
>> -
>> -static inline struct moxart_gpio_chip *to_moxart_gpio(struct gpio_chip
>> *chip)
>> -{
>> - return container_of(chip, struct moxart_gpio_chip, gpio);
>> -}
>> -
>> static int moxart_gpio_request(struct gpio_chip *chip, unsigned offset)
>> {
>> return pinctrl_request_gpio(offset);
>> @@ -48,90 +39,60 @@ static void moxart_gpio_free(struct gpio_chip *chip,
>> unsigned offset)
>> pinctrl_free_gpio(offset);
>> }
>>
>> -static void moxart_gpio_set(struct gpio_chip *chip, unsigned offset, int
>> value)
>> -{
>> - struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
>> - void __iomem *ioaddr = gc->base + GPIO_DATA_OUT;
>> - u32 reg = readl(ioaddr);
>> -
>> - if (value)
>> - reg = reg | BIT(offset);
>> - else
>> - reg = reg & ~BIT(offset);
>> -
>> - writel(reg, ioaddr);
>> -}
>> -
>> static int moxart_gpio_get(struct gpio_chip *chip, unsigned offset)
>> {
>> - struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
>> - u32 ret = readl(gc->base + GPIO_PIN_DIRECTION);
>> + struct bgpio_chip *bgc = to_bgpio_chip(chip);
>> + u32 ret = bgc->read_reg(bgc->reg_dir);
>>
>> if (ret & BIT(offset))
>> - return !!(readl(gc->base + GPIO_DATA_OUT) & BIT(offset));
>> + return !!(bgc->read_reg(bgc->reg_set) & BIT(offset));
>> else
>> - return !!(readl(gc->base + GPIO_DATA_IN) & BIT(offset));
>> -}
>> -
>> -static int moxart_gpio_direction_input(struct gpio_chip *chip, unsigned
>> offset)
>> -{
>> - struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
>> - void __iomem *ioaddr = gc->base + GPIO_PIN_DIRECTION;
>> -
>> - writel(readl(ioaddr) & ~BIT(offset), ioaddr);
>> - return 0;
>> -}
>> -
>> -static int moxart_gpio_direction_output(struct gpio_chip *chip,
>> - unsigned offset, int value)
>> -{
>> - struct moxart_gpio_chip *gc = to_moxart_gpio(chip);
>> - void __iomem *ioaddr = gc->base + GPIO_PIN_DIRECTION;
>> -
>> - moxart_gpio_set(chip, offset, value);
>> - writel(readl(ioaddr) | BIT(offset), ioaddr);
>> - return 0;
>> + return !!(bgc->read_reg(bgc->reg_dat) & BIT(offset));
>> }
>>
>> -static struct gpio_chip moxart_template_chip = {
>> - .label = "moxart-gpio",
>> - .request = moxart_gpio_request,
>> - .free = moxart_gpio_free,
>> - .direction_input = moxart_gpio_direction_input,
>> - .direction_output = moxart_gpio_direction_output,
>> - .set = moxart_gpio_set,
>> - .get = moxart_gpio_get,
>> - .ngpio = 32,
>> - .owner = THIS_MODULE,
>> -};
>> -
>> static int moxart_gpio_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> struct resource *res;
>> - struct moxart_gpio_chip *mgc;
>> + struct bgpio_chip *bgc;
>> + void __iomem *base;
>> int ret;
>>
>> - mgc = devm_kzalloc(dev, sizeof(*mgc), GFP_KERNEL);
>> - if (!mgc)
>> + bgc = devm_kzalloc(dev, sizeof(*bgc), GFP_KERNEL);
>> + if (!bgc)
>> return -ENOMEM;
>> - mgc->gpio = moxart_template_chip;
>>
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> - mgc->base = devm_ioremap_resource(dev, res);
>> - if (IS_ERR(mgc->base))
>> - return PTR_ERR(mgc->base);
>> + base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>>
>> - mgc->gpio.dev = dev;
>> + ret = bgpio_init(bgc, dev, 4, base + GPIO_DATA_IN,
>> + base + GPIO_DATA_OUT, NULL,
>> + base + GPIO_PIN_DIRECTION, NULL, 0);
>> + if (ret) {
>> + dev_err(&pdev->dev, "bgpio_init failed\n");
>> + return ret;
>> + }
>>
>> - ret = gpiochip_add(&mgc->gpio);
>> + bgc->gc.label = "moxart-gpio";
>> + bgc->gc.request = moxart_gpio_request;
>> + bgc->gc.free = moxart_gpio_free;
>> + bgc->gc.get = moxart_gpio_get;
>> + bgc->data = bgc->read_reg(bgc->reg_set);
>> + bgc->gc.base = 0;
>
> Do we actually want all instances of this driver to clain GPIOs 0..31?
>
Here is what I got from Jonas in previous discussion:
...
Thanks, it works, tested on UC-7112-LX hardware.
I have one additional nit..
The GPIO base number is implicitly changed from 0 to 224
(ARCH_NR_GPIOS (256) - ngpio (32)) which happen because of
bgpio_init() (it sets base -1 / gpiochip_find_base() on
gpiochip_add()). Which is confusing since the valid range (from user
space) used to be 0-31. So on export we now get:
[root@zurkon ~]# echo 24 > /sys/class/gpio/export
[ 61.640000] gpio-24 (?): gpiod_request: status -517
[ 61.650000] export_store: status -19
I see other drivers explicitly set base after bgpio_init(), my
suggestion is that we do the same here e.g. :
> + bgc->gc.label = "moxart-gpio";
> + bgc->gc.request = moxart_gpio_request;
> + bgc->gc.free = moxart_gpio_free;
> + bgc->gc.get = moxart_gpio_get;
> + bgc->data = bgc->read_reg(bgc->reg_set);
> + bgc->gc.ngpio = 32;
> + bgc->gc.dev = dev;
> + bgc->gc.owner = THIS_MODULE;
bgc->gc.base = 0;
Tested-by: Jonas Jensen <[email protected]>
...
> If so,
>
> Acked-by: Alexandre Courbot <[email protected]>
>
> Since this patch greatly simplifies the code and has been properly tested.
Thanks,
Kamlakant Patel
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
