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

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

[Shawn Guo]

In case of ACPI boot, GPIO core does the right thing to parse GPIO pin
configs from ACPI table, and call into gpio_chip's .set_config hook for
setting them up.  It enables such support on qcom platform by using
generic config function, which in turn calls into .pin_config_set of
pinconf for setting up hardware.  For qcom platform, it's possible to
reuse pin group config functions for pin config hooks, because every pin
is maintained as a single group.

This change fixes the problem that Touchpad of Lenovo Flex 5G laptop
doesn't work with ACPI boot, because PullUp config of Touchpad GpioInt
pin is not set up by kernel driver.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index a591be9f380a..2526f299bdce 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -493,6 +493,8 @@ static const struct pinconf_ops msm_pinconf_ops = {
 	.is_generic		= true,
 	.pin_config_group_get	= msm_config_group_get,
 	.pin_config_group_set	= msm_config_group_set,
+	.pin_config_get		= msm_config_group_get,
+	.pin_config_set		= msm_config_group_set,
 };
 
 static int msm_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
@@ -717,6 +719,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,
 	.request          = gpiochip_generic_request,
 	.free             = gpiochip_generic_free,
 	.dbg_show         = msm_gpio_dbg_show,
-- 
2.17.1

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

[Andy Shevchenko]

On Sun, Feb 28, 2021 at 4:55 AM Shawn Guo <shawn.guo@linaro.org> wrote:
>
> In case of ACPI boot, GPIO core does the right thing to parse GPIO pin
> configs from ACPI table, and call into gpio_chip's .set_config hook for
> setting them up.  It enables such support on qcom platform by using
> generic config function, which in turn calls into .pin_config_set of
> pinconf for setting up hardware.  For qcom platform, it's possible to
> reuse pin group config functions for pin config hooks, because every pin
> is maintained as a single group.
>
> This change fixes the problem that Touchpad of Lenovo Flex 5G laptop
> doesn't work with ACPI boot, because PullUp config of Touchpad GpioInt
> pin is not set up by kernel driver.

by the kernel

...

>         .pin_config_group_get   = msm_config_group_get,
>         .pin_config_group_set   = msm_config_group_set,
> +       .pin_config_get         = msm_config_group_get,
> +       .pin_config_set         = msm_config_group_set,

This can't be right. They have different semantics.

-- 
With Best Regards,
Andy Shevchenko

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

[Shawn Guo]

On Mon, Mar 01, 2021 at 07:07:03PM +0200, Andy Shevchenko wrote:
> On Sun, Feb 28, 2021 at 4:55 AM Shawn Guo <shawn.guo@linaro.org> wrote:
> >
> > In case of ACPI boot, GPIO core does the right thing to parse GPIO pin
> > configs from ACPI table, and call into gpio_chip's .set_config hook for
> > setting them up.  It enables such support on qcom platform by using
> > generic config function, which in turn calls into .pin_config_set of
> > pinconf for setting up hardware.  For qcom platform, it's possible to
> > reuse pin group config functions for pin config hooks, because every pin
> > is maintained as a single group.
> >
> > This change fixes the problem that Touchpad of Lenovo Flex 5G laptop
> > doesn't work with ACPI boot, because PullUp config of Touchpad GpioInt
> > pin is not set up by kernel driver.
> 
> by the kernel
> 
> ...
> 
> >         .pin_config_group_get   = msm_config_group_get,
> >         .pin_config_group_set   = msm_config_group_set,
> > +       .pin_config_get         = msm_config_group_get,
> > +       .pin_config_set         = msm_config_group_set,
> 
> This can't be right. They have different semantics.

As mentioned in the commit log, this works considering every pin is
maintained as a single group on Qualcomm platform.  So configuring one
pin is essentially to configure the group containing the pin.  I can do
something like below, if you think that's easier to understand.

static int msm_config_pin_get(struct pinctrl_dev *pctldev, unsigned int pin,
                              unsigned long *config)
{
        return msm_config_group_get(pctldev, pin, config);
}

static int msm_config_pin_set(struct pinctrl_dev *pctldev, unsigned pin,
                              unsigned long *configs, unsigned num_configs)
{
        return msm_config_group_set(pctldev, pin, configs, num_configs);
}

static const struct pinconf_ops msm_pinconf_ops = {
	...
        .pin_config_get         = msm_config_pin_get,
        .pin_config_set         = msm_config_pin_set,
};

Shawn

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

[Andy Shevchenko]

On Tue, Mar 2, 2021 at 3:02 AM Shawn Guo <shawn.guo@linaro.org> wrote:
>
> On Mon, Mar 01, 2021 at 07:07:03PM +0200, Andy Shevchenko wrote:
> > On Sun, Feb 28, 2021 at 4:55 AM Shawn Guo <shawn.guo@linaro.org> wrote:
> > >
> > > In case of ACPI boot, GPIO core does the right thing to parse GPIO pin
> > > configs from ACPI table, and call into gpio_chip's .set_config hook for
> > > setting them up.  It enables such support on qcom platform by using
> > > generic config function, which in turn calls into .pin_config_set of
> > > pinconf for setting up hardware.  For qcom platform, it's possible to
> > > reuse pin group config functions for pin config hooks, because every pin
> > > is maintained as a single group.
> > >
> > > This change fixes the problem that Touchpad of Lenovo Flex 5G laptop
> > > doesn't work with ACPI boot, because PullUp config of Touchpad GpioInt
> > > pin is not set up by kernel driver.
> >
> > by the kernel
> >
> > ...
> >
> > >         .pin_config_group_get   = msm_config_group_get,
> > >         .pin_config_group_set   = msm_config_group_set,
> > > +       .pin_config_get         = msm_config_group_get,
> > > +       .pin_config_set         = msm_config_group_set,
> >
> > This can't be right. They have different semantics.
>
> As mentioned in the commit log, this works considering every pin is
> maintained as a single group on Qualcomm platform.  So configuring one
> pin is essentially to configure the group containing the pin.  I can do
> something like below, if you think that's easier to understand.

Yes, in your case you must have a selector == # of a pin for each
individual pin (not just declared that you have enough selectors to
cover the amount of pins and beyond).

If there is such a requirement, go with it.

-- 
With Best Regards,
Andy Shevchenko