Re: [PATCH] PCI: mvebu: Mark driver as BROKEN

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Krzysztof]

On Fri, Aug 04, 2023 at 12:35:13PM +0100, Russell King (Oracle) wrote:
> So it seems this patch got applied, but it wasn't Cc'd to
> linux-arm-kernel or anyone else, so those of us with platforms never
> had a chance to comment on it.
> 
> *** This change causes a regression to working setups. ***
> 
> It appears that the *only* reason this patch was proposed is to stop a
> kernel developer receiving problem reports from a set of users, but
> completely ignores that there is another group of users where this works
> fine - and thus the addition of this patch causes working setups to
> regress.
> 
> Because one is being bothered with problem reports is not a reason to
> mark a driver broken - and especially not doing so in a way that those
> who may be affected don't get an opportunity to comment on the patch!
> Also, there is _zero_ information provided on what the reported problems
> actually are, so no one else can guess what these issues are.
> 
> However, given that there are working setups and this change causes
> those to regress, it needs to be reverted.
> 
> For example, I have an Atheros PCIe WiFi card in an Armada 388 Clearfog
> platform, and this works fine.
> 
> Uwe has a SATA controller for a bunch of disks in an Armada 370 based
> NAS platform that is connected to PCIe, and removing PCIe support
> effectively makes his platform utterly useless.
> 
> Please revert this patch.

Sorry for the inconvenience.

I was under the mistaken impression that making the driver depend on
CONFIG_BROKEN would keep the driver available but only if the user
explicitly requested it, similar to how 
CONFIG_COMPILE_TEST works.  But obviously that's not the case, so
we'll revert the change.

I queued up the revert below, including a note in the Kconfig help
text about the known issues.

commit 814b6bb15367 ("Revert "PCI: mvebu: Mark driver as BROKEN"")
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Fri Aug 4 11:54:43 2023 -0500

    Revert "PCI: mvebu: Mark driver as BROKEN"
    
    b3574f579ece ("PCI: mvebu: Mark driver as BROKEN") made it impossible to
    enable the pci-mvebu driver.  The driver does have known problems, but as
    Russell and Uwe reported, it does work in some configurations, so removing
    it broke some working setups.
    
    Revert b3574f579ece so pci-mvebu is available.  Mention the known problems
    in the Kconfig help text.
    
    Reported-by: Russell King (Oracle) <linux@armlinux.org.uk>
    Link: https://lore.kernel.org/r/ZMzicVQEyHyZzBOc@shell.armlinux.org.uk
    Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
    Link: https://lore.kernel.org/r/20230804134622.pmbymxtzxj2yfhri@pengutronix.de
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>


diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 8d49bad7f847..478f158b2dfb 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -179,13 +179,15 @@ config PCI_MVEBU
 	depends on MVEBU_MBUS
 	depends on ARM
 	depends on OF
-	depends on BROKEN
 	select PCI_BRIDGE_EMUL
 	help
 	 Add support for Marvell EBU PCIe controller. This PCIe controller
 	 is used on 32-bit Marvell ARM SoCs: Dove, Kirkwood, Armada 370,
 	 Armada XP, Armada 375, Armada 38x and Armada 39x.
 
+	 This driver has known problems that may cause crashes during boot
+	 and failure to detect PCIe devices in some cases.
+
 config PCIE_MEDIATEK
 	tristate "MediaTek PCIe controller"
 	depends on ARCH_AIROHA || ARCH_MEDIATEK || COMPILE_TEST