2019-07-18 02:08:13

by Yue Haibing

[permalink] [raw]
Subject: [PATCH] Input: applespi: Fix build error without CONFIG_PCI

If CONFIG_KEYBOARD_APPLESPI is set to y, but
CONFIG_PCI is not set, building will fails:

drivers/spi/spi-pxa2xx-pci.c: In function pxa2xx_spi_pci_probe:
drivers/spi/spi-pxa2xx-pci.c:208:8: error: implicit declaration of function pcim_enable_device;
did you mean pci_enable_device? [-Werror=implicit-function-declaration]
ret = pcim_enable_device(dev);
^~~~~~~~~~~~~~~~~~
pci_enable_device
drivers/spi/spi-pxa2xx-pci.c:239:8: error: implicit declaration of function pci_alloc_irq_vectors;
did you mean pci_alloc_consistent? [-Werror=implicit-function-declaration]
ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES);
^~~~~~~~~~~~~~~~~~~~~

Make CONFIG_KEYBOARD_APPLESPI depends on CONFIG_PCI
to fix this.

Reported-by: Hulk Robot <[email protected]>
Fixes: b426ac045209 ("Input: add Apple SPI keyboard and trackpad driver")
Signed-off-by: YueHaibing <[email protected]>
---
drivers/input/keyboard/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index dd934c4..fefcc46 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -74,7 +74,7 @@ config ATARI_KBD_CORE
config KEYBOARD_APPLESPI
tristate "Apple SPI keyboard and trackpad"
depends on ACPI && EFI
- depends on SPI
+ depends on SPI && PCI
depends on X86 || COMPILE_TEST
imply SPI_PXA2XX
imply SPI_PXA2XX_PCI
--
2.7.4



2019-07-18 07:17:20

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] Input: applespi: Fix build error without CONFIG_PCI

On Thu, Jul 18, 2019 at 4:07 AM YueHaibing <[email protected]> wrote:
>
> If CONFIG_KEYBOARD_APPLESPI is set to y, but
> CONFIG_PCI is not set, building will fails:
>
> drivers/spi/spi-pxa2xx-pci.c: In function pxa2xx_spi_pci_probe:
> drivers/spi/spi-pxa2xx-pci.c:208:8: error: implicit declaration of function pcim_enable_device;
> did you mean pci_enable_device? [-Werror=implicit-function-declaration]
> ret = pcim_enable_device(dev);
> ^~~~~~~~~~~~~~~~~~
> pci_enable_device
> drivers/spi/spi-pxa2xx-pci.c:239:8: error: implicit declaration of function pci_alloc_irq_vectors;
> did you mean pci_alloc_consistent? [-Werror=implicit-function-declaration]
> ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES);
> ^~~~~~~~~~~~~~~~~~~~~
>
> Make CONFIG_KEYBOARD_APPLESPI depends on CONFIG_PCI
> to fix this.
>
> Reported-by: Hulk Robot <[email protected]>
> Fixes: b426ac045209 ("Input: add Apple SPI keyboard and trackpad driver")
> Signed-off-by: YueHaibing <[email protected]>

I found the same build bug, plus another issue:

arch/arm/Kconfig:1942:error: recursive dependency detected!
arch/arm/Kconfig:1942: symbol XIP_KERNEL depends on KASAN
lib/Kconfig.kasan:15: symbol KASAN depends on SYSFS
fs/sysfs/Kconfig:2: symbol SYSFS is selected by CONFIGFS_FS
fs/configfs/Kconfig:2: symbol CONFIGFS_FS is selected by USB_LIBCOMPOSITE
drivers/usb/gadget/Kconfig:145: symbol USB_LIBCOMPOSITE is
selected by USB_ZERO
drivers/usb/gadget/legacy/Kconfig:17: symbol USB_ZERO is part of
choice <choice>
drivers/usb/gadget/Kconfig:486: choice <choice> contains symbol USB_G_WEBCAM
drivers/usb/gadget/legacy/Kconfig:479: symbol USB_G_WEBCAM is
part of choice VIDEO_V4L2
drivers/media/v4l2-core/Kconfig:7: symbol VIDEO_V4L2 depends on I2C
drivers/i2c/Kconfig:8: symbol I2C is selected by FB_DDC
drivers/video/fbdev/Kconfig:63: symbol FB_DDC depends on FB
drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER
drivers/gpu/drm/Kconfig:75: symbol DRM_KMS_FB_HELPER depends
on DRM_KMS_HELPER
drivers/gpu/drm/Kconfig:69: symbol DRM_KMS_HELPER is selected
by DRM_ARMADA
drivers/gpu/drm/armada/Kconfig:2: symbol DRM_ARMADA depends
on HAVE_CLK
arch/Kconfig:314: symbol HAVE_CLK is selected by CLKDEV_LOOKUP
drivers/clk/Kconfig:3: symbol CLKDEV_LOOKUP is selected by COMMON_CLK
drivers/clk/Kconfig:10: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
drivers/mfd/Kconfig:600: symbol MFD_INTEL_LPSS is selected
by MFD_INTEL_LPSS_PCI
drivers/mfd/Kconfig:614: symbol MFD_INTEL_LPSS_PCI is
implied by KEYBOARD_APPLESPI
drivers/input/keyboard/Kconfig:74: symbol KEYBOARD_APPLESPI
depends on EFI
arch/arm/Kconfig:2031: symbol EFI depends on XIP_KERNEL

Your patch correctly solves the spi_pxa2xx issue, but I'd prefer to instead
drop the three 'imply' statements altogether, they seem to do more harm
than good.

(the circular dependency I saw might only happen when applying the
arm32 KASAN patches, but I expect to see them merged for linux-5.4)

Arnd

Subject: Re: [PATCH] Input: applespi: Fix build error without CONFIG_PCI


On Thu, Jul 18, 2019 at 10:06:54AM +0800, YueHaibing wrote:
> If CONFIG_KEYBOARD_APPLESPI is set to y, but
> CONFIG_PCI is not set, building will fails:
>
> drivers/spi/spi-pxa2xx-pci.c: In function pxa2xx_spi_pci_probe:
> drivers/spi/spi-pxa2xx-pci.c:208:8: error: implicit declaration of function pcim_enable_device;
> did you mean pci_enable_device? [-Werror=implicit-function-declaration]
> ret = pcim_enable_device(dev);
> ^~~~~~~~~~~~~~~~~~
> pci_enable_device
> drivers/spi/spi-pxa2xx-pci.c:239:8: error: implicit declaration of function pci_alloc_irq_vectors;
> did you mean pci_alloc_consistent? [-Werror=implicit-function-declaration]
> ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES);
> ^~~~~~~~~~~~~~~~~~~~~
>
> Make CONFIG_KEYBOARD_APPLESPI depends on CONFIG_PCI
> to fix this.
>
> Reported-by: Hulk Robot <[email protected]>
> Fixes: b426ac045209 ("Input: add Apple SPI keyboard and trackpad driver")
> Signed-off-by: YueHaibing <[email protected]>
> ---
> drivers/input/keyboard/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index dd934c4..fefcc46 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -74,7 +74,7 @@ config ATARI_KBD_CORE
> config KEYBOARD_APPLESPI
> tristate "Apple SPI keyboard and trackpad"
> depends on ACPI && EFI
> - depends on SPI
> + depends on SPI && PCI
> depends on X86 || COMPILE_TEST
> imply SPI_PXA2XX
> imply SPI_PXA2XX_PCI
> --
> 2.7.4

I think this is more properly fixed by Dmitry's suggestion of making
SPI_PXA2XX_PCI depend on PCI, since it's that module, not applespi,
that actually needs PCI - see
https://www.spinics.net/lists/linux-input/msg62351.html


Cheers,

Ronald

2019-07-18 11:58:32

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] Input: applespi: Fix build error without CONFIG_PCI

On Thu, Jul 18, 2019 at 1:40 PM Life is hard, and then you die
<[email protected]> wrote:
> On Thu, Jul 18, 2019 at 10:06:54AM +0800, YueHaibing wrote:

> > @@ -74,7 +74,7 @@ config ATARI_KBD_CORE
> > config KEYBOARD_APPLESPI
> > tristate "Apple SPI keyboard and trackpad"
> > depends on ACPI && EFI
> > - depends on SPI
> > + depends on SPI && PCI
> > depends on X86 || COMPILE_TEST
> > imply SPI_PXA2XX
> > imply SPI_PXA2XX_PCI
> > --
> > 2.7.4
>
> I think this is more properly fixed by Dmitry's suggestion of making
> SPI_PXA2XX_PCI depend on PCI, since it's that module, not applespi,
> that actually needs PCI - see
> https://www.spinics.net/lists/linux-input/msg62351.html

I'll put that into my randconfig setup to see if it's sufficient. I'm
a little bit
suspicious as the circular dependency was not avoided by the 'depends
on X86' for MFD_INTEL_LPSS_PCI.

Arnd

Subject: Re: [PATCH] Input: applespi: Fix build error without CONFIG_PCI


Hi Arnd,

On Thu, Jul 18, 2019 at 09:15:59AM +0200, Arnd Bergmann wrote:
> On Thu, Jul 18, 2019 at 4:07 AM YueHaibing <[email protected]> wrote:
> >
> > If CONFIG_KEYBOARD_APPLESPI is set to y, but
> > CONFIG_PCI is not set, building will fails:
> >
> > drivers/spi/spi-pxa2xx-pci.c: In function pxa2xx_spi_pci_probe:
> > drivers/spi/spi-pxa2xx-pci.c:208:8: error: implicit declaration of function pcim_enable_device;
> > did you mean pci_enable_device? [-Werror=implicit-function-declaration]
> > ret = pcim_enable_device(dev);
> > ^~~~~~~~~~~~~~~~~~
> > pci_enable_device
> > drivers/spi/spi-pxa2xx-pci.c:239:8: error: implicit declaration of function pci_alloc_irq_vectors;
> > did you mean pci_alloc_consistent? [-Werror=implicit-function-declaration]
> > ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES);
> > ^~~~~~~~~~~~~~~~~~~~~
> >
> > Make CONFIG_KEYBOARD_APPLESPI depends on CONFIG_PCI
> > to fix this.
> >
> > Reported-by: Hulk Robot <[email protected]>
> > Fixes: b426ac045209 ("Input: add Apple SPI keyboard and trackpad driver")
> > Signed-off-by: YueHaibing <[email protected]>
>
> I found the same build bug, plus another issue:
>
> arch/arm/Kconfig:1942:error: recursive dependency detected!
> arch/arm/Kconfig:1942: symbol XIP_KERNEL depends on KASAN
> lib/Kconfig.kasan:15: symbol KASAN depends on SYSFS
> fs/sysfs/Kconfig:2: symbol SYSFS is selected by CONFIGFS_FS
> fs/configfs/Kconfig:2: symbol CONFIGFS_FS is selected by USB_LIBCOMPOSITE
> drivers/usb/gadget/Kconfig:145: symbol USB_LIBCOMPOSITE is
> selected by USB_ZERO
> drivers/usb/gadget/legacy/Kconfig:17: symbol USB_ZERO is part of
> choice <choice>
> drivers/usb/gadget/Kconfig:486: choice <choice> contains symbol USB_G_WEBCAM
> drivers/usb/gadget/legacy/Kconfig:479: symbol USB_G_WEBCAM is
> part of choice VIDEO_V4L2
> drivers/media/v4l2-core/Kconfig:7: symbol VIDEO_V4L2 depends on I2C
> drivers/i2c/Kconfig:8: symbol I2C is selected by FB_DDC
> drivers/video/fbdev/Kconfig:63: symbol FB_DDC depends on FB
> drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER
> drivers/gpu/drm/Kconfig:75: symbol DRM_KMS_FB_HELPER depends
> on DRM_KMS_HELPER
> drivers/gpu/drm/Kconfig:69: symbol DRM_KMS_HELPER is selected
> by DRM_ARMADA
> drivers/gpu/drm/armada/Kconfig:2: symbol DRM_ARMADA depends
> on HAVE_CLK
> arch/Kconfig:314: symbol HAVE_CLK is selected by CLKDEV_LOOKUP
> drivers/clk/Kconfig:3: symbol CLKDEV_LOOKUP is selected by COMMON_CLK
> drivers/clk/Kconfig:10: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
> drivers/mfd/Kconfig:600: symbol MFD_INTEL_LPSS is selected
> by MFD_INTEL_LPSS_PCI
> drivers/mfd/Kconfig:614: symbol MFD_INTEL_LPSS_PCI is
> implied by KEYBOARD_APPLESPI
> drivers/input/keyboard/Kconfig:74: symbol KEYBOARD_APPLESPI
> depends on EFI
> arch/arm/Kconfig:2031: symbol EFI depends on XIP_KERNEL
>
> Your patch correctly solves the spi_pxa2xx issue, but I'd prefer to instead
> drop the three 'imply' statements altogether, they seem to do more harm
> than good.
>
> (the circular dependency I saw might only happen when applying the
> arm32 KASAN patches, but I expect to see them merged for linux-5.4)

Isn't there more generally a problem here that this is selecting
MFD_INTEL_LPSS_PCI even though that depends on X86? I.e. are both ARM
and X86 selected at the same time? (sorry if I'm being na?ve, but I
assumed only one arch can be selected at a time)


Cheers,

Ronald

2019-07-18 12:35:34

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] Input: applespi: Fix build error without CONFIG_PCI

On Thu, Jul 18, 2019 at 1:58 PM Life is hard, and then you die
<[email protected]> wrote:
> On Thu, Jul 18, 2019 at 09:15:59AM +0200, Arnd Bergmann wrote:
> > On Thu, Jul 18, 2019 at 4:07 AM YueHaibing <[email protected]> wrote:
> > Your patch correctly solves the spi_pxa2xx issue, but I'd prefer to instead
> > drop the three 'imply' statements altogether, they seem to do more harm
> > than good.
> >
> > (the circular dependency I saw might only happen when applying the
> > arm32 KASAN patches, but I expect to see them merged for linux-5.4)
>
> Isn't there more generally a problem here that this is selecting
> MFD_INTEL_LPSS_PCI even though that depends on X86? I.e. are both ARM
> and X86 selected at the same time? (sorry if I'm being naïve, but I
> assumed only one arch can be selected at a time)

You can't have ARM and X86 defined at the same time, but Kconfig does
not know that, it just sees X86 as an undefined symbol, and ARM as
as always-enabled symbol when building for ARM.

In theory, 'imply' should deal with that and have no effect when there
are missing dependencies, but it appears that this only works for
deciding whether to turn on MFD_INTEL_LPSS_PCI, not for figuring
out loops in the dependency chain.

Arnd

2019-07-19 04:31:36

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] Input: applespi: Fix build error without CONFIG_PCI

On 7/18/19 4:40 AM, Life is hard, and then you die wrote:
>
> On Thu, Jul 18, 2019 at 10:06:54AM +0800, YueHaibing wrote:
>> If CONFIG_KEYBOARD_APPLESPI is set to y, but
>> CONFIG_PCI is not set, building will fails:
>>
>> drivers/spi/spi-pxa2xx-pci.c: In function pxa2xx_spi_pci_probe:
>> drivers/spi/spi-pxa2xx-pci.c:208:8: error: implicit declaration of function pcim_enable_device;
>> did you mean pci_enable_device? [-Werror=implicit-function-declaration]
>> ret = pcim_enable_device(dev);
>> ^~~~~~~~~~~~~~~~~~
>> pci_enable_device
>> drivers/spi/spi-pxa2xx-pci.c:239:8: error: implicit declaration of function pci_alloc_irq_vectors;
>> did you mean pci_alloc_consistent? [-Werror=implicit-function-declaration]
>> ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES);
>> ^~~~~~~~~~~~~~~~~~~~~
>>
>> Make CONFIG_KEYBOARD_APPLESPI depends on CONFIG_PCI
>> to fix this.
>>
>> Reported-by: Hulk Robot <[email protected]>
>> Fixes: b426ac045209 ("Input: add Apple SPI keyboard and trackpad driver")
>> Signed-off-by: YueHaibing <[email protected]>
>> ---
>> drivers/input/keyboard/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>> index dd934c4..fefcc46 100644
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -74,7 +74,7 @@ config ATARI_KBD_CORE
>> config KEYBOARD_APPLESPI
>> tristate "Apple SPI keyboard and trackpad"
>> depends on ACPI && EFI
>> - depends on SPI
>> + depends on SPI && PCI
>> depends on X86 || COMPILE_TEST
>> imply SPI_PXA2XX
>> imply SPI_PXA2XX_PCI
>> --
>> 2.7.4
>
> I think this is more properly fixed by Dmitry's suggestion of making
> SPI_PXA2XX_PCI depend on PCI, since it's that module, not applespi,
> that actually needs PCI - see
> https://www.spinics.net/lists/linux-input/msg62351.html

Dmitry's patch works for my failing test case.

Acked-by: Randy Dunlap <[email protected]> # build-tested

Thanks.

--
~Randy