2020-12-27 21:17:54

by Hans de Goede

[permalink] [raw]
Subject: [PATCH 04/14] mfd: arizona: Allow building arizona MFD-core as module

There is no reason why the arizona core,irq and codec model specific
regmap bits cannot be build as a module. All they do is export symbols
which are used by the arizona-spi and/or arizona-i2c modules, which
themselves can be built as module.

Change the Kconfig and Makefile arizona bits so that the arizona MFD-core
can be built as a module.

This is especially useful on x86 platforms with a WM5102 codec, this
allows the arizona MFD driver necessary for the WM5102 codec to be
enabled in generic distro-kernels without growing the base kernel-image
size.

Signed-off-by: Hans de Goede <[email protected]>
---
drivers/mfd/Kconfig | 2 +-
drivers/mfd/Makefile | 14 +++++++-------
drivers/mfd/arizona-core.c | 2 ++
3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index cc0b73280c68..8fe9e10eaf51 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1831,7 +1831,7 @@ config MFD_ARIZONA
select REGMAP
select REGMAP_IRQ
select MFD_CORE
- bool
+ tristate

config MFD_ARIZONA_I2C
tristate "Cirrus Logic/Wolfson Microelectronics Arizona platform with I2C"
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 14fdb188af02..3f208af1664f 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -41,24 +41,24 @@ obj-$(CONFIG_MFD_TQMX86) += tqmx86.o

obj-$(CONFIG_MFD_LOCHNAGAR) += lochnagar-i2c.o

-obj-$(CONFIG_MFD_ARIZONA) += arizona-core.o
-obj-$(CONFIG_MFD_ARIZONA) += arizona-irq.o
+arizona-objs := arizona-core.o arizona-irq.o
+obj-$(CONFIG_MFD_ARIZONA) += arizona.o
obj-$(CONFIG_MFD_ARIZONA_I2C) += arizona-i2c.o
obj-$(CONFIG_MFD_ARIZONA_SPI) += arizona-spi.o
ifeq ($(CONFIG_MFD_WM5102),y)
-obj-$(CONFIG_MFD_ARIZONA) += wm5102-tables.o
+arizona-objs += wm5102-tables.o
endif
ifeq ($(CONFIG_MFD_WM5110),y)
-obj-$(CONFIG_MFD_ARIZONA) += wm5110-tables.o
+arizona-objs += wm5110-tables.o
endif
ifeq ($(CONFIG_MFD_WM8997),y)
-obj-$(CONFIG_MFD_ARIZONA) += wm8997-tables.o
+arizona-objs += wm8997-tables.o
endif
ifeq ($(CONFIG_MFD_WM8998),y)
-obj-$(CONFIG_MFD_ARIZONA) += wm8998-tables.o
+arizona-objs += wm8998-tables.o
endif
ifeq ($(CONFIG_MFD_CS47L24),y)
-obj-$(CONFIG_MFD_ARIZONA) += cs47l24-tables.o
+arizona-objs += cs47l24-tables.o
endif
obj-$(CONFIG_MFD_WCD934X) += wcd934x.o
obj-$(CONFIG_MFD_WM8400) += wm8400-core.o
diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index 000cb82023e3..a9ba1c865abf 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -1478,3 +1478,5 @@ int arizona_dev_exit(struct arizona *arizona)
return 0;
}
EXPORT_SYMBOL_GPL(arizona_dev_exit);
+
+MODULE_LICENSE("GPL v2");
--
2.28.0


2020-12-29 12:02:42

by Charles Keepax

[permalink] [raw]
Subject: Re: [PATCH 04/14] mfd: arizona: Allow building arizona MFD-core as module

On Sun, Dec 27, 2020 at 10:12:22PM +0100, Hans de Goede wrote:
> There is no reason why the arizona core,irq and codec model specific
> regmap bits cannot be build as a module. All they do is export symbols
> which are used by the arizona-spi and/or arizona-i2c modules, which
> themselves can be built as module.
>
> Change the Kconfig and Makefile arizona bits so that the arizona MFD-core
> can be built as a module.
>
> This is especially useful on x86 platforms with a WM5102 codec, this
> allows the arizona MFD driver necessary for the WM5102 codec to be
> enabled in generic distro-kernels without growing the base kernel-image
> size.
>
> Signed-off-by: Hans de Goede <[email protected]>
> ---

I think this patch might still cause some issues. ASoC has an
idiom where the machine driver does a select on the necessary
CODEC drivers. Select doesn't take care of dependencies etc. So I
believe if you build the machine driver as built in, it then
selects the CODEC as built in. If you have the MFD as a module
the build then fails due to the the CODEC calling some Arizona
functions.

arizona_request_irq, arizona_free_irq, arizona_set_irq_wake

On Madera we made the equivalents inline functions to avoid the
issue, the same should work here.

include/linux/irqchip/irq-madera.h

Thanks,
Charles

2021-01-12 09:20:13

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 04/14] mfd: arizona: Allow building arizona MFD-core as module

Hi,

First of all thank you for the review and for all your comments
(also in the other part of the thread).

On 12/29/20 1:00 PM, Charles Keepax wrote:
> On Sun, Dec 27, 2020 at 10:12:22PM +0100, Hans de Goede wrote:
>> There is no reason why the arizona core,irq and codec model specific
>> regmap bits cannot be build as a module. All they do is export symbols
>> which are used by the arizona-spi and/or arizona-i2c modules, which
>> themselves can be built as module.
>>
>> Change the Kconfig and Makefile arizona bits so that the arizona MFD-core
>> can be built as a module.
>>
>> This is especially useful on x86 platforms with a WM5102 codec, this
>> allows the arizona MFD driver necessary for the WM5102 codec to be
>> enabled in generic distro-kernels without growing the base kernel-image
>> size.
>>
>> Signed-off-by: Hans de Goede <[email protected]>
>> ---
>
> I think this patch might still cause some issues. ASoC has an
> idiom where the machine driver does a select on the necessary
> CODEC drivers. Select doesn't take care of dependencies etc. So I
> believe if you build the machine driver as built in, it then
> selects the CODEC as built in. If you have the MFD as a module
> the build then fails due to the the CODEC calling some Arizona
> functions.

The rules for using select in Kconfig say that the Kconfig
symbol doing the selecting must either have a "requires on
or a "select" on any dependencies of the Kconfig symbol which
it is selecting. Looking at the new machine-driver which this
series adds:

config SND_SOC_INTEL_BYTCR_WM5102_MACH
tristate "Baytrail and Baytrail-CR with WM5102 codec"
depends on SPI && ACPI
depends on X86_INTEL_LPSS || COMPILE_TEST
select SND_SOC_ACPI
select SND_SOC_WM5102
help
This adds support for ASoC machine driver for Intel(R) Baytrail and Baytrail-CR
platforms with WM5102 audio codec.
Say Y if you have such a device.
If unsure select "N".

I now see that I'm breaking that rule myself and this is
missing a "depends on MFD_WM5102" I do see a "depends on MFD_WM5102"
here:

config SND_SOC_WM5102
tristate
depends on MFD_WM5102

So I would expect some sort of error if I unselect MFD_WM5102 and indeed
the following happens if I unselect MFD_WM5102:

[hans@x1 linux]$ make oldconfig

WARNING: unmet direct dependencies detected for SND_SOC_WM5102
Depends on [n]: SOUND [=m] && !UML && SND [=m] && SND_SOC [=m] && MFD_WM5102 [=n]
Selected by [m]:
- SND_SOC_INTEL_BYTCR_WM5102_MACH [=m] && SOUND [=m] && !UML && SND [=m] && SND_SOC [=m] && SND_SOC_INTEL_MACH [=y] && (SND_SST_ATOM_HIFI2_PLATFORM [=m] || SND_SOC_SOF_BAYTRAIL [=m]) && SPI [=y] && ACPI [=y] && (X86_INTEL_LPSS [=y] || COMPILE_TEST [=n])

Which of course is not supposed to happen and is caused by the
config SND_SOC_INTEL_BYTCR_WM5102_MACH missing
"depends on MFD_WM5102", which is my bad.

But typing this (rubber ducky effect) has made me realized what the
problem is, the codec Kconfig symbols depend on the: MFD_WM5102,
MFD_WM5110, etc. Kconfig symbols. But those are booleans which enable/disable
support for those codecs inside arizona-core.c. So you are right that
this ("mfd: arizona: Allow building arizona MFD-core as module") could
cause a scenario where the core is built as a module and the codec
driver is builtin.

But I do not think that the solution is to inline all the functions used
from the codec drivers. The solution is to extend:

config SND_SOC_WM5102
tristate
depends on MFD_WM5102

to:

config SND_SOC_WM5102
tristate
depends on MFD_WM5102
depends on MFD_ARIZONA

And to update machine drivers to still match the:
"The Kconfig symbol doing the selecting must either have a "requires
on" or a "select" on any dependencies of the Kconfig symbol which
it is selecting." rule which I formulated above.

(and idem for all the other codecs which use symbols from MFD_ARIZONA)

> arizona_request_irq, arizona_free_irq, arizona_set_irq_wake
>
> On Madera we made the equivalents inline functions to avoid the
> issue, the same should work here.
>
> include/linux/irqchip/irq-madera.h

Yes that will work too, but I would prefer to solve what is a Kconfig
issue with Kconfig changes.

Note I will simply drop this patch for the next version of this series (*)
since the whole jack rework thing we discussed is really orthogonal to
this and that one will be interesting enough the review all by itself :)

Regards,

Hans



*) I will resubmit a fixed version later after the other stuff has landed