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

Commit Message

Rafał Miłecki May 11, 2018, 9:17 a.m. UTC

From: Rafał Miłecki <[e-mail]>

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 <[e-mail]>
---
 drivers/ssb/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rafał Miłecki May 11, 2018, 9:21 a.m. UTC | #1

On 11 May 2018 at 11:17, Rafał Miłecki <[e-mail]> wrote:
> From: Rafał Miłecki <[e-mail]>
>
> 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 <[e-mail]>

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

Kalle Valo May 11, 2018, 10:13 a.m. UTC | #2

Rafał Miłecki <[e-mail]> writes:

> On 11 May 2018 at 11:17, Rafał Miłecki <[e-mail]> wrote:
>> From: Rafał Miłecki <[e-mail]>
>>
>> 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 <[e-mail]>
>
> 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.

Rafał Miłecki May 11, 2018, 10:25 a.m. UTC | #3

On 11 May 2018 at 12:13, Kalle Valo <[e-mail]> wrote:
> Rafał Miłecki <[e-mail]> writes:
>
>> On 11 May 2018 at 11:17, Rafał Miłecki <[e-mail]> wrote:
>>> From: Rafał Miłecki <[e-mail]>
>>>
>>> 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 <[e-mail]>
>>
>> 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 = 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 = m

Larry Finger May 11, 2018, 12:08 p.m. UTC | #4

On 05/11/2018 05:13 AM, Kalle Valo wrote:
> Rafał Miłecki <[e-mail]> writes:
> 
>> On 11 May 2018 at 11:17, Rafał Miłecki <[e-mail]> wrote:
>>> From: Rafał Miłecki <[e-mail]>
>>>
>>> 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 <[e-mail]>
>>
>> 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

Kalle Valo May 12, 2018, 7:50 a.m. UTC | #5

Larry Finger <[e-mail]> writes:

> On 05/11/2018 05:13 AM, Kalle Valo wrote:
>> Rafał Miłecki <[e-mail]> writes:
>>
>>> On 11 May 2018 at 11:17, Rafał Miłecki <[e-mail]> wrote:
>>>> From: Rafał Miłecki <[e-mail]>
>>>>
>>>> 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 <[e-mail]>
>>>
>>> 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.

Michael Büsch May 12, 2018, 8:01 a.m. UTC | #6

On Sat, 12 May 2018 10:50:42 +0300
Kalle Valo <[e-mail]> wrote:

> Larry Finger <[e-mail]> writes:
> 
> > On 05/11/2018 05:13 AM, Kalle Valo wrote:  
> >> Rafał Miłecki <[e-mail]> writes:
> >>  
> >>> On 11 May 2018 at 11:17, Rafał Miłecki <[e-mail]> 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.

Kalle Valo May 12, 2018, 8:40 a.m. UTC | #7

Rafał Miłecki wrote:

> From: Rafał Miłecki <[e-mail]>
> 
> 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 <[e-mail]>

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

Rafał Miłecki May 12, 2018, 10 a.m. UTC | #8

On 2018-05-12 10:01, Michael Büsch wrote:
> On Sat, 12 May 2018 10:50:42 +0300
> Kalle Valo <[e-mail]> wrote:
> 
>> Larry Finger <[e-mail]> writes:
>> 
>> > On 05/11/2018 05:13 AM, Kalle Valo wrote:
>> >> Rafał Miłecki <[e-mail]> writes:
>> >>
>> >>> On 11 May 2018 at 11:17, Rafał Miłecki <[e-mail]> 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.

Michael Büsch May 12, 2018, 10:14 a.m. UTC | #9

On Sat, 12 May 2018 12:00:07 +0200
Rafał Miłecki <[e-mail]> 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.

Patch

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