All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Michael Walle" <michael@walle.cc>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Robert Marko" <robert.marko@sartura.hr>,
	"Luka Perkov" <luka.perkov@sartura.hr>,
	linux-kernel@vger.kernel.org,
	"Randy Dunlap" <rdunlap@infradead.org>,
	"Chen-Yu Tsai" <wenst@chromium.org>,
	"Daniel Golle" <daniel@makrotopia.org>,
	"Rafał Miłecki" <rafal@milecki.pl>
Subject: Re: [PATCH v10 1/3] nvmem: core: Rework layouts to become platform devices
Date: Tue, 3 Oct 2023 11:43:26 +0200	[thread overview]
Message-ID: <20231003114326.7c61b07c@xps-13> (raw)
In-Reply-To: <84ff0dd8-706e-4b44-5313-3dd77b83100c@linaro.org>

Hi Srinivas,

> > +static int nvmem_dev_match_available_layout(struct device *dev, void *data)
> > +{
> > +	struct nvmem_device *nvmem = to_nvmem_device(dev);
> > +
> > +	return nvmem_match_available_layout(nvmem);
> > +}
> > +
> > +static int nvmem_for_each_dev(int (*fn)(struct device *dev, void *data))
> > +{
> > +	return bus_for_each_dev(&nvmem_bus_type, NULL, NULL, fn);
> > +}
> > +
> > +/*
> > + * When an NVMEM device is registered, try to match against a layout and
> > + * populate the cells. When an NVMEM layout is probed, ensure all NVMEM devices
> > + * which could use it properly expose their cells.
> > + */
> > +static int nvmem_notifier_call(struct notifier_block *notifier,
> > +			       unsigned long event_flags, void *context)
> > +{
> > +	struct nvmem_device *nvmem = NULL;
> > +	int ret;
> > +
> > +	switch (event_flags) {
> > +	case NVMEM_ADD:
> > +		nvmem = context;
> > +		break;
> > +	case NVMEM_LAYOUT_ADD:
> > +		break;
> > +	default:
> > +		return NOTIFY_DONE;
> > +	}  
> 
> It looks bit unnatural for core to register notifier for its own events.
> 
> 
> Why do we need the notifier at core level, can we not just handle this in core before raising these events, instead of registering a notifier cb?

There is no good place to do that "synchronously". We need some kind of
notification mechanism in these two cases:
* A memory device is being probed -> if a matching layout driver is
  already available, we need to parse the device and expose the cells,
  but not in the thread registering the memory device.
* A layout driver is being insmod'ed -> if a memory device needs it to
  create cells we need to parse the device content, but I find it
  crappy to start device-specific parsing in the registration handler.

So probe of the memory device is not a good place for this, nor is the
registration of the layout driver. Yet, we need to do the same
operation upon two different "events".

This notifier mechanism is a clean and easy way to get notified and
implement a callback which is also not blocking the thread doing the
initial registration. I am personally not bothered using it only
internally. If you have another mechanism in mind to perform a similar
operation, or a way to avoid this need I'll do the switch.

Thanks,
Miquèl

  reply	other threads:[~2023-10-03  9:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-22 17:48 [PATCH v10 0/3] NVMEM cells in sysfs Miquel Raynal
2023-09-22 17:48 ` [PATCH v10 1/3] nvmem: core: Rework layouts to become platform devices Miquel Raynal
2023-09-28 20:24   ` Rafał Miłecki
2023-10-02  9:23   ` Srinivas Kandagatla
2023-10-03  9:43     ` Miquel Raynal [this message]
2023-10-05 14:58       ` Miquel Raynal
2023-10-02  9:35   ` Greg Kroah-Hartman
2023-10-02 15:16     ` Miquel Raynal
2023-09-22 17:48 ` [PATCH v10 2/3] ABI: sysfs-nvmem-cells: Expose cells through sysfs Miquel Raynal
2023-09-22 17:48 ` [PATCH v10 3/3] nvmem: core: " Miquel Raynal
2023-09-28 15:31   ` Rafał Miłecki
2023-09-29  5:18     ` Rafał Miłecki
2023-10-01 15:59       ` Miquel Raynal

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=20231003114326.7c61b07c@xps-13 \
    --to=miquel.raynal@bootlin.com \
    --cc=daniel@makrotopia.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luka.perkov@sartura.hr \
    --cc=michael@walle.cc \
    --cc=rafal@milecki.pl \
    --cc=rdunlap@infradead.org \
    --cc=robert.marko@sartura.hr \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=wenst@chromium.org \
    /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.