[U-Boot] [PATCH 10/14] dm: mmc: sunxi: Add support for driver model

[Maxime Ripard]

Hi Siarhei,

On Fri, Jul 28, 2017 at 11:15:44PM +0300, Siarhei Siamashka wrote:
> On Fri, 28 Jul 2017 18:08:31 +0200
> Maxime Ripard <maxime.ripard at free-electrons.com> wrote:
> 
> > On Thu, Jul 27, 2017 at 10:19:44PM -0600, Simon Glass wrote:
> > > Hi Maxime,
> > > 
> > > On 17 July 2017 at 03:26, Maxime Ripard
> > > <maxime.ripard at free-electrons.com> wrote:  
> > > > Hi Simon,
> > > >
> > > > On Fri, Jul 14, 2017 at 07:47:45AM -0600, Simon Glass wrote:  
> > > >> On 5 July 2017 at 14:14, Maxime Ripard <maxime.ripard at free-electrons.com> wrote:  
> > > >> > On Wed, Jul 05, 2017 at 04:57:40PM +0200, Maxime Ripard wrote:  
> > > >> >> On Tue, Jul 04, 2017 at 01:33:25PM -0600, Simon Glass wrote:  
> > > >> >> > Hi Maxime,
> > > >> >> >
> > > >> >> > On 21 June 2017 at 01:31, Maxime Ripard
> > > >> >> > <maxime.ripard at free-electrons.com> wrote:  
> > > >> >> > > Hi Simon,
> > > >> >> > >
> > > >> >> > > On Mon, Jun 19, 2017 at 11:11:27AM -0600, Simon Glass wrote:  
> > > >> >> > >> Add a driver-model version of this driver which mostly uses the existing
> > > >> >> > >> code. The old code can be removed once all boards are switched over.
> > > >> >> > >>
> > > >> >> > >> Signed-off-by: Simon Glass <sjg at chromium.org>  
> > > >> >> > >
> > > >> >> > > I'm not sure if you tested that, but we have some code that switches
> > > >> >> > > the MMC indices when using both an eMMC and an external MMC.
> > > >> >> > >
> > > >> >> > > http://git.denx.de/?p=u-boot.git;a=blob;f=board/sunxi/board.c#l494
> > > >> >> > >
> > > >> >> > > This predates my time, but it seems that it was done to have a
> > > >> >> > > consistent boot MMC device ID.
> > > >> >> > >
> > > >> >> > > I'm not really sure we can get rid of it (even if it creates some
> > > >> >> > > issues of it's own), but what would be the impact of a switch to the
> > > >> >> > > device model on that logic?  
> > > >> >> >
> > > >> >> > That is a pretty terrible hack.  
> > > >> >>
> > > >> >> Yes, I know. This is especially bad when used together with other
> > > >> >> tools that rely on one MMC index for example (such as fastboot).
> > > >> >>
> > > >> >> I wanted to kill it for quite some time, but I'm a bit reluctant due
> > > >> >> to the possible side effects.
> > > >> >>  
> > > >> >> > I'm not sure whether it will continue to work with DM. It does still
> > > >> >> > use the device number in the block device, so maybe...  Do you have
> > > >> >> > a board would use this?  
> > > >> >>
> > > >> >> I guess I do. I'll give it a try or tonight and let you know.  
> > > >> >
> > > >> > I just tested. Even with an eMMC (which was the first use case for
> > > >> > that hack), it works, even things that are not mainline yet (fastboot,
> > > >> > etc).
> > > >> >
> > > >> > It obviously break the old scripts relying on the mmc index, but I
> > > >> > guess that's ok.
> > > >> >
> > > >> > There's one regression though. Our eMMC will always be the second one,
> > > >> > which means that the distro bootargs will always boot on the external
> > > >> > SD first (which is always going to be mmc0).
> > > >> >
> > > >> > That's due to the fact that the eMMC controller is the third one, and
> > > >> > is thus probed last. We obviously want something deterministic for
> > > >> > fastboot for example, but booting partitions of the media you started
> > > >> > from make sense I guess. And this is what this hack was trying to
> > > >> > address.  
> > > >>
> > > >> OK...so what should we do here?  
> > > >
> > > > I guess we should just drop the hack. We'll have to at some point
> > > > anyway. But I guess we should also find a way to tweak the distro
> > > > bootcmd at boot time to search for the medium that we booted on first.
> > > >
> > > > I'm not really sure how to do this though.  
> 
> You can check how this is done for USB FEL boot. Basically, we set an
> environment variable if we see that the boot source is FEL:
>    http://git.denx.de/?p=u-boot.git;a=commitdiff;h=af654d14613f22f4910601d86e584030ee392b94
> 
> And then check this environment variable from bootcmd:
>    http://git.denx.de/?p=u-boot.git;a=commitdiff;h=f3b589c09b43a231706f11ab391e5ea7f9670f12
> 
> This can be done in a pretty much the same way for eMMC. Or maybe even
> generalized to also use this for different boot sources and non-sunxi
> SoCs.

That looks like a good solution, we'll give it a shot.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170822/75144143/attachment.sig>