[PATCH] powerpc/44x: Force PCI on for CURRITUCK

[PATCH] powerpc/44x: Force PCI on for CURRITUCK

[Michael Ellerman]

The recent rework of PCI kconfig symbols exposed an existing bug in
the CURRITUCK kconfig logic.

It selects PPC4xx_PCI_EXPRESS which depends on PCI, but PCI is user
selectable and might be disabled, leading to a warning:

  WARNING: unmet direct dependencies detected for PPC4xx_PCI_EXPRESS
    Depends on [n]: PCI [=n] && 4xx [=y]
    Selected by [y]:
    - CURRITUCK [=y] && PPC_47x [=y]

Prior to commit eb01d42a7778 ("PCI: consolidate PCI config entry in
drivers/pci") PCI was enabled by default for currituck_defconfig so we
didn't see the warning. The bad logic was still there, it just
required someone disabling PCI in their .config to hit it.

Fix it by forcing PCI on for CURRITUCK, which seems was always the
expectation anyway.

Fixes: eb01d42a7778 ("PCI: consolidate PCI config entry in drivers/pci")
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/platforms/44x/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index 4a9a72d01c3c..35be81fd2dc2 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -180,6 +180,7 @@ config CURRITUCK
 	depends on PPC_47x
 	select SWIOTLB
 	select 476FPE
+	select FORCE_PCI
 	select PPC4xx_PCI_EXPRESS
 	help
 	  This option enables support for the IBM Currituck (476fpe) evaluation board
-- 
2.20.1

Re: [PATCH] powerpc/44x: Force PCI on for CURRITUCK

[Christoph Hellwig]

Thanks, this looks good to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] powerpc/44x: Force PCI on for CURRITUCK

[Geert Uytterhoeven]

Hi Michael,

On Thu, Feb 7, 2019 at 3:49 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
> The recent rework of PCI kconfig symbols exposed an existing bug in
> the CURRITUCK kconfig logic.
>
> It selects PPC4xx_PCI_EXPRESS which depends on PCI, but PCI is user
> selectable and might be disabled, leading to a warning:
>
>   WARNING: unmet direct dependencies detected for PPC4xx_PCI_EXPRESS
>     Depends on [n]: PCI [=n] && 4xx [=y]
>     Selected by [y]:
>     - CURRITUCK [=y] && PPC_47x [=y]
>
> Prior to commit eb01d42a7778 ("PCI: consolidate PCI config entry in
> drivers/pci") PCI was enabled by default for currituck_defconfig so we
> didn't see the warning. The bad logic was still there, it just
> required someone disabling PCI in their .config to hit it.
>
> Fix it by forcing PCI on for CURRITUCK, which seems was always the
> expectation anyway.
>
> Fixes: eb01d42a7778 ("PCI: consolidate PCI config entry in drivers/pci")
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/platforms/44x/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
> index 4a9a72d01c3c..35be81fd2dc2 100644
> --- a/arch/powerpc/platforms/44x/Kconfig
> +++ b/arch/powerpc/platforms/44x/Kconfig
> @@ -180,6 +180,7 @@ config CURRITUCK
>         depends on PPC_47x
>         select SWIOTLB
>         select 476FPE
> +       select FORCE_PCI
>         select PPC4xx_PCI_EXPRESS

Would "select PPC4xx_PCI_EXPRESS if PCI" be a suitable alternative?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] powerpc/44x: Force PCI on for CURRITUCK

[Michael Ellerman]

Geert Uytterhoeven <geert@linux-m68k.org> writes:
> Hi Michael,
>
> On Thu, Feb 7, 2019 at 3:49 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> The recent rework of PCI kconfig symbols exposed an existing bug in
>> the CURRITUCK kconfig logic.
>>
>> It selects PPC4xx_PCI_EXPRESS which depends on PCI, but PCI is user
>> selectable and might be disabled, leading to a warning:
>>
>>   WARNING: unmet direct dependencies detected for PPC4xx_PCI_EXPRESS
>>     Depends on [n]: PCI [=n] && 4xx [=y]
>>     Selected by [y]:
>>     - CURRITUCK [=y] && PPC_47x [=y]
>>
>> Prior to commit eb01d42a7778 ("PCI: consolidate PCI config entry in
>> drivers/pci") PCI was enabled by default for currituck_defconfig so we
>> didn't see the warning. The bad logic was still there, it just
>> required someone disabling PCI in their .config to hit it.
>>
>> Fix it by forcing PCI on for CURRITUCK, which seems was always the
>> expectation anyway.
>>
>> Fixes: eb01d42a7778 ("PCI: consolidate PCI config entry in drivers/pci")
>> Reported-by: Randy Dunlap <rdunlap@infradead.org>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>>  arch/powerpc/platforms/44x/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
>> index 4a9a72d01c3c..35be81fd2dc2 100644
>> --- a/arch/powerpc/platforms/44x/Kconfig
>> +++ b/arch/powerpc/platforms/44x/Kconfig
>> @@ -180,6 +180,7 @@ config CURRITUCK
>>         depends on PPC_47x
>>         select SWIOTLB
>>         select 476FPE
>> +       select FORCE_PCI
>>         select PPC4xx_PCI_EXPRESS
>
> Would "select PPC4xx_PCI_EXPRESS if PCI" be a suitable alternative?

It would work, but I don't really like it because it means the
dependency on PCI is now encoded in two places.

I also doubt it reflects the intention of the original authors, because
at the time PCI was default y I suspect they never intended for PCI to
be disabled for that board.

cheers

Re: powerpc/44x: Force PCI on for CURRITUCK

[Michael Ellerman]

On Thu, 2019-02-07 at 02:49:35 UTC, Michael Ellerman wrote:
> The recent rework of PCI kconfig symbols exposed an existing bug in
> the CURRITUCK kconfig logic.
> 
> It selects PPC4xx_PCI_EXPRESS which depends on PCI, but PCI is user
> selectable and might be disabled, leading to a warning:
> 
>   WARNING: unmet direct dependencies detected for PPC4xx_PCI_EXPRESS
>     Depends on [n]: PCI [=n] && 4xx [=y]
>     Selected by [y]:
>     - CURRITUCK [=y] && PPC_47x [=y]
> 
> Prior to commit eb01d42a7778 ("PCI: consolidate PCI config entry in
> drivers/pci") PCI was enabled by default for currituck_defconfig so we
> didn't see the warning. The bad logic was still there, it just
> required someone disabling PCI in their .config to hit it.
> 
> Fix it by forcing PCI on for CURRITUCK, which seems was always the
> expectation anyway.
> 
> Fixes: eb01d42a7778 ("PCI: consolidate PCI config entry in drivers/pci")
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Applied to powerpc next.

https://git.kernel.org/powerpc/c/aa7150ba378650d0e9d84b8e4d805946

cheers