Can't access mmc #0 on mt7623 when booted from external SD

On Thu, 2020-06-18 at 16:52 +0100, David Woodhouse wrote:
> So... whose bug is that? :)

Looks like the pinctrl driver. This *ought* to work (with the caveat
that I really ought to make two pinctrl setups and change between them
according to the voltage, like the Linux DT and mtk-sd driver do).

 
                conf-rst {


But it doesn't work. Partly because the U-Boot mtk pinctrl driver
doesn't support the R0 R1 bits encoded in that <3>, and partly because
it doesn't *even* get pullup/pulldown right at all; it sets the bit in
the GPIO_PULLSEL registers but *not* the PUPD bit in the correct
register for MSDC and other pins.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5174 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200618/5f1d8324/attachment.bin>

Hi David

On Thu, Jun 18, 2020 at 9:21 PM David Woodhouse <dwmw2 at infradead.org> wrote:
>
> On Thu, 2020-06-18 at 16:52 +0100, David Woodhouse wrote:
> > So... whose bug is that? :)
>
> Looks like the pinctrl driver. This *ought* to work (with the caveat
> that I really ought to make two pinctrl setups and change between them
> according to the voltage, like the Linux DT and mtk-sd driver do).
>
> --- a/arch/arm/dts/mt7623n-bananapi-bpi-r2.dts
> +++ b/arch/arm/dts/mt7623n-bananapi-bpi-r2.dts
> @@ -133,12 +133,14 @@
>                                "MSDC0_DAT2", "MSDC0_DAT3",
> "MSDC0_DAT4",
>                                "MSDC0_DAT5", "MSDC0_DAT6",
> "MSDC0_DAT7";
>                         input-enable;
> -                       bias-pull-up;
> +                       drive-strength = <2>;
> +                       bias-pull-up = <3>;
>                 };
>
>                 conf-clk {
>                         pins = "MSDC0_CLK";
> -                       bias-pull-down;
> +                       drive-strength = <2>;
> +                       bias-pull-down = <3>;
>                 };
>
>                 conf-rst {
>
>
> But it doesn't work. Partly because the U-Boot mtk pinctrl driver
> doesn't support the R0 R1 bits encoded in that <3>, and partly because
> it doesn't *even* get pullup/pulldown right at all; it sets the bit in
> the GPIO_PULLSEL registers but *not* the PUPD bit in the correct
> register for MSDC and other pins.

So change them manually, make it work, You have the board. I don't
have any MediaTek board at the moment. Can you implement the missed part?

Michael

On Thu, 2020-06-18 at 21:34 +0200, Michael Nazzareno Trimarchi wrote:
> > Looks like the pinctrl driver. This *ought* to work (with the caveat
> > that I really ought to make two pinctrl setups and change between them
> > according to the voltage, like the Linux DT and mtk-sd driver do).
> > 
> > --- a/arch/arm/dts/mt7623n-bananapi-bpi-r2.dts
> > +++ b/arch/arm/dts/mt7623n-bananapi-bpi-r2.dts
> > @@ -133,12 +133,14 @@
> >                                 "MSDC0_DAT2", "MSDC0_DAT3", "MSDC0_DAT4",
> >                                 "MSDC0_DAT5", "MSDC0_DAT6", "MSDC0_DAT7";
> >                          input-enable;
> > -                       bias-pull-up;
> > +                       drive-strength = <2>;
> > +                       bias-pull-up = <3>;
> >                  };
> > 
> >                  conf-clk {
> >                          pins = "MSDC0_CLK";
> > -                       bias-pull-down;
> > +                       drive-strength = <2>;
> > +                       bias-pull-down = <3>;
> >                  };
> > 
> >                  conf-rst {
> > 
> > 
> > But it doesn't work. Partly because the U-Boot mtk pinctrl driver
> > doesn't support the R0 R1 bits encoded in that <3>, and partly because
> > it doesn't *even* get pullup/pulldown right at all; it sets the bit in
> > the GPIO_PULLSEL registers but *not* the PUPD bit in the correct
> > register for MSDC and other pins.
> 
> So change them manually, make it work, You have the board. I don't
> have any MediaTek board at the moment. Can you implement the missed part?

I think I'd rather let the MediaTek folks handle it, as it affects
other SoCs too.

It turns out Linux has a pinctrl-mt7623.c driver which looks a lot like
the U-Boot one, but *isn't* actually the one being used, because in
Linux it only matches "mediatek,mt7623-moore-pinctrl" which isn't in
this board's DT.

Linux also has a pinctrl-mt2701.c driver that matches the
"mediatek,mt7623-pinctrl" that *is* on this board, and which *does*
work correctly.

So I don't know if we just have the wrong driver in U-Boot. Or if the
mediatek,mt7623-moore-pinctrl compatible string is a temporary name for
the *same* hardware, to invoke the new driver that isn't complete yet
so it isn't being used in Linux... but is in U-Boot?

Sean?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5174 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200618/04c310d1/attachment.bin>

Hi David,

1. mt7623-moore-pinctrl is written for introducing more supports for new mediatek soc like mt7622, mt7629, mt8183 and so on and even more easily maintained than before

2. mt7623-moore-pinctrl/mt2701-pinctrl drives the same hardware, but with different driver style, the latter one is already hard to extend to newer and variety mediatek SoC. 

3.  mt7623 pinctrl on uboot would prefer to apply mt7623-moore-pinctrl just since pinctrl on uboot is developed later than mt7623-moore-pinctrl.
      mt7623 pinctrl on linux still would prefer to apply mt2701-pinctrl since they have been supported for a while and be stable enough.

4.  it's still welcome to submit patches to mt7623-moore-pinctrl from your experiences in mt7623 uboot to make it complete.

Thanks,
Sean

-----Original Message-----
From: David Woodhouse [mailto:dwmw2 at infradead.org] 
Sent: Thursday, June 18, 2020 2:03 PM
To: Michael Nazzareno Trimarchi <michael at amarulasolutions.com>
Cc: U-Boot-Denx <u-boot at lists.denx.de>; Ryder Lee (???) <Ryder.Lee at mediatek.com>; Weijie Gao (???) <Weijie.Gao at mediatek.com>; frank wunderlich <frank-w at public-files.de>; Sean Wang <Sean.Wang at mediatek.com>
Subject: Re: Can't access mmc #0 on mt7623 when booted from external SD

On Thu, 2020-06-18 at 21:34 +0200, Michael Nazzareno Trimarchi wrote:
> > Looks like the pinctrl driver. This *ought* to work (with the caveat
> > that I really ought to make two pinctrl setups and change between them
> > according to the voltage, like the Linux DT and mtk-sd driver do).
> > 
> > --- a/arch/arm/dts/mt7623n-bananapi-bpi-r2.dts
> > +++ b/arch/arm/dts/mt7623n-bananapi-bpi-r2.dts
> > @@ -133,12 +133,14 @@
> >                                 "MSDC0_DAT2", "MSDC0_DAT3", "MSDC0_DAT4",
> >                                 "MSDC0_DAT5", "MSDC0_DAT6", "MSDC0_DAT7";
> >                          input-enable;
> > -                       bias-pull-up;
> > +                       drive-strength = <2>;
> > +                       bias-pull-up = <3>;
> >                  };
> > 
> >                  conf-clk {
> >                          pins = "MSDC0_CLK";
> > -                       bias-pull-down;
> > +                       drive-strength = <2>;
> > +                       bias-pull-down = <3>;
> >                  };
> > 
> >                  conf-rst {
> > 
> > 
> > But it doesn't work. Partly because the U-Boot mtk pinctrl driver
> > doesn't support the R0 R1 bits encoded in that <3>, and partly because
> > it doesn't *even* get pullup/pulldown right at all; it sets the bit in
> > the GPIO_PULLSEL registers but *not* the PUPD bit in the correct
> > register for MSDC and other pins.
> 
> So change them manually, make it work, You have the board. I don't
> have any MediaTek board at the moment. Can you implement the missed part?

I think I'd rather let the MediaTek folks handle it, as it affects
other SoCs too.

It turns out Linux has a pinctrl-mt7623.c driver which looks a lot like
the U-Boot one, but *isn't* actually the one being used, because in
Linux it only matches "mediatek,mt7623-moore-pinctrl" which isn't in
this board's DT.

Linux also has a pinctrl-mt2701.c driver that matches the
"mediatek,mt7623-pinctrl" that *is* on this board, and which *does*
work correctly.

So I don't know if we just have the wrong driver in U-Boot. Or if the
mediatek,mt7623-moore-pinctrl compatible string is a temporary name for
the *same* hardware, to invoke the new driver that isn't complete yet
so it isn't being used in Linux... but is in U-Boot?

Sean?