| Message ID | 20201227211232.117801-5-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MFD/extcon/ASoC: Add support for Intel Bay Trail boards with WM5102 codec | expand |
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 <hdegoede@redhat.com> > --- 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
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 <hdegoede@redhat.com> >> --- > > 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
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");
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 <hdegoede@redhat.com> --- drivers/mfd/Kconfig | 2 +- drivers/mfd/Makefile | 14 +++++++------- drivers/mfd/arizona-core.c | 2 ++ 3 files changed, 10 insertions(+), 8 deletions(-)