Re: [PATCH v4 1/4] pinctrl: mediatek: add support for MTK_PULL_PD_TYPE

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 09/10/24 18:52, Frank Wunderlich ha scritto:
> From: Daniel Golle <daniel@makrotopia.org>
> 
> The MediaTek MT7988 SoC got some pins which only got configurable
> pull-down but unlike previous designs there is no pull-up option.
> Add new type MTK_PULL_PD_TYPE to support configuring such pins.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>   .../pinctrl/mediatek/pinctrl-mtk-common-v2.c  | 59 +++++++++++++++++++
>   .../pinctrl/mediatek/pinctrl-mtk-common-v2.h  |  1 +
>   2 files changed, 60 insertions(+)
> 
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> index 54301fbba524..eff2aecd31dd 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> @@ -601,6 +601,30 @@ static int mtk_pinconf_bias_set_pu_pd(struct mtk_pinctrl *hw,
>   	return err;
>   }
>   
> +static int mtk_pinconf_bias_set_pd(struct mtk_pinctrl *hw,
> +				const struct mtk_pin_desc *desc,
> +				u32 pullup, u32 arg)
> +{
> +	int err, pd;
> +
> +	if (arg == MTK_DISABLE)

if (arg != MTK_DISABLE && arg != MTK_ENABLE)
	return -EINVAL

/* Either this */
if (arg == MTK_DISABLE || pullup)
	pd = 0;
else if (!pullup)
	pd = 1

/* Or this (but it's probably a bit too cryptic) */
pd = !(arg == MTK_DISABLE || pullup);

return mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_PD, pd);



...but then, you could otherwise modify mtk_pinconf_bias_set_pu_pd(), so that

static int mtk_pinconf_bias_set_pu_pd(struct mtk_pinctrl *hw,
				const struct mtk_pin_desc *desc,
				u32 pullup, u32 arg, bool pd_only)
{
	int err, pu, pd;

	if (arg == MTK_DISABLE) {
		pu = 0;
		pd = 0;
	} else if ((arg == MTK_ENABLE) && pullup) {
		pu = 1;
		pd = 0;
	} else if ((arg == MTK_ENABLE) && !pullup) {
		pu = 0;
		pd = 1;
	} else {
		return -EINVAL;
	}

	if (!pd_only) {
		err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_PU, pu);
		if (err)
			return err;
	}

	return mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_PD, pd);
}

> +		pd = 0;
> +	else if ((arg == MTK_ENABLE) && pullup)
> +		pd = 0;
> +	else if ((arg == MTK_ENABLE) && !pullup)
> +		pd = 1;
> +	else {
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_PD, pd);
> +
> +out:
> +	return err;
> +
> +}
> +
>   static int mtk_pinconf_bias_set_pullsel_pullen(struct mtk_pinctrl *hw,
>   				const struct mtk_pin_desc *desc,
>   				u32 pullup, u32 arg)
> @@ -758,6 +782,12 @@ int mtk_pinconf_bias_set_combo(struct mtk_pinctrl *hw,
>   			return 0;
>   	}
>   
> +	if (try_all_type & MTK_PULL_PD_TYPE) {
> +		err = mtk_pinconf_bias_set_pd(hw, desc, pullup, arg);

so if it is PD_TYPE, mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, arg, true);

> +		if (!err)
> +			return err;
> +	}
> +
>   	if (try_all_type & MTK_PULL_PU_PD_TYPE) {
>   		err = mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, arg);

mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, arg, false);

>   		if (!err)
> @@ -878,6 +908,29 @@ static int mtk_pinconf_bias_get_pu_pd(struct mtk_pinctrl *hw,
>   	return err;
>   }
>   
> +static int mtk_pinconf_bias_get_pd(struct mtk_pinctrl *hw,
> +				const struct mtk_pin_desc *desc,
> +				u32 *pullup, u32 *enable)
> +{

this one you can keep it as it is, because I don't think that you can get
the get_pu_pd function to work with pd_only without making it .. well, messy.

> +	int err, pd;
> +
> +	err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_PD, &pd);
> +	if (err)
> +		goto out;
> +
> +	if (pd == 0) {
> +		*pullup = 0;
> +		*enable = MTK_DISABLE;
> +	} else if (pd == 1) {
> +		*pullup = 0;
> +		*enable = MTK_ENABLE;
> +	} else
> +		err = -EINVAL;
> +
> +out:
> +	return err;
> +}
> +
>   static int mtk_pinconf_bias_get_pullsel_pullen(struct mtk_pinctrl *hw,
>   				const struct mtk_pin_desc *desc,
>   				u32 *pullup, u32 *enable)
> @@ -947,6 +1000,12 @@ int mtk_pinconf_bias_get_combo(struct mtk_pinctrl *hw,
>   			return 0;
>   	}
>   
> +	if (try_all_type & MTK_PULL_PD_TYPE) {
> +		err = mtk_pinconf_bias_get_pd(hw, desc, pullup, enable);
> +		if (!err)
> +			return err;
> +	}
> +
>   	if (try_all_type & MTK_PULL_PU_PD_TYPE) {
>   		err = mtk_pinconf_bias_get_pu_pd(hw, desc, pullup, enable);
>   		if (!err)
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h
> index 23688ca6d04e..9c271dc2b521 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h
> @@ -24,6 +24,7 @@
>    * turned on/off itself. But it can't be selected pull up/down
>    */
>   #define MTK_PULL_RSEL_TYPE		BIT(3)
> +#define MTK_PULL_PD_TYPE        BIT(4)
>   /* MTK_PULL_PU_PD_RSEL_TYPE is a type which is controlled by
>    * MTK_PULL_PU_PD_TYPE and MTK_PULL_RSEL_TYPE.
>    */


Cheers,
Angelo