Messages in this thread Patch in this message | | | Date | Tue, 7 Dec 2021 16:50:01 +0100 | Subject | Re: [patch V2 01/23] powerpc/4xx: Remove MSI support which never worked | From | Cédric Le Goater <> |
| |
On 12/7/21 12:36, Michael Ellerman wrote: > Cédric Le Goater <clg@kaod.org> writes: >> Hello Thomas, >> >> On 12/6/21 23:27, Thomas Gleixner wrote: >>> This code is broken since day one. ppc4xx_setup_msi_irqs() has the >>> following gems: >>> >>> 1) The handling of the result of msi_bitmap_alloc_hwirqs() is completely >>> broken: >>> >>> When the result is greater than or equal 0 (bitmap allocation >>> successful) then the loop terminates and the function returns 0 >>> (success) despite not having installed an interrupt. >>> >>> When the result is less than 0 (bitmap allocation fails), it prints an >>> error message and continues to "work" with that error code which would >>> eventually end up in the MSI message data. >>> >>> 2) On every invocation the file global pp4xx_msi::msi_virqs bitmap is >>> allocated thereby leaking the previous one. >>> >>> IOW, this has never worked and for more than 10 years nobody cared. Remove >>> the gunk. >>> >>> Fixes: 3fb7933850fa ("powerpc/4xx: Adding PCIe MSI support") >> >> Shouldn't we remove all of it ? including the updates in the device trees >> and the Kconfig changes under : >> >> arch/powerpc/platforms/44x/Kconfig: select PPC4xx_MSI >> arch/powerpc/platforms/44x/Kconfig: select PPC4xx_MSI >> arch/powerpc/platforms/44x/Kconfig: select PPC4xx_MSI >> arch/powerpc/platforms/44x/Kconfig: select PPC4xx_MSI >> arch/powerpc/platforms/40x/Kconfig: select PPC4xx_MSI > > This patch should drop those selects I guess. Can you send an > incremental diff for Thomas to squash in?
Sure.
> Removing all the tendrils in various device tree files will probably > require some archaeology, and it should be perfectly safe to leave those > in the tree with the driver gone. So I think we can do that as a > subsequent patch, rather than in this series.
Here are the changes. Compiled tested with ppc40x and ppc44x defconfigs.
Thanks,
C.
diff --git a/arch/powerpc/boot/dts/bluestone.dts b/arch/powerpc/boot/dts/bluestone.dts index aa1ae94cd776..6971595319c1 100644 --- a/arch/powerpc/boot/dts/bluestone.dts +++ b/arch/powerpc/boot/dts/bluestone.dts @@ -366,30 +366,5 @@ PCIE0: pcie@d00000000 { 0x0 0x0 0x0 0x3 &UIC3 0xe 0x4 /* swizzled int C */ 0x0 0x0 0x0 0x4 &UIC3 0xf 0x4 /* swizzled int D */>; }; - - MSI: ppc4xx-msi@C10000000 { - compatible = "amcc,ppc4xx-msi", "ppc4xx-msi"; - reg = < 0xC 0x10000000 0x100 - 0xC 0x10000000 0x100>; - sdr-base = <0x36C>; - msi-data = <0x00004440>; - msi-mask = <0x0000ffe0>; - interrupts =<0 1 2 3 4 5 6 7>; - interrupt-parent = <&MSI>; - #interrupt-cells = <1>; - #address-cells = <0>; - #size-cells = <0>; - msi-available-ranges = <0x0 0x100>; - interrupt-map = < - 0 &UIC3 0x18 1 - 1 &UIC3 0x19 1 - 2 &UIC3 0x1A 1 - 3 &UIC3 0x1B 1 - 4 &UIC3 0x1C 1 - 5 &UIC3 0x1D 1 - 6 &UIC3 0x1E 1 - 7 &UIC3 0x1F 1 - >; - }; }; }; diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts index c5fbb08e0a6e..5db1bff6b23d 100644 --- a/arch/powerpc/boot/dts/canyonlands.dts +++ b/arch/powerpc/boot/dts/canyonlands.dts @@ -544,23 +544,5 @@ PCIE1: pcie@d20000000 { 0x0 0x0 0x0 0x3 &UIC3 0x12 0x4 /* swizzled int C */ 0x0 0x0 0x0 0x4 &UIC3 0x13 0x4 /* swizzled int D */>; }; - - MSI: ppc4xx-msi@C10000000 { - compatible = "amcc,ppc4xx-msi", "ppc4xx-msi"; - reg = < 0xC 0x10000000 0x100>; - sdr-base = <0x36C>; - msi-data = <0x00000000>; - msi-mask = <0x44440000>; - interrupt-count = <3>; - interrupts = <0 1 2 3>; - interrupt-parent = <&UIC3>; - #interrupt-cells = <1>; - #address-cells = <0>; - #size-cells = <0>; - interrupt-map = <0 &UIC3 0x18 1 - 1 &UIC3 0x19 1 - 2 &UIC3 0x1A 1 - 3 &UIC3 0x1B 1>; - }; }; }; diff --git a/arch/powerpc/boot/dts/katmai.dts b/arch/powerpc/boot/dts/katmai.dts index a8f353229fb7..4262b2bbd6de 100644 --- a/arch/powerpc/boot/dts/katmai.dts +++ b/arch/powerpc/boot/dts/katmai.dts @@ -442,24 +442,6 @@ PCIE2: pcie@d40000000 { 0x0 0x0 0x0 0x4 &UIC3 0xb 0x4 /* swizzled int D */>; }; - MSI: ppc4xx-msi@400300000 { - compatible = "amcc,ppc4xx-msi", "ppc4xx-msi"; - reg = < 0x4 0x00300000 0x100>; - sdr-base = <0x3B0>; - msi-data = <0x00000000>; - msi-mask = <0x44440000>; - interrupt-count = <3>; - interrupts =<0 1 2 3>; - interrupt-parent = <&UIC0>; - #interrupt-cells = <1>; - #address-cells = <0>; - #size-cells = <0>; - interrupt-map = <0 &UIC0 0xC 1 - 1 &UIC0 0x0D 1 - 2 &UIC0 0x0E 1 - 3 &UIC0 0x0F 1>; - }; - I2O: i2o@400100000 { compatible = "ibm,i2o-440spe"; reg = <0x00000004 0x00100000 0x100>; diff --git a/arch/powerpc/boot/dts/kilauea.dts b/arch/powerpc/boot/dts/kilauea.dts index a709fb47a180..c07a7525a72c 100644 --- a/arch/powerpc/boot/dts/kilauea.dts +++ b/arch/powerpc/boot/dts/kilauea.dts @@ -403,33 +403,5 @@ PCIE1: pcie@c0000000 { 0x0 0x0 0x0 0x3 &UIC2 0xd 0x4 /* swizzled int C */ 0x0 0x0 0x0 0x4 &UIC2 0xe 0x4 /* swizzled int D */>; }; - - MSI: ppc4xx-msi@C10000000 { - compatible = "amcc,ppc4xx-msi", "ppc4xx-msi"; - reg = <0xEF620000 0x100>; - sdr-base = <0x4B0>; - msi-data = <0x00000000>; - msi-mask = <0x44440000>; - interrupt-count = <12>; - interrupts = <0 1 2 3 4 5 6 7 8 9 0xA 0xB 0xC 0xD>; - interrupt-parent = <&UIC2>; - #interrupt-cells = <1>; - #address-cells = <0>; - #size-cells = <0>; - interrupt-map = <0 &UIC2 0x10 1 - 1 &UIC2 0x11 1 - 2 &UIC2 0x12 1 - 2 &UIC2 0x13 1 - 2 &UIC2 0x14 1 - 2 &UIC2 0x15 1 - 2 &UIC2 0x16 1 - 2 &UIC2 0x17 1 - 2 &UIC2 0x18 1 - 2 &UIC2 0x19 1 - 2 &UIC2 0x1A 1 - 2 &UIC2 0x1B 1 - 2 &UIC2 0x1C 1 - 3 &UIC2 0x1D 1>; - }; }; }; diff --git a/arch/powerpc/boot/dts/redwood.dts b/arch/powerpc/boot/dts/redwood.dts index f38035a1f4a1..3c849e23e5f3 100644 --- a/arch/powerpc/boot/dts/redwood.dts +++ b/arch/powerpc/boot/dts/redwood.dts @@ -358,25 +358,6 @@ PCIE2: pcie@d40000000 { 0x0 0x0 0x0 0x4 &UIC3 0xb 0x4 /* swizzled int D */>; }; - MSI: ppc4xx-msi@400300000 { - compatible = "amcc,ppc4xx-msi", "ppc4xx-msi"; - reg = < 0x4 0x00300000 0x100 - 0x4 0x00300000 0x100>; - sdr-base = <0x3B0>; - msi-data = <0x00000000>; - msi-mask = <0x44440000>; - interrupt-count = <3>; - interrupts =<0 1 2 3>; - interrupt-parent = <&UIC0>; - #interrupt-cells = <1>; - #address-cells = <0>; - #size-cells = <0>; - interrupt-map = <0 &UIC0 0xC 1 - 1 &UIC0 0x0D 1 - 2 &UIC0 0x0E 1 - 3 &UIC0 0x0F 1>; - }; - }; diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig index e3e5217c9822..614ea6dc994c 100644 --- a/arch/powerpc/platforms/40x/Kconfig +++ b/arch/powerpc/platforms/40x/Kconfig @@ -23,7 +23,6 @@ config KILAUEA select PPC4xx_PCI_EXPRESS select FORCE_PCI select PCI_MSI - select PPC4xx_MSI help This option enables support for the AMCC PPC405EX evaluation board. diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig index 83975ef50975..25b80cd558f8 100644 --- a/arch/powerpc/platforms/44x/Kconfig +++ b/arch/powerpc/platforms/44x/Kconfig @@ -23,7 +23,6 @@ config BLUESTONE select APM821xx select FORCE_PCI select PCI_MSI - select PPC4xx_MSI select PPC4xx_PCI_EXPRESS select IBM_EMAC_RGMII if IBM_EMAC help @@ -73,7 +72,6 @@ config KATMAI select FORCE_PCI select PPC4xx_PCI_EXPRESS select PCI_MSI - select PPC4xx_MSI help This option enables support for the AMCC PPC440SPe evaluation board. @@ -115,7 +113,6 @@ config CANYONLANDS select FORCE_PCI select PPC4xx_PCI_EXPRESS select PCI_MSI - select PPC4xx_MSI select IBM_EMAC_RGMII if IBM_EMAC select IBM_EMAC_ZMII if IBM_EMAC help @@ -141,7 +138,6 @@ config REDWOOD select FORCE_PCI select PPC4xx_PCI_EXPRESS select PCI_MSI - select PPC4xx_MSI help This option enables support for the AMCC PPC460SX Redwood board. -- 2.31.1
| |