All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.