| Message ID | 20230911131638.443055-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v2] pinctrl: renesas: pinctrl-rzg2l: Add validation of GPIO pin in rzg2l_gpio_request() | expand |
Hi Prabhakar, On Mon, Sep 11, 2023 at 3:17 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Validate the GPIO pin request in rzg2l_gpio_request() callback using > rzg2l_validate_gpio_pin() function. This stops any accidental usage > of GPIO pins which are not supported by the SoCs. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > v1->v2 > * Maintained reverse christmas tree order. > * Included RB/TB tags from Claudiu. Thanks for your patch! > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -795,12 +795,18 @@ static const struct pinconf_ops rzg2l_pinctrl_confops = { > static int rzg2l_gpio_request(struct gpio_chip *chip, unsigned int offset) > { > struct rzg2l_pinctrl *pctrl = gpiochip_get_data(chip); > + const struct pinctrl_pin_desc *pin = &pctrl->desc.pins[offset]; > u32 port = RZG2L_PIN_ID_TO_PORT(offset); > u8 bit = RZG2L_PIN_ID_TO_PIN(offset); > + u64 *pin_data = pin->drv_data; isn't drv_data a pointer to unsigned int aka u32? > unsigned long flags; > u8 reg8; > int ret; > > + ret = rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit); > + if (ret) > + return ret; > + > ret = pinctrl_gpio_request(chip->base + offset); > if (ret) > return ret; The rest LGTM. Gr{oetje,eeting}s, Geert
Hi Geert, Thank you for the review. On Mon, Sep 18, 2023 at 1:31 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Mon, Sep 11, 2023 at 3:17 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Validate the GPIO pin request in rzg2l_gpio_request() callback using > > rzg2l_validate_gpio_pin() function. This stops any accidental usage > > of GPIO pins which are not supported by the SoCs. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > --- > > v1->v2 > > * Maintained reverse christmas tree order. > > * Included RB/TB tags from Claudiu. > > Thanks for your patch! > > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > @@ -795,12 +795,18 @@ static const struct pinconf_ops rzg2l_pinctrl_confops = { > > static int rzg2l_gpio_request(struct gpio_chip *chip, unsigned int offset) > > { > > struct rzg2l_pinctrl *pctrl = gpiochip_get_data(chip); > > + const struct pinctrl_pin_desc *pin = &pctrl->desc.pins[offset]; > > u32 port = RZG2L_PIN_ID_TO_PORT(offset); > > u8 bit = RZG2L_PIN_ID_TO_PIN(offset); > > + u64 *pin_data = pin->drv_data; > > isn't drv_data a pointer to unsigned int aka u32? > Agreed. I will fix this and send a v3. Cheers, Prabhakar > > unsigned long flags; > > u8 reg8; > > int ret; > > > > + ret = rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit); > > + if (ret) > > + return ret; > > + > > ret = pinctrl_gpio_request(chip->base + offset); > > if (ret) > > return ret; > > The rest LGTM. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > 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
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 37cdfe4b04f9..cc52a154d201 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -795,12 +795,18 @@ static const struct pinconf_ops rzg2l_pinctrl_confops = { static int rzg2l_gpio_request(struct gpio_chip *chip, unsigned int offset) { struct rzg2l_pinctrl *pctrl = gpiochip_get_data(chip); + const struct pinctrl_pin_desc *pin = &pctrl->desc.pins[offset]; u32 port = RZG2L_PIN_ID_TO_PORT(offset); u8 bit = RZG2L_PIN_ID_TO_PIN(offset); + u64 *pin_data = pin->drv_data; unsigned long flags; u8 reg8; int ret; + ret = rzg2l_validate_gpio_pin(pctrl, *pin_data, port, bit); + if (ret) + return ret; + ret = pinctrl_gpio_request(chip->base + offset); if (ret) return ret;