Re: Regression caused by commit 882164a4a928

[Matt Redfearn]

Hi Michael,

On 09/05/18 17:27, Michael BÃsch wrote:
> On Wed, 9 May 2018 13:55:43 +0100
> Matt Redfearn <matt.redfearn@xxxxxxxx> 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 <matt.redfearn@xxxxxxxx>
>
>
> 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 <matt.redfearn@xxxxxxxx>

Thanks,
Matt