linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alban <albeu@free.fr>
To: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Aban Bedel <albeu@free.fr>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-mtd@lists.infradead.org,
	Cyrille Pitchen <cyrille.pitchen@atmel.com>,
	Richard Weinberger <richard@nod.at>,
	Marek Vasut <marek.vasut@gmail.com>,
	Brian Norris <computersforpeace@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Maxime Ripard <maxime.ripard@free-electrons.com>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	Moritz Fischer <moritz.fischer@ettus.com>
Subject: Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API
Date: Fri, 3 Mar 2017 14:57:26 +0100	[thread overview]
Message-ID: <20170303145726.16cd67fd@tock> (raw)
In-Reply-To: <20170303143658.5d89a329@bbrezillon>

[-- Attachment #1: Type: text/plain, Size: 2889 bytes --]

On Fri, 3 Mar 2017 14:36:58 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Fri, 3 Mar 2017 13:36:29 +0100
> Alban <albeu@free.fr> wrote:
> 
> > On Thu, 2 Mar 2017 22:18:03 +0100
> > Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> >   
> > > On Thu,  2 Mar 2017 20:50:22 +0100
> > > Alban <albeu@free.fr> wrote:
> > > 
> > > [snip]
> > >    
> [snip]
>   
> > > Maybe we should let notifiers return an error if they want to cancel
> > > the removal, or maybe this is a good reason to put the nvmem pointer
> > > directly in mtd_info and call mtd_nvmem_add/remove() directly from
> > > add/del_mtd_device() and allow them to return an error.
> > > 
> > > Not that, if you go for this solution, you'll also get rid of the
> > > global mtd_nvmem_list list and the associated lock.    
> > 
> > IMHO the MTD users framework has to be re-worked to be useful. First
> > both the add and remove callbacks should have return values. Users where
> > the add failed shouldn't be removed later and users where the remove
> > fails should block the removal of the MTD.  
> 
> As said in my previous reply, it's not just about returning an error. I
> had a closer look at the code, and it seems that using
> __get_mtd_device() properly should prevent the problem we are talking
> about (call __get_mtd_device() after your nvmem_register() and call
> __put_mtd_device() only if nvmem_unregister() succeed).

That's not going to work. If the notifier add increase the MTD reference
count it can never be removed again. What could work would be to
propagate the nvmem device ref counting down to the MTD device, but that
sound complex and would require some non-trivial locking to still allow
for an "always succeed" removal.

> > 
> > Furthermore only passing the MTD device to the add/remove callback
> > force the users to keep their own list, which is annoying to say the
> > least. A simple fix would be to have the add callback return a pointer
> > that would be passed back to the remove callback. Trivial to implement
> > and the MTD user wouldn't have to keep any list. I will look into this
> > in the next days.  
> 
> That's a different problem, and I'm not sure I like the idea of
> changing the ->add() prototype into
> 
> 	void *(*add)(struct mtd_info *);
> 
> If we want to do that, I'd rather see an API extension allowing one to
> attach/detach/query/update user data to an MTD device.

Under which condition would these be triggered? That sound more than is
needed. I would just use the above add along with:

 int (*remove)(struct mtd_info *, void *);

And add a list of successfully added notifiers, along with their
data pointer, to the MTD device. That's simple and would also remove
the need for notifier to have a private list of their instances as I
had to do here.

Alban

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2017-03-03 19:59 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-02 19:50 [PATCH 0/3] " Alban
2017-03-02 19:50 ` [PATCH 1/3] doc: bindings: Add bindings documentation for mtd nvmem Alban
2017-03-02 20:22   ` Boris Brezillon
2017-03-03 12:17     ` Alban
2017-03-03 12:37       ` Boris Brezillon
2017-03-03 13:12         ` Alban
2017-03-03 11:27   ` Srinivas Kandagatla
2017-03-03 12:19     ` Boris Brezillon
2017-03-03 12:22     ` Alban
2017-03-02 19:50 ` [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API Alban
2017-03-02 21:18   ` Boris Brezillon
2017-03-03 12:36     ` Alban
2017-03-03 13:36       ` Boris Brezillon
2017-03-03 13:57         ` Alban [this message]
2017-03-03 14:11           ` Boris Brezillon
2017-03-03 22:21             ` Richard Weinberger
2017-03-06 17:21               ` Alban
2017-03-06 19:03                 ` Richard Weinberger
2017-03-06 21:02                   ` Boris Brezillon
2017-03-03 11:23   ` Srinivas Kandagatla
2017-03-03 12:34     ` Boris Brezillon
2017-03-03 13:30       ` Alban
2017-03-03 14:03         ` Boris Brezillon
2017-03-02 19:50 ` [PATCH 3/3] nvmem: core: Allow allocating several anonymous nvmem devices Alban
2017-03-02 20:03   ` Boris Brezillon
2017-03-03  1:50     ` Moritz Fischer
2017-03-03 10:08   ` Srinivas Kandagatla

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=20170303145726.16cd67fd@tock \
    --to=albeu@free.fr \
    --cc=boris.brezillon@free-electrons.com \
    --cc=computersforpeace@gmail.com \
    --cc=cyrille.pitchen@atmel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marek.vasut@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=maxime.ripard@free-electrons.com \
    --cc=moritz.fischer@ettus.com \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --cc=srinivas.kandagatla@linaro.org \
    --subject='Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API' \
    /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

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).