Hi Biju
On Fri, Aug 3, 2018 at 6:47 PM Biju Das <biju.das@xxxxxxxxxxxxxx> wrote:
> > Subject: Re: [PATCH v2 1/5] gpio: rcar: Add GPIO hole support
> > On Thu, Aug 2, 2018 at 4:17 PM Biju Das <biju.das@xxxxxxxxxxxxxx> wrote:
> > > GPIO hole is present in RZ/G1C SoC. Valid GPIO pins on bank3 are in
> > > the range GP3_0 to GP3_16 and GP3_27 to GP3_29. The GPIO pins
> > between
> > > GP3_17 to GP3_26 are unused. Add support for handling unused GPIO's.
> > >
> > > Signed-off-by: Biju Das <biju.das@xxxxxxxxxxxxxx>
> > > Reviewed-by: Fabrizio Castro <fabrizio.castro@xxxxxxxxxxxxxx>
> > > @@ -414,6 +422,22 @@ static int gpio_rcar_parse_dt(struct gpio_rcar_priv
> > *p, unsigned int *npins)
> > > *npins = RCAR_MAX_GPIO_PER_BANK;
> > > }
> > >
> > > + p->bank_info.gpiomsk = 0;
> > > + len = of_property_count_u32_elems(np, "gpio-reserved-ranges");
> > > + if (len < 0 || len % 2 != 0)
> > > + return 0;
> > > +
> > > + for (i = 0; i < len; i += 2) {
> > > + of_property_read_u32_index(np, "gpio-reserved-ranges",
> > > + i, &start);
> > > + of_property_read_u32_index(np, "gpio-reserved-ranges",
> > > + i + 1, &count);
> > > + if (start >= *npins || start + count >= *npins)
> > > + continue;
> > > +
> > > + p->bank_info.gpiomsk = GENMASK(start + count - 1, start);
> > > + }
> >
> > of_gpiochip_init_valid_mask() already does the same, for chip->valid_mask.
>
> Currently we are not setting "gpio_chip.of_node" variable in the driver.
> So the below function in gpiochip_init_valid_mask() will return negative value and won't allocate memory for valid_mask.
> size = of_property_count_u32_elems(np, "gpio-reserved-ranges");
Nice catch, I had missed that!
> We need to either set " of_node" variable or parse "gpio-reserved-ranges" and set " need_valid_mask=true ;"
> so that gpiochip_init_valid_mask() will allocate the valid_mask.
Agreed.
>
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -471,6 +495,7 @@ static int gpio_rcar_probe(struct platform_device
> > *pdev)
> > > gpio_chip->owner = THIS_MODULE;
> > > gpio_chip->base = -1;
> > > gpio_chip->ngpio = npins;
> > > + gpio_chip->need_valid_mask = p->bank_info.gpiomsk ? true :
> > > + false;
> >
> > gpiochip_init_valid_mask() already sets gpio_chip->need_valid_mask to true
> > if "gpio-reserved-ranges" is detected.
>
> The plan is to set "of_node" variable in driver. So that gpiochip_init_valid_mask() sets gpio_chip->need_valid_mask to true.
> What is your opinion on this?
Sounds fine to me.
You said on IRC that it currently crashes when "gpio-reserved-ranges" is used.
IMHO that's something that should be fixed separately, possibly for v4.19.
Looks like all other Renesas gpio drivers (some combined pinctrl/gpio), except
for gpio-em.c, also fail to set gpio_chip.of_node. Will have a closer look...
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
