Re: [PATCH] pinctrl: mediatek: common-v2: Fix broken bias-disable for PULL_PU_PD_RSEL_TYPE

[Linus Walleij <linus.walleij@linaro.org>]

On Fri, Aug 9, 2024 at 1:27 AM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:

> Despite its name, commit fed74d75277d ("pinctrl: mediatek: common-v2:
> Fix bias-disable for PULL_PU_PD_RSEL_TYPE") actually broke bias-disable
> for PULL_PU_PD_RSEL_TYPE.
>
> mtk_pinconf_bias_set_combo() tries every bias method supported by the
> pin until one succeeds. For PULL_PU_PD_RSEL_TYPE pins, before the
> breaking commit, mtk_pinconf_bias_set_rsel() would be called first to
> try and set the RSEL value (as well as PU and PD), and if that failed,
> the only other valid option was that bias-disable was specified, which
> would then be handled by calling mtk_pinconf_bias_set_pu_pd() and
> disabling both PU and PD.
>
> The breaking commit misunderstood this logic and added an early "return
> 0" in mtk_pinconf_bias_set_rsel(). The result was that in the
> bias-disable case, the bias was left unchanged, since by returning
> success, mtk_pinconf_bias_set_combo() no longer tried calling
> mtk_pinconf_bias_set_pu_pd() to disable the bias.
>
> Since the logic for configuring bias-disable on PULL_PU_PD_RSEL_TYPE
> pins required mtk_pinconf_bias_set_rsel() to fail first, in that case,
> an error was printed to the log, eg:
>
>   mt8195-pinctrl 10005000.pinctrl: Not support rsel value 0 Ohm for pin = 29 (GPIO29)
>
> This is what the breaking commit actually got rid of, and likely part of
> the reason why that commit was thought to be fixing functionality, while
> in reality it was breaking it.
>
> Instead of simply reverting that commit, restore the functionality but
> in a way that avoids the error from being printed and makes the code
> less confusing:
> * Return 0 explicitly if a bias method was successful
> * Introduce an extra function mtk_pinconf_bias_set_pu_pd_rsel() that
>   calls both mtk_pinconf_bias_set_rsel() (only if needed) and
>   mtk_pinconf_bias_set_pu_pd()
>   * And analogously for the corresponding getters
>
> Fixes: fed74d75277d ("pinctrl: mediatek: common-v2: Fix bias-disable for PULL_PU_PD_RSEL_TYPE")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Patch applied for fixes.

Thanks!
Linus Walleij