public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
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 15:50:41 +0100	[thread overview]
Message-ID: <20180225145041.2AD1B24018D@gemini.denx.de> (raw)
In-Reply-To: <20180225134810.GU4311@bill-the-cat>

Dear Tom,

In message <20180225134810.GU4311@bill-the-cat> you wrote:
> 
> > 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.

Maybe I was not really clear.  You are right as this is not generic
file system code.  Instead, it is the EXT4 support code for the
environment handling.

The patch adds mmc_initialize() to env_ext4_load(), so whenever we
try to load the environment from an EXT4 file system, it will
initialize the MMC subsystem.

However - what if we want to load the environment from an EXT4 file
system on any other storage device - say, USB, or SATA, or flash?

In all such cases the initialization of MMC would be plain wrong.


And what if we want to load the environment from any other type of
file system - say, cramfs, zfs, etc. - on SDCard or eMMC?  These do
not initialize MMC, so it would fail?

Yes, we have the same crappy code in env/fat.c, but this is the
wrong thing to do, and must be cleaned up there as well.


This is what I meant when I wrote that the file system (interface)
code and the storage device support code are independent of each
other, and code should be kept orthogonal - storage support like MMC
etc. has no place in code dealing with a specific file system type.

I still think my NAK is reasonable.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I had the rare misfortune of being one of the first people to try and
implement a PL/1 compiler.                             -- T. Cheatham

  reply	other threads:[~2018-02-25 14:50 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
2018-02-25 14:50           ` Wolfgang Denk [this message]
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=20180225145041.2AD1B24018D@gemini.denx.de \
    --to=wd@denx.de \
    --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