Hi Jacopo,
Thank you for your review....again ;)
On Tuesday, November 13, 2018, jacopo mondi wrote:
> I would prefer the reverse xmas order too, but I'm not saying out loud
> as I understand is something hard to enforce, as it's a very minor
> issue :)
The next version will be very festive!
#
#
.#%###%##%##.
.##%###%##.
.%##%###.
.#%##%.
.###.
.#.
t
Christmas in Australia
> > + if (!of_property_read_bool(np, "gpio-controller")) {
> > + dev_info(priv->dev, "No gpio controller registered\n");
> > + return 0;
> > + }
>
> The bindings say this is mandatory... What do you think, would that
> make sense to have no gpio-controller support for this driver (testing
> apart :)
Good point. I don't need to check for this.
If it doesn't work..."RTFM"
> > + gpio_range.id = of_args.args[0];
>
> Do you think of_args need to be validated?
> As I understand them, id and pin_base are fixed at 0, while the pin
> number depends on the soc version/revision.
>
> Now I wonder if you would need a gpio-ranges property at all, but
> that's fine, it doesn't hurt...
My idea was that if another RZ/A2 with a different package size is
created, or even a RZ/A3 with the same pin controller, nothing in the driver
will have to change because the gpio-ranges is passed in and will tell
you how many ports you have.
As for validating the values, the only thing I can really check is that:
of_args.args[2] == RZA2_NPINS
Of course, now that I say that, I realize that if/when it does come time
to expand this driver beyond the 1 SOC that exists today, I will have
to stop using that hard coded RZA2_NPINS value...but I'll deal with that
when the time comes.
> > + ret = rza2_pinctrl_register(priv);
> > + if (ret)
> > + return ret;
> > +
> > + pr_info("RZ/A2 pin controller registered\n");
> nit: dev_info
Actually, I changed it to this to anticipate the day when more than one
SOC is supported:
dev_info(&pdev->dev, "Registered ports P0 - P%c\n",
port_names[priv->desc.npins / RZA2_PINS_PER_PORT - 1]);
> With this minor fixed, please add my
> Reviewed-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
Thank you.
Chris
