[U-Boot] [linux-sunxi] Re: [PATCH v3 0/9] mmc: sunxi: Enable DM_MMC

[Andre Przywara]

On Fri, 25 Jan 2019 19:42:22 +0800
Chen-Yu Tsai <wens at csie.org> wrote:

> On Fri, Jan 25, 2019 at 7:37 PM Andre Przywara
> <andre.przywara at arm.com> wrote:
> >
> > On Fri, 25 Jan 2019 19:01:12 +0800
> > Chen-Yu Tsai <wens at csie.org> wrote:
> >
> > Hi,
> >  
> > > On Fri, Jan 25, 2019 at 6:41 PM Jagan Teki
> > > <jagan at amarulasolutions.com> wrote:  
> > > >
> > > > On Fri, Jan 25, 2019 at 2:05 PM Chen-Yu Tsai <wens at csie.org>
> > > > wrote:  
> > > > >
> > > > > On Fri, Jan 25, 2019 at 4:27 PM Jagan Teki
> > > > > <jagan at amarulasolutions.com> wrote:  
> > > > > >
> > > > > > On Tue, Jan 22, 2019 at 7:55 AM Chen-Yu Tsai <wens at csie.org>
> > > > > > wrote:  
> > > > > > >
> > > > > > > On Mon, Jan 21, 2019 at 6:31 PM Jagan Teki
> > > > > > > <jagan at amarulasolutions.com> wrote:  
> > > > > > > >
> > > > > > > > Compared to previous version changes[1] this version do
> > > > > > > > manage ahb clocks/resets via CLK framework.
> > > > > > > >
> > > > > > > > This version created changes along with Andre patches to
> > > > > > > > support clock/resets[2]
> > > > > > > >
> > > > > > > > Tested A64, SD, eMMC and respective changes available at
> > > > > > > > u-boot-sunxi/next
> > > > > > > >
> > > > > > > > Changes for v3:
> > > > > > > > - Handle clock via CLK framework.
> > > > > > > > Changes for v2:
> > > > > > > > - update the 'reset enablement' logic to do
> > > > > > > >   required SoC's
> > > > > > > >
> > > > > > > > [1] https://patchwork.ozlabs.org/cover/1026828/
> > > > > > > > [2] https://patchwork.ozlabs.org/cover/1027850/
> > > > > > > >
> > > > > > > > Andre Przywara (2):
> > > > > > > >   sunxi: clk: add MMC gates/resets
> > > > > > > >   sunxi: clk: A80: add MMC clock support
> > > > > > > >
> > > > > > > > Jagan Teki (7):
> > > > > > > >   mmc: sunxi: Add A83T emmc compatible
> > > > > > > >   mmc: sunxi: Add mmc, emmc H5/A64 compatible
> > > > > > > >   mmc: sunxi: Add DM_MMC support for H6
> > > > > > > >   mmc: sunxi: Add DM_MMC support for A80
> > > > > > > >   dm: mmc: sunxi: Add CLK and RESET support
> > > > > > > >   arm: sunxi: Enable DM_MMC
> > > > > > > >   arm: dts: sunxi: Enumerate MMC2 as MMC1  
> > > > > > >
> > > > > > > Tested-by: Chen-Yu Tsai <wens at csie.org> # on the Bananapi
> > > > > > > M2M
> > > > > > >
> > > > > > > However, the A80 MMC clock patch is still missing code to
> > > > > > > toggle the upstream bus clock gate and reset control in
> > > > > > > the CCU.  
> > > > > >
> > > > > > Sent it with original mmc clock and reset patch[1], let me
> > > > > > know your comments.  
> > > > >
> > > > > Right. The problem is nowhere in this series are those
> > > > > enabled / deasserted. The A80 CCU only has one gate & reset
> > > > > for the whole MMC subsystem, including the MMC config clocks
> > > > > block. Ideally the driver for that block should enable the
> > > > > gate and deassert the reset control.  
> > > >
> > > > Yes, that same has been included with v3.1 patch which I linked
> > > > above.  
> > >
> > > The gates/resets are added in that patch. No one is using them.
> > > The device tree node for the MMC controllers do _not_ reference
> > > this clock and reset, only the individual ones in the MMC config
> > > clock block.
> > >
> > > Unless you are expecting BROM or SPL to have set that up for you,
> > > which IMO is quite fragile. You could very well boot from SPI or
> > > even FEL, and it wouldn't be correctly setup, in which case MMC
> > > would be completely broken.
> > >
> > > FYI the clk-sun9i-mmc driver in Linux deasserts the reset at
> > > probe:
> > >
> > >     https://elixir.bootlin.com/linux/latest/source/drivers/clk/sunxi/clk-sun9i-mmc.c#L145
> > >
> > > And enables the bus gate when it needs to toggle bits in its
> > > register space:
> > >
> > >     https://elixir.bootlin.com/linux/latest/source/drivers/clk/sunxi/clk-sun9i-mmc.c#L53
> > >
> > > This is what is missing. Andre's patch doesn't have them. He
> > > specifically mentioned this.  
> >
> > But this is addressed by this, isn't it?
> > https://lists.denx.de/pipermail/u-boot/2019-January/355856.html
> > (combined with Jagan's addition of the shared gates in v3.1)  
> 
> Yes it does, however nowhere is it mentioned in the cover letter.

True, I realised this myself while searching for it in the archive ;-)

Will fix that in v2.

> That and I managed to purge it from my short term memory.
> 
> So with the two series combined it should work properly on the A80.

Cool. We need some serious testing of this, once we have settled on
something ;-)
Fortunately we now have more than two months until the release.

Cheers,
Andre.

> BTW, Jagan, vN.M patches are very hard to track...
> 
> ChenYu
> 
> > Btw.: Somehow I managed to send it from the wrong branch (doesn't
> > even compile), will send a fix later.
> >
> > Jagan, we need this patch for A80, and also the pinmux setup patch
> > (1/2 in this series) for it to work. That together with the H3
> > reset hiccup should be enough to justify a v4, I believe. Also I
> > had some comments on your v3 (commit messages outdated, 7/9 too
> > complicated), are you going to address them?
> >
> > Meanwhile I prepared a branch to have everything we need:
> > https://github.com/apritzel/u-boot/commits/sunxi-dm-gates
> > Please ignore the top 4 commits for now (adding SPI flash support).
> >
> > Cheers,
> > Andre.