Hi Grant,
Thanks for the review.
On Wednesday 12 June 2013 12:49:11 Grant Likely wrote:
> On Tue, 21 May 2013 13:40:06 +0200, Laurent Pinchart wrote:
> > Add DT bindings for the gpio-rcar driver and read the device
> > configuration from the DT node at probe time if available.
> >
> > Cc: devicetree-discuss@lists.ozlabs.org
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+rene...@ideasonboard.com>
> > ---
> >
> > .../devicetree/bindings/gpio/renesas,gpio-rcar.txt | 52 +++++++++++++++++
> > drivers/gpio/gpio-rcar.c | 66 ++++++++++++++---
> > 2 files changed, 108 insertions(+), 10 deletions(-)
> > create mode 100644
> > Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
[snip]
> > diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
> > index 0f3d647..c153837 100644
> > --- a/drivers/gpio/gpio-rcar.c
> > +++ b/drivers/gpio/gpio-rcar.c
[snip]
> > +static void gpio_rcar_parse_pdata(struct gpio_rcar_priv *p)
> > +{
> > + struct gpio_rcar_config *pdata = p->pdev->dev.platform_data;
> > +#ifdef CONFIG_OF
> > + struct device_node *np = p->pdev->dev.of_node;
> > + struct of_phandle_args args;
> > + int ret;
> > +#endif
> > +
> > + if (pdata)
> > + p->config = *pdata;
> > +#ifdef CONFIG_OF
> > + else if (np) {
>
> Try this:
> else if ((IS_ENABLED(CONFIG_OF)) && np) {
>
> It's much better than adding #ifdef blocks to .c files. In v3.11 a bunch
> of the OF forward declarations get pulled out from under the #ifdef
> block so that you can use the above construct.
As a pull request has already been sent to the ARM SoC maintainers, and given
that this isn't a critical issue, I'll address it as a follow-up patch if
that's fine with you.
> > + ret = of_parse_phandle_with_args(np, "gpio-ranges",
> > + "#gpio-range-cells", 0, &args);
> > + p->config.number_of_pins = ret == 0 && args.args_count == 3
> > + ? args.args[2]
> > + : RCAR_MAX_GPIO_PER_BANK;
> > + p->config.gpio_base = -1;
> > + }
> > +#endif
> > +
> > + if (p->config.number_of_pins == 0 ||
> > + p->config.number_of_pins > RCAR_MAX_GPIO_PER_BANK) {
> > + dev_warn(&p->pdev->dev,
> > + "Invalid number of gpio lines %u, using %u\n",
> > + p->config.number_of_pins, RCAR_MAX_GPIO_PER_BANK);
> > + p->config.number_of_pins = RCAR_MAX_GPIO_PER_BANK;
> > + }
> > +}
--
Regards,
Laurent Pinchart
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
