public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Siarhei Siamashka <siarhei.siamashka@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 10/14] dm: mmc: sunxi: Add support for driver model
Date: Fri, 28 Jul 2017 23:15:44 +0300	[thread overview]
Message-ID: <20170728231544.378e2130@i7> (raw)
In-Reply-To: <20170728160831.oxpylxqppuyorala@flea.lan>

On Fri, 28 Jul 2017 18:08:31 +0200
Maxime Ripard <maxime.ripard@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@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@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@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@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.

> > 
> > Well in that case let's drop the hack and someone will pick it up when
> > it hits them.  
> 
> That works for me.

Deliberately breaking something just to see if some end user runs into
troubles, wastes time to bisect the problem and comes up with a fix
looks like a dick move.

-- 
Best regards,
Siarhei Siamashka

  reply	other threads:[~2017-07-28 20:15 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-19 17:11 [U-Boot] [PATCH 00/14] dm: sata: mmc: Convert a sunxi board to driver model for MMC, SATA Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 01/14] ahci: Support non-PCI controllers Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 02/14] dm: mmc: Allow disabling driver model in SPL Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 03/14] fdt: Correct fdt_get_base_address() Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 04/14] dm: scsi: Drop duplicate SCSI and DM_SCSI options Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 05/14] dm: ahci: Correct uclass private data Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 06/14] dm: mmc: sunxi: Rename struct sunxi_mmc_host to sunxi_mmc_priv Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 07/14] dm: mmc: sunxi: Rename mmchost to priv Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 08/14] dm: mmc: sunxi: Pass private data around explicitly Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 09/14] dm: mmc: sunxi: Drop mmc_clk_io_on() Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 10/14] dm: mmc: sunxi: Add support for driver model Simon Glass
2017-06-21  7:31   ` Maxime Ripard
2017-07-04 19:33     ` Simon Glass
2017-07-05 14:57       ` Maxime Ripard
2017-07-05 20:14         ` Maxime Ripard
2017-07-14 13:47           ` Simon Glass
2017-07-17  9:26             ` Maxime Ripard
2017-07-28  4:19               ` Simon Glass
2017-07-28  5:03                 ` Suneel Garapati
2017-07-28 18:44                   ` Simon Glass
2017-07-28 21:34                     ` Suneel Garapati
2017-07-31 14:45                       ` Simon Glass
2017-07-28 16:08                 ` Maxime Ripard
2017-07-28 20:15                   ` Siarhei Siamashka [this message]
2017-08-22 19:57                     ` Maxime Ripard
2017-06-19 17:11 ` [U-Boot] [PATCH 11/14] dm: scsi: Don't scan the SCSI bus when probing Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 12/14] dm: sunxi: sata: Don't build sata support into SPL Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 13/14] dm: sata: sunxi: Add support for driver model Simon Glass
2017-06-19 17:11 ` [U-Boot] [PATCH 14/14] dm: sunxi: Move Linksprite_pcDuino3 to use DM for MMC, SATA Simon Glass
2017-06-20  6:45   ` Maxime Ripard
2017-06-20 18:26     ` Simon Glass
2017-06-21  7:18       ` Maxime Ripard
2017-07-04 19:33         ` Simon Glass
2017-07-02  1:32 ` [U-Boot] [PATCH 00/14] dm: sata: mmc: Convert a sunxi board to driver model " Simon Glass
2017-07-03 20:35   ` Maxime Ripard
2017-07-04 19:33     ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170728231544.378e2130@i7 \
    --to=siarhei.siamashka@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox