Re: [PATCH v4 2/7] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()

From: Greg KH <gregkh@linuxfoundation.org>
To: "Uwe Kleine-König" <u.kleine-koenig@baylibre.com>
Cc: David Lechner <dlechner@baylibre.com>,
	Andy Shevchenko <andriy.shevchenko@intel.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	sboyd@kernel.org, jic23@kernel.org, nuno.sa@analog.com,
	andy@kernel.org, arnd@arndb.de, srini@kernel.org,
	vkoul@kernel.org, kishon@kernel.org, sre@kernel.org,
	krzysztof.kozlowski@linaro.org, linux-arm-msm@vger.kernel.org,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-phy@lists.infradead.org, linux-pm@vger.kernel.org,
	kernel@collabora.com, wenst@chromium.org,
	casey.connolly@linaro.org,
	Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>,
	Neil Armstrong <neil.armstrong@linaro.org>
Subject: Re: [PATCH v4 2/7] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Date: Wed, 24 Sep 2025 14:32:47 +0200	[thread overview]
Message-ID: <2025092451-immortal-synopsis-51fa@gregkh> (raw)
In-Reply-To: <x5ot622jqzz67imvswtdacqeeclqaw7my3pj6ne7tureec6ufg@fuzltifrkcae>

On Sat, Sep 20, 2025 at 06:41:57PM +0200, Uwe Kleine-König wrote:
> On Fri, Sep 19, 2025 at 05:37:03PM +0200, Greg KH wrote:
> > On Fri, Sep 19, 2025 at 10:20:29AM -0500, David Lechner wrote:
> > > Up to now, my (naive) understanding was that the point module namespaces
> > > is to reduce the number of symbols in the global namespace because having
> > > too many symbols there was starting to cause problems. So moving symbols
> > > to another namespace was a "good thing".
> > 
> > Yes, it is a "good thing" overall, but by just making all of your
> > symbols in a namespace, and then including it in the .h file, that does
> > the same exact thing as before (i.e. anyone that includes that .h file
> > puts the symbols into the global namespace with that prefix.)
> 
> I fail to parse the part in parenthesis. The symbols of the pwm
> subsystem are defined in the "PWM" namespace (using `#define
> DEFAULT_SYMBOL_NAMESPACE "PWM"` in drivers/pwm/core.c). In <linux/pwm.h>
> there is a `MODULE_IMPORT_NS("PWM");`, so a consumer (say
> `drivers/regulator/pwm-regulator.c`) only needs
> 
> 	#include <linux/pwm.h>
> 
> to import the "PWM" namespace. So pwm-regulator.c puts the symbols
> into the global namespace with that prefix. What symbols? What prefix?
> 
> The thing that is different is that the pwm functions are in the "PWM"
> namespace, so a module without an import for "PWM" has a smaller pool of
> global symbols and so loading that module is a tad more effective,
> right?
> 
> I agree that for the consumer source it doesn't make a difference if pwm
> is using a namespace or not. I'd count that as an advantage of the
> "import in a header" approach.
> 
> > Ideally, the goal was to be able to easily see in a module, what symbol
> > namespaces they depend on, which requires them to put MODULE_IMPORT_NS()
> > in the module to get access to those symbols.  dmabuf has done this very
> > well, making it obvious to the maintainers of that subsystem that they
> > should be paying attention to those users.
> 
> For me as pwm maintainer it doesn't matter much if I pay attention to
> `MODULE_IMPORT_NS("PWM");` or `#include <linux/pwm.h>`.

I think this is the primary thing here.  It's easier for some
maintainers and reviewers to notice the MODULE_IMPORT_NS() thing than a
simple include line.  Especially as includes are often hidden in other
include files.

So sure, as a maintainer, you are free to do things this way, it's just
not really what we thought about when namespaces were first created.  We
assumed that an explict MODULE_INPORT_NS() was what would be used, not
worked around :)

> > For other "tiny" subsystems, it just slots away their symbols so that no
> > one else should ever be using them, and it makes it blindingly obvious
> > if they do.  For example, the usb-storage symbols, anyone that does:
> > 	MODULE_IMPORT_NS("USB_STORAGE");
> > had better be living in drivers/usb/storage/ otherwise I need to have a
> > word with those offenders :)
> 
> All symbols in the "USB_STORAGE" namespace (apart from
> `fill_inquiry_response`) start with `usb_stor_`. If you grep for that
> string you find all the (probably illegitimate) users of the usb-storage
> symbols, but also those that define their own symbols with that prefix.
> 
> Do you actually look out for such offenders, i.e. have a lei mailbox
> with `MODULE_IMPORT_NS("USB_STORAGE")` as search string or a cron job
> grepping your tree for that?

Some maintainers do just this, yes.  I think the dmabuf maintainers do
as an example.

> > So it's a way of "tidying" up things, and to make things more explicit
> > than just having to rely on searching a tree and looking for .h include
> > usage. 
> 
> For some reason in your eyes grepping for MODULE_IMPORT_NS is superior
> to grepping for an #include. Can you explain that?

#include files are often included in other include files, and can easily
be "hidden" in the chain of what is needed by what .c file.

That's it, nothing major here, if you want to use namespaces this way,
that's fine, just feels kind of counter-productive :)

thanks,

greg k-h

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy