[PATCH] pinctrl: mediatek: set R1/R0 in case pullen/pullsel succeeded

[PATCH] pinctrl: mediatek: set R1/R0 in case pullen/pullsel succeeded

[Daniel Golle]

Commit dafe0fbfb0f3 ("pinctrl: mediatek: rewrite mtk_pinconf_set and
related functions") changed the logic deciding to set R0 and R1
registers for V1 devices.

Before:
	/* Also set PUPD/R0/R1 if the pin has them */
	err = mtk_hw_set_value(dev, pin, PINCTRL_PIN_REG_PUPD, !pullup);
	if (err != -EINVAL) {
		mtk_hw_set_value(dev, pin, PINCTRL_PIN_REG_R0, r0);
		mtk_hw_set_value(dev, pin, PINCTRL_PIN_REG_R1, r1);
	}

After:
	/* try pupd_r1_r0 if pullen_pullsel return error */
	err = mtk_pinconf_bias_set_pullen_pullsel(dev, pin, disable, pullup,
						  val);
	if (err)
		return mtk_pinconf_bias_set_pupd_r1_r0(dev, pin, disable,
						       pullup, val);

Tracing mtk_pinconf_bias_set_pullen_pullsel shows that the function
always either returns 0 in case of success or -EINVAL in case any error
has occurred. Hence the logic responsible of the decision to program R0
and R1 has been inverted.

This leads to problems on BananaPi R2 (MT7623N) when booting from
SDMMC, it turns out accessing eMMC no longer works since
U-Boot 2022.07:

MT7623> mmc dev 0
Card did not respond to voltage select! : -110

The problem wasn't detected for a long time as both eMMC and SDMMC work
fine if they are used to boot from, and hence R0 and R1 were already
setup by the bootrom and/or preloader.

Fix the logic to restore the originally intended and correct behavior
and also change the descriptive comment accordingly.

Fixes: dafe0fbfb0f3 ("pinctrl: mediatek: rewrite mtk_pinconf_set and related functions")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
index 4ae328699e2..23d759aa2d8 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
@@ -319,10 +319,10 @@ int mtk_pinconf_bias_set_v1(struct udevice *dev, u32 pin, bool disable,
 {
 	int err;
 
-	/* try pupd_r1_r0 if pullen_pullsel return error */
+	/* set pupd_r1_r0 if pullen_pullsel succeeded */
 	err = mtk_pinconf_bias_set_pullen_pullsel(dev, pin, disable, pullup,
 						  val);
-	if (err)
+	if (!err)
 		return mtk_pinconf_bias_set_pupd_r1_r0(dev, pin, disable,
 						       pullup, val);
 
-- 
2.40.0

Aw: [PATCH] pinctrl: mediatek: set R1/R0 in case pullen/pullsel succeeded

[Frank Wunderlich]

FYI...this issue affects only specific eMMC-Chips (vendor SEC) on the Bananapi-R2 (v1.2).

i have a v1.1 with a Samsung eMMC where i had no problems with the missing pullups/pulldowns
(maybe this has internal resistors)

more information about this special case:
https://forum.banana-pi.org/t/problem-trying-to-use-emmc/11253

tested the patch on my device for regressions...no problems

Tested-By: Frank Wunderlich <frank-w@public-files.de>

Re: [PATCH] pinctrl: mediatek: set R1/R0 in case pullen/pullsel succeeded

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1843 bytes --]

On Wed, Apr 12, 2023 at 09:36:43PM +0100, Daniel Golle wrote:

> Commit dafe0fbfb0f3 ("pinctrl: mediatek: rewrite mtk_pinconf_set and
> related functions") changed the logic deciding to set R0 and R1
> registers for V1 devices.
> 
> Before:
> 	/* Also set PUPD/R0/R1 if the pin has them */
> 	err = mtk_hw_set_value(dev, pin, PINCTRL_PIN_REG_PUPD, !pullup);
> 	if (err != -EINVAL) {
> 		mtk_hw_set_value(dev, pin, PINCTRL_PIN_REG_R0, r0);
> 		mtk_hw_set_value(dev, pin, PINCTRL_PIN_REG_R1, r1);
> 	}
> 
> After:
> 	/* try pupd_r1_r0 if pullen_pullsel return error */
> 	err = mtk_pinconf_bias_set_pullen_pullsel(dev, pin, disable, pullup,
> 						  val);
> 	if (err)
> 		return mtk_pinconf_bias_set_pupd_r1_r0(dev, pin, disable,
> 						       pullup, val);
> 
> Tracing mtk_pinconf_bias_set_pullen_pullsel shows that the function
> always either returns 0 in case of success or -EINVAL in case any error
> has occurred. Hence the logic responsible of the decision to program R0
> and R1 has been inverted.
> 
> This leads to problems on BananaPi R2 (MT7623N) when booting from
> SDMMC, it turns out accessing eMMC no longer works since
> U-Boot 2022.07:
> 
> MT7623> mmc dev 0
> Card did not respond to voltage select! : -110
> 
> The problem wasn't detected for a long time as both eMMC and SDMMC work
> fine if they are used to boot from, and hence R0 and R1 were already
> setup by the bootrom and/or preloader.
> 
> Fix the logic to restore the originally intended and correct behavior
> and also change the descriptive comment accordingly.
> 
> Fixes: dafe0fbfb0f3 ("pinctrl: mediatek: rewrite mtk_pinconf_set and related functions")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Tested-By: Frank Wunderlich <frank-w@public-files.de>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]