LinuxPPC-Dev Archive on lore.kernel.org
 help / Atom feed
* [PATCH] powerpc/44x: Force PCI on for CURRITUCK
@ 2019-02-07  2:49 Michael Ellerman
  2019-02-07  5:37 ` Christoph Hellwig
  2019-02-07  8:18 ` Geert Uytterhoeven
  0 siblings, 2 replies; 4+ messages in thread
From: Michael Ellerman @ 2019-02-07  2:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: bhelgaas, rdunlap, geert, hch

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/44x: Force PCI on for CURRITUCK
  2019-02-07  2:49 [PATCH] powerpc/44x: Force PCI on for CURRITUCK Michael Ellerman
@ 2019-02-07  5:37 ` Christoph Hellwig
  2019-02-07  8:18 ` Geert Uytterhoeven
  1 sibling, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2019-02-07  5:37 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: bhelgaas, linuxppc-dev, rdunlap, geert, hch

Thanks, this looks good to me:

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/44x: Force PCI on for CURRITUCK
  2019-02-07  2:49 [PATCH] powerpc/44x: Force PCI on for CURRITUCK Michael Ellerman
  2019-02-07  5:37 ` Christoph Hellwig
@ 2019-02-07  8:18 ` Geert Uytterhoeven
  2019-02-08 11:46   ` Michael Ellerman
  1 sibling, 1 reply; 4+ messages in thread
From: Geert Uytterhoeven @ 2019-02-07  8:18 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Bjorn Helgaas, Linuxppc-dev, Randy Dunlap, Christoph Hellwig

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/44x: Force PCI on for CURRITUCK
  2019-02-07  8:18 ` Geert Uytterhoeven
@ 2019-02-08 11:46   ` Michael Ellerman
  0 siblings, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2019-02-08 11:46 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Bjorn Helgaas, Linuxppc-dev, Randy Dunlap, Christoph Hellwig

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07  2:49 [PATCH] powerpc/44x: Force PCI on for CURRITUCK Michael Ellerman
2019-02-07  5:37 ` Christoph Hellwig
2019-02-07  8:18 ` Geert Uytterhoeven
2019-02-08 11:46   ` Michael Ellerman

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org linuxppc-dev@archiver.kernel.org
	public-inbox-index linuxppc-dev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox