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

[Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>]

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