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

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

[YueHaibing]

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 <hulkci@huawei.com>
Fixes: b426ac045209 ("Input: add Apple SPI keyboard and trackpad driver")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 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

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

[Arnd Bergmann]

On Thu, Jul 18, 2019 at 4:07 AM YueHaibing <yuehaibing@huawei.com> 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 <hulkci@huawei.com>
> Fixes: b426ac045209 ("Input: add Apple SPI keyboard and trackpad driver")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

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

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

[Life is hard, and then you die]

  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 <yuehaibing@huawei.com> 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 <hulkci@huawei.com>
> > Fixes: b426ac045209 ("Input: add Apple SPI keyboard and trackpad driver")
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> 
> 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

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

[Arnd Bergmann]

On Thu, Jul 18, 2019 at 1:58 PM Life is hard, and then you die
<ronald@innovation.ch> wrote:
> On Thu, Jul 18, 2019 at 09:15:59AM +0200, Arnd Bergmann wrote:
> > On Thu, Jul 18, 2019 at 4:07 AM YueHaibing <yuehaibing@huawei.com> 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

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

[Life is hard, and then you die]

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 <hulkci@huawei.com>
> Fixes: b426ac045209 ("Input: add Apple SPI keyboard and trackpad driver")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  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

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

[Arnd Bergmann]

On Thu, Jul 18, 2019 at 1:40 PM Life is hard, and then you die
<ronald@innovation.ch> 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

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

[Randy Dunlap]

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 <hulkci@huawei.com>
>> Fixes: b426ac045209 ("Input: add Apple SPI keyboard and trackpad driver")
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> ---
>>  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 <rdunlap@infradead.org> # build-tested

Thanks.

-- 
~Randy