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