[RFC,v2,3/5] pinctrl: mediatek: common: Expose more configurations to GPIO set_config

Currently the set_config callback in the gpio_chip registered by the
pinctrl-mtk-common driver only supports configuring a single parameter
on specific pins (the input debounce of the EINT controller, on pins
that support it), even though many other configurations are already
implemented and available through the pinctrl API for configuration of
pins by the Devicetree and other drivers.

Expose all configurations currently implemented through the GPIO API so
they can also be set from userspace, which is particularly useful to
allow testing them from userspace.
---
 drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 48 ++++++++++++++++-----------
 1 file changed, 28 insertions(+), 20 deletions(-)

On Sat, Oct 26, 2024 at 5:16 AM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> Currently the set_config callback in the gpio_chip registered by the
> pinctrl-mtk-common driver only supports configuring a single parameter
> on specific pins (the input debounce of the EINT controller, on pins
> that support it), even though many other configurations are already
> implemented and available through the pinctrl API for configuration of
> pins by the Devicetree and other drivers.
>
> Expose all configurations currently implemented through the GPIO API so
> they can also be set from userspace, which is particularly useful to
> allow testing them from userspace.

Missing signed-off-by?

Otherwise,

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

> ---
>  drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 48 ++++++++++++++++-----------
>  1 file changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> index 91edb539925a49b4302866b9ac36f580cc189fb5..7f9764b474c4e7d0d4c3d6e542bdb7df0264daec 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> @@ -62,13 +62,12 @@ static unsigned int mtk_get_port(struct mtk_pinctrl *pctl, unsigned long pin)
>                         << pctl->devdata->port_shf;
>  }
>
> -static int mtk_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
> -                       struct pinctrl_gpio_range *range, unsigned offset,
> -                       bool input)
> +static int mtk_common_pin_set_direction(struct mtk_pinctrl *pctl,
> +                                       unsigned int offset,
> +                                       bool input)
>  {
>         unsigned int reg_addr;
>         unsigned int bit;
> -       struct mtk_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
>
>         reg_addr = mtk_get_port(pctl, offset) + pctl->devdata->dir_offset;
>         bit = BIT(offset & pctl->devdata->mode_mask);
> @@ -86,6 +85,15 @@ static int mtk_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
>         return 0;
>  }
>
> +static int mtk_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
> +                       struct pinctrl_gpio_range *range, unsigned int offset,
> +                       bool input)
> +{
> +       struct mtk_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +       return mtk_common_pin_set_direction(pctl, offset, input);
> +}
> +
>  static void mtk_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
>  {
>         unsigned int reg_addr;
> @@ -363,12 +371,11 @@ static int mtk_pconf_set_pull_select(struct mtk_pinctrl *pctl,
>         return 0;
>  }
>
> -static int mtk_pconf_parse_conf(struct pinctrl_dev *pctldev,
> +static int mtk_pconf_parse_conf(struct mtk_pinctrl *pctl,
>                 unsigned int pin, enum pin_config_param param,
> -               enum pin_config_param arg)
> +               u32 arg)
>  {
>         int ret = 0;
> -       struct mtk_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
>
>         switch (param) {
>         case PIN_CONFIG_BIAS_DISABLE:
> @@ -381,15 +388,15 @@ static int mtk_pconf_parse_conf(struct pinctrl_dev *pctldev,
>                 ret = mtk_pconf_set_pull_select(pctl, pin, true, false, arg);
>                 break;
>         case PIN_CONFIG_INPUT_ENABLE:
> -               mtk_pmx_gpio_set_direction(pctldev, NULL, pin, true);
> +               mtk_common_pin_set_direction(pctl, pin, true);
>                 ret = mtk_pconf_set_ies_smt(pctl, pin, arg, param);
>                 break;
>         case PIN_CONFIG_OUTPUT:
>                 mtk_gpio_set(pctl->chip, pin, arg);
> -               ret = mtk_pmx_gpio_set_direction(pctldev, NULL, pin, false);
> +               ret = mtk_common_pin_set_direction(pctl, pin, false);
>                 break;
>         case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
> -               mtk_pmx_gpio_set_direction(pctldev, NULL, pin, true);
> +               mtk_common_pin_set_direction(pctl, pin, true);
>                 ret = mtk_pconf_set_ies_smt(pctl, pin, arg, param);
>                 break;
>         case PIN_CONFIG_DRIVE_STRENGTH:
> @@ -421,7 +428,7 @@ static int mtk_pconf_group_set(struct pinctrl_dev *pctldev, unsigned group,
>         int i, ret;
>
>         for (i = 0; i < num_configs; i++) {
> -               ret = mtk_pconf_parse_conf(pctldev, g->pin,
> +               ret = mtk_pconf_parse_conf(pctl, g->pin,
>                         pinconf_to_config_param(configs[i]),
>                         pinconf_to_config_argument(configs[i]));
>                 if (ret < 0)
> @@ -870,19 +877,20 @@ static int mtk_gpio_set_config(struct gpio_chip *chip, unsigned offset,
>         struct mtk_pinctrl *pctl = gpiochip_get_data(chip);
>         const struct mtk_desc_pin *pin;
>         unsigned long eint_n;
> -       u32 debounce;
> +       enum pin_config_param param = pinconf_to_config_param(config);
> +       u32 arg = pinconf_to_config_argument(config);
>
> -       if (pinconf_to_config_param(config) != PIN_CONFIG_INPUT_DEBOUNCE)
> -               return -ENOTSUPP;
> +       if (param == PIN_CONFIG_INPUT_DEBOUNCE) {
> +               pin = pctl->devdata->pins + offset;
> +               if (pin->eint.eintnum == NO_EINT_SUPPORT)
> +                       return -EINVAL;
>
> -       pin = pctl->devdata->pins + offset;
> -       if (pin->eint.eintnum == NO_EINT_SUPPORT)
> -               return -EINVAL;
> +               eint_n = pin->eint.eintnum;
>
> -       debounce = pinconf_to_config_argument(config);
> -       eint_n = pin->eint.eintnum;
> +               return mtk_eint_set_debounce(pctl->eint, eint_n, arg);
> +       }
>
> -       return mtk_eint_set_debounce(pctl->eint, eint_n, debounce);
> +       return mtk_pconf_parse_conf(pctl, offset, param, arg);
>  }
>
>  static const struct gpio_chip mtk_gpio_chip = {
>
> --
> 2.47.0
>
>

On Fri, Nov 01, 2024 at 03:54:58PM +0800, Chen-Yu Tsai wrote:
> On Sat, Oct 26, 2024 at 5:16 AM Nícolas F. R. A. Prado
> <nfraprado@collabora.com> wrote:
> >
> > Currently the set_config callback in the gpio_chip registered by the
> > pinctrl-mtk-common driver only supports configuring a single parameter
> > on specific pins (the input debounce of the EINT controller, on pins
> > that support it), even though many other configurations are already
> > implemented and available through the pinctrl API for configuration of
> > pins by the Devicetree and other drivers.
> >
> > Expose all configurations currently implemented through the GPIO API so
> > they can also be set from userspace, which is particularly useful to
> > allow testing them from userspace.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

> 
> Missing signed-off-by?

Huh, I don't know how the pre-send checks didn't catch it, will take a look,
thanks for pointing it out! I've added the SoB above so it can
still be merged if no further versions are required.

Thanks,
Nícolas