| Message ID | 1371205241-17205-3-git-send-email-avinashphilipk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 14, 2013 at 12:20 PM, Philip Avinash <avinashphilipk@gmail.com> wrote: > From: KV Sujith <sujithkv@ti.com> > > - Add of_device_id for Davinci GPIO driver. > - Add function to populate data from DT. > - Modify the probe to read from DT if DT match is found. > - Add DT binding documentation for Davinci GPIO properties in a new file > gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. > > Signed-off-by: KV Sujith <sujithkv@ti.com> > Signed-off-by: Philip Avinash <avinashphilip@ti.com> > --- > Changes since v1: > - description for interrupts changed to reflecti > interrupt array usage. (...) > +static struct davinci_gpio_platform_data *davinci_gpio_set_pdata_of( > + struct platform_device *pdev) > +{ > + struct device_node *dn = pdev->dev.of_node; > + struct davinci_gpio_platform_data *pdata; > + u32 val, ret; > + > + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > + if (pdata) { Instead of this arrow pattern: if (!pdata) { dev_err( ... "out of memory allocating platform data"); return NULL; } And de-indent the rest. With that change: Acked-by: Linus Walleij <linus.walleij@linaro.org> I guess this may go through the DaVinci tree with the rest of the DaVinci patches? Yours, Linus Walleij
On 6/20/2013 12:40 AM, Linus Walleij wrote: > On Fri, Jun 14, 2013 at 12:20 PM, Philip Avinash > <avinashphilipk@gmail.com> wrote: > >> From: KV Sujith <sujithkv@ti.com> >> >> - Add of_device_id for Davinci GPIO driver. >> - Add function to populate data from DT. >> - Modify the probe to read from DT if DT match is found. >> - Add DT binding documentation for Davinci GPIO properties in a new file >> gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. >> >> Signed-off-by: KV Sujith <sujithkv@ti.com> >> Signed-off-by: Philip Avinash <avinashphilip@ti.com> >> --- >> Changes since v1: >> - description for interrupts changed to reflecti >> interrupt array usage. > (...) > >> +static struct davinci_gpio_platform_data *davinci_gpio_set_pdata_of( >> + struct platform_device *pdev) >> +{ >> + struct device_node *dn = pdev->dev.of_node; >> + struct davinci_gpio_platform_data *pdata; >> + u32 val, ret; >> + >> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); >> + if (pdata) { > > Instead of this arrow pattern: > > if (!pdata) { > dev_err( ... "out of memory allocating platform data"); > return NULL; > } > > And de-indent the rest. > > With that change: > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > I guess this may go through the DaVinci tree with the rest of the DaVinci > patches? Linus, yes, I will pick these up with your acks. Thanks, Sekhar
On 6/14/2013 3:50 PM, Philip Avinash wrote: > From: KV Sujith <sujithkv@ti.com> > > - Add of_device_id for Davinci GPIO driver. > - Add function to populate data from DT. > - Modify the probe to read from DT if DT match is found. > - Add DT binding documentation for Davinci GPIO properties in a new file > gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. > > Signed-off-by: KV Sujith <sujithkv@ti.com> > Signed-off-by: Philip Avinash <avinashphilip@ti.com> > --- > Changes since v1: > - description for interrupts changed to reflecti > interrupt array usage. > > .../devicetree/bindings/gpio/gpio-davinci.txt | 32 +++++++++++ > drivers/gpio/gpio-davinci.c | 57 ++++++++++++++++++-- > 2 files changed, 86 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt > new file mode 100644 > index 0000000..1c31638 > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt > @@ -0,0 +1,32 @@ > +Davinci GPIO controller bindings > + > +Required Properties: > +- compatible:"ti,da830-gpio" "ti,dm6441-gpio" instead since this came first on DM6441? > + > +- reg: Physical base address of the controller and length of memory mapped > + region. > + > +- interrupts: Array of GPIO interrupt number. > + > +- ngpio: The number of GPIO pins supported Is this a generic GPIO property? I could not find it in documentation. Looks like Marvell GPIO uses a similar property too, but even there it is not marked as Marvell specific. Should this be added as a generic GPIO property? > +- intc_irq_num: The number of IRQs supported by the Interrupt Controller You are not actually looking at the number of IRQs interrupt controller on the SoC supports (which is quite unrelated to GPIO module). What you are actually looking for is the base from where GPIO interrupt numbering can start. So "ti,davinci-gpio-irq-base" is more meaningful. That said, this property is probably not required if irqdomains are used (I myself have to read about that). Not sure if it is okay to add this property now waiting for irqdomain conversion. I will let Linus take a call. > + > +- gpio_unbanked: The number of GPIOs that have an individual interrupt > + line to processor. This is also a TI specific property so it should be "ti,davinci-gpio-unbanked". Thanks, Sekhar
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt new file mode 100644 index 0000000..1c31638 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,32 @@ +Davinci GPIO controller bindings + +Required Properties: +- compatible:"ti,da830-gpio" + +- reg: Physical base address of the controller and length of memory mapped + region. + +- interrupts: Array of GPIO interrupt number. + +- ngpio: The number of GPIO pins supported + +- intc_irq_num: The number of IRQs supported by the Interrupt Controller + +- gpio_unbanked: The number of GPIOs that have an individual interrupt + line to processor. + +Example: +#include <dt-bindings/interrupt-controller/irq.h> + +gpio: gpio@1e26000 { + compatible = "ti,da830-gpio"; + reg = <0x226000 0x1000>; + interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH + 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH + 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH + 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH + 50 IRQ_TYPE_EDGE_BOTH>; + ngpio = <144>; + intc_irq_num = <101>; + gpio_unbanked = <0>; +}; diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c index 475a5ece..cd2ed25 100644 --- a/drivers/gpio/gpio-davinci.c +++ b/drivers/gpio/gpio-davinci.c @@ -20,6 +20,8 @@ #include <linux/irqdomain.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/platform_data/gpio-davinci.h> @@ -137,6 +139,50 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned offset, int value) __raw_writel((1 << offset), value ? &g->set_data : &g->clr_data); } +static struct davinci_gpio_platform_data *davinci_gpio_set_pdata_of( + struct platform_device *pdev) +{ + struct device_node *dn = pdev->dev.of_node; + struct davinci_gpio_platform_data *pdata; + u32 val, ret; + + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); + if (pdata) { + ret = of_property_read_u32(dn, "ngpio", &val); + if (ret) + goto of_err; + + pdata->ngpio = val; + + ret = of_property_read_u32(dn, "gpio_unbanked", &val); + if (ret) + goto of_err; + + pdata->gpio_unbanked = val; + + ret = of_property_read_u32(dn, "intc_irq_num", &val); + if (ret) + goto of_err; + + pdata->intc_irq_num = val; + } + + return pdata; + +of_err: + dev_err(&pdev->dev, "Populating pdata from DT failed: err %d\n", ret); + return NULL; +} + +static const struct of_device_id davinci_gpio_ids[] = { + { + .compatible = "ti,da830-gpio", + }, + { }, +}; + +MODULE_DEVICE_TABLE(of, davinci_gpio_ids); + static int davinci_gpio_probe(struct platform_device *pdev) { int i, base; @@ -146,13 +192,17 @@ static int davinci_gpio_probe(struct platform_device *pdev) struct davinci_gpio_regs __iomem *regs; struct device *dev = &pdev->dev; struct resource *res; + const struct of_device_id *match = + of_match_device(of_match_ptr(davinci_gpio_ids), &pdev->dev); - pdata = dev->platform_data; + pdata = match ? davinci_gpio_set_pdata_of(pdev) : dev->platform_data; if (!pdata) { dev_err(dev, "No platform data found\n"); return -EINVAL; } + dev->platform_data = pdata; + /* * The gpio banks conceptually expose a segmented bitmap, * and "ngpio" is one more than the largest zero-based @@ -497,8 +547,9 @@ done: static struct platform_driver davinci_gpio_driver = { .probe = davinci_gpio_probe, .driver = { - .name = "davinci_gpio", - .owner = THIS_MODULE, + .name = "davinci_gpio", + .owner = THIS_MODULE, + .of_match_table = of_match_ptr(davinci_gpio_ids), }, };