linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Brian Norris <computersforpeace@gmail.com>
To: Daniel Ehrenberg <dehrenberg@google.com>
Cc: richard.weinberger@gmail.com,
	Ezequiel Garcia <ezequiel.garcia@imgtec.com>,
	Nam Nguyen <namnguyen@chromium.org>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	Gwendal Grignou <gwendal@chromium.org>
Subject: Re: [PATCH] mtd: part: Create the master device node when partitioned
Date: Tue, 10 Mar 2015 13:23:17 -0700	[thread overview]
Message-ID: <20150310202317.GB32500@ld-irv-0074> (raw)
In-Reply-To: <CAAK6Zt2xY=maQOQyC1sg7U2pAthbvE2QaWN_UaJC+CA6KBp1sA@mail.gmail.com>

On Tue, Mar 10, 2015 at 01:00:35PM -0700, Daniel Ehrenberg wrote:
> On Mon, Mar 9, 2015 at 6:53 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> > I kinda like this strategy. If users were flexible enough, I'd like to
> > be able to make this the default eventually. But that probably won't
> > happen, so we may have to live with this Kconfig forever, in that case.
> 
> :( It'd also be nice if we could name things like mtda, mtda0, mtda1,
> etc like SCSI. Alas!

Yeah...

> >
> > BTW, is there any way to tell the difference between a partition and a
> > master device, from user space?
> 
> Not that I know of.
> >
> > Also, would we want to address these comments from mtdpart.c?
> >
> >  * We don't register the master, or expect the caller to have done so,
> >  * for reasons of data integrity.
> >
> >
> >         /* NOTE:  we don't arrange MTDs as a tree; it'd be error-prone
> >          * to have the same data be in two different partitions.
> >          */
> >
> > First of all, as I mentioned previously, I don't think the argument is
> > valid. We can't (and shouldn't have to) protect users from themselves
> > here.
> 
> Agreed. Anything I should do to address this comment besides deleting it?

This depends on the answer to the next part.

> >
> > Secondly, I think maybe this should be changed if we register both
> > master and partition(s). The partition *should* use the master as its
> > parent if we register both devices.
> >
> While I'd like that change, it looks to me like it'd have to be
> conditional on the new sysfs variable. The master's sysfs entries
> won't be populated otherwise.

Right.

> Or should we build the sysfs node in
> either case?

No, I don't think we want to change the current behavior.

> What if people are depending on the current sysfs
> semantics (not too hard to imagine actually)?

If we're making these kinds of changes, they should only be for when you
have the new Kconfig entry enabled. I think it's fair to induce a few
sensible, related ABI changes when the kernel is configured to use a new
option. So:
1. register both master and partition(s)
2. use master as the parent of the partition (i.e., tree structure)

We should briefly describe those changes in the Kconfig, and adjust the
code comments to something like, "Historically, we did not register both
the master and partition(s) as devices--and therefore didn't arrange
them as a tree structure--because <fill in bogus reasons>."

> > Side ntoe: that might provide the means for differentiating master and
> > partition -- check the device parent in sysfs.
> >
> > Brian
> 
> Yeah, that'd be easy. It'd also be easy to add sysfs properties
> describing the partition more.

Sure. Off the top of my head: I don't think we have a way to read what
the offset of a partition is within the master. This info is only
printed to the kernel log. (I'm not specifically requesting such
information, but I can imagine it being useful.)

Brian

  reply	other threads:[~2015-03-10 20:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-06 22:22 [PATCH] mtd: part: Create the master device node when partitioned Dan Ehrenberg
2015-03-10  1:53 ` Brian Norris
2015-03-10 20:00   ` Daniel Ehrenberg
2015-03-10 20:23     ` Brian Norris [this message]
2015-03-10 20:36       ` Brian Norris

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=20150310202317.GB32500@ld-irv-0074 \
    --to=computersforpeace@gmail.com \
    --cc=dehrenberg@google.com \
    --cc=ezequiel.garcia@imgtec.com \
    --cc=gwendal@chromium.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=namnguyen@chromium.org \
    --cc=richard.weinberger@gmail.com \
    /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;
as well as URLs for NNTP newsgroup(s).