linux-kernel - Re: [PATCH] pinctrl: mediatek: common-v2: Fix broken bias-disable for PULL_PU_PD_RSEL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACRpkdZy9F-oh0sT+YhvgzoSrKQL78gK46wRbQ6d6jHYS5nzfA@mail.gmail.com>
Date: Fri, 23 Aug 2024 18:07:39 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: Nícolas F. R. A. Prado <nfraprado@...labora.com>
Cc: Sean Wang <sean.wang@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, kernel@...labora.com, 
	linux-mediatek@...ts.infradead.org, linux-gpio@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] pinctrl: mediatek: common-v2: Fix broken bias-disable for PULL_PU_PD_RSEL_TYPE

On Fri, Aug 9, 2024 at 1:27 AM Nícolas F. R. A. Prado
<nfraprado@...labora.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@...labora.com>

Patch applied for fixes.

Thanks!
Linus Walleij