2018-05-10 11:17:23

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 4.17 1/2] Revert "ssb: Prevent build of PCI host features in module"

From: Rafał Miłecki <[email protected]>

This reverts commit 882164a4a928bcaa53280940436ca476e6b1db8e.

Above commit added "SSB = y" dependency to the wrong symbol
SSB_DRIVER_PCICORE_POSSIBLE and prevented SSB_DRIVER_PCICORE from being
selected when needed. PCI core driver for core running in clienthost
mode is important for bus initialization. It's perfectly valid scenario
to have ssb built as module and use it with buses on PCI cards.

This fixes regression that affected all *module* users with PCI cards.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1572349
Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/ssb/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index 9371651d8017..b3f5cae98ea6 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -117,7 +117,7 @@ config SSB_SERIAL

config SSB_DRIVER_PCICORE_POSSIBLE
bool
- depends on SSB_PCIHOST && SSB = y
+ depends on SSB_PCIHOST
default y

config SSB_DRIVER_PCICORE
--
2.13.6



2018-05-10 11:16:35

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 4.17 2/2] ssb: make SSB_PCICORE_HOSTMODE depend on SSB = y

From: Rafał Miłecki <[email protected]>

SSB_PCICORE_HOSTMODE protects MIPS specific code that calls not exported
symbols pcibios_enable_device and register_pci_controller. This code is
supposed to be compiled only with ssb builtin.

This fixes:
ERROR: "pcibios_enable_device" [drivers/ssb/ssb.ko] undefined!
ERROR: "register_pci_controller" [drivers/ssb/ssb.ko] undefined!
make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/ssb/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index b3f5cae98ea6..c574dd210500 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -131,7 +131,7 @@ config SSB_DRIVER_PCICORE

config SSB_PCICORE_HOSTMODE
bool "Hostmode support for SSB PCI core"
- depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS
+ depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && SSB = y
help
PCIcore hostmode operation (external PCI bus).

--
2.13.6


2018-05-10 11:19:02

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 4.17 2/2] ssb: make SSB_PCICORE_HOSTMODE depend on SSB = y

On Thu, 10 May 2018 13:14:01 +0200
Rafał Miłecki <[email protected]> wrote:

> From: Rafał Miłecki <[email protected]>
>
> SSB_PCICORE_HOSTMODE protects MIPS specific code that calls not exported
> symbols pcibios_enable_device and register_pci_controller. This code is
> supposed to be compiled only with ssb builtin.
>
> This fixes:
> ERROR: "pcibios_enable_device" [drivers/ssb/ssb.ko] undefined!
> ERROR: "register_pci_controller" [drivers/ssb/ssb.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
>
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> drivers/ssb/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> index b3f5cae98ea6..c574dd210500 100644
> --- a/drivers/ssb/Kconfig
> +++ b/drivers/ssb/Kconfig
> @@ -131,7 +131,7 @@ config SSB_DRIVER_PCICORE
>
> config SSB_PCICORE_HOSTMODE
> bool "Hostmode support for SSB PCI core"
> - depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS
> + depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && SSB = y
> help
> PCIcore hostmode operation (external PCI bus).
>


I think we also need to depend on PCI_DRIVERS_LEGACY.
See the other patch that floats around.


--
Michael


Attachments:
(No filename) (849.00 B)
OpenPGP digital signature

2018-05-10 11:21:39

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 4.17 2/2] ssb: make SSB_PCICORE_HOSTMODE depend on SSB = y

On 10 May 2018 at 13:17, Michael Büsch <[email protected]> wrote:
> On Thu, 10 May 2018 13:14:01 +0200
> Rafał Miłecki <[email protected]> wrote:
>
>> From: Rafał Miłecki <[email protected]>
>>
>> SSB_PCICORE_HOSTMODE protects MIPS specific code that calls not exported
>> symbols pcibios_enable_device and register_pci_controller. This code is
>> supposed to be compiled only with ssb builtin.
>>
>> This fixes:
>> ERROR: "pcibios_enable_device" [drivers/ssb/ssb.ko] undefined!
>> ERROR: "register_pci_controller" [drivers/ssb/ssb.ko] undefined!
>> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
>>
>> Signed-off-by: Rafał Miłecki <[email protected]>
>> ---
>> drivers/ssb/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
>> index b3f5cae98ea6..c574dd210500 100644
>> --- a/drivers/ssb/Kconfig
>> +++ b/drivers/ssb/Kconfig
>> @@ -131,7 +131,7 @@ config SSB_DRIVER_PCICORE
>>
>> config SSB_PCICORE_HOSTMODE
>> bool "Hostmode support for SSB PCI core"
>> - depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS
>> + depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && SSB = y
>> help
>> PCIcore hostmode operation (external PCI bus).
>>
>
>
> I think we also need to depend on PCI_DRIVERS_LEGACY.
> See the other patch that floats around.

I believe it's already handled by SSB_PCIHOST_POSSIBLE's dependency on
PCI_DRIVERS_LEGACY.

--
Rafał

2018-05-10 11:29:16

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 4.17 2/2] ssb: make SSB_PCICORE_HOSTMODE depend on SSB = y

On Thu, 10 May 2018 13:20:01 +0200
Rafał Miłecki <[email protected]> wrote:

> On 10 May 2018 at 13:17, Michael Büsch <[email protected]> wrote:
> > On Thu, 10 May 2018 13:14:01 +0200
> > Rafał Miłecki <[email protected]> wrote:
> >
> >> From: Rafał Miłecki <[email protected]>
> >>
> >> SSB_PCICORE_HOSTMODE protects MIPS specific code that calls not exported
> >> symbols pcibios_enable_device and register_pci_controller. This code is
> >> supposed to be compiled only with ssb builtin.
> >>
> >> This fixes:
> >> ERROR: "pcibios_enable_device" [drivers/ssb/ssb.ko] undefined!
> >> ERROR: "register_pci_controller" [drivers/ssb/ssb.ko] undefined!
> >> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
> >>
> >> Signed-off-by: Rafał Miłecki <[email protected]>
> >> ---
> >> drivers/ssb/Kconfig | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> >> index b3f5cae98ea6..c574dd210500 100644
> >> --- a/drivers/ssb/Kconfig
> >> +++ b/drivers/ssb/Kconfig
> >> @@ -131,7 +131,7 @@ config SSB_DRIVER_PCICORE
> >>
> >> config SSB_PCICORE_HOSTMODE
> >> bool "Hostmode support for SSB PCI core"
> >> - depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS
> >> + depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && SSB = y
> >> help
> >> PCIcore hostmode operation (external PCI bus).
> >>
> >
> >
> > I think we also need to depend on PCI_DRIVERS_LEGACY.
> > See the other patch that floats around.
>
> I believe it's already handled by SSB_PCIHOST_POSSIBLE's dependency on
> PCI_DRIVERS_LEGACY.


That dependency seems to be wrong there.
Was it added among some other "let's just unbreak some random
build" change as well?

SSB_PCIHOST enables support for SSB on top of PCI. (Which is 99% of it
uses). I don't see how this uses the legacy API.

SSB_PCICORE_HOSTMODE enables PCI on top of SSB. Which is a MIPS corner
case. This uses the legacy MIPS API to register a PCI bus.

--
Michael


Attachments:
(No filename) (849.00 B)
OpenPGP digital signature

2018-05-10 11:35:59

by Matt Redfearn

[permalink] [raw]
Subject: Re: [PATCH 4.17 2/2] ssb: make SSB_PCICORE_HOSTMODE depend on SSB = y

Hi,

On 10/05/18 12:26, Michael Büsch wrote:
> On Thu, 10 May 2018 13:20:01 +0200
> Rafał Miłecki <[email protected]> wrote:
>
>> On 10 May 2018 at 13:17, Michael Büsch <[email protected]> wrote:
>>> On Thu, 10 May 2018 13:14:01 +0200
>>> Rafał Miłecki <[email protected]> wrote:
>>>
>>>> From: Rafał Miłecki <[email protected]>
>>>>
>>>> SSB_PCICORE_HOSTMODE protects MIPS specific code that calls not exported
>>>> symbols pcibios_enable_device and register_pci_controller. This code is
>>>> supposed to be compiled only with ssb builtin.
>>>>
>>>> This fixes:
>>>> ERROR: "pcibios_enable_device" [drivers/ssb/ssb.ko] undefined!
>>>> ERROR: "register_pci_controller" [drivers/ssb/ssb.ko] undefined!
>>>> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
>>>>
>>>> Signed-off-by: Rafał Miłecki <[email protected]>
>>>> ---
>>>> drivers/ssb/Kconfig | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
>>>> index b3f5cae98ea6..c574dd210500 100644
>>>> --- a/drivers/ssb/Kconfig
>>>> +++ b/drivers/ssb/Kconfig
>>>> @@ -131,7 +131,7 @@ config SSB_DRIVER_PCICORE
>>>>
>>>> config SSB_PCICORE_HOSTMODE
>>>> bool "Hostmode support for SSB PCI core"
>>>> - depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS
>>>> + depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && SSB = y
>>>> help
>>>> PCIcore hostmode operation (external PCI bus).
>>>>
>>>
>>>
>>> I think we also need to depend on PCI_DRIVERS_LEGACY.
>>> See the other patch that floats around.
>>
>> I believe it's already handled by SSB_PCIHOST_POSSIBLE's dependency on
>> PCI_DRIVERS_LEGACY.
>
>
> That dependency seems to be wrong there.
> Was it added among some other "let's just unbreak some random
> build" change as well?

Yeah - that was commit 58eae1416b80 ("ssb: Disable PCI host for
PCI_DRIVERS_GENERIC").

>
> SSB_PCIHOST enables support for SSB on top of PCI. (Which is 99% of it
> uses). I don't see how this uses the legacy API.
>
> SSB_PCICORE_HOSTMODE enables PCI on top of SSB. Which is a MIPS corner
> case. This uses the legacy MIPS API to register a PCI bus.
>

Yeah the dependency would seem to be in the wrong place and should be on
SSB_PCICORE_HOSTMODE, in the same way as the bcma driver - commits
664eadd6f44b ("bcma: Fix 'allmodconfig' and BCMA builds on MIPS
targets") & 79ca239a68f8 ("bcma: Prevent build of PCI host features in
module").

Thanks,
Matt

2018-05-10 11:36:46

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 4.17 2/2] ssb: make SSB_PCICORE_HOSTMODE depend on SSB = y

On 10 May 2018 at 13:26, Michael Büsch <[email protected]> wrote:
> On Thu, 10 May 2018 13:20:01 +0200
> Rafał Miłecki <[email protected]> wrote:
>
>> On 10 May 2018 at 13:17, Michael Büsch <[email protected]> wrote:
>> > On Thu, 10 May 2018 13:14:01 +0200
>> > Rafał Miłecki <[email protected]> wrote:
>> >
>> >> From: Rafał Miłecki <[email protected]>
>> >>
>> >> SSB_PCICORE_HOSTMODE protects MIPS specific code that calls not exported
>> >> symbols pcibios_enable_device and register_pci_controller. This code is
>> >> supposed to be compiled only with ssb builtin.
>> >>
>> >> This fixes:
>> >> ERROR: "pcibios_enable_device" [drivers/ssb/ssb.ko] undefined!
>> >> ERROR: "register_pci_controller" [drivers/ssb/ssb.ko] undefined!
>> >> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
>> >>
>> >> Signed-off-by: Rafał Miłecki <[email protected]>
>> >> ---
>> >> drivers/ssb/Kconfig | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
>> >> index b3f5cae98ea6..c574dd210500 100644
>> >> --- a/drivers/ssb/Kconfig
>> >> +++ b/drivers/ssb/Kconfig
>> >> @@ -131,7 +131,7 @@ config SSB_DRIVER_PCICORE
>> >>
>> >> config SSB_PCICORE_HOSTMODE
>> >> bool "Hostmode support for SSB PCI core"
>> >> - depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS
>> >> + depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && SSB = y
>> >> help
>> >> PCIcore hostmode operation (external PCI bus).
>> >>
>> >
>> >
>> > I think we also need to depend on PCI_DRIVERS_LEGACY.
>> > See the other patch that floats around.
>>
>> I believe it's already handled by SSB_PCIHOST_POSSIBLE's dependency on
>> PCI_DRIVERS_LEGACY.
>
>
> That dependency seems to be wrong there.
> Was it added among some other "let's just unbreak some random
> build" change as well?

I guess so.


> SSB_PCIHOST enables support for SSB on top of PCI. (Which is 99% of it
> uses). I don't see how this uses the legacy API.
>
> SSB_PCICORE_HOSTMODE enables PCI on top of SSB. Which is a MIPS corner
> case. This uses the legacy MIPS API to register a PCI bus.

I agree current dependency on PCI_DRIVERS_LEGACY doesn't make sense.
It doesn't break thing though, thanks to the || !MIPS. I'm going to
fix that up in separated patch.

--
Rafał

2018-05-10 16:07:40

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 4.17 2/2] ssb: make SSB_PCICORE_HOSTMODE depend on SSB = y

On 05/10/2018 06:14 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> SSB_PCICORE_HOSTMODE protects MIPS specific code that calls not exported
> symbols pcibios_enable_device and register_pci_controller. This code is
> supposed to be compiled only with ssb builtin.
>
> This fixes:
> ERROR: "pcibios_enable_device" [drivers/ssb/ssb.ko] undefined!
> ERROR: "register_pci_controller" [drivers/ssb/ssb.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
>
> Signed-off-by: Rafał Miłecki <[email protected]>

This patch needs a "Reported-by: Matt Redfearn <[email protected]>".

Applying both patches leads to a correct configuration for PCI. I cannot test on
my present hardware, but the patches seem to be correct.

Reviewed-by: Larry Finger <[email protected]>

@Kalle: Please drop my patch from yesterday. This solution is much better.

Larry

> ---
> drivers/ssb/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> index b3f5cae98ea6..c574dd210500 100644
> --- a/drivers/ssb/Kconfig
> +++ b/drivers/ssb/Kconfig
> @@ -131,7 +131,7 @@ config SSB_DRIVER_PCICORE
>
> config SSB_PCICORE_HOSTMODE
> bool "Hostmode support for SSB PCI core"
> - depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS
> + depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && SSB = y
> help
> PCIcore hostmode operation (external PCI bus).
>
>


2018-05-11 10:19:02

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 4.17 2/2] ssb: make SSB_PCICORE_HOSTMODE depend on SSB = y

Larry Finger <[email protected]> writes:

> On 05/10/2018 06:14 AM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <[email protected]>
>>
>> SSB_PCICORE_HOSTMODE protects MIPS specific code that calls not exported
>> symbols pcibios_enable_device and register_pci_controller. This code is
>> supposed to be compiled only with ssb builtin.
>>
>> This fixes:
>> ERROR: "pcibios_enable_device" [drivers/ssb/ssb.ko] undefined!
>> ERROR: "register_pci_controller" [drivers/ssb/ssb.ko] undefined!
>> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
>>
>> Signed-off-by: Rafał Miłecki <[email protected]>
>
> This patch needs a "Reported-by: Matt Redfearn <[email protected]>".
>
> Applying both patches leads to a correct configuration for PCI. I
> cannot test on my present hardware, but the patches seem to be
> correct.
>
> Reviewed-by: Larry Finger <[email protected]>
>
> @Kalle: Please drop my patch from yesterday. This solution is much better.

Dropped, thanks for letting me know.

--
Kalle Valo