Re: Re: [PATCH v2] arm64: dts: allwinner: h6: add eMMC voltage property for Beelink GS1

["Jernej Škrabec" <jernej.skrabec@siol.net>]

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 <maxime@cerno.tech> 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 <maxime@cerno.tech> 
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 <peron.clem@gmail.com>
> > > > > > > > ---
> > > > > > > >  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..3f20d2c9bbbb 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 @@ &mmc2 {
> > > > > > > >       vqmmc-supply = <&reg_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 back and couldn't find any board with
> HS200/HS400 since.

Yes, I think all H6 boards with eMMC support HS400. If you didn't already 
erase Android from eMMC, I suggest to run it and grep dmesg for HS400. I'm 
pretty sure it will be there.

> 
> > 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.
> 
> If it work fine on the H6, we should set the max-frequency property on
> the DT, just like I did on the A64.

Oh, I missed that property. I'll check. However, we can't blindly set max-
frequency. According to user manual, HS400 supports lower max. frequency than 
HS200.

> 
> > 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 driver:
> > 1. Module clock is disabled before adjusting DDR bit and afterwards it's 
re-
> > enabled . That could fix some kind of glitches.
> > 2. SDMMC peripheral runs on higher clock than on mainline.
> > 
> > > And for the delay calibration, iirc, that's only necessary for HS400
> > > that we don't support?
> > 
> > According to BSP driver and its DT, HS200 also needs calibration. However, 
it 
> > seems that using it on lower speed it isn't needed.
> 
> Which calibration do you mean? IIRC, the data strobe signal is only used
> (optionally) in HS400. The controller might have some internal
> calibration for other modes though.

Actually both. BSP always adjust both calibration values at the same time 
based on table from DT.

Best regards,
Jernej