Re: [PATCH v4 13/39] pinctrl: qcom: stub support for special GPIOs

[Sumit Garg <sumit.garg@linaro.org>]

On Fri, 16 Feb 2024 at 02:22, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Most platforms have a handful of "special" GPIOs, like the MMC
> clock/data lanes, UFS reset, etc. These don't follow the usually naming

s/usually/usual/

> scheme of "gpioX" and also have unique capabilities and registers. We
> can get away without supporting them all for now, but DT compatibility
> is still an issue.
>
> Add support for allowing these to be specified after the other pins, and
> make all pinmux/pinconf calls for them nop.

Yeah earlier incorrect configuration was done for these pins. So
having them nop is an improvement.

>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  arch/arm/mach-snapdragon/include/mach/gpio.h |  2 ++
>  drivers/gpio/msm_gpio.c                      | 20 ++++++++++++++++++++
>  drivers/pinctrl/qcom/pinctrl-qcom.c          | 12 ++++++++++++
>  3 files changed, 34 insertions(+)
>

Reviewed-by: Sumit Garg <sumit.garg@linaro.org>

-Sumit

> diff --git a/arch/arm/mach-snapdragon/include/mach/gpio.h b/arch/arm/mach-snapdragon/include/mach/gpio.h
> index 8dac62f870b9..c373f5a4cf3d 100644
> --- a/arch/arm/mach-snapdragon/include/mach/gpio.h
> +++ b/arch/arm/mach-snapdragon/include/mach/gpio.h
> @@ -13,6 +13,8 @@
>  struct msm_pin_data {
>         int pin_count;
>         const unsigned int *pin_offsets;
> +       /* Index of first special pin, these are ignored for now */
> +       unsigned int special_pins_start;
>  };
>
>  static inline u32 qcom_pin_offset(const unsigned int *offs, unsigned int selector)
> diff --git a/drivers/gpio/msm_gpio.c b/drivers/gpio/msm_gpio.c
> index 80cd28bb231f..8a5e8730e911 100644
> --- a/drivers/gpio/msm_gpio.c
> +++ b/drivers/gpio/msm_gpio.c
> @@ -39,6 +39,10 @@ static int msm_gpio_direction_input(struct udevice *dev, unsigned int gpio)
>  {
>         struct msm_gpio_bank *priv = dev_get_priv(dev);
>
> +       /* Always NOP for special pins, assume they're in the correct state */
> +       if (gpio >= priv->pin_data->special_pins_start)
> +               return 0;
> +
>         /* Disable OE bit */
>         clrsetbits_le32(priv->base + GPIO_CONFIG_REG(dev, gpio),
>                         GPIO_OE_MASK, GPIO_OE_DISABLE);
> @@ -50,6 +54,10 @@ static int msm_gpio_set_value(struct udevice *dev, unsigned int gpio, int value)
>  {
>         struct msm_gpio_bank *priv = dev_get_priv(dev);
>
> +       /* Always NOP for special pins, assume they're in the correct state */
> +       if (gpio >= priv->pin_data->special_pins_start)
> +               return 0;
> +
>         value = !!value;
>         /* set value */
>         writel(value << GPIO_OUT, priv->base + GPIO_IN_OUT_REG(dev, gpio));
> @@ -62,6 +70,10 @@ static int msm_gpio_direction_output(struct udevice *dev, unsigned int gpio,
>  {
>         struct msm_gpio_bank *priv = dev_get_priv(dev);
>
> +       /* Always NOP for special pins, assume they're in the correct state */
> +       if (gpio >= priv->pin_data->special_pins_start)
> +               return 0;
> +
>         value = !!value;
>         /* set value */
>         writel(value << GPIO_OUT, priv->base + GPIO_IN_OUT_REG(dev, gpio));
> @@ -76,6 +88,10 @@ static int msm_gpio_get_value(struct udevice *dev, unsigned int gpio)
>  {
>         struct msm_gpio_bank *priv = dev_get_priv(dev);
>
> +       /* Always NOP for special pins, assume they're in the correct state */
> +       if (gpio >= priv->pin_data->special_pins_start)
> +               return 0;
> +
>         return !!(readl(priv->base + GPIO_IN_OUT_REG(dev, gpio)) >> GPIO_IN);
>  }
>
> @@ -83,6 +99,10 @@ static int msm_gpio_get_function(struct udevice *dev, unsigned int gpio)
>  {
>         struct msm_gpio_bank *priv = dev_get_priv(dev);
>
> +       /* Always NOP for special pins, assume they're in the correct state */
> +       if (gpio >= priv->pin_data->special_pins_start)
> +               return 0;
> +
>         if (readl(priv->base + GPIO_CONFIG_REG(dev, gpio)) & GPIO_OE_ENABLE)
>                 return GPIOF_OUTPUT;
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-qcom.c b/drivers/pinctrl/qcom/pinctrl-qcom.c
> index dc3d8c4d9034..1ea4d21c41fc 100644
> --- a/drivers/pinctrl/qcom/pinctrl-qcom.c
> +++ b/drivers/pinctrl/qcom/pinctrl-qcom.c
> @@ -83,6 +83,10 @@ static int msm_pinmux_set(struct udevice *dev, unsigned int pin_selector,
>  {
>         struct msm_pinctrl_priv *priv = dev_get_priv(dev);
>
> +       /* Always NOP for special pins, assume they're in the correct state */
> +       if (pin_selector >= priv->data->pin_data.special_pins_start)
> +               return 0;
> +
>         clrsetbits_le32(priv->base + GPIO_CONFIG_REG(priv, pin_selector),
>                         TLMM_FUNC_SEL_MASK | TLMM_GPIO_DISABLE,
>                         priv->data->get_function_mux(func_selector) << 2);
> @@ -94,6 +98,10 @@ static int msm_pinconf_set(struct udevice *dev, unsigned int pin_selector,
>  {
>         struct msm_pinctrl_priv *priv = dev_get_priv(dev);
>
> +       /* Always NOP for special pins */
> +       if (pin_selector >= priv->data->pin_data.special_pins_start)
> +               return 0;
> +
>         switch (param) {
>         case PIN_CONFIG_DRIVE_STRENGTH:
>                 argument = (argument / 2) - 1;
> @@ -136,6 +144,10 @@ int msm_pinctrl_bind(struct udevice *dev)
>         const char *name;
>         int ret;
>
> +       /* Make sure we don't indadvertently treat all pins as special pins. */
> +       if (!data->pin_data.special_pins_start)
> +               data->pin_data.special_pins_start = data->pin_data.pin_count;
> +
>         drv = lists_driver_lookup_name("pinctrl_qcom");
>         if (!drv)
>                 return -ENOENT;
>
> --
> 2.43.1
>