| Message ID | e6da9398355648fc72dc3f674a7c11e114c37d61.1517795461.git.baolin.wang@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/2] dt-bindings: gpio: Add Spreadtrum GPIO controller documentation | expand |
On Mon, Feb 5, 2018 at 2:55 AM, Baolin Wang <baolin.wang@linaro.org> wrote: > The Spreadtrum SC9860 platform GPIO controller contains 16 groups and > each group contains 16 GPIOs. Each GPIO can set input/output and has > the interrupt capability. > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > --- > Changes since v2: Hi Baolin, sorry for taking so long to review :( I think you need to add depends on OF_GPIO to the dependencies. Else the build will break on compile test on things like Usermode Linux that doesn't have IOMEM. > +/* GPIO registers definition */ > +#define SPRD_GPIO_DATA 0x0 > +#define SPRD_GPIO_DMSK 0x4 > +#define SPRD_GPIO_DIR 0x8 > +#define SPRD_GPIO_IS 0xc > +#define SPRD_GPIO_IBE 0x10 > +#define SPRD_GPIO_IEV 0x14 > +#define SPRD_GPIO_IE 0x18 > +#define SPRD_GPIO_RIS 0x1c > +#define SPRD_GPIO_MIS 0x20 > +#define SPRD_GPIO_IC 0x24 > +#define SPRD_GPIO_INEN 0x28 So this is very simple. And the only reason we cannot use GPIO_GENERIC is that we have these groups inside the controller and a shared interrupt line :/ Hm yeah I cannot think of anything better anyway. Have you contemplated just putting them into the device tree like this: ap_gpio0: gpio@40280000 { compatible = "sprd,sc9860-gpio"; reg = <0 0x40280000 0 0x80>; gpio-controller; #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>; }; ap_gpio1: gpio@40280080 { compatible = "sprd,sc9860-gpio"; reg = <0 0x40280080 0 0x80>; gpio-controller; #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>; }; ap_gpio2: gpio@40280100 { compatible = "sprd,sc9860-gpio"; reg = <0 0x40280100 0 0x80>; gpio-controller; #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>; }; (...) ? It is fine to have 16 driver instances if you grab the interrupt with IRQF_SHARED and really just handle the IRQ if it is for "your" instance. The upside is that you could then use GPIO_GENERIC and get a very small and simple driver. I understand that the current approach is also appealing though and I see why. I'm not gonna say no to it, so if you strongly prefer this approach we can go for it. Just wanted to point out alternatives. > +/* We have 16 groups GPIOs and each group contain 16 GPIOs */ > +#define SPRD_GPIO_GROUP_NR 16 > +#define SPRD_GPIO_NR 256 > +#define SPRD_GPIO_GROUP_SIZE 0x80 > +#define SPRD_GPIO_GROUP_MASK GENMASK(15, 0) > +#define SPRD_GPIO_BIT(x) ((x) & (SPRD_GPIO_GROUP_NR - 1)) Please rename this from "groups" to "banks" because in the GPIO subsystem everyone talks about "banks" not "groups". This last thing many drivers do like this: bit = x % 15; but it is up to you, either way works (and probably result in the same assembly). > +static inline void __iomem *sprd_gpio_group_base(struct sprd_gpio *sprd_gpio, > + unsigned int group) > +{ > + return sprd_gpio->base + SPRD_GPIO_GROUP_SIZE * group; > +} Since you're always using this like this: void __iomem *base = sprd_gpio_group_base(sprd_gpio, offset / SPRD_GPIO_GROUP_NR); Why not simply have the offset as parameter to the function instead of group number and do the division inside this static inline? > +static void sprd_gpio_update(struct gpio_chip *chip, unsigned int offset, > + unsigned int reg, unsigned int val) I would use u16 reg. > +{ > + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip); > + void __iomem *base = sprd_gpio_group_base(sprd_gpio, > + offset / SPRD_GPIO_GROUP_NR); > + u32 shift = SPRD_GPIO_BIT(offset); > + unsigned long flags; > + u32 orig, tmp; > + > + spin_lock_irqsave(&sprd_gpio->lock, flags); > + orig = readl_relaxed(base + reg); > + > + tmp = (orig & ~BIT(shift)) | (val << shift); > + writel_relaxed(tmp, base + reg); > + spin_unlock_irqrestore(&sprd_gpio->lock, flags); > +} You don't need shift, orig and tmp variables here, I think it gets hard to read. I would do it like this: u32 tmp; tmp = readl_relaxed(base + reg); if (val) tmp |= BIT(SPRD_GPIO_BIT(offset)); else tmp &= ~BIT(SPRD_GPIO_BIT(offset)); writel_relaxed(tmp, base + reg); I don't know if the macros really help much. Maybe just inline it: tmp = readl_relaxed(base + reg); if (val) tmp |= BIT(offset % 15); else tmp &= ~BIT(offset % 15); writel_relaxed(tmp, base + reg); It depends on taste. Just consider my options. (I'll go with what you feel is easiest to read.) > +static int sprd_gpio_read(struct gpio_chip *chip, unsigned int offset, > + unsigned int reg) > +{ > + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip); > + void __iomem *base = sprd_gpio_group_base(sprd_gpio, > + offset / SPRD_GPIO_GROUP_NR); > + u32 value = readl_relaxed(base + reg) & SPRD_GPIO_GROUP_MASK; > + u32 shift = SPRD_GPIO_BIT(offset); > + > + return !!(value & BIT(shift)); I would just return !!(readl_relaxed(base + reg) & BIT(offset % 15)): But again it is a matter of taste. (the rest looks fine!) Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, On 13 February 2018 at 16:13, Linus Walleij <linus.walleij@linaro.org> wrote: > On Mon, Feb 5, 2018 at 2:55 AM, Baolin Wang <baolin.wang@linaro.org> wrote: > >> The Spreadtrum SC9860 platform GPIO controller contains 16 groups and >> each group contains 16 GPIOs. Each GPIO can set input/output and has >> the interrupt capability. >> >> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> >> --- >> Changes since v2: > > Hi Baolin, > > sorry for taking so long to review :( > > I think you need to add > > depends on OF_GPIO to the dependencies. Else the build > will break on compile test on things like Usermode Linux > that doesn't have IOMEM. OK. > >> +/* GPIO registers definition */ >> +#define SPRD_GPIO_DATA 0x0 >> +#define SPRD_GPIO_DMSK 0x4 >> +#define SPRD_GPIO_DIR 0x8 >> +#define SPRD_GPIO_IS 0xc >> +#define SPRD_GPIO_IBE 0x10 >> +#define SPRD_GPIO_IEV 0x14 >> +#define SPRD_GPIO_IE 0x18 >> +#define SPRD_GPIO_RIS 0x1c >> +#define SPRD_GPIO_MIS 0x20 >> +#define SPRD_GPIO_IC 0x24 >> +#define SPRD_GPIO_INEN 0x28 > > So this is very simple. And the only reason we cannot use > GPIO_GENERIC is that we have these groups inside the controller > and a shared interrupt line :/ > > Hm yeah I cannot think of anything better anyway. > > Have you contemplated just putting them into the device tree > like this: > > ap_gpio0: gpio@40280000 { > compatible = "sprd,sc9860-gpio"; > reg = <0 0x40280000 0 0x80>; > gpio-controller; > #gpio-cells = <2>; > interrupt-controller; > #interrupt-cells = <2>; > interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>; > }; > > ap_gpio1: gpio@40280080 { > compatible = "sprd,sc9860-gpio"; > reg = <0 0x40280080 0 0x80>; > gpio-controller; > #gpio-cells = <2>; > interrupt-controller; > #interrupt-cells = <2>; > interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>; > }; > > ap_gpio2: gpio@40280100 { > compatible = "sprd,sc9860-gpio"; > reg = <0 0x40280100 0 0x80>; > gpio-controller; > #gpio-cells = <2>; > interrupt-controller; > #interrupt-cells = <2>; > interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>; > }; > > (...) > > ? > > It is fine to have 16 driver instances if you grab the interrupt > with IRQF_SHARED and really just handle the IRQ if it is for > "your" instance. The upside is that you could then use > GPIO_GENERIC and get a very small and simple driver. > > I understand that the current approach is also appealing though > and I see why. I'm not gonna say no to it, so if you strongly > prefer this approach we can go for it. Just wanted to point out > alternatives. Thanks for pointing out another approach. But we only have one controller with multiple banks, so I think it is not easy to maintain if we split one GPIO controller into multiple ones. Moreover if we have more banks in future, there will be more device node to maintain. So I still would like to use one device node to describe the only one GPIO device. > >> +/* We have 16 groups GPIOs and each group contain 16 GPIOs */ >> +#define SPRD_GPIO_GROUP_NR 16 >> +#define SPRD_GPIO_NR 256 >> +#define SPRD_GPIO_GROUP_SIZE 0x80 >> +#define SPRD_GPIO_GROUP_MASK GENMASK(15, 0) >> +#define SPRD_GPIO_BIT(x) ((x) & (SPRD_GPIO_GROUP_NR - 1)) > > Please rename this from "groups" to "banks" because in the GPIO > subsystem everyone talks about "banks" not "groups". Sure. > > This last thing many drivers do like this: > > bit = x % 15; > > but it is up to you, either way works (and probably result in the > same assembly). OK. > >> +static inline void __iomem *sprd_gpio_group_base(struct sprd_gpio *sprd_gpio, >> + unsigned int group) >> +{ >> + return sprd_gpio->base + SPRD_GPIO_GROUP_SIZE * group; >> +} > > Since you're always using this like this: > > void __iomem *base = sprd_gpio_group_base(sprd_gpio, > offset / SPRD_GPIO_GROUP_NR); > > Why not simply have the offset as parameter to the function > instead of group number and do the division inside this > static inline? In sprd_gpio_irq_handler() function, we should pass the group number as parameter, so to consistent with them, I use group number as parameter. > >> +static void sprd_gpio_update(struct gpio_chip *chip, unsigned int offset, >> + unsigned int reg, unsigned int val) > > I would use u16 reg. Sure. > >> +{ >> + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip); >> + void __iomem *base = sprd_gpio_group_base(sprd_gpio, >> + offset / SPRD_GPIO_GROUP_NR); >> + u32 shift = SPRD_GPIO_BIT(offset); >> + unsigned long flags; >> + u32 orig, tmp; >> + >> + spin_lock_irqsave(&sprd_gpio->lock, flags); >> + orig = readl_relaxed(base + reg); >> + >> + tmp = (orig & ~BIT(shift)) | (val << shift); >> + writel_relaxed(tmp, base + reg); >> + spin_unlock_irqrestore(&sprd_gpio->lock, flags); >> +} > > You don't need shift, orig and tmp variables here, I think it > gets hard to read. > > I would do it like this: > > u32 tmp; > > tmp = readl_relaxed(base + reg); > if (val) > tmp |= BIT(SPRD_GPIO_BIT(offset)); > else > tmp &= ~BIT(SPRD_GPIO_BIT(offset)); > writel_relaxed(tmp, base + reg); OK. This seems more simpler and clearer. > > I don't know if the macros really help much. Maybe just inline it: > > tmp = readl_relaxed(base + reg); > if (val) > tmp |= BIT(offset % 15); > else > tmp &= ~BIT(offset % 15); > writel_relaxed(tmp, base + reg); > > It depends on taste. Just consider my options. > (I'll go with what you feel is easiest to read.) OK. > >> +static int sprd_gpio_read(struct gpio_chip *chip, unsigned int offset, >> + unsigned int reg) >> +{ >> + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip); >> + void __iomem *base = sprd_gpio_group_base(sprd_gpio, >> + offset / SPRD_GPIO_GROUP_NR); >> + u32 value = readl_relaxed(base + reg) & SPRD_GPIO_GROUP_MASK; >> + u32 shift = SPRD_GPIO_BIT(offset); >> + >> + return !!(value & BIT(shift)); > > I would just > > return !!(readl_relaxed(base + reg) & BIT(offset % 15)): > > But again it is a matter of taste. OK. Thanks for your useful comments.
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index d6a8e85..2ed1a88 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -404,6 +404,13 @@ config GPIO_SPEAR_SPICS help Say yes here to support ST SPEAr SPI Chip Select as GPIO device +config GPIO_SPRD + tristate "Spreadtrum GPIO support" + depends on ARCH_SPRD || COMPILE_TEST + select GPIOLIB_IRQCHIP + help + Say yes here to support Spreadtrum GPIO device. + config GPIO_STA2X11 bool "STA2x11/ConneXt GPIO support" depends on MFD_STA2X11 diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 4bc24fe..5b633a0 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -108,6 +108,7 @@ obj-$(CONFIG_GPIO_SCH) += gpio-sch.o obj-$(CONFIG_GPIO_SCH311X) += gpio-sch311x.o obj-$(CONFIG_GPIO_SODAVILLE) += gpio-sodaville.o obj-$(CONFIG_GPIO_SPEAR_SPICS) += gpio-spear-spics.o +obj-$(CONFIG_GPIO_SPRD) += gpio-sprd.o obj-$(CONFIG_GPIO_STA2X11) += gpio-sta2x11.o obj-$(CONFIG_GPIO_STMPE) += gpio-stmpe.o obj-$(CONFIG_GPIO_STP_XWAY) += gpio-stp-xway.o diff --git a/drivers/gpio/gpio-sprd.c b/drivers/gpio/gpio-sprd.c new file mode 100644 index 0000000..7c0f9f1 --- /dev/null +++ b/drivers/gpio/gpio-sprd.c @@ -0,0 +1,290 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Spreadtrum Communications Inc. + * Copyright (c) 2018 Linaro Ltd. + */ + +#include <linux/bitops.h> +#include <linux/gpio/driver.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/spinlock.h> + +/* GPIO registers definition */ +#define SPRD_GPIO_DATA 0x0 +#define SPRD_GPIO_DMSK 0x4 +#define SPRD_GPIO_DIR 0x8 +#define SPRD_GPIO_IS 0xc +#define SPRD_GPIO_IBE 0x10 +#define SPRD_GPIO_IEV 0x14 +#define SPRD_GPIO_IE 0x18 +#define SPRD_GPIO_RIS 0x1c +#define SPRD_GPIO_MIS 0x20 +#define SPRD_GPIO_IC 0x24 +#define SPRD_GPIO_INEN 0x28 + +/* We have 16 groups GPIOs and each group contain 16 GPIOs */ +#define SPRD_GPIO_GROUP_NR 16 +#define SPRD_GPIO_NR 256 +#define SPRD_GPIO_GROUP_SIZE 0x80 +#define SPRD_GPIO_GROUP_MASK GENMASK(15, 0) +#define SPRD_GPIO_BIT(x) ((x) & (SPRD_GPIO_GROUP_NR - 1)) + +struct sprd_gpio { + struct gpio_chip chip; + void __iomem *base; + spinlock_t lock; + int irq; +}; + +static inline void __iomem *sprd_gpio_group_base(struct sprd_gpio *sprd_gpio, + unsigned int group) +{ + return sprd_gpio->base + SPRD_GPIO_GROUP_SIZE * group; +} + +static void sprd_gpio_update(struct gpio_chip *chip, unsigned int offset, + unsigned int reg, unsigned int val) +{ + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip); + void __iomem *base = sprd_gpio_group_base(sprd_gpio, + offset / SPRD_GPIO_GROUP_NR); + u32 shift = SPRD_GPIO_BIT(offset); + unsigned long flags; + u32 orig, tmp; + + spin_lock_irqsave(&sprd_gpio->lock, flags); + orig = readl_relaxed(base + reg); + + tmp = (orig & ~BIT(shift)) | (val << shift); + writel_relaxed(tmp, base + reg); + spin_unlock_irqrestore(&sprd_gpio->lock, flags); +} + +static int sprd_gpio_read(struct gpio_chip *chip, unsigned int offset, + unsigned int reg) +{ + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip); + void __iomem *base = sprd_gpio_group_base(sprd_gpio, + offset / SPRD_GPIO_GROUP_NR); + u32 value = readl_relaxed(base + reg) & SPRD_GPIO_GROUP_MASK; + u32 shift = SPRD_GPIO_BIT(offset); + + return !!(value & BIT(shift)); +} + +static int sprd_gpio_request(struct gpio_chip *chip, unsigned int offset) +{ + sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 1); + return 0; +} + +static void sprd_gpio_free(struct gpio_chip *chip, unsigned int offset) +{ + sprd_gpio_update(chip, offset, SPRD_GPIO_DMSK, 0); +} + +static int sprd_gpio_direction_input(struct gpio_chip *chip, + unsigned int offset) +{ + sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 0); + sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 1); + return 0; +} + +static int sprd_gpio_direction_output(struct gpio_chip *chip, + unsigned int offset, int value) +{ + sprd_gpio_update(chip, offset, SPRD_GPIO_DIR, 1); + sprd_gpio_update(chip, offset, SPRD_GPIO_INEN, 0); + sprd_gpio_update(chip, offset, SPRD_GPIO_DATA, value); + return 0; +} + +static int sprd_gpio_get(struct gpio_chip *chip, unsigned int offset) +{ + return sprd_gpio_read(chip, offset, SPRD_GPIO_DATA); +} + +static void sprd_gpio_set(struct gpio_chip *chip, unsigned int offset, + int value) +{ + sprd_gpio_update(chip, offset, SPRD_GPIO_DATA, value); +} + +static void sprd_gpio_irq_mask(struct irq_data *data) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + u32 offset = irqd_to_hwirq(data); + + sprd_gpio_update(chip, offset, SPRD_GPIO_IE, 0); +} + +static void sprd_gpio_irq_ack(struct irq_data *data) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + u32 offset = irqd_to_hwirq(data); + + sprd_gpio_update(chip, offset, SPRD_GPIO_IC, 1); +} + +static void sprd_gpio_irq_unmask(struct irq_data *data) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + u32 offset = irqd_to_hwirq(data); + + sprd_gpio_update(chip, offset, SPRD_GPIO_IE, 1); +} + +static int sprd_gpio_irq_set_type(struct irq_data *data, + unsigned int flow_type) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + u32 offset = irqd_to_hwirq(data); + + switch (flow_type) { + case IRQ_TYPE_EDGE_RISING: + sprd_gpio_update(chip, offset, SPRD_GPIO_IS, 0); + sprd_gpio_update(chip, offset, SPRD_GPIO_IBE, 0); + sprd_gpio_update(chip, offset, SPRD_GPIO_IEV, 1); + irq_set_handler_locked(data, handle_edge_irq); + break; + case IRQ_TYPE_EDGE_FALLING: + sprd_gpio_update(chip, offset, SPRD_GPIO_IS, 0); + sprd_gpio_update(chip, offset, SPRD_GPIO_IBE, 0); + sprd_gpio_update(chip, offset, SPRD_GPIO_IEV, 0); + irq_set_handler_locked(data, handle_edge_irq); + break; + case IRQ_TYPE_EDGE_BOTH: + sprd_gpio_update(chip, offset, SPRD_GPIO_IS, 0); + sprd_gpio_update(chip, offset, SPRD_GPIO_IBE, 1); + irq_set_handler_locked(data, handle_edge_irq); + break; + case IRQ_TYPE_LEVEL_HIGH: + sprd_gpio_update(chip, offset, SPRD_GPIO_IS, 1); + sprd_gpio_update(chip, offset, SPRD_GPIO_IBE, 0); + sprd_gpio_update(chip, offset, SPRD_GPIO_IEV, 1); + irq_set_handler_locked(data, handle_level_irq); + break; + case IRQ_TYPE_LEVEL_LOW: + sprd_gpio_update(chip, offset, SPRD_GPIO_IS, 1); + sprd_gpio_update(chip, offset, SPRD_GPIO_IBE, 0); + sprd_gpio_update(chip, offset, SPRD_GPIO_IEV, 0); + irq_set_handler_locked(data, handle_level_irq); + break; + default: + return -EINVAL; + } + + return 0; +} + +static void sprd_gpio_irq_handler(struct irq_desc *desc) +{ + struct gpio_chip *chip = irq_desc_get_handler_data(desc); + struct irq_chip *ic = irq_desc_get_chip(desc); + struct sprd_gpio *sprd_gpio = gpiochip_get_data(chip); + u32 group, n, girq; + + chained_irq_enter(ic, desc); + + for (group = 0; group * SPRD_GPIO_GROUP_NR < chip->ngpio; group++) { + void __iomem *base = sprd_gpio_group_base(sprd_gpio, group); + unsigned long reg = readl_relaxed(base + SPRD_GPIO_MIS) & + SPRD_GPIO_GROUP_MASK; + + for_each_set_bit(n, ®, SPRD_GPIO_GROUP_NR) { + girq = irq_find_mapping(chip->irq.domain, + group * SPRD_GPIO_GROUP_NR + n); + + generic_handle_irq(girq); + } + + } + chained_irq_exit(ic, desc); +} + +static struct irq_chip sprd_gpio_irqchip = { + .name = "sprd-gpio", + .irq_ack = sprd_gpio_irq_ack, + .irq_mask = sprd_gpio_irq_mask, + .irq_unmask = sprd_gpio_irq_unmask, + .irq_set_type = sprd_gpio_irq_set_type, + .flags = IRQCHIP_SKIP_SET_WAKE, +}; + +static int sprd_gpio_probe(struct platform_device *pdev) +{ + struct gpio_irq_chip *irq; + struct sprd_gpio *sprd_gpio; + struct resource *res; + int ret; + + sprd_gpio = devm_kzalloc(&pdev->dev, sizeof(*sprd_gpio), GFP_KERNEL); + if (!sprd_gpio) + return -ENOMEM; + + sprd_gpio->irq = platform_get_irq(pdev, 0); + if (sprd_gpio->irq < 0) { + dev_err(&pdev->dev, "Failed to get GPIO interrupt.\n"); + return sprd_gpio->irq; + } + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + sprd_gpio->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(sprd_gpio->base)) + return PTR_ERR(sprd_gpio->base); + + spin_lock_init(&sprd_gpio->lock); + + sprd_gpio->chip.label = dev_name(&pdev->dev); + sprd_gpio->chip.ngpio = SPRD_GPIO_NR; + sprd_gpio->chip.base = -1; + sprd_gpio->chip.parent = &pdev->dev; + sprd_gpio->chip.of_node = pdev->dev.of_node; + sprd_gpio->chip.request = sprd_gpio_request; + sprd_gpio->chip.free = sprd_gpio_free; + sprd_gpio->chip.get = sprd_gpio_get; + sprd_gpio->chip.set = sprd_gpio_set; + sprd_gpio->chip.direction_input = sprd_gpio_direction_input; + sprd_gpio->chip.direction_output = sprd_gpio_direction_output; + + irq = &sprd_gpio->chip.irq; + irq->chip = &sprd_gpio_irqchip; + irq->handler = handle_simple_irq; + irq->default_type = IRQ_TYPE_NONE; + irq->parent_handler = sprd_gpio_irq_handler; + irq->parent_handler_data = sprd_gpio; + irq->num_parents = 1; + irq->parents = &sprd_gpio->irq; + + ret = devm_gpiochip_add_data(&pdev->dev, &sprd_gpio->chip, sprd_gpio); + if (ret < 0) { + dev_err(&pdev->dev, "Could not register gpiochip %d\n", ret); + return ret; + } + + platform_set_drvdata(pdev, sprd_gpio); + return 0; +} + +static const struct of_device_id sprd_gpio_of_match[] = { + { .compatible = "sprd,sc9860-gpio", }, + { /* end of list */ }, +}; +MODULE_DEVICE_TABLE(of, sprd_gpio_of_match); + +static struct platform_driver sprd_gpio_driver = { + .probe = sprd_gpio_probe, + .driver = { + .name = "sprd-gpio", + .of_match_table = sprd_gpio_of_match, + }, +}; + +module_platform_driver_probe(sprd_gpio_driver, sprd_gpio_probe); + +MODULE_DESCRIPTION("Spreadtrum GPIO driver"); +MODULE_LICENSE("GPL v2");