Re: [patch V2 01/23] powerpc/4xx: Remove MSI support which never worked

["Cédric Le Goater" <clg@kaod.org>]

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