public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [U-Boot, v2] env: mmc/fat/ext4: make sure that the MMC sub-system is initialized before using it
Date: Sun, 25 Feb 2018 08:48:10 -0500	[thread overview]
Message-ID: <20180225134810.GU4311@bill-the-cat> (raw)
In-Reply-To: <20180225085310.248C324018D@gemini.denx.de>

On Sun, Feb 25, 2018 at 09:53:10AM +0100, Wolfgang Denk wrote:
> Dear Tom Rini,
> 
> In message <20180224215325.GQ4311@bill-the-cat> you wrote:
> > 
> > > Why do you ignore this NAK, and why do you add this patch so late in
> > > the release cycle anyway?
> > 
> > Sorry, didn't v2 address your concerns?  We don't initialize the device
> > because maybe we'll have env there, we initalize mmc because we need to
> > check that it is there.  Thanks!
> 
> No, it does not.  As is, we initialize MMC in the EXT4 code (in
> env_ext4_load()). If we go that route, we would have to add similar
> init calls to all other file systemn load routines as well.
> 
> This does not make sense to me.  This pollutes the code with
> unrelated things - file system code should not depend on any
> underlaying driver suport.  It increases code size, makes the code
> harder to maintain (if you need to change this, there is good
> chances to forget the same change in other file systems), and there
> is a good chance that in the end you will initialzie MMC even if you
> don't use it.
> 
> We should keep the code clean and orthogonal.  Driver init code has
> no place in file system code.
> 
> If needed, the drivers have to make sure to auto--initialize on
> first access.
> 
> I hold my NAK on this patch.  It is the wrong thing to do.

I think you have this a little bit wrong.  But, it's also where we are
with the DM conversion.  This _is_ the first place we're trying to
access the MMC.  And it's not in the filesystem code, it's in the
environment code.

That said, Faiz, what exactly is the failure sequence (and hardware)?
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180225/83cb01de/attachment.sig>

  reply	other threads:[~2018-02-25 13:48 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-12 13:54 [U-Boot] [PATCH v2] env: mmc/fat/ext4: make sure that the MMC sub-system is initialized before using it Faiz Abbas
2018-02-19  5:32 ` Faiz Abbas
2018-02-20 22:03 ` [U-Boot] [U-Boot, " Tom Rini
2018-02-24 20:58   ` Wolfgang Denk
2018-02-24 21:53     ` Tom Rini
2018-02-25  8:53       ` Wolfgang Denk
2018-02-25 13:48         ` Tom Rini [this message]
2018-02-25 14:50           ` Wolfgang Denk
2018-02-25 18:35             ` Tom Rini
2018-02-25 15:18           ` Lukasz Majewski
2018-02-25 17:38             ` Wolfgang Denk
2018-02-26 14:29           ` Faiz Abbas
2018-02-28  9:08             ` Lukasz Majewski
2018-03-05  9:59               ` Faiz Abbas

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=20180225134810.GU4311@bill-the-cat \
    --to=trini@konsulko.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