[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGXv+5ESMm3t_kJckubDf4jMLYehz66o8B42sTTM=vEF1WMpCA@mail.gmail.com>
Date: Fri, 18 Oct 2024 13:17:40 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Bo Ye <bo.ye@...iatek.com>
Cc: Sean Wang <sean.wang@...nel.org>, Linus Walleij <linus.walleij@...aro.org>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Yongdong Zhang <yongdong.zhang@...iatek.com>, Xiujuan Tan <xiujuan.tan@...iatek.com>,
Browse Zhang <browse.zhang@...iatek.com>, Light Hsieh <light.hsieh@...iatek.com>,
Evan Cao <ot_evan.cao@...iatek.com>, linux-mediatek@...ts.infradead.org,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [RESEND. PATCH v1] pinctrl: mediatek: paris: Revert "Rework
support for PIN_CONFIG_{INPUT,OUTPUT}_ENABLE"
On Thu, Oct 17, 2024 at 5:14 PM Bo Ye <bo.ye@...iatek.com> wrote:
Please avoid sending more than one version per day. Most people in
the community are not in the Asian time zones. Give people time
to respond.
> [This reverts commit c5d3b64c568a344e998830e0e94a7c04e372f89b.]
>
> For MTK HW,
> 1. to enable GPIO input direction: set DIR=0, IES=1
> 2. to enable GPIO output direction: set DIR=1, and set DO=1 to output high, set DO=0 to out low
>
> The PIN_CONFIG_INPUT/PIN_CONFIG_OUTPUT/PIN_CONFIG_INPUT_ENABLE/PIN_CONFIG_OUTPUT_ENABLE shall
> be implemented according to view of its purpose - set GPIO direction and output value (for
> output only) according to specific HW design.
>
> However, the reverted patch implement according to author's own explanation of IES without
> understanding of MTK's HW. Such patch does not correctly set DIR/IES bit to control GPIO
> direction on MTK's HW.
>
> Fixes: c5d3b64c568 ("pinctrl: mediatek: paris: Rework support for PIN_CONFIG_{INPUT,OUTPUT}_ENABLE")
As Macpaul mentioned, you changed the commit message by adding a tag, and
thus this patch should be marked "v2". Please also provide a changelog
on what changed in this version in the footer, i.e. under the "---" line
but before the actual patch context.
ChenYu
> Signed-off-by: Light Hsieh <light.hsieh@...iatek.com>
> Signed-off-by: Evan Cao <ot_evan.cao@...iatek.com>
> Signed-off-by: Bo Ye <bo.ye@...iatek.com>
> ---
> drivers/pinctrl/mediatek/pinctrl-paris.c | 38 +++++++++++++++++-------
> 1 file changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c
> index 87e958d827bf..a8af62e6f8ca 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-paris.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c
> @@ -165,21 +165,20 @@ static int mtk_pinconf_get(struct pinctrl_dev *pctldev,
> err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_SR, &ret);
> break;
> case PIN_CONFIG_INPUT_ENABLE:
> - err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_IES, &ret);
> - if (!ret)
> - err = -EINVAL;
> - break;
> - case PIN_CONFIG_OUTPUT:
> + case PIN_CONFIG_OUTPUT_ENABLE:
> err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_DIR, &ret);
> if (err)
> break;
> + /* CONFIG Current direction return value
> + * ------------- ----------------- ----------------------
> + * OUTPUT_ENABLE output 1 (= HW value)
> + * input 0 (= HW value)
> + * INPUT_ENABLE output 0 (= reverse HW value)
> + * input 1 (= reverse HW value)
> + */
> + if (param == PIN_CONFIG_INPUT_ENABLE)
> + ret = !ret;
>
> - if (!ret) {
> - err = -EINVAL;
> - break;
> - }
> -
> - err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_DO, &ret);
> break;
> case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
> err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_DIR, &ret);
> @@ -284,9 +283,26 @@ static int mtk_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> break;
> err = hw->soc->bias_set_combo(hw, desc, 0, arg);
> break;
> + case PIN_CONFIG_OUTPUT_ENABLE:
> + err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> + MTK_DISABLE);
> + /* Keep set direction to consider the case that a GPIO pin
> + * does not have SMT control
> + */
> + if (err != -ENOTSUPP)
> + break;
> +
> + err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_DIR,
> + MTK_OUTPUT);
> + break;
> case PIN_CONFIG_INPUT_ENABLE:
> /* regard all non-zero value as enable */
> err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_IES, !!arg);
> + if (err)
> + break;
> +
> + err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_DIR,
> + MTK_INPUT);
> break;
> case PIN_CONFIG_SLEW_RATE:
> /* regard all non-zero value as enable */
> --
> 2.17.0
>
Powered by blists - more mailing lists