Re: [PATCH v5 1/3] pinctrl: imx: answer OUTPUT_ENABLE/INPUT_ENABLE queries from the pad register

[Bough Chen <haibo.chen@oss.nxp.com>]

On Thu, Sep 03, 2026 at 09:59:38AM +0200, Mehmet Fide wrote:
> From: Mehmet Fide <mehmet.fide@screeningeagle.com>
> 
> The mmio pinconf get callback ignores which parameter was requested and
> returns the raw conf register, so a generic query through
> pinctrl_gpio_get_config() gets register bits back instead of the packed
> parameter it asked for.
> 
> Decode the requested parameter and answer PIN_CONFIG_OUTPUT_ENABLE and
> PIN_CONFIG_INPUT_ENABLE on SoCs that declare where those bits live in
> the pad register; Vybrid has OBE at bit 1 and IBE at bit 0. The answer
> is 0 with the bit value as the argument, which is what the
> pinctrl_gpio_get_config() users (gpio-by-pinctrl, and gpio-mmio in the
> next patch) expect. Other parameters and the SCU based SoCs, whose
> firmware call returns the raw pad value as well, get -ENOTSUPP; a pin
> the device tree never configured gets -EINVAL, as the raw helper already
> does, so a caller can tell "no answer for this pin" from "this
> controller never answers".
> 
> The only in-tree user of the raw register was the debugfs group dump,
> which called the callback with an uninitialized config; it now reads
> the register through its own helper, like the single pin dump already
> did.
> 
> The set callback is not touched: the fsl,pins binding hands it the raw
> pad register value and that stays the only thing it accepts. Nothing
> in-tree sends generic parameters to it on these SoCs; making it
> understand them is a separate change.
> 
> This gives gpio-mmio a way to read back the line direction on chips
> whose direction lives in the pin controller.

Since vf610 and imx7ulp/imx8ulp share one gpio driver(gpio-vf610.c), 
I double check the gpio driver, imx7ulp/imx8ulp has PDDR, so it will go
another path in mmio, not impacted by this patch. So even the pin config
register of imx7ulp/imx8ulp also include IBE/OBE, no need to add obe_mask/
ibe_mask as vf610.

Reviewed-by: Haibo Chen <haibo.chen@nxp.com>

Regards
Haibo Chen
> 
> Suggested-by: Bartosz Golaszewski <brgl@kernel.org>
> Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
> ---
>  drivers/pinctrl/freescale/pinctrl-imx.c   | 54 +++++++++++++++++++++--
>  drivers/pinctrl/freescale/pinctrl-imx.h   |  4 ++
>  drivers/pinctrl/freescale/pinctrl-vf610.c |  2 +
>  3 files changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> index 9a45b376d36f..f08ef52bf4f8 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> @@ -21,6 +21,7 @@
>  
>  #include <linux/pinctrl/machine.h>
>  #include <linux/pinctrl/pinconf.h>
> +#include <linux/pinctrl/pinconf-generic.h>
>  #include <linux/pinctrl/pinctrl.h>
>  #include <linux/pinctrl/pinmux.h>
>  
> @@ -291,8 +292,8 @@ struct pinmux_ops imx_pmx_ops = {
>  	.set_mux = imx_pmx_set,
>  };
>  
> -static int imx_pinconf_get_mmio(struct pinctrl_dev *pctldev, unsigned pin_id,
> -				unsigned long *config)
> +static int imx_pinconf_get_raw_mmio(struct pinctrl_dev *pctldev,
> +				    unsigned int pin_id, unsigned long *config)
>  {
>  	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
>  	const struct imx_pinctrl_soc_info *info = ipctl->info;
> @@ -312,16 +313,61 @@ static int imx_pinconf_get_mmio(struct pinctrl_dev *pctldev, unsigned pin_id,
>  	return 0;
>  }
>  
> +static int imx_pinconf_get_mmio(struct pinctrl_dev *pctldev,
> +				unsigned int pin_id, unsigned long *config)
> +{
> +	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +	const struct imx_pinctrl_soc_info *info = ipctl->info;
> +	const struct imx_pin_reg *pin_reg = &ipctl->pin_regs[pin_id];
> +	enum pin_config_param param = pinconf_to_config_param(*config);
> +	unsigned int mask;
> +	u32 raw;
> +
> +	switch (param) {
> +	case PIN_CONFIG_OUTPUT_ENABLE:
> +		mask = info->obe_mask;
> +		break;
> +	case PIN_CONFIG_INPUT_ENABLE:
> +		mask = info->ibe_mask;
> +		break;
> +	default:
> +		mask = 0;
> +		break;
> +	}
> +
> +	if (!mask)
> +		return -ENOTSUPP;
> +	if (pin_reg->conf_reg == -1)
> +		return -EINVAL;
> +
> +	raw = readl(ipctl->base + pin_reg->conf_reg);
> +	*config = pinconf_to_config_packed(param, !!(raw & mask));
> +
> +	return 0;
> +}
> +
>  static int imx_pinconf_get(struct pinctrl_dev *pctldev,
>  			   unsigned pin_id, unsigned long *config)
>  {
>  	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
>  	const struct imx_pinctrl_soc_info *info = ipctl->info;
>  
> +	if (info->flags & IMX_USE_SCU)
> +		return -ENOTSUPP;
> +
> +	return imx_pinconf_get_mmio(pctldev, pin_id, config);
> +}
> +
> +static int imx_pinconf_get_raw(struct pinctrl_dev *pctldev,
> +			       unsigned int pin_id, unsigned long *config)
> +{
> +	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +	const struct imx_pinctrl_soc_info *info = ipctl->info;
> +
>  	if (info->flags & IMX_USE_SCU)
>  		return info->imx_pinconf_get(pctldev, pin_id, config);
>  	else
> -		return imx_pinconf_get_mmio(pctldev, pin_id, config);
> +		return imx_pinconf_get_raw_mmio(pctldev, pin_id, config);
>  }
>  
>  static int imx_pinconf_set_mmio(struct pinctrl_dev *pctldev,
> @@ -426,7 +472,7 @@ static void imx_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
>  		struct imx_pin *pin = &((struct imx_pin *)(grp->data))[i];
>  
>  		name = pin_get_name(pctldev, pin->pin);
> -		ret = imx_pinconf_get(pctldev, pin->pin, &config);
> +		ret = imx_pinconf_get_raw(pctldev, pin->pin, &config);
>  		if (ret)
>  			return;
>  		seq_printf(s, "  %s: 0x%lx\n", name, config);
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
> index f65ff45b4003..8fa7e1e2521d 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> @@ -91,6 +91,10 @@ struct imx_pinctrl_soc_info {
>  	unsigned int mux_mask;
>  	u8 mux_shift;
>  
> +	/* OBE/IBE bits in the conf register, 0 if the pad does not have them */
> +	unsigned int obe_mask;
> +	unsigned int ibe_mask;
> +
>  	int (*gpio_set_direction)(struct pinctrl_dev *pctldev,
>  				  struct pinctrl_gpio_range *range,
>  				  unsigned offset,
> diff --git a/drivers/pinctrl/freescale/pinctrl-vf610.c b/drivers/pinctrl/freescale/pinctrl-vf610.c
> index 76a4bc0181a0..77d077618782 100644
> --- a/drivers/pinctrl/freescale/pinctrl-vf610.c
> +++ b/drivers/pinctrl/freescale/pinctrl-vf610.c
> @@ -319,6 +319,8 @@ static const struct imx_pinctrl_soc_info vf610_pinctrl_info = {
>  	.gpio_set_direction = vf610_pmx_gpio_set_direction,
>  	.mux_mask = 0x700000,
>  	.mux_shift = 20,
> +	.obe_mask = 0x2,
> +	.ibe_mask = 0x1,
>  };
>  
>  static const struct of_device_id vf610_pinctrl_of_match[] = {
> -- 
> 2.54.0
>