Matt,
Although commit 882164a4a928 ("ssb: Prevent build of PCI host features in
module") appeared to be harmless, it leads to complete failure of drivers b43.
and b43legacy, and likely affects b44 as well. The problem is that
CONFIG_SSB_PCIHOST is undefined, which prevents the compilation of the code that
controls the PCI cores of the device. See
https://bugzilla.redhat.com/show_bug.cgi?id=1572349 for details.
As the underlying errors ("pcibios_enable_device" undefined, and
"register_pci_controller" undefined) do not appear on the architectures that I
have tested (x86_64, x86, and ppc), I suspect something in the arch-specific
code for your setup (MIPS?). As I have no idea on how to fix that problem, would
the following patch work for you?
diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index 9371651d8017..3743533c8057 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 && (SSB = y || !MIPS)
default y
config SSB_DRIVER_PCICORE
Thanks,
Larry
Michael B=C3=BCsch <[email protected]> writes:
> On Mon, 07 May 2018 22:03:58 +0300
> Kalle Valo <[email protected]> wrote:
>
>> Michael B=C3=BCsch <[email protected]> writes:
>>=20
>> > On Mon, 7 May 2018 10:44:34 -0500
>> > Larry Finger <[email protected]> wrote:
>> >=20=20
>> >> Although commit 882164a4a928 ("ssb: Prevent build of PCI host feature=
s in=20
>> >> module") appeared to be harmless, it leads to complete failure of dri=
vers b43.=20=20=20
>> >=20=20
>> >> config SSB_DRIVER_PCICORE_POSSIBLE
>> >> bool
>> >> - depends on SSB_PCIHOST && SSB =3D y
>> >> + depends on SSB_PCIHOST && (SSB =3D y || !MIPS)
>> >> default y
>> >>=20
>> >> config SSB_DRIVER_PCICORE=20=20
>> >
>> >
>> > https://patchwork.kernel.org/patch/10161131/
>> >
>> > Could we _please_ switch to not applying patches to ssb or b43, if
>> > nobody acked (or better reviewed) a patch?
>> >
>> > We had multiple changes to ssb and b43 in the recent past that did not
>> > have a review at all and broke something. I don't think such software
>> > quality is acceptable at all.
>> > So please revert 882164a4a928.=20=20
>>=20
>> Yes, someone please send a revert so that this can be fixed quickly for
>> v4.17.
>
> Uhm, can you just type git revert 882164a4a928? :)
But it needs a proper commit log explaining why it's reverted (links to
bugzilla report etc). And I prefer also reverts to be reviewed on the
list.
> Or do I have to send you a pull request?
A revert is a regular commit, so you can submit it using git
format-patch and git send-email.
>> > I'm sorry that this patch slipped through the cracks of my inbox.
>> > But the reaction to that shall not be to just apply the patch. It
>> > shall be to resubmit it for review.=20=20
>>=20
>> The thing is that in general I do not have time to ping people for every
>> patch, I get enough of emails as is. If there are no review comments I
>> have to assume the patch is ok to apply.
>
> Yes, I understand that pinging people can be annoying and time
> consuming. But we have tools like patchwork. Why isn't pinging
> (semi)automated? Patchwork should really track the review status of a
> patch.
That would be awesome but patchwork is nowhere near that kind of
sophistication. I like it but to be honest it's really simple at the
moment. My custom client script has a simple way to ping about patches
but even that is too much work on the long run. Some people do send Acks
to the driver they maintain but not always, I guess because too busy
with real life or something which is totally understandable. But it
would not scale at all if I would start pinging for the 25% of patches
that they have not acked.
> I think the concept of no-comments =3D everything-ok is
> fundamentally broken. But it has always been that way for wireless and
> lots of other subsystems.
It's a balance between bureaucracy and getting things done. From my POV
the only viable solution is that maintainers actively follow the patches
on the mailing list.
--=20
Kalle Valo
On 10 May 2018 at 12:41, Rafa=C5=82 Mi=C5=82ecki <[email protected]> wrote:
> On 7 May 2018 at 17:44, Larry Finger <[email protected]> wrote:
>> Although commit 882164a4a928 ("ssb: Prevent build of PCI host features i=
n
>> module") appeared to be harmless, it leads to complete failure of driver=
s
>> b43. and b43legacy, and likely affects b44 as well. The problem is that
>> CONFIG_SSB_PCIHOST is undefined, which prevents the compilation of the c=
ode
>> that controls the PCI cores of the device. See
>> https://bugzilla.redhat.com/show_bug.cgi?id=3D1572349 for details.
>>
>> As the underlying errors ("pcibios_enable_device" undefined, and
>> "register_pci_controller" undefined) do not appear on the architectures =
that
>> I have tested (x86_64, x86, and ppc), I suspect something in the
>> arch-specific code for your setup (MIPS?). As I have no idea on how to f=
ix
>> that problem, would the following patch work for you?
>>
>> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
>> index 9371651d8017..3743533c8057 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 =3D y
>> + depends on SSB_PCIHOST && (SSB =3D y || !MIPS)
>> default y
>>
>> config SSB_DRIVER_PCICORE
>
> I strongly suggest we take a step back, slow down a bit and look at
> the original problem.
>
> In driver_pcicore.c there is MIPS specific code. It's protected using
> #ifdef CONFIG_SSB_PCICORE_HOSTMODE
> (...)
> #endif
>
> If anyone has ever seen
> 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
> it means he managed to get CONFIG_SSB_PCICORE_HOSTMODE set on non-MIPS sy=
stem.
>
> We should rather answer how did that happen and fix it.
>
> SSB_PCICORE_HOSTMODE depends on SSB_DRIVER_MIPS
> SSB_DRIVER_MIPS depends on MIPS
>
> How is that possible to set SSB_PCICORE_HOSTMODE with non-MIPS config?
> Is there some mistake in Kconfig I can't see?
I think SSB =3D y should be added as dependency for
SSB_PCICORE_HOSTMODE. See also commit 79ca239a68f8f ("bcma: Prevent
build of PCI host features in module").
--=20
Rafa=C5=82
Hi Michael,
On 09/05/18 17:27, Michael Büsch wrote:
> On Wed, 9 May 2018 13:55:43 +0100
> Matt Redfearn <[email protected]> wrote:
>
>> Hi Larry
>>
>> On 07/05/18 16:44, Larry Finger wrote:
>>> Matt,
>>>
>>> Although commit 882164a4a928 ("ssb: Prevent build of PCI host features
>>> in module") appeared to be harmless, it leads to complete failure of
>>> drivers b43. and b43legacy, and likely affects b44 as well. The problem
>>> is that CONFIG_SSB_PCIHOST is undefined, which prevents the compilation
>>> of the code that controls the PCI cores of the device. See
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1572349 for details.
>>
>> Sorry for the breakage :-/
>>
>>>
>>> As the underlying errors ("pcibios_enable_device" undefined, and
>>> "register_pci_controller" undefined) do not appear on the architectures
>>> that I have tested (x86_64, x86, and ppc), I suspect something in the
>>> arch-specific code for your setup (MIPS?). As I have no idea on how to
>>> fix that problem, would the following patch work for you?
>>>
>>> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
>>> index 9371651d8017..3743533c8057 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 && (SSB = y || !MIPS)
>>> default y
>>>
>>> config SSB_DRIVER_PCICORE
>>
>> I believe that the problem stems from these drivers being used for some
>> wireless AP functionality built into some MIPS based SoCs. The Kconfig
>> rules sort out building this additional functionality when configured
>> for MIPS (in a round about sort of way), but it allowed it even when SSB
>> is a module, leading to build failures. My patch was intended to prevent
>> that.
>>
>> There was a similar issue in the same Kconfig file, introduced by
>> c5611df96804 and fixed by a9e6d44ddecc. It was fixed the same way as you
>> suggest. I've tested the above patch and it does work for MIPS
>> (preventing the PCICORE being built into the module).
>>
>> Tested-by: Matt Redfearn <[email protected]>
>
>
> Could you please try this?
>
> config SSB_DRIVER_PCICORE_POSSIBLE
> depends on SSB_PCIHOST
>
> config SSB_PCICORE_HOSTMODE
> depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && (SSB = y) && PCI_DRIVERS_LEGACY
>
>
> The affected API pcibios_enable_device() and register_pci_controller()
> is only used in HOSTMODE. So I think it makes sense to make HOSTMODE
> depend on SSB=y and PCI_DRIVERS_LEGACY.
>
> PCICore itself does not use the API, if hostmode is disabled.
>
Sure - I've tested the patch:
--- 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
@@ -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) &&
PCI_DRIVERS_LEGACY
help
PCIcore hostmode operation (external PCI bus).
And this seems to work for MIPS, we don't get the build error from
building the SSB module under nec_markeins allmodconfig, and
SSB_PCICORE_HOSTMODE=y for bcm47xx allmodconfig, which selects SSB=y.
So this looks like a good fix for MIPS, at least.
Tested-by: Matt Redfearn <[email protected]>
Thanks,
Matt
On Mon, 07 May 2018 22:03:58 +0300
Kalle Valo <[email protected]> wrote:
> Michael Büsch <[email protected]> writes:
>
> > On Mon, 7 May 2018 10:44:34 -0500
> > Larry Finger <[email protected]> wrote:
> >
> >> Although commit 882164a4a928 ("ssb: Prevent build of PCI host features in
> >> module") appeared to be harmless, it leads to complete failure of drivers b43.
> >
> >> config SSB_DRIVER_PCICORE_POSSIBLE
> >> bool
> >> - depends on SSB_PCIHOST && SSB = y
> >> + depends on SSB_PCIHOST && (SSB = y || !MIPS)
> >> default y
> >>
> >> config SSB_DRIVER_PCICORE
> >
> >
> > https://patchwork.kernel.org/patch/10161131/
> >
> > Could we _please_ switch to not applying patches to ssb or b43, if
> > nobody acked (or better reviewed) a patch?
> >
> > We had multiple changes to ssb and b43 in the recent past that did not
> > have a review at all and broke something. I don't think such software
> > quality is acceptable at all.
> > So please revert 882164a4a928.
>
> Yes, someone please send a revert so that this can be fixed quickly for
> v4.17.
Uhm, can you just type git revert 882164a4a928? :)
Or do I have to send you a pull request?
> > I'm sorry that this patch slipped through the cracks of my inbox.
> > But the reaction to that shall not be to just apply the patch. It
> > shall be to resubmit it for review.
>
> The thing is that in general I do not have time to ping people for every
> patch, I get enough of emails as is. If there are no review comments I
> have to assume the patch is ok to apply.
Yes, I understand that pinging people can be annoying and time
consuming. But we have tools like patchwork. Why isn't pinging
(semi)automated? Patchwork should really track the review status of a
patch.
I think the concept of no-comments = everything-ok is
fundamentally broken. But it has always been that way for wireless and
lots of other subsystems.
> But as ssb has had two major regressions recently I'm going to
> significantly raise the bar for ssb patches, and will refuse to apply
> random patches if they have not been tested with b43/b44.
Thanks.
--
Michael
On Mon, 7 May 2018 10:44:34 -0500
Larry Finger <[email protected]> wrote:
> Although commit 882164a4a928 ("ssb: Prevent build of PCI host features in
> module") appeared to be harmless, it leads to complete failure of drivers b43.
> config SSB_DRIVER_PCICORE_POSSIBLE
> bool
> - depends on SSB_PCIHOST && SSB = y
> + depends on SSB_PCIHOST && (SSB = y || !MIPS)
> default y
>
> config SSB_DRIVER_PCICORE
https://patchwork.kernel.org/patch/10161131/
Could we _please_ switch to not applying patches to ssb or b43, if
nobody acked (or better reviewed) a patch?
We had multiple changes to ssb and b43 in the recent past that did not
have a review at all and broke something. I don't think such software
quality is acceptable at all.
So please revert 882164a4a928.
I'm sorry that this patch slipped through the cracks of my inbox.
But the reaction to that shall not be to just apply the patch. It
shall be to resubmit it for review.
But back to the technical topic.
I don't remember why SSB_DRIVER_PCICORE_POSSIBLE depends on SSB_PCIHOST.
But that looks and feels wrong.
I would say it should rather look like
config SSB_DRIVER_PCICORE_POSSIBLE
depends on SSB && (PCI = y || PCI = SSB)
completely untested, though.
--
Michael
On 7 May 2018 at 17:44, Larry Finger <[email protected]> wrote:
> Although commit 882164a4a928 ("ssb: Prevent build of PCI host features in
> module") appeared to be harmless, it leads to complete failure of drivers
> b43. and b43legacy, and likely affects b44 as well. The problem is that
> CONFIG_SSB_PCIHOST is undefined, which prevents the compilation of the co=
de
> that controls the PCI cores of the device. See
> https://bugzilla.redhat.com/show_bug.cgi?id=3D1572349 for details.
>
> As the underlying errors ("pcibios_enable_device" undefined, and
> "register_pci_controller" undefined) do not appear on the architectures t=
hat
> I have tested (x86_64, x86, and ppc), I suspect something in the
> arch-specific code for your setup (MIPS?). As I have no idea on how to fi=
x
> that problem, would the following patch work for you?
>
> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> index 9371651d8017..3743533c8057 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 =3D y
> + depends on SSB_PCIHOST && (SSB =3D y || !MIPS)
> default y
>
> config SSB_DRIVER_PCICORE
I strongly suggest we take a step back, slow down a bit and look at
the original problem.
In driver_pcicore.c there is MIPS specific code. It's protected using
#ifdef CONFIG_SSB_PCICORE_HOSTMODE
(...)
#endif
If anyone has ever seen
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
it means he managed to get CONFIG_SSB_PCICORE_HOSTMODE set on non-MIPS syst=
em.
We should rather answer how did that happen and fix it.
SSB_PCICORE_HOSTMODE depends on SSB_DRIVER_MIPS
SSB_DRIVER_MIPS depends on MIPS
How is that possible to set SSB_PCICORE_HOSTMODE with non-MIPS config?
Is there some mistake in Kconfig I can't see?
--=20
Rafa=C5=82
On Thu, 10 May 2018 11:24:12 +0100
Matt Redfearn <[email protected]> wrote:
> > Could you please try this?
> >
> > config SSB_DRIVER_PCICORE_POSSIBLE
> > depends on SSB_PCIHOST
> >
> > config SSB_PCICORE_HOSTMODE
> > depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && (SSB = y) && PCI_DRIVERS_LEGACY
> >
> >
> > The affected API pcibios_enable_device() and register_pci_controller()
> > is only used in HOSTMODE. So I think it makes sense to make HOSTMODE
> > depend on SSB=y and PCI_DRIVERS_LEGACY.
> >
> > PCICore itself does not use the API, if hostmode is disabled.
> >
>
> Sure - I've tested the patch:
>
> --- 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
> @@ -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) &&
> PCI_DRIVERS_LEGACY
> help
> PCIcore hostmode operation (external PCI bus).
>
>
> And this seems to work for MIPS, we don't get the build error from
> building the SSB module under nec_markeins allmodconfig, and
> SSB_PCICORE_HOSTMODE=y for bcm47xx allmodconfig, which selects SSB=y.
>
> So this looks like a good fix for MIPS, at least.
>
> Tested-by: Matt Redfearn <[email protected]>
Thanks.
Could you please submit it?
You can add my Acked-by.
--
Michael
Hi Larry
On 07/05/18 16:44, Larry Finger wrote:
> Matt,
>
> Although commit 882164a4a928 ("ssb: Prevent build of PCI host features
> in module") appeared to be harmless, it leads to complete failure of
> drivers b43. and b43legacy, and likely affects b44 as well. The problem
> is that CONFIG_SSB_PCIHOST is undefined, which prevents the compilation
> of the code that controls the PCI cores of the device. See
> https://bugzilla.redhat.com/show_bug.cgi?id=1572349 for details.
Sorry for the breakage :-/
>
> As the underlying errors ("pcibios_enable_device" undefined, and
> "register_pci_controller" undefined) do not appear on the architectures
> that I have tested (x86_64, x86, and ppc), I suspect something in the
> arch-specific code for your setup (MIPS?). As I have no idea on how to
> fix that problem, would the following patch work for you?
>
> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> index 9371651d8017..3743533c8057 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 && (SSB = y || !MIPS)
> default y
>
> config SSB_DRIVER_PCICORE
I believe that the problem stems from these drivers being used for some
wireless AP functionality built into some MIPS based SoCs. The Kconfig
rules sort out building this additional functionality when configured
for MIPS (in a round about sort of way), but it allowed it even when SSB
is a module, leading to build failures. My patch was intended to prevent
that.
There was a similar issue in the same Kconfig file, introduced by
c5611df96804 and fixed by a9e6d44ddecc. It was fixed the same way as you
suggest. I've tested the above patch and it does work for MIPS
(preventing the PCICORE being built into the module).
Tested-by: Matt Redfearn <[email protected]>
Thanks & sorry again for the breakage,
Matt
>
> Thanks,
>
> Larry
Michael B=C3=BCsch <[email protected]> writes:
> On Mon, 7 May 2018 10:44:34 -0500
> Larry Finger <[email protected]> wrote:
>
>> Although commit 882164a4a928 ("ssb: Prevent build of PCI host features i=
n=20
>> module") appeared to be harmless, it leads to complete failure of driver=
s b43.=20
>
>> config SSB_DRIVER_PCICORE_POSSIBLE
>> bool
>> - depends on SSB_PCIHOST && SSB =3D y
>> + depends on SSB_PCIHOST && (SSB =3D y || !MIPS)
>> default y
>>=20
>> config SSB_DRIVER_PCICORE
>
>
> https://patchwork.kernel.org/patch/10161131/
>
> Could we _please_ switch to not applying patches to ssb or b43, if
> nobody acked (or better reviewed) a patch?
>
> We had multiple changes to ssb and b43 in the recent past that did not
> have a review at all and broke something. I don't think such software
> quality is acceptable at all.
> So please revert 882164a4a928.
Yes, someone please send a revert so that this can be fixed quickly for
v4.17.
> I'm sorry that this patch slipped through the cracks of my inbox.
> But the reaction to that shall not be to just apply the patch. It
> shall be to resubmit it for review.
The thing is that in general I do not have time to ping people for every
patch, I get enough of emails as is. If there are no review comments I
have to assume the patch is ok to apply.
But as ssb has had two major regressions recently I'm going to
significantly raise the bar for ssb patches, and will refuse to apply
random patches if they have not been tested with b43/b44.
--=20
Kalle Valo
On Wed, 9 May 2018 13:55:43 +0100
Matt Redfearn <[email protected]> wrote:
> Hi Larry
>
> On 07/05/18 16:44, Larry Finger wrote:
> > Matt,
> >
> > Although commit 882164a4a928 ("ssb: Prevent build of PCI host features
> > in module") appeared to be harmless, it leads to complete failure of
> > drivers b43. and b43legacy, and likely affects b44 as well. The problem
> > is that CONFIG_SSB_PCIHOST is undefined, which prevents the compilation
> > of the code that controls the PCI cores of the device. See
> > https://bugzilla.redhat.com/show_bug.cgi?id=1572349 for details.
>
> Sorry for the breakage :-/
>
> >
> > As the underlying errors ("pcibios_enable_device" undefined, and
> > "register_pci_controller" undefined) do not appear on the architectures
> > that I have tested (x86_64, x86, and ppc), I suspect something in the
> > arch-specific code for your setup (MIPS?). As I have no idea on how to
> > fix that problem, would the following patch work for you?
> >
> > diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> > index 9371651d8017..3743533c8057 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 && (SSB = y || !MIPS)
> > default y
> >
> > config SSB_DRIVER_PCICORE
>
> I believe that the problem stems from these drivers being used for some
> wireless AP functionality built into some MIPS based SoCs. The Kconfig
> rules sort out building this additional functionality when configured
> for MIPS (in a round about sort of way), but it allowed it even when SSB
> is a module, leading to build failures. My patch was intended to prevent
> that.
>
> There was a similar issue in the same Kconfig file, introduced by
> c5611df96804 and fixed by a9e6d44ddecc. It was fixed the same way as you
> suggest. I've tested the above patch and it does work for MIPS
> (preventing the PCICORE being built into the module).
>
> Tested-by: Matt Redfearn <[email protected]>
Could you please try this?
config SSB_DRIVER_PCICORE_POSSIBLE
depends on SSB_PCIHOST
config SSB_PCICORE_HOSTMODE
depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && (SSB = y) && PCI_DRIVERS_LEGACY
The affected API pcibios_enable_device() and register_pci_controller()
is only used in HOSTMODE. So I think it makes sense to make HOSTMODE
depend on SSB=y and PCI_DRIVERS_LEGACY.
PCICore itself does not use the API, if hostmode is disabled.
--
Michael
Hi Rafał,
On 10/05/18 11:41, Rafał Miłecki wrote:
> On 7 May 2018 at 17:44, Larry Finger <[email protected]> wrote:
>> Although commit 882164a4a928 ("ssb: Prevent build of PCI host features in
>> module") appeared to be harmless, it leads to complete failure of drivers
>> b43. and b43legacy, and likely affects b44 as well. The problem is that
>> CONFIG_SSB_PCIHOST is undefined, which prevents the compilation of the code
>> that controls the PCI cores of the device. See
>> https://bugzilla.redhat.com/show_bug.cgi?id=1572349 for details.
>>
>> As the underlying errors ("pcibios_enable_device" undefined, and
>> "register_pci_controller" undefined) do not appear on the architectures that
>> I have tested (x86_64, x86, and ppc), I suspect something in the
>> arch-specific code for your setup (MIPS?). As I have no idea on how to fix
>> that problem, would the following patch work for you?
>>
>> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
>> index 9371651d8017..3743533c8057 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 && (SSB = y || !MIPS)
>> default y
>>
>> config SSB_DRIVER_PCICORE
>
> I strongly suggest we take a step back, slow down a bit and look at
> the original problem.
>
> In driver_pcicore.c there is MIPS specific code. It's protected using
> #ifdef CONFIG_SSB_PCICORE_HOSTMODE
> (...)
> #endif
>
> If anyone has ever seen
> 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
> it means he managed to get CONFIG_SSB_PCICORE_HOSTMODE set on non-MIPS system.
I saw this on a MIPS system (to my knowledge, this does not happen on
other arches due to the Kconfig rules you describe), which is what my
original patch was attempting to fix, but appears to have caused
problems on other arches.
Thanks,
Matt
>
> We should rather answer how did that happen and fix it.
>
> SSB_PCICORE_HOSTMODE depends on SSB_DRIVER_MIPS
> SSB_DRIVER_MIPS depends on MIPS
>
> How is that possible to set SSB_PCICORE_HOSTMODE with non-MIPS config?
> Is there some mistake in Kconfig I can't see?
>