[linux-sunxi] Re: Re: [PATCH v2] arm64: dts: allwinner: h6: add eMMC voltage property for Beelink GS1
Dne četrtek, 15. oktober 2020 ob 11:35:44 CEST je Maxime Ripard napisal(a):
> On Tue, Oct 13, 2020 at 11:27:33PM +0200, Jernej Škrabec wrote:
> > Dne petek, 09. oktober 2020 ob 09:36:51 CEST je Maxime Ripard napisal(a):
> > > On Thu, Oct 08, 2020 at 10:00:06PM +0200, Clément Péron wrote:
> > > > Hi Maxime,
> > > >
> > > > Adding linux-sunxi and Jernej Skrabec to this discussion.
> > > >
> > > > On Thu, 8 Oct 2020 at 17:10, Maxime Ripard wrote:
> > > > >
> > > > > Hi Clément,
> > > > >
> > > > > On Mon, Oct 05, 2020 at 08:47:19PM +0200, Clément Péron wrote:
> > > > > > On Mon, 5 Oct 2020 at 11:21, Maxime Ripard
wrote:
> > > > > > >
> > > > > > > Hi Clément,
> > > > > > >
> > > > > > > On Sat, Oct 03, 2020 at 11:20:01AM +0200, Clément Péron wrote:
> > > > > > > > Sunxi MMC driver can't distinguish at runtime what's the I/O
> > voltage
> > > > > > > > for HS200 mode.
> > > > > > >
> > > > > > > Unfortunately, that's not true (or at least, that's not related
to
> > your patch).
> > > > > > >
> > > > > > > > Add a property in the device-tree to notify MMC core about
this
> > > > > > > > configuration.
> > > > > > > >
> > > > > > > > Fixes: 089bee8dd119 ("arm64: dts: allwinner: h6: Introduce
Beelink
> > GS1 board")
> > > > > > > > Signed-off-by: Clément Péron
> > > > > > > > ---
> > > > > > > > arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 +
> > > > > > > > 1 file changed, 1 insertion(+)
> > > > > > > >
> > > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-
> > gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> > > > > > > > index 049c21718846..3f20d2c9 100644
> > > > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> > > > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> > > > > > > > @@ -145,6 +145,7 @@ {
> > > > > > > > vqmmc-supply = <_bldo2>;
> > > > > > > > non-removable;
> > > > > > > > cap-mmc-hw-reset;
> > > > > > > > + mmc-hs200-1_8v;
> > > > > > > > bus-width = <8>;
> > > > > > > > status = "okay";
> > > > > > > > };
> > > > > > >
> > > > > > > I'm not really sure what you're trying to fix here, but as far as
MMC
> > > > > > > goes, eMMC's can support io voltage of 3.3, 1.8 and 1.2V. Modes
up
> > until
> > > > > > > HS DDR (50MHz in DDR) will use an IO voltage of 3.3V, higher
speed
> > modes
> > > > > > > (HS200 and HS400) supporting 1.8V and 1.2V.
> > > > > >
> > > > > > Some users report that the eMMC is not working properly on their
> > > > > > Beelink GS1 boards.
> > > > > >
> > > > > > > The mmc-hs200-1_8v property states that the MMC controller
supports
> > the
> > > > > > > HS200 mode at 1.8V. Now, I can only assume that since BLDO2 is
set
> > up at
> > > > > > > 1.8V then otherwise, the MMC core will rightfully decide to use
the
> > > > > > > highest supported mode. In this case, since the driver sets it,
it
> > would
> > > > > > > be HS-DDR at 3.3V, which won't work with that fixed regulator.
> > > > > > >
> > > > > > > I can only assume that enabling HS200 at 1.8V only fixes the
issue
> > you
> > > > > > > have because otherwise it would use HS-DDR at 3.3V, ie not
actually
> > > > > > > fixing the issue but sweeping it under the rug.
> > > > > > >
> > > > > > > Trying to add mmc-ddr-1_8v would be a good idea
> > > > > >
> > > > > > Thanks for the explanation, this is indeed the correct one.
> > > > > > So It looks like the SDIO controller has an issue on some boards
when
> > > > > > using HS-DDR mode.
> > > > > >
> > > > > > Is this patch acceptable with the proper commit log?
> > > > >
> > > > > If HS-DDR works, yes, but I assume it doesn't?
> > > >
> > > > After discussing with Jernej about this issue, I understood that:
> > > > - Automatic delay calibration is not implemented
> > > > - We also miss some handling of DDR related bits in control register
> > > >
> > > > So none of H5/H6 boards should actually work.
> > > > (Some 'lucky' boards seem to work enough to switch to HS200 mode...)
> > > >
> > > > To "fix" this the H5 disable the HS-DDR mode in sunxi mmc driver :
> > > > https://github.com/torvalds/linux/blob/master/drivers/mmc/host/sunxi-mmc.c#L1409
> > >
> > > I find it suspicious that some boards would have traces not good enough
> > > for HS-DDR (50MHz in DDR) but would work fine in HS200 (200MHz in SDR).
> > > If there's some mismatch on the traces, it will only be worse in HS200.
> >
> > FYI, similar situation is also with Tanix TX6 board. Mine works well in
HS-DDR
> > mode, but some people reported that it doesn't work for them. The only
> > possible difference could be different eMMC IC. I'll try to confirm that.
> >
> > Anyway, I did some tests on OrangePi 3 board which also have eMMC. Both
modes
> > (HS-DDR and HS200) are supported and work well.
>
> The Orange Pi 3 has an HS400-capable eMMC ?!
>
> That's really good news, I've initially done the HS200 support on a
> custom board I had to give
[linux-sunxi] Re: Re: [PATCH v2] arm64: dts: allwinner: h6: add eMMC voltage property for Beelink GS1
Dne petek, 09. oktober 2020 ob 09:36:51 CEST je Maxime Ripard napisal(a):
> On Thu, Oct 08, 2020 at 10:00:06PM +0200, Clément Péron wrote:
> > Hi Maxime,
> >
> > Adding linux-sunxi and Jernej Skrabec to this discussion.
> >
> > On Thu, 8 Oct 2020 at 17:10, Maxime Ripard wrote:
> > >
> > > Hi Clément,
> > >
> > > On Mon, Oct 05, 2020 at 08:47:19PM +0200, Clément Péron wrote:
> > > > On Mon, 5 Oct 2020 at 11:21, Maxime Ripard wrote:
> > > > >
> > > > > Hi Clément,
> > > > >
> > > > > On Sat, Oct 03, 2020 at 11:20:01AM +0200, Clément Péron wrote:
> > > > > > Sunxi MMC driver can't distinguish at runtime what's the I/O
voltage
> > > > > > for HS200 mode.
> > > > >
> > > > > Unfortunately, that's not true (or at least, that's not related to
your patch).
> > > > >
> > > > > > Add a property in the device-tree to notify MMC core about this
> > > > > > configuration.
> > > > > >
> > > > > > Fixes: 089bee8dd119 ("arm64: dts: allwinner: h6: Introduce Beelink
GS1 board")
> > > > > > Signed-off-by: Clément Péron
> > > > > > ---
> > > > > > arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 +
> > > > > > 1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-
gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> > > > > > index 049c21718846..3f20d2c9 100644
> > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> > > > > > @@ -145,6 +145,7 @@ {
> > > > > > vqmmc-supply = <_bldo2>;
> > > > > > non-removable;
> > > > > > cap-mmc-hw-reset;
> > > > > > + mmc-hs200-1_8v;
> > > > > > bus-width = <8>;
> > > > > > status = "okay";
> > > > > > };
> > > > >
> > > > > I'm not really sure what you're trying to fix here, but as far as MMC
> > > > > goes, eMMC's can support io voltage of 3.3, 1.8 and 1.2V. Modes up
until
> > > > > HS DDR (50MHz in DDR) will use an IO voltage of 3.3V, higher speed
modes
> > > > > (HS200 and HS400) supporting 1.8V and 1.2V.
> > > >
> > > > Some users report that the eMMC is not working properly on their
> > > > Beelink GS1 boards.
> > > >
> > > > > The mmc-hs200-1_8v property states that the MMC controller supports
the
> > > > > HS200 mode at 1.8V. Now, I can only assume that since BLDO2 is set
up at
> > > > > 1.8V then otherwise, the MMC core will rightfully decide to use the
> > > > > highest supported mode. In this case, since the driver sets it, it
would
> > > > > be HS-DDR at 3.3V, which won't work with that fixed regulator.
> > > > >
> > > > > I can only assume that enabling HS200 at 1.8V only fixes the issue
you
> > > > > have because otherwise it would use HS-DDR at 3.3V, ie not actually
> > > > > fixing the issue but sweeping it under the rug.
> > > > >
> > > > > Trying to add mmc-ddr-1_8v would be a good idea
> > > >
> > > > Thanks for the explanation, this is indeed the correct one.
> > > > So It looks like the SDIO controller has an issue on some boards when
> > > > using HS-DDR mode.
> > > >
> > > > Is this patch acceptable with the proper commit log?
> > >
> > > If HS-DDR works, yes, but I assume it doesn't?
> >
> > After discussing with Jernej about this issue, I understood that:
> > - Automatic delay calibration is not implemented
> > - We also miss some handling of DDR related bits in control register
> >
> > So none of H5/H6 boards should actually work.
> > (Some 'lucky' boards seem to work enough to switch to HS200 mode...)
> >
> > To "fix" this the H5 disable the HS-DDR mode in sunxi mmc driver :
> > https://github.com/torvalds/linux/blob/master/drivers/mmc/host/sunxi-mmc.c#L1409
>
> I find it suspicious that some boards would have traces not good enough
> for HS-DDR (50MHz in DDR) but would work fine in HS200 (200MHz in SDR).
> If there's some mismatch on the traces, it will only be worse in HS200.
FYI, similar situation is also with Tanix TX6 board. Mine works well in HS-DDR
mode, but some people reported that it doesn't work for them. The only
possible difference could be different eMMC IC. I'll try to confirm that.
Anyway, I did some tests on OrangePi 3 board which also have eMMC. Both modes
(HS-DDR and HS200) are supported and work well. Interesting observation is
that speed test (hdparm -t) reported 80.58 MB/sec for HS-DDR mode and 43.40
MB/sec for HS200. As it can be seen here, HS-DDR is quicker by a factor of 2,
but it should be the other way around. Reason for this is that both modes use
same base clock and thus HS-DDR produces higher speed.
If I change f_max to 150 MHz (max. per datasheet for SDR @ 1.8 V) then
naturally HS200 mode is faster (124.63 MB/sec) as HS-DDR as it should be. This
would be actually correct test for problematic boards but unfortunately I
don't have it. I also hacked in support for HS400 (~143 MB/s) and this mode is
the only one which really needs calibration on my board.
Two observations from BSP
