Re: [PATCH 2/3] nvmem: Add spmi-nvmem driver

[Srinivas Kandagatla <srini@kernel.org>]

On Thu, Apr 17, 2025 at 04:30:53PM +0200, Janne Grunau wrote:
> On Thu, Apr 17, 2025 at 02:34:37PM +0100, Srinivas Kandagatla wrote:
> > 
> > 
> > On 15/04/2025 22:52, Sasha Finkelstein via B4 Relay wrote:
> > > From: Hector Martin <marcan@marcan.st>
> > > 
> > > This driver exposes a SPMI device as an NVMEM device.
> > > It is intended to be used with e.g. PMUs/PMICs that are used to
> > > hold power management configuration, such as used on Apple Silicon
> > > Macs.
> > > 
> > > Signed-off-by: Hector Martin <marcan@marcan.st>
> > > Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
> > > ---
> > >   MAINTAINERS                |  1 +
> > >   drivers/nvmem/Kconfig      | 14 +++++++++++
> > >   drivers/nvmem/Makefile     |  2 ++
> > >   drivers/nvmem/spmi-nvmem.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++
> > >   4 files changed, 79 insertions(+)
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index e7b2d0df81b387ba5398957131971588dc7b89dc..63c12f901aed1f3e6de8227d6db34af1bd046fe6 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -2298,6 +2298,7 @@ F:	drivers/iommu/io-pgtable-dart.c
> > >   F:	drivers/irqchip/irq-apple-aic.c
> > >   F:	drivers/nvme/host/apple.c
> > >   F:	drivers/nvmem/apple-efuses.c
> > > +F:	drivers/nvmem/spmi-nvmem.c
> > >   F:	drivers/pinctrl/pinctrl-apple-gpio.c
> > >   F:	drivers/pwm/pwm-apple.c
> > >   F:	drivers/soc/apple/*
> > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> > > index 8671b7c974b933e147154bb40b5d41b5730518d2..9ec907d8aa6ef7df0ea45cc35e92d8239d2705ee 100644
> > > --- a/drivers/nvmem/Kconfig
> > > +++ b/drivers/nvmem/Kconfig
> > > @@ -310,6 +310,20 @@ config NVMEM_SNVS_LPGPR
> > >   	  This driver can also be built as a module. If so, the module
> > >   	  will be called nvmem-snvs-lpgpr.
> > >   
> > > +config NVMEM_SPMI
> > > +	tristate "Generic SPMI NVMEM"
> > > +	default ARCH_APPLE
> > Why default is set to ARCH_APPLE?
> > 
> > This will endup with y in arm64 defconfig, means increasing the size of 
> > kernel.
> > 
> > should it be:
> > 
> > depends on ARCH_APPLE || COMPILE_TEST
> 
> I don't think it should depend on ARCH_APPLE. There is nothing
> ARCH_APPLE specific in the driver or dt-bindings even apple platforms
> are currently only user.

irrespective of this is generic or not none of the drivers should have
default set to y.


> 
> `default m if ARCH_APPLE` might an alternative but in this specific case
> the driver which will uses the nvmem cells should just select it. So I
> would remove the default.

remove the default, and let it be selected in defconfig as m as
required, like any other drivers.

--srini
> 
> Janne