2018-05-11 09:19:11

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH V2 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-11 10:13:39

by Kalle Valo

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

Rafa=C5=82 Mi=C5=82ecki <[email protected]> writes:

> On 11 May 2018 at 11:17, Rafa=C5=82 Mi=C5=82ecki <[email protected]> wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>
>> This reverts commit 882164a4a928bcaa53280940436ca476e6b1db8e.
>>
>> Above commit added "SSB =3D 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=3D1572349
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>
> As these patches fix regression/build error, I believe both should get
> into 4.17.

How much confidence do we have that we don't need to end up reverting
patch 2 as well? I rather be pushing patch 2 to 4.18 so that there's
more time for testing and waiting for feedback.

--=20
Kalle Valo

2018-05-12 07:50:49

by Kalle Valo

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

Larry Finger <[email protected]> writes:

> On 05/11/2018 05:13 AM, Kalle Valo wrote:
>> Rafa=C5=82 Mi=C5=82ecki <[email protected]> writes:
>>
>>> On 11 May 2018 at 11:17, Rafa=C5=82 Mi=C5=82ecki <[email protected]> wro=
te:
>>>> From: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>>>
>>>> This reverts commit 882164a4a928bcaa53280940436ca476e6b1db8e.
>>>>
>>>> Above commit added "SSB =3D 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=3D1572349
>>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>>
>>> As these patches fix regression/build error, I believe both should get
>>> into 4.17.
>>
>> How much confidence do we have that we don't need to end up reverting
>> patch 2 as well? I rather be pushing patch 2 to 4.18 so that there's
>> more time for testing and waiting for feedback.
>
> Although I do not have the hardware to test the builds, I worked
> closely with the OP in the bug at b.r.c noted above. From that effort,
> it became clear what configuration variables were missing to cause the
> x86 failures. Patch 2 satisfies the requirement, and prevents the
> build problems found by the MIPS users. Both patches are needed in
> 4.17.

And I assume Michael is ok with this approach as well as I haven't heard
from him. I'll then push both of these to 4.17.

--=20
Kalle Valo

2018-05-11 10:25:36

by Rafał Miłecki

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

On 11 May 2018 at 12:13, Kalle Valo <[email protected]> wrote:
> Rafa=C5=82 Mi=C5=82ecki <[email protected]> writes:
>
>> On 11 May 2018 at 11:17, Rafa=C5=82 Mi=C5=82ecki <[email protected]> wrot=
e:
>>> From: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>>
>>> This reverts commit 882164a4a928bcaa53280940436ca476e6b1db8e.
>>>
>>> Above commit added "SSB =3D 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=3D1572349
>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>
>> As these patches fix regression/build error, I believe both should get
>> into 4.17.
>
> How much confidence do we have that we don't need to end up reverting
> patch 2 as well? I rather be pushing patch 2 to 4.18 so that there's
> more time for testing and waiting for feedback.

Solution from 2/2 seems pretty obvious.

1) Enabling SSB_PCICORE_HOSTMODE compiles code that requires
non-exported symbols. Requiring "SSB =3D y" seems pretty obvious.
2) As pointed in another e-mail bcma has pretty identical solution
that seems to be working well, see commit 79ca239a68f8f ("bcma:
Prevent build of PCI host features in module").

That's just my opinion though, I shared since you asked.

If you prefer to queue that for 4.18, I'm OK. After all:
1) This problem affects MIPS arch only
2) It can be fixed by not selecting BCMA_DRIVER_PCI_HOSTMODE for SSB =3D m

--=20
Rafa=C5=82

2018-05-11 09:19:13

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH V2 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

Reported-by: Matt Redfearn <[email protected]>
Signed-off-by: Rafał Miłecki <[email protected]>
Reviewed-by: Larry Finger <[email protected]>
---
V2: Add Reported-by and Reviewed-by
---
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-11 12:08:51

by Larry Finger

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

On 05/11/2018 05:13 AM, Kalle Valo wrote:
> Rafał Miłecki <[email protected]> writes:
>
>> On 11 May 2018 at 11:17, Rafał Miłecki <[email protected]> wrote:
>>> 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]>
>>
>> As these patches fix regression/build error, I believe both should get
>> into 4.17.
>
> How much confidence do we have that we don't need to end up reverting
> patch 2 as well? I rather be pushing patch 2 to 4.18 so that there's
> more time for testing and waiting for feedback.

Although I do not have the hardware to test the builds, I worked closely with
the OP in the bug at b.r.c noted above. From that effort, it became clear what
configuration variables were missing to cause the x86 failures. Patch 2
satisfies the requirement, and prevents the build problems found by the MIPS
users. Both patches are needed in 4.17.

Larry

2018-05-12 08:01:57

by Michael Büsch

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

On Sat, 12 May 2018 10:50:42 +0300
Kalle Valo <[email protected]> wrote:

> Larry Finger <[email protected]> writes:
>
> > On 05/11/2018 05:13 AM, Kalle Valo wrote:
> >> Rafał Miłecki <[email protected]> writes:
> >>
> >>> On 11 May 2018 at 11:17, Rafał Miłecki <[email protected]> wrote:
> [...]
> >>>
> >>> As these patches fix regression/build error, I believe both should get
> >>> into 4.17.
> >>
> >> How much confidence do we have that we don't need to end up reverting
> >> patch 2 as well? I rather be pushing patch 2 to 4.18 so that there's
> >> more time for testing and waiting for feedback.
> >
> > Although I do not have the hardware to test the builds, I worked
> > closely with the OP in the bug at b.r.c noted above. From that effort,
> > it became clear what configuration variables were missing to cause the
> > x86 failures. Patch 2 satisfies the requirement, and prevents the
> > build problems found by the MIPS users. Both patches are needed in
> > 4.17.
>
> And I assume Michael is ok with this approach as well as I haven't heard
> from him. I'll then push both of these to 4.17.
>

Yes, I'm OK with the patch, if we have a third patch that cleans up the
PCI_DRIVERS_LEGACY dependency by moving it to SSB_PCICORE_HOSTMODE
where it belongs. (This doesn't need to go into the stable tree.)
We currently implicitly get that via dependency chain, so this is OK
for now as-is.

--
Michael


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

2018-05-12 08:40:12

by Kalle Valo

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

Rafał Miłecki wrote:

> 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]>

2 patches applied to wireless-drivers.git, thanks.

36910d82a80c Revert "ssb: Prevent build of PCI host features in module"
ebd27d3317c6 ssb: make SSB_PCICORE_HOSTMODE depend on SSB = y

--
https://patchwork.kernel.org/patch/10393729/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2018-05-12 10:15:42

by Michael Büsch

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

On Sat, 12 May 2018 12:00:07 +0200
Rafał Miłecki <[email protected]> wrote:

> > Yes, I'm OK with the patch, if we have a third patch that cleans up the
> > PCI_DRIVERS_LEGACY dependency by moving it to SSB_PCICORE_HOSTMODE
> > where it belongs. (This doesn't need to go into the stable tree.)
> > We currently implicitly get that via dependency chain, so this is OK
> > for now as-is.
>
> I'm planning to handle PCI_DRIVERS_LEGACY cleanup once my patches hit
> net-next.git and then wireless-drivers-next.git. It's to avoid
> conflicts.

Yes, thanks. Take your time. We're not in a hurry. :)
This change should not make a functional difference.

--
Michael


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

2018-05-12 13:36:01

by Rafał Miłecki

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

On 2018-05-12 10:01, Michael Büsch wrote:
> On Sat, 12 May 2018 10:50:42 +0300
> Kalle Valo <[email protected]> wrote:
>
>> Larry Finger <[email protected]> writes:
>>
>> > On 05/11/2018 05:13 AM, Kalle Valo wrote:
>> >> Rafał Miłecki <[email protected]> writes:
>> >>
>> >>> On 11 May 2018 at 11:17, Rafał Miłecki <[email protected]> wrote:
>> [...]
>> >>>
>> >>> As these patches fix regression/build error, I believe both should get
>> >>> into 4.17.
>> >>
>> >> How much confidence do we have that we don't need to end up reverting
>> >> patch 2 as well? I rather be pushing patch 2 to 4.18 so that there's
>> >> more time for testing and waiting for feedback.
>> >
>> > Although I do not have the hardware to test the builds, I worked
>> > closely with the OP in the bug at b.r.c noted above. From that effort,
>> > it became clear what configuration variables were missing to cause the
>> > x86 failures. Patch 2 satisfies the requirement, and prevents the
>> > build problems found by the MIPS users. Both patches are needed in
>> > 4.17.
>>
>> And I assume Michael is ok with this approach as well as I haven't
>> heard
>> from him. I'll then push both of these to 4.17.
>>
>
> Yes, I'm OK with the patch, if we have a third patch that cleans up the
> PCI_DRIVERS_LEGACY dependency by moving it to SSB_PCICORE_HOSTMODE
> where it belongs. (This doesn't need to go into the stable tree.)
> We currently implicitly get that via dependency chain, so this is OK
> for now as-is.

I'm planning to handle PCI_DRIVERS_LEGACY cleanup once my patches hit
net-next.git and then wireless-drivers-next.git. It's to avoid
conflicts.

2018-05-11 09:21:13

by Rafał Miłecki

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

On 11 May 2018 at 11:17, Rafa=C5=82 Mi=C5=82ecki <[email protected]> wrote:
> From: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>
> This reverts commit 882164a4a928bcaa53280940436ca476e6b1db8e.
>
> Above commit added "SSB =3D 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=3D1572349
> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <[email protected]>

As these patches fix regression/build error, I believe both should get
into 4.17.