Re: [PATCH v2] pinctrl: qcom: support gpio_chip .set_config call

Bjorn Andersson
2021-03-04T05:07:19Z

On Wed 03 Mar 20:24 CST 2021, Shawn Guo wrote:

> On Wed, Mar 03, 2021 at 08:51:06AM -0600, Bjorn Andersson wrote:
> > > @@ -717,6 +731,7 @@ static const struct gpio_chip msm_gpio_template = {
> > > .get_direction = msm_gpio_get_direction,
> > > .get = msm_gpio_get,
> > > .set = msm_gpio_set,
> > > + .set_config = gpiochip_generic_config,
> >
> > Generally the pinconf/pinmux part of the driver deals with groups, and
> > the gpio_chip deals with gpio numbers. So I think that either
> > gpiochip_generic_config() should somehow do the translation, or we
> > should use a different function that does it (even though there's no
> > translation).
>
> The transition from GPIO to PINCTRL world is being done by
> pinctrl_gpio_set_config() which is wrapped by gpiochip_generic_config().
> This is nothing new from gpiochip_generic_request() and
> gpiochip_generic_free() right below.
>

You're right, this seems analog to the two other gpiochip_generic_*
helpers used below, I should have made this comment on the previous
hunk.

I don't think it's right to have the driver implement both group based
and pin based pin_conf_set/get functions (at least not for the same
entities), but I've not found the time to review the core code to
determine if this would cause any issues.

Regards,
Bjorn

> > > .request = gpiochip_generic_request,
> > > .free = gpiochip_generic_free,
> > > .dbg_show = msm_gpio_dbg_show,
>
> Shawn