linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description
@ 2022-08-19 19:15 Pali Rohár
  2022-08-19 19:15 ` [PATCH 1/7] powerpc/85xx: Mark mpc85xx_rdb_pic_init() as static Pali Rohár
                   ` (8 more replies)
  0 siblings, 9 replies; 30+ messages in thread
From: Pali Rohár @ 2022-08-19 19:15 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Scott Wood, Christophe Leroy, Sinan Akman
  Cc: linuxppc-dev, linux-kernel

This patch series unifies all P2020 boards and machine descriptions into
one generic unified P2020 machine description. With this generic machine
description, kernel can boot on any P2020-based board with correct DTS
file.

Tested on CZ.NIC Turris 1.1 board with has Freescale P2020 processor.
Kernel during booting correctly detects P2020 and prints:
[    0.000000] Using Freescale P2020 machine description

Pali Rohár (7):
  powerpc/85xx: Mark mpc85xx_rdb_pic_init() as static
  powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
  powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
  powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks
  powerpc/85xx: p2020: Define just one machine description
  powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020
  powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string

 arch/powerpc/boot/dts/turris1x.dts            |   2 +-
 arch/powerpc/platforms/85xx/Kconfig           |  22 ++-
 arch/powerpc/platforms/85xx/Makefile          |   1 +
 arch/powerpc/platforms/85xx/mpc85xx_ds.c      |  25 +--
 arch/powerpc/platforms/85xx/mpc85xx_rdb.c     |  46 +-----
 .../platforms/85xx/{mpc85xx_ds.c => p2020.c}  | 144 +++++++-----------
 6 files changed, 75 insertions(+), 165 deletions(-)
 copy arch/powerpc/platforms/85xx/{mpc85xx_ds.c => p2020.c} (53%)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/7] powerpc/85xx: Mark mpc85xx_rdb_pic_init() as static
  2022-08-19 19:15 [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
@ 2022-08-19 19:15 ` Pali Rohár
  2022-08-19 19:15 ` [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() " Pali Rohár
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2022-08-19 19:15 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Scott Wood, Christophe Leroy, Sinan Akman
  Cc: linuxppc-dev, linux-kernel

Function mpc85xx_rdb_pic_init() is not used out of the mpc85xx_rdb.c file.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
index d99aba158235..b6129c148fea 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
@@ -38,7 +38,7 @@
 #endif
 
 
-void __init mpc85xx_rdb_pic_init(void)
+static void __init mpc85xx_rdb_pic_init(void)
 {
 	struct mpic *mpic;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
  2022-08-19 19:15 [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
  2022-08-19 19:15 ` [PATCH 1/7] powerpc/85xx: Mark mpc85xx_rdb_pic_init() as static Pali Rohár
@ 2022-08-19 19:15 ` Pali Rohár
  2022-09-26  9:43   ` Christophe Leroy
  2022-08-19 19:15 ` [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c Pali Rohár
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-08-19 19:15 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Scott Wood, Christophe Leroy, Sinan Akman
  Cc: linuxppc-dev, linux-kernel

Function mpc85xx_ds_pic_init() is not used out of the mpc85xx_ds.c file.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/powerpc/platforms/85xx/mpc85xx_ds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
index f8d2c97f39bd..9a6d637ef54a 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
@@ -54,7 +54,7 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
 }
 #endif	/* CONFIG_PPC_I8259 */
 
-void __init mpc85xx_ds_pic_init(void)
+static void __init mpc85xx_ds_pic_init(void)
 {
 	struct mpic *mpic;
 #ifdef CONFIG_PPC_I8259
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
  2022-08-19 19:15 [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
  2022-08-19 19:15 ` [PATCH 1/7] powerpc/85xx: Mark mpc85xx_rdb_pic_init() as static Pali Rohár
  2022-08-19 19:15 ` [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() " Pali Rohár
@ 2022-08-19 19:15 ` Pali Rohár
  2022-09-26  9:48   ` Christophe Leroy
  2022-08-19 19:15 ` [PATCH 4/7] powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks Pali Rohár
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-08-19 19:15 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Scott Wood, Christophe Leroy, Sinan Akman
  Cc: linuxppc-dev, linux-kernel

This moves machine descriptions and all related code for all P2020 boards
into new p2020.c source file. This is preparation for code deduplication
and providing one unified machine description for all P2020 boards.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/powerpc/platforms/85xx/Makefile          |   2 +
 arch/powerpc/platforms/85xx/mpc85xx_ds.c      |  23 ---
 arch/powerpc/platforms/85xx/mpc85xx_rdb.c     |  44 ------
 .../platforms/85xx/{mpc85xx_ds.c => p2020.c}  | 134 ++++++++++++------
 4 files changed, 91 insertions(+), 112 deletions(-)
 copy arch/powerpc/platforms/85xx/{mpc85xx_ds.c => p2020.c} (65%)

diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index 260fbad7967b..1ad261b4eeb6 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -23,6 +23,8 @@ obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
 obj-$(CONFIG_P1022_DS)    += p1022_ds.o
 obj-$(CONFIG_P1022_RDK)   += p1022_rdk.o
 obj-$(CONFIG_P1023_RDB)   += p1023_rdb.o
+obj-$(CONFIG_MPC85xx_DS)  += p2020.o
+obj-$(CONFIG_MPC85xx_RDB) += p2020.o
 obj-$(CONFIG_TWR_P102x)   += twr_p102x.o
 obj-$(CONFIG_CORENET_GENERIC)   += corenet_generic.o
 obj-$(CONFIG_FB_FSL_DIU)	+= t1042rdb_diu.o
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
index 9a6d637ef54a..05aac997b5ed 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
@@ -168,7 +168,6 @@ static int __init mpc8544_ds_probe(void)
 
 machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
 machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
-machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
 
 /*
  * Called very early, device-tree isn't unflattened
@@ -178,14 +177,6 @@ static int __init mpc8572_ds_probe(void)
 	return !!of_machine_is_compatible("fsl,MPC8572DS");
 }
 
-/*
- * Called very early, device-tree isn't unflattened
- */
-static int __init p2020_ds_probe(void)
-{
-	return !!of_machine_is_compatible("fsl,P2020DS");
-}
-
 define_machine(mpc8544_ds) {
 	.name			= "MPC8544 DS",
 	.probe			= mpc8544_ds_probe,
@@ -213,17 +204,3 @@ define_machine(mpc8572_ds) {
 	.calibrate_decr		= generic_calibrate_decr,
 	.progress		= udbg_progress,
 };
-
-define_machine(p2020_ds) {
-	.name			= "P2020 DS",
-	.probe			= p2020_ds_probe,
-	.setup_arch		= mpc85xx_ds_setup_arch,
-	.init_IRQ		= mpc85xx_ds_pic_init,
-#ifdef CONFIG_PCI
-	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
-	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
-#endif
-	.get_irq		= mpic_get_irq,
-	.calibrate_decr		= generic_calibrate_decr,
-	.progress		= udbg_progress,
-};
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
index b6129c148fea..05f1ed635735 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
@@ -108,8 +108,6 @@ static void __init mpc85xx_rdb_setup_arch(void)
 	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
 }
 
-machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
-machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
 machine_arch_initcall(p1020_mbg_pc, mpc85xx_common_publish_devices);
 machine_arch_initcall(p1020_rdb, mpc85xx_common_publish_devices);
 machine_arch_initcall(p1020_rdb_pc, mpc85xx_common_publish_devices);
@@ -122,13 +120,6 @@ machine_arch_initcall(p1024_rdb, mpc85xx_common_publish_devices);
 /*
  * Called very early, device-tree isn't unflattened
  */
-static int __init p2020_rdb_probe(void)
-{
-	if (of_machine_is_compatible("fsl,P2020RDB"))
-		return 1;
-	return 0;
-}
-
 static int __init p1020_rdb_probe(void)
 {
 	if (of_machine_is_compatible("fsl,P1020RDB"))
@@ -153,13 +144,6 @@ static int __init p1021_rdb_pc_probe(void)
 	return 0;
 }
 
-static int __init p2020_rdb_pc_probe(void)
-{
-	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
-		return 1;
-	return 0;
-}
-
 static int __init p1025_rdb_probe(void)
 {
 	return of_machine_is_compatible("fsl,P1025RDB");
@@ -180,20 +164,6 @@ static int __init p1024_rdb_probe(void)
 	return of_machine_is_compatible("fsl,P1024RDB");
 }
 
-define_machine(p2020_rdb) {
-	.name			= "P2020 RDB",
-	.probe			= p2020_rdb_probe,
-	.setup_arch		= mpc85xx_rdb_setup_arch,
-	.init_IRQ		= mpc85xx_rdb_pic_init,
-#ifdef CONFIG_PCI
-	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
-	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
-#endif
-	.get_irq		= mpic_get_irq,
-	.calibrate_decr		= generic_calibrate_decr,
-	.progress		= udbg_progress,
-};
-
 define_machine(p1020_rdb) {
 	.name			= "P1020 RDB",
 	.probe			= p1020_rdb_probe,
@@ -222,20 +192,6 @@ define_machine(p1021_rdb_pc) {
 	.progress		= udbg_progress,
 };
 
-define_machine(p2020_rdb_pc) {
-	.name			= "P2020RDB-PC",
-	.probe			= p2020_rdb_pc_probe,
-	.setup_arch		= mpc85xx_rdb_setup_arch,
-	.init_IRQ		= mpc85xx_rdb_pic_init,
-#ifdef CONFIG_PCI
-	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
-	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
-#endif
-	.get_irq		= mpic_get_irq,
-	.calibrate_decr		= generic_calibrate_decr,
-	.progress		= udbg_progress,
-};
-
 define_machine(p1025_rdb) {
 	.name			= "P1025 RDB",
 	.probe			= p1025_rdb_probe,
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/p2020.c
similarity index 65%
copy from arch/powerpc/platforms/85xx/mpc85xx_ds.c
copy to arch/powerpc/platforms/85xx/p2020.c
index 9a6d637ef54a..d65d4c88ac47 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
+++ b/arch/powerpc/platforms/85xx/p2020.c
@@ -1,11 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * MPC85xx DS Board Setup
+ * Freescale P2020 board Setup
  *
- * Author Xianghua Xiao (x.xiao@freescale.com)
- * Roy Zang <tie-fei.zang@freescale.com>
- * 	- Add PCI/PCI Exprees support
- * Copyright 2007 Freescale Semiconductor Inc.
+ * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
+ * Copyright 2022 Pali Rohár <pali@kernel.org>
  */
 
 #include <linux/stddef.h>
@@ -17,6 +15,7 @@
 #include <linux/interrupt.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
+#include <linux/fsl/guts.h>
 
 #include <asm/time.h>
 #include <asm/machdep.h>
@@ -27,6 +26,8 @@
 #include <asm/i8259.h>
 #include <asm/swiotlb.h>
 
+#include <soc/fsl/qe/qe.h>
+
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
 #include "smp.h"
@@ -41,6 +42,8 @@
 #define DBG(fmt, args...)
 #endif
 
+#ifdef CONFIG_MPC85xx_DS
+
 #ifdef CONFIG_PPC_I8259
 static void mpc85xx_8259_cascade(struct irq_desc *desc)
 {
@@ -62,18 +65,11 @@ static void __init mpc85xx_ds_pic_init(void)
 	struct device_node *cascade_node = NULL;
 	int cascade_irq;
 #endif
-	if (of_machine_is_compatible("fsl,MPC8572DS-CAMP")) {
-		mpic = mpic_alloc(NULL, 0,
-			MPIC_NO_RESET |
-			MPIC_BIG_ENDIAN |
-			MPIC_SINGLE_DEST_CPU,
-			0, 256, " OpenPIC  ");
-	} else {
-		mpic = mpic_alloc(NULL, 0,
-			  MPIC_BIG_ENDIAN |
-			  MPIC_SINGLE_DEST_CPU,
-			0, 256, " OpenPIC  ");
-	}
+
+	mpic = mpic_alloc(NULL, 0,
+		  MPIC_BIG_ENDIAN |
+		  MPIC_SINGLE_DEST_CPU,
+		0, 256, " OpenPIC  ");
 
 	BUG_ON(mpic == NULL);
 	mpic_init(mpic);
@@ -142,9 +138,27 @@ static void __init mpc85xx_ds_uli_init(void)
 #endif
 }
 
+#endif /* CONFIG_MPC85xx_DS */
+
+#ifdef CONFIG_MPC85xx_RDB
+static void __init mpc85xx_rdb_pic_init(void)
+{
+	struct mpic *mpic;
+
+	mpic = mpic_alloc(NULL, 0,
+	  MPIC_BIG_ENDIAN |
+	  MPIC_SINGLE_DEST_CPU,
+	  0, 256, " OpenPIC  ");
+
+	BUG_ON(mpic == NULL);
+	mpic_init(mpic);
+}
+#endif /* CONFIG_MPC85xx_RDB */
+
 /*
  * Setup the architecture
  */
+#ifdef CONFIG_MPC85xx_DS
 static void __init mpc85xx_ds_setup_arch(void)
 {
 	if (ppc_md.progress)
@@ -157,38 +171,65 @@ static void __init mpc85xx_ds_setup_arch(void)
 
 	printk("MPC85xx DS board from Freescale Semiconductor\n");
 }
+#endif /* CONFIG_MPC85xx_DS */
 
-/*
- * Called very early, device-tree isn't unflattened
- */
-static int __init mpc8544_ds_probe(void)
+#ifdef CONFIG_MPC85xx_RDB
+static void __init mpc85xx_rdb_setup_arch(void)
 {
-	return !!of_machine_is_compatible("MPC8544DS");
+	if (ppc_md.progress)
+		ppc_md.progress("mpc85xx_rdb_setup_arch()", 0);
+
+	mpc85xx_smp_init();
+
+	fsl_pci_assign_primary();
+
+#ifdef CONFIG_QUICC_ENGINE
+	mpc85xx_qe_par_io_init();
+#endif	/* CONFIG_QUICC_ENGINE */
+
+	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
 }
+#endif /* CONFIG_MPC85xx_RDB */
 
-machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
-machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
+#ifdef CONFIG_MPC85xx_DS
 machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
+#endif /* CONFIG_MPC85xx_DS */
 
-/*
- * Called very early, device-tree isn't unflattened
- */
-static int __init mpc8572_ds_probe(void)
-{
-	return !!of_machine_is_compatible("fsl,MPC8572DS");
-}
+#ifdef CONFIG_MPC85xx_RDB
+machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
+machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
+#endif /* CONFIG_MPC85xx_RDB */
 
 /*
  * Called very early, device-tree isn't unflattened
  */
+#ifdef CONFIG_MPC85xx_DS
 static int __init p2020_ds_probe(void)
 {
 	return !!of_machine_is_compatible("fsl,P2020DS");
 }
+#endif /* CONFIG_MPC85xx_DS */
+
+#ifdef CONFIG_MPC85xx_RDB
+static int __init p2020_rdb_probe(void)
+{
+	if (of_machine_is_compatible("fsl,P2020RDB"))
+		return 1;
+	return 0;
+}
+
+static int __init p2020_rdb_pc_probe(void)
+{
+	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
+		return 1;
+	return 0;
+}
+#endif /* CONFIG_MPC85xx_RDB */
 
-define_machine(mpc8544_ds) {
-	.name			= "MPC8544 DS",
-	.probe			= mpc8544_ds_probe,
+#ifdef CONFIG_MPC85xx_DS
+define_machine(p2020_ds) {
+	.name			= "P2020 DS",
+	.probe			= p2020_ds_probe,
 	.setup_arch		= mpc85xx_ds_setup_arch,
 	.init_IRQ		= mpc85xx_ds_pic_init,
 #ifdef CONFIG_PCI
@@ -199,12 +240,14 @@ define_machine(mpc8544_ds) {
 	.calibrate_decr		= generic_calibrate_decr,
 	.progress		= udbg_progress,
 };
-
-define_machine(mpc8572_ds) {
-	.name			= "MPC8572 DS",
-	.probe			= mpc8572_ds_probe,
-	.setup_arch		= mpc85xx_ds_setup_arch,
-	.init_IRQ		= mpc85xx_ds_pic_init,
+#endif /* CONFIG_MPC85xx_DS */
+
+#ifdef CONFIG_MPC85xx_RDB
+define_machine(p2020_rdb) {
+	.name			= "P2020 RDB",
+	.probe			= p2020_rdb_probe,
+	.setup_arch		= mpc85xx_rdb_setup_arch,
+	.init_IRQ		= mpc85xx_rdb_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
 	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
@@ -214,11 +257,11 @@ define_machine(mpc8572_ds) {
 	.progress		= udbg_progress,
 };
 
-define_machine(p2020_ds) {
-	.name			= "P2020 DS",
-	.probe			= p2020_ds_probe,
-	.setup_arch		= mpc85xx_ds_setup_arch,
-	.init_IRQ		= mpc85xx_ds_pic_init,
+define_machine(p2020_rdb_pc) {
+	.name			= "P2020RDB-PC",
+	.probe			= p2020_rdb_pc_probe,
+	.setup_arch		= mpc85xx_rdb_setup_arch,
+	.init_IRQ		= mpc85xx_rdb_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
 	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
@@ -227,3 +270,4 @@ define_machine(p2020_ds) {
 	.calibrate_decr		= generic_calibrate_decr,
 	.progress		= udbg_progress,
 };
+#endif /* CONFIG_MPC85xx_RDB */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/7] powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks
  2022-08-19 19:15 [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
                   ` (2 preceding siblings ...)
  2022-08-19 19:15 ` [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c Pali Rohár
@ 2022-08-19 19:15 ` Pali Rohár
  2022-09-26  9:59   ` Christophe Leroy
  2022-08-19 19:15 ` [PATCH 5/7] powerpc/85xx: p2020: Define just one machine description Pali Rohár
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-08-19 19:15 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Scott Wood, Christophe Leroy, Sinan Akman
  Cc: linuxppc-dev, linux-kernel

Make just one .setup_arch and one .init_IRQ callback implementation for all
P2020 board code. This deduplicate repeated and same code.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/powerpc/platforms/85xx/p2020.c | 97 +++++++++--------------------
 1 file changed, 30 insertions(+), 67 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/p2020.c b/arch/powerpc/platforms/85xx/p2020.c
index d65d4c88ac47..d327e6c9b838 100644
--- a/arch/powerpc/platforms/85xx/p2020.c
+++ b/arch/powerpc/platforms/85xx/p2020.c
@@ -42,9 +42,8 @@
 #define DBG(fmt, args...)
 #endif
 
-#ifdef CONFIG_MPC85xx_DS
-
 #ifdef CONFIG_PPC_I8259
+
 static void mpc85xx_8259_cascade(struct irq_desc *desc)
 {
 	struct irq_chip *chip = irq_desc_get_chip(desc);
@@ -55,37 +54,21 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
 	}
 	chip->irq_eoi(&desc->irq_data);
 }
-#endif	/* CONFIG_PPC_I8259 */
 
-static void __init mpc85xx_ds_pic_init(void)
+static void mpc85xx_8259_init(void)
 {
-	struct mpic *mpic;
-#ifdef CONFIG_PPC_I8259
 	struct device_node *np;
 	struct device_node *cascade_node = NULL;
 	int cascade_irq;
-#endif
-
-	mpic = mpic_alloc(NULL, 0,
-		  MPIC_BIG_ENDIAN |
-		  MPIC_SINGLE_DEST_CPU,
-		0, 256, " OpenPIC  ");
 
-	BUG_ON(mpic == NULL);
-	mpic_init(mpic);
-
-#ifdef CONFIG_PPC_I8259
-	/* Initialize the i8259 controller */
 	for_each_node_by_type(np, "interrupt-controller")
 	    if (of_device_is_compatible(np, "chrp,iic")) {
 		cascade_node = np;
 		break;
 	}
 
-	if (cascade_node == NULL) {
-		printk(KERN_DEBUG "Could not find i8259 PIC\n");
+	if (cascade_node == NULL)
 		return;
-	}
 
 	cascade_irq = irq_of_parse_and_map(cascade_node, 0);
 	if (!cascade_irq) {
@@ -93,12 +76,30 @@ static void __init mpc85xx_ds_pic_init(void)
 		return;
 	}
 
-	DBG("mpc85xxds: cascade mapped to irq %d\n", cascade_irq);
+	DBG("i8259: cascade mapped to irq %d\n", cascade_irq);
 
 	i8259_init(cascade_node, 0);
 	of_node_put(cascade_node);
 
 	irq_set_chained_handler(cascade_irq, mpc85xx_8259_cascade);
+}
+
+#endif	/* CONFIG_PPC_I8259 */
+
+static void __init p2020_pic_init(void)
+{
+	struct mpic *mpic;
+
+	mpic = mpic_alloc(NULL, 0,
+		  MPIC_BIG_ENDIAN |
+		  MPIC_SINGLE_DEST_CPU,
+		0, 256, " OpenPIC  ");
+
+	BUG_ON(mpic == NULL);
+	mpic_init(mpic);
+
+#ifdef CONFIG_PPC_I8259
+	mpc85xx_8259_init();
 #endif	/* CONFIG_PPC_I8259 */
 }
 
@@ -138,58 +139,20 @@ static void __init mpc85xx_ds_uli_init(void)
 #endif
 }
 
-#endif /* CONFIG_MPC85xx_DS */
-
-#ifdef CONFIG_MPC85xx_RDB
-static void __init mpc85xx_rdb_pic_init(void)
-{
-	struct mpic *mpic;
-
-	mpic = mpic_alloc(NULL, 0,
-	  MPIC_BIG_ENDIAN |
-	  MPIC_SINGLE_DEST_CPU,
-	  0, 256, " OpenPIC  ");
-
-	BUG_ON(mpic == NULL);
-	mpic_init(mpic);
-}
-#endif /* CONFIG_MPC85xx_RDB */
-
 /*
  * Setup the architecture
  */
-#ifdef CONFIG_MPC85xx_DS
-static void __init mpc85xx_ds_setup_arch(void)
+static void __init p2020_setup_arch(void)
 {
-	if (ppc_md.progress)
-		ppc_md.progress("mpc85xx_ds_setup_arch()", 0);
-
 	swiotlb_detect_4g();
 	fsl_pci_assign_primary();
 	mpc85xx_ds_uli_init();
 	mpc85xx_smp_init();
 
-	printk("MPC85xx DS board from Freescale Semiconductor\n");
-}
-#endif /* CONFIG_MPC85xx_DS */
-
-#ifdef CONFIG_MPC85xx_RDB
-static void __init mpc85xx_rdb_setup_arch(void)
-{
-	if (ppc_md.progress)
-		ppc_md.progress("mpc85xx_rdb_setup_arch()", 0);
-
-	mpc85xx_smp_init();
-
-	fsl_pci_assign_primary();
-
 #ifdef CONFIG_QUICC_ENGINE
 	mpc85xx_qe_par_io_init();
-#endif	/* CONFIG_QUICC_ENGINE */
-
-	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
+#endif
 }
-#endif /* CONFIG_MPC85xx_RDB */
 
 #ifdef CONFIG_MPC85xx_DS
 machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
@@ -230,8 +193,8 @@ static int __init p2020_rdb_pc_probe(void)
 define_machine(p2020_ds) {
 	.name			= "P2020 DS",
 	.probe			= p2020_ds_probe,
-	.setup_arch		= mpc85xx_ds_setup_arch,
-	.init_IRQ		= mpc85xx_ds_pic_init,
+	.setup_arch		= p2020_setup_arch,
+	.init_IRQ		= p2020_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
 	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
@@ -246,8 +209,8 @@ define_machine(p2020_ds) {
 define_machine(p2020_rdb) {
 	.name			= "P2020 RDB",
 	.probe			= p2020_rdb_probe,
-	.setup_arch		= mpc85xx_rdb_setup_arch,
-	.init_IRQ		= mpc85xx_rdb_pic_init,
+	.setup_arch		= p2020_setup_arch,
+	.init_IRQ		= p2020_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
 	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
@@ -260,8 +223,8 @@ define_machine(p2020_rdb) {
 define_machine(p2020_rdb_pc) {
 	.name			= "P2020RDB-PC",
 	.probe			= p2020_rdb_pc_probe,
-	.setup_arch		= mpc85xx_rdb_setup_arch,
-	.init_IRQ		= mpc85xx_rdb_pic_init,
+	.setup_arch		= p2020_setup_arch,
+	.init_IRQ		= p2020_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
 	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 5/7] powerpc/85xx: p2020: Define just one machine description
  2022-08-19 19:15 [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
                   ` (3 preceding siblings ...)
  2022-08-19 19:15 ` [PATCH 4/7] powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks Pali Rohár
@ 2022-08-19 19:15 ` Pali Rohár
  2022-09-26 10:02   ` Christophe Leroy
  2022-08-19 19:15 ` [PATCH 6/7] powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020 Pali Rohár
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-08-19 19:15 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Scott Wood, Christophe Leroy, Sinan Akman
  Cc: linuxppc-dev, linux-kernel

Combine machine descriptions and code of all P2020 boards into just one
generic unified P2020 machine description. This allows kernel to boot on
any P2020-based board with P2020 DTS file without need to patch kernel and
define a new machine description in 85xx powerpc platform directory.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/powerpc/platforms/85xx/p2020.c | 83 +++++++----------------------
 1 file changed, 19 insertions(+), 64 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/p2020.c b/arch/powerpc/platforms/85xx/p2020.c
index d327e6c9b838..1a3ffeb47dfc 100644
--- a/arch/powerpc/platforms/85xx/p2020.c
+++ b/arch/powerpc/platforms/85xx/p2020.c
@@ -154,83 +154,38 @@ static void __init p2020_setup_arch(void)
 #endif
 }
 
-#ifdef CONFIG_MPC85xx_DS
-machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
-#endif /* CONFIG_MPC85xx_DS */
-
-#ifdef CONFIG_MPC85xx_RDB
-machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
-machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
-#endif /* CONFIG_MPC85xx_RDB */
+machine_arch_initcall(p2020, mpc85xx_common_publish_devices);
 
 /*
  * Called very early, device-tree isn't unflattened
  */
-#ifdef CONFIG_MPC85xx_DS
-static int __init p2020_ds_probe(void)
-{
-	return !!of_machine_is_compatible("fsl,P2020DS");
-}
-#endif /* CONFIG_MPC85xx_DS */
-
-#ifdef CONFIG_MPC85xx_RDB
-static int __init p2020_rdb_probe(void)
-{
-	if (of_machine_is_compatible("fsl,P2020RDB"))
-		return 1;
-	return 0;
-}
-
-static int __init p2020_rdb_pc_probe(void)
+static int __init p2020_probe(void)
 {
-	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
-		return 1;
-	return 0;
+	struct device_node *p2020_cpu;
+
+	/*
+	 * There is no common compatible string for all P2020 boards.
+	 * The only common thing is "PowerPC,P2020@0" cpu node.
+	 * So check for P2020 board via this cpu node.
+	 */
+	p2020_cpu = of_find_node_by_path("/cpus/PowerPC,P2020@0");
+	if (!p2020_cpu)
+		return 0;
+
+	of_node_put(p2020_cpu);
+	return 1;
 }
-#endif /* CONFIG_MPC85xx_RDB */
-
-#ifdef CONFIG_MPC85xx_DS
-define_machine(p2020_ds) {
-	.name			= "P2020 DS",
-	.probe			= p2020_ds_probe,
-	.setup_arch		= p2020_setup_arch,
-	.init_IRQ		= p2020_pic_init,
-#ifdef CONFIG_PCI
-	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
-	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
-#endif
-	.get_irq		= mpic_get_irq,
-	.calibrate_decr		= generic_calibrate_decr,
-	.progress		= udbg_progress,
-};
-#endif /* CONFIG_MPC85xx_DS */
-
-#ifdef CONFIG_MPC85xx_RDB
-define_machine(p2020_rdb) {
-	.name			= "P2020 RDB",
-	.probe			= p2020_rdb_probe,
-	.setup_arch		= p2020_setup_arch,
-	.init_IRQ		= p2020_pic_init,
-#ifdef CONFIG_PCI
-	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
-	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
-#endif
-	.get_irq		= mpic_get_irq,
-	.calibrate_decr		= generic_calibrate_decr,
-	.progress		= udbg_progress,
-};
 
-define_machine(p2020_rdb_pc) {
-	.name			= "P2020RDB-PC",
-	.probe			= p2020_rdb_pc_probe,
+define_machine(p2020) {
+	.name			= "Freescale P2020",
+	.probe			= p2020_probe,
 	.setup_arch		= p2020_setup_arch,
 	.init_IRQ		= p2020_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
-	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
+	.pcibios_fixup_phb	= fsl_pcibios_fixup_phb,
 #endif
 	.get_irq		= mpic_get_irq,
 	.calibrate_decr		= generic_calibrate_decr,
 	.progress		= udbg_progress,
 };
-#endif /* CONFIG_MPC85xx_RDB */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 6/7] powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020
  2022-08-19 19:15 [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
                   ` (4 preceding siblings ...)
  2022-08-19 19:15 ` [PATCH 5/7] powerpc/85xx: p2020: Define just one machine description Pali Rohár
@ 2022-08-19 19:15 ` Pali Rohár
  2022-09-26 10:08   ` Christophe Leroy
  2022-08-19 19:15 ` [PATCH 7/7] powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string Pali Rohár
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-08-19 19:15 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Scott Wood, Christophe Leroy, Sinan Akman
  Cc: linuxppc-dev, linux-kernel

Generic unified P2020 machine description which supports all P2020-based
boards is now in separate file p2020.c. So create a separate config option
CONFIG_P2020 for it.

Previously machine descriptions for P2020 boards were enabled by
CONFIG_MPC85xx_DS or CONFIG_MPC85xx_RDB option. So set CONFIG_P2020 to be
enabled by default when one of those option is enabled.

This allows to compile support for P2020 boards without need to have
enabled support for older mpc85xx boards. And to compile kernel for old
mpc85xx boards without having enabled support for new P2020 boards.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/powerpc/platforms/85xx/Kconfig  | 22 ++++++++++++++++++----
 arch/powerpc/platforms/85xx/Makefile |  3 +--
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
index be16eba0f704..2cb4e9248b42 100644
--- a/arch/powerpc/platforms/85xx/Kconfig
+++ b/arch/powerpc/platforms/85xx/Kconfig
@@ -78,16 +78,16 @@ config MPC8536_DS
 	  This option enables support for the MPC8536 DS board
 
 config MPC85xx_DS
-	bool "Freescale MPC8544 DS / MPC8572 DS / P2020 DS"
+	bool "Freescale MPC8544 DS / MPC8572 DS"
 	select PPC_I8259
 	select DEFAULT_UIMAGE
 	select FSL_ULI1575 if PCI
 	select SWIOTLB
 	help
-	  This option enables support for the MPC8544 DS, MPC8572 DS and P2020 DS boards
+	  This option enables support for the MPC8544 DS and MPC8572 DS boards
 
 config MPC85xx_RDB
-	bool "Freescale P102x MBG/UTM/RDB and P2020 RDB"
+	bool "Freescale P102x MBG/UTM/RDB"
 	select PPC_I8259
 	select DEFAULT_UIMAGE
 	select FSL_ULI1575 if PCI
@@ -95,7 +95,21 @@ config MPC85xx_RDB
 	help
 	  This option enables support for the P1020 MBG PC, P1020 UTM PC,
 	  P1020 RDB PC, P1020 RDB PD, P1020 RDB, P1021 RDB PC, P1024 RDB,
-	  P1025 RDB, P2020 RDB and P2020 RDB PC boards
+	  and P1025 RDB boards
+
+config P2020
+	bool "Freescale P2020"
+	default y if MPC85xx_DS || MPC85xx_RDB
+	select DEFAULT_UIMAGE
+	select SWIOTLB
+	imply PPC_I8259
+	imply FSL_ULI1575 if PCI
+	help
+	  This option enables generic unified support for any board with the
+	  Freescale P2020 processor.
+
+	  For example: P2020 DS board, P2020 RDB board, P2020 RDB PC board or
+	  CZ.NIC Turris 1.x boards.
 
 config P1010_RDB
 	bool "Freescale P1010 RDB"
diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index 1ad261b4eeb6..021e168442d7 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -23,8 +23,7 @@ obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
 obj-$(CONFIG_P1022_DS)    += p1022_ds.o
 obj-$(CONFIG_P1022_RDK)   += p1022_rdk.o
 obj-$(CONFIG_P1023_RDB)   += p1023_rdb.o
-obj-$(CONFIG_MPC85xx_DS)  += p2020.o
-obj-$(CONFIG_MPC85xx_RDB) += p2020.o
+obj-$(CONFIG_P2020)       += p2020.o
 obj-$(CONFIG_TWR_P102x)   += twr_p102x.o
 obj-$(CONFIG_CORENET_GENERIC)   += corenet_generic.o
 obj-$(CONFIG_FB_FSL_DIU)	+= t1042rdb_diu.o
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 7/7] powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string
  2022-08-19 19:15 [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
                   ` (5 preceding siblings ...)
  2022-08-19 19:15 ` [PATCH 6/7] powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020 Pali Rohár
@ 2022-08-19 19:15 ` Pali Rohár
  2022-09-26 10:10   ` Christophe Leroy
  2022-09-24 12:42 ` [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
  2022-12-04 10:54 ` Pali Rohár
  8 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-08-19 19:15 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Scott Wood, Christophe Leroy, Sinan Akman
  Cc: linuxppc-dev, linux-kernel

"fsl,P2020RDB-PC" compatible string was present in Turris 1.x DTS file just
because Linux kernel required it for proper detection of P2020 processor
during boot.

This was quite a hack as CZ,NIC Turris 1.x is not compatible with
Freescale P2020-RDB-PC board.

Now when kernel has generic unified support for boards with P2020
processors, there is no need to have this "hack" in turris1x.dts file.

So remove incorrect "fsl,P2020RDB-PC" compatible string from turris1x.dts.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/powerpc/boot/dts/turris1x.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/turris1x.dts b/arch/powerpc/boot/dts/turris1x.dts
index 12e08271e61f..69c38ed8a3a5 100644
--- a/arch/powerpc/boot/dts/turris1x.dts
+++ b/arch/powerpc/boot/dts/turris1x.dts
@@ -15,7 +15,7 @@
 
 / {
 	model = "Turris 1.x";
-	compatible = "cznic,turris1x", "fsl,P2020RDB-PC"; /* fsl,P2020RDB-PC is required for booting Linux */
+	compatible = "cznic,turris1x";
 
 	aliases {
 		ethernet0 = &enet0;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description
  2022-08-19 19:15 [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
                   ` (6 preceding siblings ...)
  2022-08-19 19:15 ` [PATCH 7/7] powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string Pali Rohár
@ 2022-09-24 12:42 ` Pali Rohár
  2022-12-04 10:54 ` Pali Rohár
  8 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2022-09-24 12:42 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Scott Wood, Christophe Leroy, Sinan Akman
  Cc: linuxppc-dev, linux-kernel

Hello! Any comments for these patches?

On Friday 19 August 2022 21:15:50 Pali Rohár wrote:
> This patch series unifies all P2020 boards and machine descriptions into
> one generic unified P2020 machine description. With this generic machine
> description, kernel can boot on any P2020-based board with correct DTS
> file.
> 
> Tested on CZ.NIC Turris 1.1 board with has Freescale P2020 processor.
> Kernel during booting correctly detects P2020 and prints:
> [    0.000000] Using Freescale P2020 machine description
> 
> Pali Rohár (7):
>   powerpc/85xx: Mark mpc85xx_rdb_pic_init() as static
>   powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
>   powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
>   powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks
>   powerpc/85xx: p2020: Define just one machine description
>   powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020
>   powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string
> 
>  arch/powerpc/boot/dts/turris1x.dts            |   2 +-
>  arch/powerpc/platforms/85xx/Kconfig           |  22 ++-
>  arch/powerpc/platforms/85xx/Makefile          |   1 +
>  arch/powerpc/platforms/85xx/mpc85xx_ds.c      |  25 +--
>  arch/powerpc/platforms/85xx/mpc85xx_rdb.c     |  46 +-----
>  .../platforms/85xx/{mpc85xx_ds.c => p2020.c}  | 144 +++++++-----------
>  6 files changed, 75 insertions(+), 165 deletions(-)
>  copy arch/powerpc/platforms/85xx/{mpc85xx_ds.c => p2020.c} (53%)
> 
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
  2022-08-19 19:15 ` [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() " Pali Rohár
@ 2022-09-26  9:43   ` Christophe Leroy
  2022-09-26  9:47     ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Christophe Leroy @ 2022-09-26  9:43 UTC (permalink / raw)
  To: Pali Rohár, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Scott Wood, Sinan Akman
  Cc: linuxppc-dev, linux-kernel



Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> Function mpc85xx_ds_pic_init() is not used out of the mpc85xx_ds.c file.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

This patch should be squashed into patch 1.

> ---
>   arch/powerpc/platforms/85xx/mpc85xx_ds.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> index f8d2c97f39bd..9a6d637ef54a 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> @@ -54,7 +54,7 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
>   }
>   #endif	/* CONFIG_PPC_I8259 */
>   
> -void __init mpc85xx_ds_pic_init(void)
> +static void __init mpc85xx_ds_pic_init(void)
>   {
>   	struct mpic *mpic;
>   #ifdef CONFIG_PPC_I8259

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
  2022-09-26  9:43   ` Christophe Leroy
@ 2022-09-26  9:47     ` Pali Rohár
  2022-10-16 11:05       ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-09-26  9:47 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

On Monday 26 September 2022 09:43:55 Christophe Leroy wrote:
> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> > Function mpc85xx_ds_pic_init() is not used out of the mpc85xx_ds.c file.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> 
> This patch should be squashed into patch 1.

No problem. Just to explain that I split those changes into different
patches because they touch different files and different board code.
And I thought that different things should be in different patches.

> > ---
> >   arch/powerpc/platforms/85xx/mpc85xx_ds.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > index f8d2c97f39bd..9a6d637ef54a 100644
> > --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > @@ -54,7 +54,7 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
> >   }
> >   #endif	/* CONFIG_PPC_I8259 */
> >   
> > -void __init mpc85xx_ds_pic_init(void)
> > +static void __init mpc85xx_ds_pic_init(void)
> >   {
> >   	struct mpic *mpic;
> >   #ifdef CONFIG_PPC_I8259

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
  2022-08-19 19:15 ` [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c Pali Rohár
@ 2022-09-26  9:48   ` Christophe Leroy
  2022-09-26  9:53     ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Christophe Leroy @ 2022-09-26  9:48 UTC (permalink / raw)
  To: Pali Rohár, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Scott Wood, Sinan Akman
  Cc: linuxppc-dev, linux-kernel



Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> This moves machine descriptions and all related code for all P2020 boards
> into new p2020.c source file. This is preparation for code deduplication
> and providing one unified machine description for all P2020 boards.

I'm having hard time to review this patch.

It looks like you are doing much more than just moving machine 
descriptions and related code into p2020.c

Apparently p2020.c has a lot of code that doesn't seem be move from 
somewhere else.

Maybe there is a need to tidy up in order to ease reviewing.

> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>   arch/powerpc/platforms/85xx/Makefile          |   2 +
>   arch/powerpc/platforms/85xx/mpc85xx_ds.c      |  23 ---
>   arch/powerpc/platforms/85xx/mpc85xx_rdb.c     |  44 ------
>   .../platforms/85xx/{mpc85xx_ds.c => p2020.c}  | 134 ++++++++++++------
>   4 files changed, 91 insertions(+), 112 deletions(-)
>   copy arch/powerpc/platforms/85xx/{mpc85xx_ds.c => p2020.c} (65%)
> 
> diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> index 260fbad7967b..1ad261b4eeb6 100644
> --- a/arch/powerpc/platforms/85xx/Makefile
> +++ b/arch/powerpc/platforms/85xx/Makefile
> @@ -23,6 +23,8 @@ obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
>   obj-$(CONFIG_P1022_DS)    += p1022_ds.o
>   obj-$(CONFIG_P1022_RDK)   += p1022_rdk.o
>   obj-$(CONFIG_P1023_RDB)   += p1023_rdb.o
> +obj-$(CONFIG_MPC85xx_DS)  += p2020.o
> +obj-$(CONFIG_MPC85xx_RDB) += p2020.o
>   obj-$(CONFIG_TWR_P102x)   += twr_p102x.o
>   obj-$(CONFIG_CORENET_GENERIC)   += corenet_generic.o
>   obj-$(CONFIG_FB_FSL_DIU)	+= t1042rdb_diu.o
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> index 9a6d637ef54a..05aac997b5ed 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> @@ -168,7 +168,6 @@ static int __init mpc8544_ds_probe(void)
>   
>   machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
>   machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
> -machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
>   
>   /*
>    * Called very early, device-tree isn't unflattened
> @@ -178,14 +177,6 @@ static int __init mpc8572_ds_probe(void)
>   	return !!of_machine_is_compatible("fsl,MPC8572DS");
>   }
>   
> -/*
> - * Called very early, device-tree isn't unflattened
> - */
> -static int __init p2020_ds_probe(void)
> -{
> -	return !!of_machine_is_compatible("fsl,P2020DS");
> -}
> -
>   define_machine(mpc8544_ds) {
>   	.name			= "MPC8544 DS",
>   	.probe			= mpc8544_ds_probe,
> @@ -213,17 +204,3 @@ define_machine(mpc8572_ds) {
>   	.calibrate_decr		= generic_calibrate_decr,
>   	.progress		= udbg_progress,
>   };
> -
> -define_machine(p2020_ds) {
> -	.name			= "P2020 DS",
> -	.probe			= p2020_ds_probe,
> -	.setup_arch		= mpc85xx_ds_setup_arch,
> -	.init_IRQ		= mpc85xx_ds_pic_init,
> -#ifdef CONFIG_PCI
> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> -#endif
> -	.get_irq		= mpic_get_irq,
> -	.calibrate_decr		= generic_calibrate_decr,
> -	.progress		= udbg_progress,
> -};
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> index b6129c148fea..05f1ed635735 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> @@ -108,8 +108,6 @@ static void __init mpc85xx_rdb_setup_arch(void)
>   	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
>   }
>   
> -machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> -machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
>   machine_arch_initcall(p1020_mbg_pc, mpc85xx_common_publish_devices);
>   machine_arch_initcall(p1020_rdb, mpc85xx_common_publish_devices);
>   machine_arch_initcall(p1020_rdb_pc, mpc85xx_common_publish_devices);
> @@ -122,13 +120,6 @@ machine_arch_initcall(p1024_rdb, mpc85xx_common_publish_devices);
>   /*
>    * Called very early, device-tree isn't unflattened
>    */
> -static int __init p2020_rdb_probe(void)
> -{
> -	if (of_machine_is_compatible("fsl,P2020RDB"))
> -		return 1;
> -	return 0;
> -}
> -
>   static int __init p1020_rdb_probe(void)
>   {
>   	if (of_machine_is_compatible("fsl,P1020RDB"))
> @@ -153,13 +144,6 @@ static int __init p1021_rdb_pc_probe(void)
>   	return 0;
>   }
>   
> -static int __init p2020_rdb_pc_probe(void)
> -{
> -	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> -		return 1;
> -	return 0;
> -}
> -
>   static int __init p1025_rdb_probe(void)
>   {
>   	return of_machine_is_compatible("fsl,P1025RDB");
> @@ -180,20 +164,6 @@ static int __init p1024_rdb_probe(void)
>   	return of_machine_is_compatible("fsl,P1024RDB");
>   }
>   
> -define_machine(p2020_rdb) {
> -	.name			= "P2020 RDB",
> -	.probe			= p2020_rdb_probe,
> -	.setup_arch		= mpc85xx_rdb_setup_arch,
> -	.init_IRQ		= mpc85xx_rdb_pic_init,
> -#ifdef CONFIG_PCI
> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> -#endif
> -	.get_irq		= mpic_get_irq,
> -	.calibrate_decr		= generic_calibrate_decr,
> -	.progress		= udbg_progress,
> -};
> -
>   define_machine(p1020_rdb) {
>   	.name			= "P1020 RDB",
>   	.probe			= p1020_rdb_probe,
> @@ -222,20 +192,6 @@ define_machine(p1021_rdb_pc) {
>   	.progress		= udbg_progress,
>   };
>   
> -define_machine(p2020_rdb_pc) {
> -	.name			= "P2020RDB-PC",
> -	.probe			= p2020_rdb_pc_probe,
> -	.setup_arch		= mpc85xx_rdb_setup_arch,
> -	.init_IRQ		= mpc85xx_rdb_pic_init,
> -#ifdef CONFIG_PCI
> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> -#endif
> -	.get_irq		= mpic_get_irq,
> -	.calibrate_decr		= generic_calibrate_decr,
> -	.progress		= udbg_progress,
> -};
> -
>   define_machine(p1025_rdb) {
>   	.name			= "P1025 RDB",
>   	.probe			= p1025_rdb_probe,
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/p2020.c
> similarity index 65%
> copy from arch/powerpc/platforms/85xx/mpc85xx_ds.c
> copy to arch/powerpc/platforms/85xx/p2020.c
> index 9a6d637ef54a..d65d4c88ac47 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> +++ b/arch/powerpc/platforms/85xx/p2020.c
> @@ -1,11 +1,9 @@
>   // SPDX-License-Identifier: GPL-2.0-or-later
>   /*
> - * MPC85xx DS Board Setup
> + * Freescale P2020 board Setup
>    *
> - * Author Xianghua Xiao (x.xiao@freescale.com)
> - * Roy Zang <tie-fei.zang@freescale.com>
> - * 	- Add PCI/PCI Exprees support
> - * Copyright 2007 Freescale Semiconductor Inc.
> + * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
> + * Copyright 2022 Pali Rohár <pali@kernel.org>
>    */
>   
>   #include <linux/stddef.h>
> @@ -17,6 +15,7 @@
>   #include <linux/interrupt.h>
>   #include <linux/of_irq.h>
>   #include <linux/of_platform.h>
> +#include <linux/fsl/guts.h>
>   
>   #include <asm/time.h>
>   #include <asm/machdep.h>
> @@ -27,6 +26,8 @@
>   #include <asm/i8259.h>
>   #include <asm/swiotlb.h>
>   
> +#include <soc/fsl/qe/qe.h>
> +
>   #include <sysdev/fsl_soc.h>
>   #include <sysdev/fsl_pci.h>
>   #include "smp.h"
> @@ -41,6 +42,8 @@
>   #define DBG(fmt, args...)
>   #endif
>   
> +#ifdef CONFIG_MPC85xx_DS
> +
>   #ifdef CONFIG_PPC_I8259
>   static void mpc85xx_8259_cascade(struct irq_desc *desc)
>   {
> @@ -62,18 +65,11 @@ static void __init mpc85xx_ds_pic_init(void)
>   	struct device_node *cascade_node = NULL;
>   	int cascade_irq;
>   #endif
> -	if (of_machine_is_compatible("fsl,MPC8572DS-CAMP")) {
> -		mpic = mpic_alloc(NULL, 0,
> -			MPIC_NO_RESET |
> -			MPIC_BIG_ENDIAN |
> -			MPIC_SINGLE_DEST_CPU,
> -			0, 256, " OpenPIC  ");
> -	} else {
> -		mpic = mpic_alloc(NULL, 0,
> -			  MPIC_BIG_ENDIAN |
> -			  MPIC_SINGLE_DEST_CPU,
> -			0, 256, " OpenPIC  ");
> -	}
> +
> +	mpic = mpic_alloc(NULL, 0,
> +		  MPIC_BIG_ENDIAN |
> +		  MPIC_SINGLE_DEST_CPU,
> +		0, 256, " OpenPIC  ");
>   
>   	BUG_ON(mpic == NULL);
>   	mpic_init(mpic);
> @@ -142,9 +138,27 @@ static void __init mpc85xx_ds_uli_init(void)
>   #endif
>   }
>   
> +#endif /* CONFIG_MPC85xx_DS */
> +
> +#ifdef CONFIG_MPC85xx_RDB
> +static void __init mpc85xx_rdb_pic_init(void)
> +{
> +	struct mpic *mpic;
> +
> +	mpic = mpic_alloc(NULL, 0,
> +	  MPIC_BIG_ENDIAN |
> +	  MPIC_SINGLE_DEST_CPU,
> +	  0, 256, " OpenPIC  ");
> +
> +	BUG_ON(mpic == NULL);
> +	mpic_init(mpic);
> +}
> +#endif /* CONFIG_MPC85xx_RDB */
> +
>   /*
>    * Setup the architecture
>    */
> +#ifdef CONFIG_MPC85xx_DS
>   static void __init mpc85xx_ds_setup_arch(void)
>   {
>   	if (ppc_md.progress)
> @@ -157,38 +171,65 @@ static void __init mpc85xx_ds_setup_arch(void)
>   
>   	printk("MPC85xx DS board from Freescale Semiconductor\n");
>   }
> +#endif /* CONFIG_MPC85xx_DS */
>   
> -/*
> - * Called very early, device-tree isn't unflattened
> - */
> -static int __init mpc8544_ds_probe(void)
> +#ifdef CONFIG_MPC85xx_RDB
> +static void __init mpc85xx_rdb_setup_arch(void)
>   {
> -	return !!of_machine_is_compatible("MPC8544DS");
> +	if (ppc_md.progress)
> +		ppc_md.progress("mpc85xx_rdb_setup_arch()", 0);
> +
> +	mpc85xx_smp_init();
> +
> +	fsl_pci_assign_primary();
> +
> +#ifdef CONFIG_QUICC_ENGINE
> +	mpc85xx_qe_par_io_init();
> +#endif	/* CONFIG_QUICC_ENGINE */
> +
> +	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
>   }
> +#endif /* CONFIG_MPC85xx_RDB */
>   
> -machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
> -machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
> +#ifdef CONFIG_MPC85xx_DS
>   machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
> +#endif /* CONFIG_MPC85xx_DS */
>   
> -/*
> - * Called very early, device-tree isn't unflattened
> - */
> -static int __init mpc8572_ds_probe(void)
> -{
> -	return !!of_machine_is_compatible("fsl,MPC8572DS");
> -}
> +#ifdef CONFIG_MPC85xx_RDB
> +machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> +machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
> +#endif /* CONFIG_MPC85xx_RDB */
>   
>   /*
>    * Called very early, device-tree isn't unflattened
>    */
> +#ifdef CONFIG_MPC85xx_DS
>   static int __init p2020_ds_probe(void)
>   {
>   	return !!of_machine_is_compatible("fsl,P2020DS");
>   }
> +#endif /* CONFIG_MPC85xx_DS */
> +
> +#ifdef CONFIG_MPC85xx_RDB
> +static int __init p2020_rdb_probe(void)
> +{
> +	if (of_machine_is_compatible("fsl,P2020RDB"))
> +		return 1;
> +	return 0;
> +}
> +
> +static int __init p2020_rdb_pc_probe(void)
> +{
> +	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> +		return 1;
> +	return 0;
> +}
> +#endif /* CONFIG_MPC85xx_RDB */
>   
> -define_machine(mpc8544_ds) {
> -	.name			= "MPC8544 DS",
> -	.probe			= mpc8544_ds_probe,
> +#ifdef CONFIG_MPC85xx_DS
> +define_machine(p2020_ds) {
> +	.name			= "P2020 DS",
> +	.probe			= p2020_ds_probe,
>   	.setup_arch		= mpc85xx_ds_setup_arch,
>   	.init_IRQ		= mpc85xx_ds_pic_init,
>   #ifdef CONFIG_PCI
> @@ -199,12 +240,14 @@ define_machine(mpc8544_ds) {
>   	.calibrate_decr		= generic_calibrate_decr,
>   	.progress		= udbg_progress,
>   };
> -
> -define_machine(mpc8572_ds) {
> -	.name			= "MPC8572 DS",
> -	.probe			= mpc8572_ds_probe,
> -	.setup_arch		= mpc85xx_ds_setup_arch,
> -	.init_IRQ		= mpc85xx_ds_pic_init,
> +#endif /* CONFIG_MPC85xx_DS */
> +
> +#ifdef CONFIG_MPC85xx_RDB
> +define_machine(p2020_rdb) {
> +	.name			= "P2020 RDB",
> +	.probe			= p2020_rdb_probe,
> +	.setup_arch		= mpc85xx_rdb_setup_arch,
> +	.init_IRQ		= mpc85xx_rdb_pic_init,
>   #ifdef CONFIG_PCI
>   	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>   	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> @@ -214,11 +257,11 @@ define_machine(mpc8572_ds) {
>   	.progress		= udbg_progress,
>   };
>   
> -define_machine(p2020_ds) {
> -	.name			= "P2020 DS",
> -	.probe			= p2020_ds_probe,
> -	.setup_arch		= mpc85xx_ds_setup_arch,
> -	.init_IRQ		= mpc85xx_ds_pic_init,
> +define_machine(p2020_rdb_pc) {
> +	.name			= "P2020RDB-PC",
> +	.probe			= p2020_rdb_pc_probe,
> +	.setup_arch		= mpc85xx_rdb_setup_arch,
> +	.init_IRQ		= mpc85xx_rdb_pic_init,
>   #ifdef CONFIG_PCI
>   	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>   	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> @@ -227,3 +270,4 @@ define_machine(p2020_ds) {
>   	.calibrate_decr		= generic_calibrate_decr,
>   	.progress		= udbg_progress,
>   };
> +#endif /* CONFIG_MPC85xx_RDB */

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
  2022-09-26  9:48   ` Christophe Leroy
@ 2022-09-26  9:53     ` Pali Rohár
  2022-09-26 10:17       ` Christophe Leroy
  0 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-09-26  9:53 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

On Monday 26 September 2022 09:48:02 Christophe Leroy wrote:
> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> > This moves machine descriptions and all related code for all P2020 boards
> > into new p2020.c source file. This is preparation for code deduplication
> > and providing one unified machine description for all P2020 boards.
> 
> I'm having hard time to review this patch.
> 
> It looks like you are doing much more than just moving machine 
> descriptions and related code into p2020.c
> 
> Apparently p2020.c has a lot of code that doesn't seem be move from 
> somewhere else.
> 
> Maybe there is a need to tidy up in order to ease reviewing.

This is probably harder to read due to how git format-patch generated
this email. The important is:

 copy from arch/powerpc/platforms/85xx/mpc85xx_ds.c
 copy to arch/powerpc/platforms/85xx/p2020.c

Which means that git thinks that my newly introduced file p2020.c is
similar to old file mpc85xx_ds.c and generated diff in format which do:

 1. copy mpc85xx_ds.c to p2020.c
 2. apply diff on newly introduced file p2020.c

Code is really moved from mpc85xx_ds.c and mpc85xx_rdb.c files into file
p2020.c.

File p2020.c is new in this patch.

> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >   arch/powerpc/platforms/85xx/Makefile          |   2 +
> >   arch/powerpc/platforms/85xx/mpc85xx_ds.c      |  23 ---
> >   arch/powerpc/platforms/85xx/mpc85xx_rdb.c     |  44 ------
> >   .../platforms/85xx/{mpc85xx_ds.c => p2020.c}  | 134 ++++++++++++------
> >   4 files changed, 91 insertions(+), 112 deletions(-)
> >   copy arch/powerpc/platforms/85xx/{mpc85xx_ds.c => p2020.c} (65%)
> > 
> > diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> > index 260fbad7967b..1ad261b4eeb6 100644
> > --- a/arch/powerpc/platforms/85xx/Makefile
> > +++ b/arch/powerpc/platforms/85xx/Makefile
> > @@ -23,6 +23,8 @@ obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
> >   obj-$(CONFIG_P1022_DS)    += p1022_ds.o
> >   obj-$(CONFIG_P1022_RDK)   += p1022_rdk.o
> >   obj-$(CONFIG_P1023_RDB)   += p1023_rdb.o
> > +obj-$(CONFIG_MPC85xx_DS)  += p2020.o
> > +obj-$(CONFIG_MPC85xx_RDB) += p2020.o
> >   obj-$(CONFIG_TWR_P102x)   += twr_p102x.o
> >   obj-$(CONFIG_CORENET_GENERIC)   += corenet_generic.o
> >   obj-$(CONFIG_FB_FSL_DIU)	+= t1042rdb_diu.o
> > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > index 9a6d637ef54a..05aac997b5ed 100644
> > --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > @@ -168,7 +168,6 @@ static int __init mpc8544_ds_probe(void)
> >   
> >   machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
> >   machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
> > -machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
> >   
> >   /*
> >    * Called very early, device-tree isn't unflattened
> > @@ -178,14 +177,6 @@ static int __init mpc8572_ds_probe(void)
> >   	return !!of_machine_is_compatible("fsl,MPC8572DS");
> >   }
> >   
> > -/*
> > - * Called very early, device-tree isn't unflattened
> > - */
> > -static int __init p2020_ds_probe(void)
> > -{
> > -	return !!of_machine_is_compatible("fsl,P2020DS");
> > -}
> > -
> >   define_machine(mpc8544_ds) {
> >   	.name			= "MPC8544 DS",
> >   	.probe			= mpc8544_ds_probe,
> > @@ -213,17 +204,3 @@ define_machine(mpc8572_ds) {
> >   	.calibrate_decr		= generic_calibrate_decr,
> >   	.progress		= udbg_progress,
> >   };
> > -
> > -define_machine(p2020_ds) {
> > -	.name			= "P2020 DS",
> > -	.probe			= p2020_ds_probe,
> > -	.setup_arch		= mpc85xx_ds_setup_arch,
> > -	.init_IRQ		= mpc85xx_ds_pic_init,
> > -#ifdef CONFIG_PCI
> > -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> > -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> > -#endif
> > -	.get_irq		= mpic_get_irq,
> > -	.calibrate_decr		= generic_calibrate_decr,
> > -	.progress		= udbg_progress,
> > -};
> > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> > index b6129c148fea..05f1ed635735 100644
> > --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> > +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> > @@ -108,8 +108,6 @@ static void __init mpc85xx_rdb_setup_arch(void)
> >   	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
> >   }
> >   
> > -machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> > -machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
> >   machine_arch_initcall(p1020_mbg_pc, mpc85xx_common_publish_devices);
> >   machine_arch_initcall(p1020_rdb, mpc85xx_common_publish_devices);
> >   machine_arch_initcall(p1020_rdb_pc, mpc85xx_common_publish_devices);
> > @@ -122,13 +120,6 @@ machine_arch_initcall(p1024_rdb, mpc85xx_common_publish_devices);
> >   /*
> >    * Called very early, device-tree isn't unflattened
> >    */
> > -static int __init p2020_rdb_probe(void)
> > -{
> > -	if (of_machine_is_compatible("fsl,P2020RDB"))
> > -		return 1;
> > -	return 0;
> > -}
> > -
> >   static int __init p1020_rdb_probe(void)
> >   {
> >   	if (of_machine_is_compatible("fsl,P1020RDB"))
> > @@ -153,13 +144,6 @@ static int __init p1021_rdb_pc_probe(void)
> >   	return 0;
> >   }
> >   
> > -static int __init p2020_rdb_pc_probe(void)
> > -{
> > -	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> > -		return 1;
> > -	return 0;
> > -}
> > -
> >   static int __init p1025_rdb_probe(void)
> >   {
> >   	return of_machine_is_compatible("fsl,P1025RDB");
> > @@ -180,20 +164,6 @@ static int __init p1024_rdb_probe(void)
> >   	return of_machine_is_compatible("fsl,P1024RDB");
> >   }
> >   
> > -define_machine(p2020_rdb) {
> > -	.name			= "P2020 RDB",
> > -	.probe			= p2020_rdb_probe,
> > -	.setup_arch		= mpc85xx_rdb_setup_arch,
> > -	.init_IRQ		= mpc85xx_rdb_pic_init,
> > -#ifdef CONFIG_PCI
> > -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> > -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> > -#endif
> > -	.get_irq		= mpic_get_irq,
> > -	.calibrate_decr		= generic_calibrate_decr,
> > -	.progress		= udbg_progress,
> > -};
> > -
> >   define_machine(p1020_rdb) {
> >   	.name			= "P1020 RDB",
> >   	.probe			= p1020_rdb_probe,
> > @@ -222,20 +192,6 @@ define_machine(p1021_rdb_pc) {
> >   	.progress		= udbg_progress,
> >   };
> >   
> > -define_machine(p2020_rdb_pc) {
> > -	.name			= "P2020RDB-PC",
> > -	.probe			= p2020_rdb_pc_probe,
> > -	.setup_arch		= mpc85xx_rdb_setup_arch,
> > -	.init_IRQ		= mpc85xx_rdb_pic_init,
> > -#ifdef CONFIG_PCI
> > -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> > -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> > -#endif
> > -	.get_irq		= mpic_get_irq,
> > -	.calibrate_decr		= generic_calibrate_decr,
> > -	.progress		= udbg_progress,
> > -};
> > -
> >   define_machine(p1025_rdb) {
> >   	.name			= "P1025 RDB",
> >   	.probe			= p1025_rdb_probe,
> > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/p2020.c
> > similarity index 65%
> > copy from arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > copy to arch/powerpc/platforms/85xx/p2020.c
> > index 9a6d637ef54a..d65d4c88ac47 100644
> > --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > +++ b/arch/powerpc/platforms/85xx/p2020.c
> > @@ -1,11 +1,9 @@
> >   // SPDX-License-Identifier: GPL-2.0-or-later
> >   /*
> > - * MPC85xx DS Board Setup
> > + * Freescale P2020 board Setup
> >    *
> > - * Author Xianghua Xiao (x.xiao@freescale.com)
> > - * Roy Zang <tie-fei.zang@freescale.com>
> > - * 	- Add PCI/PCI Exprees support
> > - * Copyright 2007 Freescale Semiconductor Inc.
> > + * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
> > + * Copyright 2022 Pali Rohár <pali@kernel.org>
> >    */
> >   
> >   #include <linux/stddef.h>
> > @@ -17,6 +15,7 @@
> >   #include <linux/interrupt.h>
> >   #include <linux/of_irq.h>
> >   #include <linux/of_platform.h>
> > +#include <linux/fsl/guts.h>
> >   
> >   #include <asm/time.h>
> >   #include <asm/machdep.h>
> > @@ -27,6 +26,8 @@
> >   #include <asm/i8259.h>
> >   #include <asm/swiotlb.h>
> >   
> > +#include <soc/fsl/qe/qe.h>
> > +
> >   #include <sysdev/fsl_soc.h>
> >   #include <sysdev/fsl_pci.h>
> >   #include "smp.h"
> > @@ -41,6 +42,8 @@
> >   #define DBG(fmt, args...)
> >   #endif
> >   
> > +#ifdef CONFIG_MPC85xx_DS
> > +
> >   #ifdef CONFIG_PPC_I8259
> >   static void mpc85xx_8259_cascade(struct irq_desc *desc)
> >   {
> > @@ -62,18 +65,11 @@ static void __init mpc85xx_ds_pic_init(void)
> >   	struct device_node *cascade_node = NULL;
> >   	int cascade_irq;
> >   #endif
> > -	if (of_machine_is_compatible("fsl,MPC8572DS-CAMP")) {
> > -		mpic = mpic_alloc(NULL, 0,
> > -			MPIC_NO_RESET |
> > -			MPIC_BIG_ENDIAN |
> > -			MPIC_SINGLE_DEST_CPU,
> > -			0, 256, " OpenPIC  ");
> > -	} else {
> > -		mpic = mpic_alloc(NULL, 0,
> > -			  MPIC_BIG_ENDIAN |
> > -			  MPIC_SINGLE_DEST_CPU,
> > -			0, 256, " OpenPIC  ");
> > -	}
> > +
> > +	mpic = mpic_alloc(NULL, 0,
> > +		  MPIC_BIG_ENDIAN |
> > +		  MPIC_SINGLE_DEST_CPU,
> > +		0, 256, " OpenPIC  ");
> >   
> >   	BUG_ON(mpic == NULL);
> >   	mpic_init(mpic);
> > @@ -142,9 +138,27 @@ static void __init mpc85xx_ds_uli_init(void)
> >   #endif
> >   }
> >   
> > +#endif /* CONFIG_MPC85xx_DS */
> > +
> > +#ifdef CONFIG_MPC85xx_RDB
> > +static void __init mpc85xx_rdb_pic_init(void)
> > +{
> > +	struct mpic *mpic;
> > +
> > +	mpic = mpic_alloc(NULL, 0,
> > +	  MPIC_BIG_ENDIAN |
> > +	  MPIC_SINGLE_DEST_CPU,
> > +	  0, 256, " OpenPIC  ");
> > +
> > +	BUG_ON(mpic == NULL);
> > +	mpic_init(mpic);
> > +}
> > +#endif /* CONFIG_MPC85xx_RDB */
> > +
> >   /*
> >    * Setup the architecture
> >    */
> > +#ifdef CONFIG_MPC85xx_DS
> >   static void __init mpc85xx_ds_setup_arch(void)
> >   {
> >   	if (ppc_md.progress)
> > @@ -157,38 +171,65 @@ static void __init mpc85xx_ds_setup_arch(void)
> >   
> >   	printk("MPC85xx DS board from Freescale Semiconductor\n");
> >   }
> > +#endif /* CONFIG_MPC85xx_DS */
> >   
> > -/*
> > - * Called very early, device-tree isn't unflattened
> > - */
> > -static int __init mpc8544_ds_probe(void)
> > +#ifdef CONFIG_MPC85xx_RDB
> > +static void __init mpc85xx_rdb_setup_arch(void)
> >   {
> > -	return !!of_machine_is_compatible("MPC8544DS");
> > +	if (ppc_md.progress)
> > +		ppc_md.progress("mpc85xx_rdb_setup_arch()", 0);
> > +
> > +	mpc85xx_smp_init();
> > +
> > +	fsl_pci_assign_primary();
> > +
> > +#ifdef CONFIG_QUICC_ENGINE
> > +	mpc85xx_qe_par_io_init();
> > +#endif	/* CONFIG_QUICC_ENGINE */
> > +
> > +	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
> >   }
> > +#endif /* CONFIG_MPC85xx_RDB */
> >   
> > -machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
> > -machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
> > +#ifdef CONFIG_MPC85xx_DS
> >   machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
> > +#endif /* CONFIG_MPC85xx_DS */
> >   
> > -/*
> > - * Called very early, device-tree isn't unflattened
> > - */
> > -static int __init mpc8572_ds_probe(void)
> > -{
> > -	return !!of_machine_is_compatible("fsl,MPC8572DS");
> > -}
> > +#ifdef CONFIG_MPC85xx_RDB
> > +machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> > +machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
> > +#endif /* CONFIG_MPC85xx_RDB */
> >   
> >   /*
> >    * Called very early, device-tree isn't unflattened
> >    */
> > +#ifdef CONFIG_MPC85xx_DS
> >   static int __init p2020_ds_probe(void)
> >   {
> >   	return !!of_machine_is_compatible("fsl,P2020DS");
> >   }
> > +#endif /* CONFIG_MPC85xx_DS */
> > +
> > +#ifdef CONFIG_MPC85xx_RDB
> > +static int __init p2020_rdb_probe(void)
> > +{
> > +	if (of_machine_is_compatible("fsl,P2020RDB"))
> > +		return 1;
> > +	return 0;
> > +}
> > +
> > +static int __init p2020_rdb_pc_probe(void)
> > +{
> > +	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> > +		return 1;
> > +	return 0;
> > +}
> > +#endif /* CONFIG_MPC85xx_RDB */
> >   
> > -define_machine(mpc8544_ds) {
> > -	.name			= "MPC8544 DS",
> > -	.probe			= mpc8544_ds_probe,
> > +#ifdef CONFIG_MPC85xx_DS
> > +define_machine(p2020_ds) {
> > +	.name			= "P2020 DS",
> > +	.probe			= p2020_ds_probe,
> >   	.setup_arch		= mpc85xx_ds_setup_arch,
> >   	.init_IRQ		= mpc85xx_ds_pic_init,
> >   #ifdef CONFIG_PCI
> > @@ -199,12 +240,14 @@ define_machine(mpc8544_ds) {
> >   	.calibrate_decr		= generic_calibrate_decr,
> >   	.progress		= udbg_progress,
> >   };
> > -
> > -define_machine(mpc8572_ds) {
> > -	.name			= "MPC8572 DS",
> > -	.probe			= mpc8572_ds_probe,
> > -	.setup_arch		= mpc85xx_ds_setup_arch,
> > -	.init_IRQ		= mpc85xx_ds_pic_init,
> > +#endif /* CONFIG_MPC85xx_DS */
> > +
> > +#ifdef CONFIG_MPC85xx_RDB
> > +define_machine(p2020_rdb) {
> > +	.name			= "P2020 RDB",
> > +	.probe			= p2020_rdb_probe,
> > +	.setup_arch		= mpc85xx_rdb_setup_arch,
> > +	.init_IRQ		= mpc85xx_rdb_pic_init,
> >   #ifdef CONFIG_PCI
> >   	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> >   	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> > @@ -214,11 +257,11 @@ define_machine(mpc8572_ds) {
> >   	.progress		= udbg_progress,
> >   };
> >   
> > -define_machine(p2020_ds) {
> > -	.name			= "P2020 DS",
> > -	.probe			= p2020_ds_probe,
> > -	.setup_arch		= mpc85xx_ds_setup_arch,
> > -	.init_IRQ		= mpc85xx_ds_pic_init,
> > +define_machine(p2020_rdb_pc) {
> > +	.name			= "P2020RDB-PC",
> > +	.probe			= p2020_rdb_pc_probe,
> > +	.setup_arch		= mpc85xx_rdb_setup_arch,
> > +	.init_IRQ		= mpc85xx_rdb_pic_init,
> >   #ifdef CONFIG_PCI
> >   	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> >   	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> > @@ -227,3 +270,4 @@ define_machine(p2020_ds) {
> >   	.calibrate_decr		= generic_calibrate_decr,
> >   	.progress		= udbg_progress,
> >   };
> > +#endif /* CONFIG_MPC85xx_RDB */

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 4/7] powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks
  2022-08-19 19:15 ` [PATCH 4/7] powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks Pali Rohár
@ 2022-09-26  9:59   ` Christophe Leroy
  0 siblings, 0 replies; 30+ messages in thread
From: Christophe Leroy @ 2022-09-26  9:59 UTC (permalink / raw)
  To: Pali Rohár, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Scott Wood, Sinan Akman
  Cc: linuxppc-dev, linux-kernel



Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> Make just one .setup_arch and one .init_IRQ callback implementation for all
> P2020 board code. This deduplicate repeated and same code.

I think this patch should be split in two parts:

First patch : Create function mpc85xx_8259_init
Second patch : Refactor.

> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>   arch/powerpc/platforms/85xx/p2020.c | 97 +++++++++--------------------
>   1 file changed, 30 insertions(+), 67 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/85xx/p2020.c b/arch/powerpc/platforms/85xx/p2020.c
> index d65d4c88ac47..d327e6c9b838 100644
> --- a/arch/powerpc/platforms/85xx/p2020.c
> +++ b/arch/powerpc/platforms/85xx/p2020.c
> @@ -42,9 +42,8 @@
>   #define DBG(fmt, args...)
>   #endif
>   
> -#ifdef CONFIG_MPC85xx_DS
> -
>   #ifdef CONFIG_PPC_I8259
> +
>   static void mpc85xx_8259_cascade(struct irq_desc *desc)
>   {
>   	struct irq_chip *chip = irq_desc_get_chip(desc);
> @@ -55,37 +54,21 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
>   	}
>   	chip->irq_eoi(&desc->irq_data);
>   }
> -#endif	/* CONFIG_PPC_I8259 */
>   
> -static void __init mpc85xx_ds_pic_init(void)
> +static void mpc85xx_8259_init(void)
>   {
> -	struct mpic *mpic;
> -#ifdef CONFIG_PPC_I8259
>   	struct device_node *np;
>   	struct device_node *cascade_node = NULL;
>   	int cascade_irq;
> -#endif
> -
> -	mpic = mpic_alloc(NULL, 0,
> -		  MPIC_BIG_ENDIAN |
> -		  MPIC_SINGLE_DEST_CPU,
> -		0, 256, " OpenPIC  ");
>   
> -	BUG_ON(mpic == NULL);
> -	mpic_init(mpic);
> -
> -#ifdef CONFIG_PPC_I8259
> -	/* Initialize the i8259 controller */
>   	for_each_node_by_type(np, "interrupt-controller")
>   	    if (of_device_is_compatible(np, "chrp,iic")) {
>   		cascade_node = np;
>   		break;
>   	}
>   
> -	if (cascade_node == NULL) {
> -		printk(KERN_DEBUG "Could not find i8259 PIC\n");
> +	if (cascade_node == NULL)
>   		return;
> -	}
>   
>   	cascade_irq = irq_of_parse_and_map(cascade_node, 0);
>   	if (!cascade_irq) {
> @@ -93,12 +76,30 @@ static void __init mpc85xx_ds_pic_init(void)
>   		return;
>   	}
>   
> -	DBG("mpc85xxds: cascade mapped to irq %d\n", cascade_irq);
> +	DBG("i8259: cascade mapped to irq %d\n", cascade_irq);
>   
>   	i8259_init(cascade_node, 0);
>   	of_node_put(cascade_node);
>   
>   	irq_set_chained_handler(cascade_irq, mpc85xx_8259_cascade);
> +}
> +
> +#endif	/* CONFIG_PPC_I8259 */
> +
> +static void __init p2020_pic_init(void)
> +{
> +	struct mpic *mpic;
> +
> +	mpic = mpic_alloc(NULL, 0,
> +		  MPIC_BIG_ENDIAN |
> +		  MPIC_SINGLE_DEST_CPU,
> +		0, 256, " OpenPIC  ");
> +
> +	BUG_ON(mpic == NULL);
> +	mpic_init(mpic);
> +
> +#ifdef CONFIG_PPC_I8259
> +	mpc85xx_8259_init();
>   #endif	/* CONFIG_PPC_I8259 */
>   }
>   
> @@ -138,58 +139,20 @@ static void __init mpc85xx_ds_uli_init(void)
>   #endif
>   }
>   
> -#endif /* CONFIG_MPC85xx_DS */
> -
> -#ifdef CONFIG_MPC85xx_RDB
> -static void __init mpc85xx_rdb_pic_init(void)
> -{
> -	struct mpic *mpic;
> -
> -	mpic = mpic_alloc(NULL, 0,
> -	  MPIC_BIG_ENDIAN |
> -	  MPIC_SINGLE_DEST_CPU,
> -	  0, 256, " OpenPIC  ");
> -
> -	BUG_ON(mpic == NULL);
> -	mpic_init(mpic);
> -}
> -#endif /* CONFIG_MPC85xx_RDB */
> -
>   /*
>    * Setup the architecture
>    */
> -#ifdef CONFIG_MPC85xx_DS
> -static void __init mpc85xx_ds_setup_arch(void)
> +static void __init p2020_setup_arch(void)
>   {
> -	if (ppc_md.progress)
> -		ppc_md.progress("mpc85xx_ds_setup_arch()", 0);
> -
>   	swiotlb_detect_4g();
>   	fsl_pci_assign_primary();
>   	mpc85xx_ds_uli_init();
>   	mpc85xx_smp_init();
>   
> -	printk("MPC85xx DS board from Freescale Semiconductor\n");
> -}
> -#endif /* CONFIG_MPC85xx_DS */
> -
> -#ifdef CONFIG_MPC85xx_RDB
> -static void __init mpc85xx_rdb_setup_arch(void)
> -{
> -	if (ppc_md.progress)
> -		ppc_md.progress("mpc85xx_rdb_setup_arch()", 0);
> -
> -	mpc85xx_smp_init();
> -
> -	fsl_pci_assign_primary();
> -
>   #ifdef CONFIG_QUICC_ENGINE
>   	mpc85xx_qe_par_io_init();
> -#endif	/* CONFIG_QUICC_ENGINE */
> -
> -	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
> +#endif
>   }
> -#endif /* CONFIG_MPC85xx_RDB */
>   
>   #ifdef CONFIG_MPC85xx_DS
>   machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
> @@ -230,8 +193,8 @@ static int __init p2020_rdb_pc_probe(void)
>   define_machine(p2020_ds) {
>   	.name			= "P2020 DS",
>   	.probe			= p2020_ds_probe,
> -	.setup_arch		= mpc85xx_ds_setup_arch,
> -	.init_IRQ		= mpc85xx_ds_pic_init,
> +	.setup_arch		= p2020_setup_arch,
> +	.init_IRQ		= p2020_pic_init,
>   #ifdef CONFIG_PCI
>   	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>   	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> @@ -246,8 +209,8 @@ define_machine(p2020_ds) {
>   define_machine(p2020_rdb) {
>   	.name			= "P2020 RDB",
>   	.probe			= p2020_rdb_probe,
> -	.setup_arch		= mpc85xx_rdb_setup_arch,
> -	.init_IRQ		= mpc85xx_rdb_pic_init,
> +	.setup_arch		= p2020_setup_arch,
> +	.init_IRQ		= p2020_pic_init,
>   #ifdef CONFIG_PCI
>   	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>   	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> @@ -260,8 +223,8 @@ define_machine(p2020_rdb) {
>   define_machine(p2020_rdb_pc) {
>   	.name			= "P2020RDB-PC",
>   	.probe			= p2020_rdb_pc_probe,
> -	.setup_arch		= mpc85xx_rdb_setup_arch,
> -	.init_IRQ		= mpc85xx_rdb_pic_init,
> +	.setup_arch		= p2020_setup_arch,
> +	.init_IRQ		= p2020_pic_init,
>   #ifdef CONFIG_PCI
>   	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>   	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 5/7] powerpc/85xx: p2020: Define just one machine description
  2022-08-19 19:15 ` [PATCH 5/7] powerpc/85xx: p2020: Define just one machine description Pali Rohár
@ 2022-09-26 10:02   ` Christophe Leroy
  2022-09-26 10:08     ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Christophe Leroy @ 2022-09-26 10:02 UTC (permalink / raw)
  To: Pali Rohár, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Scott Wood, Sinan Akman
  Cc: linuxppc-dev, linux-kernel



Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> Combine machine descriptions and code of all P2020 boards into just one
> generic unified P2020 machine description. This allows kernel to boot on
> any P2020-based board with P2020 DTS file without need to patch kernel and
> define a new machine description in 85xx powerpc platform directory.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>   arch/powerpc/platforms/85xx/p2020.c | 83 +++++++----------------------
>   1 file changed, 19 insertions(+), 64 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/85xx/p2020.c b/arch/powerpc/platforms/85xx/p2020.c
> index d327e6c9b838..1a3ffeb47dfc 100644
> --- a/arch/powerpc/platforms/85xx/p2020.c
> +++ b/arch/powerpc/platforms/85xx/p2020.c
> @@ -154,83 +154,38 @@ static void __init p2020_setup_arch(void)
>   #endif
>   }
>   
> -#ifdef CONFIG_MPC85xx_DS
> -machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
> -#endif /* CONFIG_MPC85xx_DS */
> -
> -#ifdef CONFIG_MPC85xx_RDB
> -machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> -machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
> -#endif /* CONFIG_MPC85xx_RDB */
> +machine_arch_initcall(p2020, mpc85xx_common_publish_devices);
>   
>   /*
>    * Called very early, device-tree isn't unflattened
>    */
> -#ifdef CONFIG_MPC85xx_DS
> -static int __init p2020_ds_probe(void)
> -{
> -	return !!of_machine_is_compatible("fsl,P2020DS");
> -}
> -#endif /* CONFIG_MPC85xx_DS */
> -
> -#ifdef CONFIG_MPC85xx_RDB
> -static int __init p2020_rdb_probe(void)
> -{
> -	if (of_machine_is_compatible("fsl,P2020RDB"))
> -		return 1;
> -	return 0;
> -}
> -
> -static int __init p2020_rdb_pc_probe(void)
> +static int __init p2020_probe(void)
>   {
> -	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> -		return 1;
> -	return 0;
> +	struct device_node *p2020_cpu;
> +
> +	/*
> +	 * There is no common compatible string for all P2020 boards.
> +	 * The only common thing is "PowerPC,P2020@0" cpu node.
> +	 * So check for P2020 board via this cpu node.
> +	 */
> +	p2020_cpu = of_find_node_by_path("/cpus/PowerPC,P2020@0");
> +	if (!p2020_cpu)
> +		return 0;

This looks odd. I though all probe were using the compatible, and in 
fact I have a series in preparation that drops all 
of_machine_is_compatible() checks in probe functions and do it in the 
caller instead, after adding a .compatible string in the machine 
description.

Is there really no compatible that can be used for all p2020 ?

> +
> +	of_node_put(p2020_cpu);
> +	return 1;
>   }
> -#endif /* CONFIG_MPC85xx_RDB */
> -
> -#ifdef CONFIG_MPC85xx_DS
> -define_machine(p2020_ds) {
> -	.name			= "P2020 DS",
> -	.probe			= p2020_ds_probe,
> -	.setup_arch		= p2020_setup_arch,
> -	.init_IRQ		= p2020_pic_init,
> -#ifdef CONFIG_PCI
> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> -#endif
> -	.get_irq		= mpic_get_irq,
> -	.calibrate_decr		= generic_calibrate_decr,
> -	.progress		= udbg_progress,
> -};
> -#endif /* CONFIG_MPC85xx_DS */
> -
> -#ifdef CONFIG_MPC85xx_RDB
> -define_machine(p2020_rdb) {
> -	.name			= "P2020 RDB",
> -	.probe			= p2020_rdb_probe,
> -	.setup_arch		= p2020_setup_arch,
> -	.init_IRQ		= p2020_pic_init,
> -#ifdef CONFIG_PCI
> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> -#endif
> -	.get_irq		= mpic_get_irq,
> -	.calibrate_decr		= generic_calibrate_decr,
> -	.progress		= udbg_progress,
> -};
>   
> -define_machine(p2020_rdb_pc) {
> -	.name			= "P2020RDB-PC",
> -	.probe			= p2020_rdb_pc_probe,
> +define_machine(p2020) {
> +	.name			= "Freescale P2020",
> +	.probe			= p2020_probe,
>   	.setup_arch		= p2020_setup_arch,
>   	.init_IRQ		= p2020_pic_init,
>   #ifdef CONFIG_PCI
>   	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> +	.pcibios_fixup_phb	= fsl_pcibios_fixup_phb,
>   #endif
>   	.get_irq		= mpic_get_irq,
>   	.calibrate_decr		= generic_calibrate_decr,
>   	.progress		= udbg_progress,
>   };
> -#endif /* CONFIG_MPC85xx_RDB */

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020
  2022-08-19 19:15 ` [PATCH 6/7] powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020 Pali Rohár
@ 2022-09-26 10:08   ` Christophe Leroy
  2022-09-26 10:19     ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Christophe Leroy @ 2022-09-26 10:08 UTC (permalink / raw)
  To: Pali Rohár, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Scott Wood, Sinan Akman
  Cc: linuxppc-dev, linux-kernel



Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> Generic unified P2020 machine description which supports all P2020-based
> boards is now in separate file p2020.c. So create a separate config option
> CONFIG_P2020 for it.

Could it be CONFIG_PPC_P2020 instead ? Nowadays, drivers seems to spread 
all over driver/ directory, so it's much better to have CONFIG_PPC_ 
prefix on all dedicated powerpc config items.

> 
> Previously machine descriptions for P2020 boards were enabled by
> CONFIG_MPC85xx_DS or CONFIG_MPC85xx_RDB option. So set CONFIG_P2020 to be
> enabled by default when one of those option is enabled.
> 
> This allows to compile support for P2020 boards without need to have
> enabled support for older mpc85xx boards. And to compile kernel for old
> mpc85xx boards without having enabled support for new P2020 boards.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>   arch/powerpc/platforms/85xx/Kconfig  | 22 ++++++++++++++++++----
>   arch/powerpc/platforms/85xx/Makefile |  3 +--
>   2 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> index be16eba0f704..2cb4e9248b42 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -78,16 +78,16 @@ config MPC8536_DS
>   	  This option enables support for the MPC8536 DS board
>   
>   config MPC85xx_DS
> -	bool "Freescale MPC8544 DS / MPC8572 DS / P2020 DS"
> +	bool "Freescale MPC8544 DS / MPC8572 DS"
>   	select PPC_I8259
>   	select DEFAULT_UIMAGE
>   	select FSL_ULI1575 if PCI
>   	select SWIOTLB
>   	help
> -	  This option enables support for the MPC8544 DS, MPC8572 DS and P2020 DS boards
> +	  This option enables support for the MPC8544 DS and MPC8572 DS boards
>   
>   config MPC85xx_RDB
> -	bool "Freescale P102x MBG/UTM/RDB and P2020 RDB"
> +	bool "Freescale P102x MBG/UTM/RDB"
>   	select PPC_I8259
>   	select DEFAULT_UIMAGE
>   	select FSL_ULI1575 if PCI
> @@ -95,7 +95,21 @@ config MPC85xx_RDB
>   	help
>   	  This option enables support for the P1020 MBG PC, P1020 UTM PC,
>   	  P1020 RDB PC, P1020 RDB PD, P1020 RDB, P1021 RDB PC, P1024 RDB,
> -	  P1025 RDB, P2020 RDB and P2020 RDB PC boards
> +	  and P1025 RDB boards
> +
> +config P2020
> +	bool "Freescale P2020"
> +	default y if MPC85xx_DS || MPC85xx_RDB

Is that necessary ?
Can you just update defconfigs ?

By the way, did you have a look at the impact on defconfigs ?

> +	select DEFAULT_UIMAGE
> +	select SWIOTLB
> +	imply PPC_I8259
> +	imply FSL_ULI1575 if PCI

Why imply and not select ?

> +	help
> +	  This option enables generic unified support for any board with the
> +	  Freescale P2020 processor.
> +
> +	  For example: P2020 DS board, P2020 RDB board, P2020 RDB PC board or
> +	  CZ.NIC Turris 1.x boards.
>   
>   config P1010_RDB
>   	bool "Freescale P1010 RDB"
> diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> index 1ad261b4eeb6..021e168442d7 100644
> --- a/arch/powerpc/platforms/85xx/Makefile
> +++ b/arch/powerpc/platforms/85xx/Makefile
> @@ -23,8 +23,7 @@ obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
>   obj-$(CONFIG_P1022_DS)    += p1022_ds.o
>   obj-$(CONFIG_P1022_RDK)   += p1022_rdk.o
>   obj-$(CONFIG_P1023_RDB)   += p1023_rdb.o
> -obj-$(CONFIG_MPC85xx_DS)  += p2020.o
> -obj-$(CONFIG_MPC85xx_RDB) += p2020.o
> +obj-$(CONFIG_P2020)       += p2020.o
>   obj-$(CONFIG_TWR_P102x)   += twr_p102x.o
>   obj-$(CONFIG_CORENET_GENERIC)   += corenet_generic.o
>   obj-$(CONFIG_FB_FSL_DIU)	+= t1042rdb_diu.o

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 5/7] powerpc/85xx: p2020: Define just one machine description
  2022-09-26 10:02   ` Christophe Leroy
@ 2022-09-26 10:08     ` Pali Rohár
  0 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2022-09-26 10:08 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

On Monday 26 September 2022 10:02:47 Christophe Leroy wrote:
> > +static int __init p2020_probe(void)
> >   {
> > -	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> > -		return 1;
> > -	return 0;
> > +	struct device_node *p2020_cpu;
> > +
> > +	/*
> > +	 * There is no common compatible string for all P2020 boards.
> > +	 * The only common thing is "PowerPC,P2020@0" cpu node.
> > +	 * So check for P2020 board via this cpu node.
> > +	 */
> > +	p2020_cpu = of_find_node_by_path("/cpus/PowerPC,P2020@0");
> > +	if (!p2020_cpu)
> > +		return 0;
> 
> This looks odd. I though all probe were using the compatible, and in 
> fact I have a series in preparation that drops all 
> of_machine_is_compatible() checks in probe functions and do it in the 
> caller instead, after adding a .compatible string in the machine 
> description.
> 
> Is there really no compatible that can be used for all p2020 ?

Really. There is none. I have looked into all available P2020 DTB files
(either externals passed by bootloader or kernel in-tree) and there is
no common compatible string. The only "common" thing is cpu node, how I
implemented it int this patch series.

And same issue is with boards with P101x and P102x DTB files.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 7/7] powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string
  2022-08-19 19:15 ` [PATCH 7/7] powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string Pali Rohár
@ 2022-09-26 10:10   ` Christophe Leroy
  2022-09-26 10:21     ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Christophe Leroy @ 2022-09-26 10:10 UTC (permalink / raw)
  To: Pali Rohár, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Scott Wood, Sinan Akman
  Cc: linuxppc-dev, linux-kernel



Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> "fsl,P2020RDB-PC" compatible string was present in Turris 1.x DTS file just
> because Linux kernel required it for proper detection of P2020 processor
> during boot.
> 
> This was quite a hack as CZ,NIC Turris 1.x is not compatible with
> Freescale P2020-RDB-PC board.
> 
> Now when kernel has generic unified support for boards with P2020
> processors, there is no need to have this "hack" in turris1x.dts file.
> 
> So remove incorrect "fsl,P2020RDB-PC" compatible string from turris1x.dts.

Oh, I thought it was not possible to modify DTSes.

If it is, can you have a common compatible to all p2020, for instance 
"fsl,p2020', so that you can use it in patch 5 instead of 
of_find_node_by_path("/cpus/PowerPC,P2020@0") ?

> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>   arch/powerpc/boot/dts/turris1x.dts | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/boot/dts/turris1x.dts b/arch/powerpc/boot/dts/turris1x.dts
> index 12e08271e61f..69c38ed8a3a5 100644
> --- a/arch/powerpc/boot/dts/turris1x.dts
> +++ b/arch/powerpc/boot/dts/turris1x.dts
> @@ -15,7 +15,7 @@
>   
>   / {
>   	model = "Turris 1.x";
> -	compatible = "cznic,turris1x", "fsl,P2020RDB-PC"; /* fsl,P2020RDB-PC is required for booting Linux */
> +	compatible = "cznic,turris1x";
>   
>   	aliases {
>   		ethernet0 = &enet0;

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
  2022-09-26  9:53     ` Pali Rohár
@ 2022-09-26 10:17       ` Christophe Leroy
  2022-09-26 10:26         ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Christophe Leroy @ 2022-09-26 10:17 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev



Le 26/09/2022 à 11:53, Pali Rohár a écrit :
> On Monday 26 September 2022 09:48:02 Christophe Leroy wrote:
>> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
>>> This moves machine descriptions and all related code for all P2020 boards
>>> into new p2020.c source file. This is preparation for code deduplication
>>> and providing one unified machine description for all P2020 boards.
>>
>> I'm having hard time to review this patch.
>>
>> It looks like you are doing much more than just moving machine
>> descriptions and related code into p2020.c
>>
>> Apparently p2020.c has a lot of code that doesn't seem be move from
>> somewhere else.
>>
>> Maybe there is a need to tidy up in order to ease reviewing.
> 
> This is probably harder to read due to how git format-patch generated
> this email. The important is:
> 
>   copy from arch/powerpc/platforms/85xx/mpc85xx_ds.c
>   copy to arch/powerpc/platforms/85xx/p2020.c
> 
> Which means that git thinks that my newly introduced file p2020.c is
> similar to old file mpc85xx_ds.c and generated diff in format which do:
> 
>   1. copy mpc85xx_ds.c to p2020.c
>   2. apply diff on newly introduced file p2020.c
> 
> Code is really moved from mpc85xx_ds.c and mpc85xx_rdb.c files into file
> p2020.c.
> 
> File p2020.c is new in this patch.

Well, I didn't really look in how the patch was generated, I imported 
your series and mainly reviewed it in git directly.

For this patch I have the following diff stat:

$ git show --stat e2d8c39e2e32855658d1c5f042a7ce88952f488a
commit e2d8c39e2e32855658d1c5f042a7ce88952f488a
Author: Pali Rohár <pali@kernel.org>
Date:   Fri Aug 19 21:15:53 2022 +0200

     powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c

     This moves machine descriptions and all related code for all P2020 
boards
     into new p2020.c source file. This is preparation for code 
deduplication
     and providing one unified machine description for all P2020 boards.

     Signed-off-by: Pali Rohár <pali@kernel.org>

  arch/powerpc/platforms/85xx/Makefile      |   2 ++
  arch/powerpc/platforms/85xx/mpc85xx_ds.c  |  23 --------------
  arch/powerpc/platforms/85xx/mpc85xx_rdb.c |  44 --------------------------
  arch/powerpc/platforms/85xx/p2020.c       | 273 
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  4 files changed, 275 insertions(+), 67 deletions(-)


So there is a lot more code added than deleted.

If it was really a code move as described in the commit message, I would 
have approximately the same number of inserts as number of deletions.


> 
>>>
>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>> ---
>>>    arch/powerpc/platforms/85xx/Makefile          |   2 +
>>>    arch/powerpc/platforms/85xx/mpc85xx_ds.c      |  23 ---
>>>    arch/powerpc/platforms/85xx/mpc85xx_rdb.c     |  44 ------
>>>    .../platforms/85xx/{mpc85xx_ds.c => p2020.c}  | 134 ++++++++++++------
>>>    4 files changed, 91 insertions(+), 112 deletions(-)
>>>    copy arch/powerpc/platforms/85xx/{mpc85xx_ds.c => p2020.c} (65%)
>>>
>>> diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
>>> index 260fbad7967b..1ad261b4eeb6 100644
>>> --- a/arch/powerpc/platforms/85xx/Makefile
>>> +++ b/arch/powerpc/platforms/85xx/Makefile
>>> @@ -23,6 +23,8 @@ obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
>>>    obj-$(CONFIG_P1022_DS)    += p1022_ds.o
>>>    obj-$(CONFIG_P1022_RDK)   += p1022_rdk.o
>>>    obj-$(CONFIG_P1023_RDB)   += p1023_rdb.o
>>> +obj-$(CONFIG_MPC85xx_DS)  += p2020.o
>>> +obj-$(CONFIG_MPC85xx_RDB) += p2020.o
>>>    obj-$(CONFIG_TWR_P102x)   += twr_p102x.o
>>>    obj-$(CONFIG_CORENET_GENERIC)   += corenet_generic.o
>>>    obj-$(CONFIG_FB_FSL_DIU)	+= t1042rdb_diu.o
>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>> index 9a6d637ef54a..05aac997b5ed 100644
>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>> @@ -168,7 +168,6 @@ static int __init mpc8544_ds_probe(void)
>>>    
>>>    machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
>>>    machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
>>> -machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
>>>    
>>>    /*
>>>     * Called very early, device-tree isn't unflattened
>>> @@ -178,14 +177,6 @@ static int __init mpc8572_ds_probe(void)
>>>    	return !!of_machine_is_compatible("fsl,MPC8572DS");
>>>    }
>>>    
>>> -/*
>>> - * Called very early, device-tree isn't unflattened
>>> - */
>>> -static int __init p2020_ds_probe(void)
>>> -{
>>> -	return !!of_machine_is_compatible("fsl,P2020DS");
>>> -}
>>> -
>>>    define_machine(mpc8544_ds) {
>>>    	.name			= "MPC8544 DS",
>>>    	.probe			= mpc8544_ds_probe,
>>> @@ -213,17 +204,3 @@ define_machine(mpc8572_ds) {
>>>    	.calibrate_decr		= generic_calibrate_decr,
>>>    	.progress		= udbg_progress,
>>>    };
>>> -
>>> -define_machine(p2020_ds) {
>>> -	.name			= "P2020 DS",
>>> -	.probe			= p2020_ds_probe,
>>> -	.setup_arch		= mpc85xx_ds_setup_arch,
>>> -	.init_IRQ		= mpc85xx_ds_pic_init,
>>> -#ifdef CONFIG_PCI
>>> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
>>> -#endif
>>> -	.get_irq		= mpic_get_irq,
>>> -	.calibrate_decr		= generic_calibrate_decr,
>>> -	.progress		= udbg_progress,
>>> -};
>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
>>> index b6129c148fea..05f1ed635735 100644
>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
>>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
>>> @@ -108,8 +108,6 @@ static void __init mpc85xx_rdb_setup_arch(void)
>>>    	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
>>>    }
>>>    
>>> -machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
>>> -machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
>>>    machine_arch_initcall(p1020_mbg_pc, mpc85xx_common_publish_devices);
>>>    machine_arch_initcall(p1020_rdb, mpc85xx_common_publish_devices);
>>>    machine_arch_initcall(p1020_rdb_pc, mpc85xx_common_publish_devices);
>>> @@ -122,13 +120,6 @@ machine_arch_initcall(p1024_rdb, mpc85xx_common_publish_devices);
>>>    /*
>>>     * Called very early, device-tree isn't unflattened
>>>     */
>>> -static int __init p2020_rdb_probe(void)
>>> -{
>>> -	if (of_machine_is_compatible("fsl,P2020RDB"))
>>> -		return 1;
>>> -	return 0;
>>> -}
>>> -
>>>    static int __init p1020_rdb_probe(void)
>>>    {
>>>    	if (of_machine_is_compatible("fsl,P1020RDB"))
>>> @@ -153,13 +144,6 @@ static int __init p1021_rdb_pc_probe(void)
>>>    	return 0;
>>>    }
>>>    
>>> -static int __init p2020_rdb_pc_probe(void)
>>> -{
>>> -	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
>>> -		return 1;
>>> -	return 0;
>>> -}
>>> -
>>>    static int __init p1025_rdb_probe(void)
>>>    {
>>>    	return of_machine_is_compatible("fsl,P1025RDB");
>>> @@ -180,20 +164,6 @@ static int __init p1024_rdb_probe(void)
>>>    	return of_machine_is_compatible("fsl,P1024RDB");
>>>    }
>>>    
>>> -define_machine(p2020_rdb) {
>>> -	.name			= "P2020 RDB",
>>> -	.probe			= p2020_rdb_probe,
>>> -	.setup_arch		= mpc85xx_rdb_setup_arch,
>>> -	.init_IRQ		= mpc85xx_rdb_pic_init,
>>> -#ifdef CONFIG_PCI
>>> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
>>> -#endif
>>> -	.get_irq		= mpic_get_irq,
>>> -	.calibrate_decr		= generic_calibrate_decr,
>>> -	.progress		= udbg_progress,
>>> -};
>>> -
>>>    define_machine(p1020_rdb) {
>>>    	.name			= "P1020 RDB",
>>>    	.probe			= p1020_rdb_probe,
>>> @@ -222,20 +192,6 @@ define_machine(p1021_rdb_pc) {
>>>    	.progress		= udbg_progress,
>>>    };
>>>    
>>> -define_machine(p2020_rdb_pc) {
>>> -	.name			= "P2020RDB-PC",
>>> -	.probe			= p2020_rdb_pc_probe,
>>> -	.setup_arch		= mpc85xx_rdb_setup_arch,
>>> -	.init_IRQ		= mpc85xx_rdb_pic_init,
>>> -#ifdef CONFIG_PCI
>>> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
>>> -#endif
>>> -	.get_irq		= mpic_get_irq,
>>> -	.calibrate_decr		= generic_calibrate_decr,
>>> -	.progress		= udbg_progress,
>>> -};
>>> -
>>>    define_machine(p1025_rdb) {
>>>    	.name			= "P1025 RDB",
>>>    	.probe			= p1025_rdb_probe,
>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/p2020.c
>>> similarity index 65%
>>> copy from arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>> copy to arch/powerpc/platforms/85xx/p2020.c
>>> index 9a6d637ef54a..d65d4c88ac47 100644
>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>> +++ b/arch/powerpc/platforms/85xx/p2020.c
>>> @@ -1,11 +1,9 @@
>>>    // SPDX-License-Identifier: GPL-2.0-or-later
>>>    /*
>>> - * MPC85xx DS Board Setup
>>> + * Freescale P2020 board Setup
>>>     *
>>> - * Author Xianghua Xiao (x.xiao@freescale.com)
>>> - * Roy Zang <tie-fei.zang@freescale.com>
>>> - * 	- Add PCI/PCI Exprees support
>>> - * Copyright 2007 Freescale Semiconductor Inc.
>>> + * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
>>> + * Copyright 2022 Pali Rohár <pali@kernel.org>
>>>     */
>>>    
>>>    #include <linux/stddef.h>
>>> @@ -17,6 +15,7 @@
>>>    #include <linux/interrupt.h>
>>>    #include <linux/of_irq.h>
>>>    #include <linux/of_platform.h>
>>> +#include <linux/fsl/guts.h>
>>>    
>>>    #include <asm/time.h>
>>>    #include <asm/machdep.h>
>>> @@ -27,6 +26,8 @@
>>>    #include <asm/i8259.h>
>>>    #include <asm/swiotlb.h>
>>>    
>>> +#include <soc/fsl/qe/qe.h>
>>> +
>>>    #include <sysdev/fsl_soc.h>
>>>    #include <sysdev/fsl_pci.h>
>>>    #include "smp.h"
>>> @@ -41,6 +42,8 @@
>>>    #define DBG(fmt, args...)
>>>    #endif
>>>    
>>> +#ifdef CONFIG_MPC85xx_DS
>>> +
>>>    #ifdef CONFIG_PPC_I8259
>>>    static void mpc85xx_8259_cascade(struct irq_desc *desc)
>>>    {
>>> @@ -62,18 +65,11 @@ static void __init mpc85xx_ds_pic_init(void)
>>>    	struct device_node *cascade_node = NULL;
>>>    	int cascade_irq;
>>>    #endif
>>> -	if (of_machine_is_compatible("fsl,MPC8572DS-CAMP")) {
>>> -		mpic = mpic_alloc(NULL, 0,
>>> -			MPIC_NO_RESET |
>>> -			MPIC_BIG_ENDIAN |
>>> -			MPIC_SINGLE_DEST_CPU,
>>> -			0, 256, " OpenPIC  ");
>>> -	} else {
>>> -		mpic = mpic_alloc(NULL, 0,
>>> -			  MPIC_BIG_ENDIAN |
>>> -			  MPIC_SINGLE_DEST_CPU,
>>> -			0, 256, " OpenPIC  ");
>>> -	}
>>> +
>>> +	mpic = mpic_alloc(NULL, 0,
>>> +		  MPIC_BIG_ENDIAN |
>>> +		  MPIC_SINGLE_DEST_CPU,
>>> +		0, 256, " OpenPIC  ");
>>>    
>>>    	BUG_ON(mpic == NULL);
>>>    	mpic_init(mpic);
>>> @@ -142,9 +138,27 @@ static void __init mpc85xx_ds_uli_init(void)
>>>    #endif
>>>    }
>>>    
>>> +#endif /* CONFIG_MPC85xx_DS */
>>> +
>>> +#ifdef CONFIG_MPC85xx_RDB
>>> +static void __init mpc85xx_rdb_pic_init(void)
>>> +{
>>> +	struct mpic *mpic;
>>> +
>>> +	mpic = mpic_alloc(NULL, 0,
>>> +	  MPIC_BIG_ENDIAN |
>>> +	  MPIC_SINGLE_DEST_CPU,
>>> +	  0, 256, " OpenPIC  ");
>>> +
>>> +	BUG_ON(mpic == NULL);
>>> +	mpic_init(mpic);
>>> +}
>>> +#endif /* CONFIG_MPC85xx_RDB */
>>> +
>>>    /*
>>>     * Setup the architecture
>>>     */
>>> +#ifdef CONFIG_MPC85xx_DS
>>>    static void __init mpc85xx_ds_setup_arch(void)
>>>    {
>>>    	if (ppc_md.progress)
>>> @@ -157,38 +171,65 @@ static void __init mpc85xx_ds_setup_arch(void)
>>>    
>>>    	printk("MPC85xx DS board from Freescale Semiconductor\n");
>>>    }
>>> +#endif /* CONFIG_MPC85xx_DS */
>>>    
>>> -/*
>>> - * Called very early, device-tree isn't unflattened
>>> - */
>>> -static int __init mpc8544_ds_probe(void)
>>> +#ifdef CONFIG_MPC85xx_RDB
>>> +static void __init mpc85xx_rdb_setup_arch(void)
>>>    {
>>> -	return !!of_machine_is_compatible("MPC8544DS");
>>> +	if (ppc_md.progress)
>>> +		ppc_md.progress("mpc85xx_rdb_setup_arch()", 0);
>>> +
>>> +	mpc85xx_smp_init();
>>> +
>>> +	fsl_pci_assign_primary();
>>> +
>>> +#ifdef CONFIG_QUICC_ENGINE
>>> +	mpc85xx_qe_par_io_init();
>>> +#endif	/* CONFIG_QUICC_ENGINE */
>>> +
>>> +	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
>>>    }
>>> +#endif /* CONFIG_MPC85xx_RDB */
>>>    
>>> -machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
>>> -machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
>>> +#ifdef CONFIG_MPC85xx_DS
>>>    machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
>>> +#endif /* CONFIG_MPC85xx_DS */
>>>    
>>> -/*
>>> - * Called very early, device-tree isn't unflattened
>>> - */
>>> -static int __init mpc8572_ds_probe(void)
>>> -{
>>> -	return !!of_machine_is_compatible("fsl,MPC8572DS");
>>> -}
>>> +#ifdef CONFIG_MPC85xx_RDB
>>> +machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
>>> +machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
>>> +#endif /* CONFIG_MPC85xx_RDB */
>>>    
>>>    /*
>>>     * Called very early, device-tree isn't unflattened
>>>     */
>>> +#ifdef CONFIG_MPC85xx_DS
>>>    static int __init p2020_ds_probe(void)
>>>    {
>>>    	return !!of_machine_is_compatible("fsl,P2020DS");
>>>    }
>>> +#endif /* CONFIG_MPC85xx_DS */
>>> +
>>> +#ifdef CONFIG_MPC85xx_RDB
>>> +static int __init p2020_rdb_probe(void)
>>> +{
>>> +	if (of_machine_is_compatible("fsl,P2020RDB"))
>>> +		return 1;
>>> +	return 0;
>>> +}
>>> +
>>> +static int __init p2020_rdb_pc_probe(void)
>>> +{
>>> +	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
>>> +		return 1;
>>> +	return 0;
>>> +}
>>> +#endif /* CONFIG_MPC85xx_RDB */
>>>    
>>> -define_machine(mpc8544_ds) {
>>> -	.name			= "MPC8544 DS",
>>> -	.probe			= mpc8544_ds_probe,
>>> +#ifdef CONFIG_MPC85xx_DS
>>> +define_machine(p2020_ds) {
>>> +	.name			= "P2020 DS",
>>> +	.probe			= p2020_ds_probe,
>>>    	.setup_arch		= mpc85xx_ds_setup_arch,
>>>    	.init_IRQ		= mpc85xx_ds_pic_init,
>>>    #ifdef CONFIG_PCI
>>> @@ -199,12 +240,14 @@ define_machine(mpc8544_ds) {
>>>    	.calibrate_decr		= generic_calibrate_decr,
>>>    	.progress		= udbg_progress,
>>>    };
>>> -
>>> -define_machine(mpc8572_ds) {
>>> -	.name			= "MPC8572 DS",
>>> -	.probe			= mpc8572_ds_probe,
>>> -	.setup_arch		= mpc85xx_ds_setup_arch,
>>> -	.init_IRQ		= mpc85xx_ds_pic_init,
>>> +#endif /* CONFIG_MPC85xx_DS */
>>> +
>>> +#ifdef CONFIG_MPC85xx_RDB
>>> +define_machine(p2020_rdb) {
>>> +	.name			= "P2020 RDB",
>>> +	.probe			= p2020_rdb_probe,
>>> +	.setup_arch		= mpc85xx_rdb_setup_arch,
>>> +	.init_IRQ		= mpc85xx_rdb_pic_init,
>>>    #ifdef CONFIG_PCI
>>>    	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>>    	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
>>> @@ -214,11 +257,11 @@ define_machine(mpc8572_ds) {
>>>    	.progress		= udbg_progress,
>>>    };
>>>    
>>> -define_machine(p2020_ds) {
>>> -	.name			= "P2020 DS",
>>> -	.probe			= p2020_ds_probe,
>>> -	.setup_arch		= mpc85xx_ds_setup_arch,
>>> -	.init_IRQ		= mpc85xx_ds_pic_init,
>>> +define_machine(p2020_rdb_pc) {
>>> +	.name			= "P2020RDB-PC",
>>> +	.probe			= p2020_rdb_pc_probe,
>>> +	.setup_arch		= mpc85xx_rdb_setup_arch,
>>> +	.init_IRQ		= mpc85xx_rdb_pic_init,
>>>    #ifdef CONFIG_PCI
>>>    	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>>    	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
>>> @@ -227,3 +270,4 @@ define_machine(p2020_ds) {
>>>    	.calibrate_decr		= generic_calibrate_decr,
>>>    	.progress		= udbg_progress,
>>>    };
>>> +#endif /* CONFIG_MPC85xx_RDB */

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020
  2022-09-26 10:08   ` Christophe Leroy
@ 2022-09-26 10:19     ` Pali Rohár
  0 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2022-09-26 10:19 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

On Monday 26 September 2022 10:08:19 Christophe Leroy wrote:
> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> > Generic unified P2020 machine description which supports all P2020-based
> > boards is now in separate file p2020.c. So create a separate config option
> > CONFIG_P2020 for it.
> 
> Could it be CONFIG_PPC_P2020 instead ? Nowadays, drivers seems to spread 
> all over driver/ directory, so it's much better to have CONFIG_PPC_ 
> prefix on all dedicated powerpc config items.

Ok! I do not have any strong preference of config option name.

> > 
> > Previously machine descriptions for P2020 boards were enabled by
> > CONFIG_MPC85xx_DS or CONFIG_MPC85xx_RDB option. So set CONFIG_P2020 to be
> > enabled by default when one of those option is enabled.
> > 
> > This allows to compile support for P2020 boards without need to have
> > enabled support for older mpc85xx boards. And to compile kernel for old
> > mpc85xx boards without having enabled support for new P2020 boards.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >   arch/powerpc/platforms/85xx/Kconfig  | 22 ++++++++++++++++++----
> >   arch/powerpc/platforms/85xx/Makefile |  3 +--
> >   2 files changed, 19 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> > index be16eba0f704..2cb4e9248b42 100644
> > --- a/arch/powerpc/platforms/85xx/Kconfig
> > +++ b/arch/powerpc/platforms/85xx/Kconfig
> > @@ -78,16 +78,16 @@ config MPC8536_DS
> >   	  This option enables support for the MPC8536 DS board
> >   
> >   config MPC85xx_DS
> > -	bool "Freescale MPC8544 DS / MPC8572 DS / P2020 DS"
> > +	bool "Freescale MPC8544 DS / MPC8572 DS"
> >   	select PPC_I8259
> >   	select DEFAULT_UIMAGE
> >   	select FSL_ULI1575 if PCI
> >   	select SWIOTLB
> >   	help
> > -	  This option enables support for the MPC8544 DS, MPC8572 DS and P2020 DS boards
> > +	  This option enables support for the MPC8544 DS and MPC8572 DS boards
> >   
> >   config MPC85xx_RDB
> > -	bool "Freescale P102x MBG/UTM/RDB and P2020 RDB"
> > +	bool "Freescale P102x MBG/UTM/RDB"
> >   	select PPC_I8259
> >   	select DEFAULT_UIMAGE
> >   	select FSL_ULI1575 if PCI
> > @@ -95,7 +95,21 @@ config MPC85xx_RDB
> >   	help
> >   	  This option enables support for the P1020 MBG PC, P1020 UTM PC,
> >   	  P1020 RDB PC, P1020 RDB PD, P1020 RDB, P1021 RDB PC, P1024 RDB,
> > -	  P1025 RDB, P2020 RDB and P2020 RDB PC boards
> > +	  and P1025 RDB boards
> > +
> > +config P2020
> > +	bool "Freescale P2020"
> > +	default y if MPC85xx_DS || MPC85xx_RDB
> 
> Is that necessary ?
> Can you just update defconfigs ?

This is for old users defconfigs, so if they update kernel to new
version it automatically selects all features which were already
enabled.

But if you think this is not necessary, just drop it.

> By the way, did you have a look at the impact on defconfigs ?
> 
> > +	select DEFAULT_UIMAGE
> > +	select SWIOTLB
> > +	imply PPC_I8259
> > +	imply FSL_ULI1575 if PCI
> 
> Why imply and not select ?

Because more P2020 boards do not have these two HW parts. So I do not
see reason for hard dependency. In my opinion, if user does not need to
enable some kernel option (because his HW does not require it) then
kernel should allow to do it, unless there is no strong reason for it.

And IIRC imply is like select but allow user to disable specified
option.

> > +	help
> > +	  This option enables generic unified support for any board with the
> > +	  Freescale P2020 processor.
> > +
> > +	  For example: P2020 DS board, P2020 RDB board, P2020 RDB PC board or
> > +	  CZ.NIC Turris 1.x boards.
> >   
> >   config P1010_RDB
> >   	bool "Freescale P1010 RDB"
> > diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> > index 1ad261b4eeb6..021e168442d7 100644
> > --- a/arch/powerpc/platforms/85xx/Makefile
> > +++ b/arch/powerpc/platforms/85xx/Makefile
> > @@ -23,8 +23,7 @@ obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
> >   obj-$(CONFIG_P1022_DS)    += p1022_ds.o
> >   obj-$(CONFIG_P1022_RDK)   += p1022_rdk.o
> >   obj-$(CONFIG_P1023_RDB)   += p1023_rdb.o
> > -obj-$(CONFIG_MPC85xx_DS)  += p2020.o
> > -obj-$(CONFIG_MPC85xx_RDB) += p2020.o
> > +obj-$(CONFIG_P2020)       += p2020.o
> >   obj-$(CONFIG_TWR_P102x)   += twr_p102x.o
> >   obj-$(CONFIG_CORENET_GENERIC)   += corenet_generic.o
> >   obj-$(CONFIG_FB_FSL_DIU)	+= t1042rdb_diu.o

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 7/7] powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string
  2022-09-26 10:10   ` Christophe Leroy
@ 2022-09-26 10:21     ` Pali Rohár
  0 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2022-09-26 10:21 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

On Monday 26 September 2022 10:10:19 Christophe Leroy wrote:
> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> > "fsl,P2020RDB-PC" compatible string was present in Turris 1.x DTS file just
> > because Linux kernel required it for proper detection of P2020 processor
> > during boot.
> > 
> > This was quite a hack as CZ,NIC Turris 1.x is not compatible with
> > Freescale P2020-RDB-PC board.
> > 
> > Now when kernel has generic unified support for boards with P2020
> > processors, there is no need to have this "hack" in turris1x.dts file.
> > 
> > So remove incorrect "fsl,P2020RDB-PC" compatible string from turris1x.dts.
> 
> Oh, I thought it was not possible to modify DTSes.

Boards which have hardcoded DTB binaries in bootloader or are
kernel out-of-tree, they obviously needs to be still supported by
kernel.

> If it is, can you have a common compatible to all p2020, for instance 
> "fsl,p2020', so that you can use it in patch 5 instead of 
> of_find_node_by_path("/cpus/PowerPC,P2020@0") ?

I can add fsl,p2020. But it does not solve issue for other boards.

This string fsl,p2020 is not used by any board (yet).

Also Turris 1.x boards have burned some older DTB file in Flash NOR.

So it is problematic.

> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >   arch/powerpc/boot/dts/turris1x.dts | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/boot/dts/turris1x.dts b/arch/powerpc/boot/dts/turris1x.dts
> > index 12e08271e61f..69c38ed8a3a5 100644
> > --- a/arch/powerpc/boot/dts/turris1x.dts
> > +++ b/arch/powerpc/boot/dts/turris1x.dts
> > @@ -15,7 +15,7 @@
> >   
> >   / {
> >   	model = "Turris 1.x";
> > -	compatible = "cznic,turris1x", "fsl,P2020RDB-PC"; /* fsl,P2020RDB-PC is required for booting Linux */
> > +	compatible = "cznic,turris1x";
> >   
> >   	aliases {
> >   		ethernet0 = &enet0;

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
  2022-09-26 10:17       ` Christophe Leroy
@ 2022-09-26 10:26         ` Pali Rohár
  2022-12-07 14:02           ` Christophe Leroy
  0 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-09-26 10:26 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

On Monday 26 September 2022 10:17:26 Christophe Leroy wrote:
> Le 26/09/2022 à 11:53, Pali Rohár a écrit :
> > On Monday 26 September 2022 09:48:02 Christophe Leroy wrote:
> >> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> >>> This moves machine descriptions and all related code for all P2020 boards
> >>> into new p2020.c source file. This is preparation for code deduplication
> >>> and providing one unified machine description for all P2020 boards.
> >>
> >> I'm having hard time to review this patch.
> >>
> >> It looks like you are doing much more than just moving machine
> >> descriptions and related code into p2020.c
> >>
> >> Apparently p2020.c has a lot of code that doesn't seem be move from
> >> somewhere else.
> >>
> >> Maybe there is a need to tidy up in order to ease reviewing.
> > 
> > This is probably harder to read due to how git format-patch generated
> > this email. The important is:
> > 
> >   copy from arch/powerpc/platforms/85xx/mpc85xx_ds.c
> >   copy to arch/powerpc/platforms/85xx/p2020.c
> > 
> > Which means that git thinks that my newly introduced file p2020.c is
> > similar to old file mpc85xx_ds.c and generated diff in format which do:
> > 
> >   1. copy mpc85xx_ds.c to p2020.c
> >   2. apply diff on newly introduced file p2020.c
> > 
> > Code is really moved from mpc85xx_ds.c and mpc85xx_rdb.c files into file
> > p2020.c.
> > 
> > File p2020.c is new in this patch.
> 
> Well, I didn't really look in how the patch was generated, I imported 
> your series and mainly reviewed it in git directly.
> 
> For this patch I have the following diff stat:
> 
> $ git show --stat e2d8c39e2e32855658d1c5f042a7ce88952f488a
> commit e2d8c39e2e32855658d1c5f042a7ce88952f488a
> Author: Pali Rohár <pali@kernel.org>
> Date:   Fri Aug 19 21:15:53 2022 +0200
> 
>      powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
> 
>      This moves machine descriptions and all related code for all P2020 
> boards
>      into new p2020.c source file. This is preparation for code 
> deduplication
>      and providing one unified machine description for all P2020 boards.
> 
>      Signed-off-by: Pali Rohár <pali@kernel.org>
> 
>   arch/powerpc/platforms/85xx/Makefile      |   2 ++
>   arch/powerpc/platforms/85xx/mpc85xx_ds.c  |  23 --------------
>   arch/powerpc/platforms/85xx/mpc85xx_rdb.c |  44 --------------------------
>   arch/powerpc/platforms/85xx/p2020.c       | 273 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 275 insertions(+), 67 deletions(-)
> 
> 
> So there is a lot more code added than deleted.
> 
> If it was really a code move as described in the commit message, I would 
> have approximately the same number of inserts as number of deletions.

I see... The reason is that helper ds/rdb functions are copies (not
moved) because they are needed still in ds/rdb boards. And in later
patches in this patch series are then p2020 helper function cleaned and
simplified.

So as I see basically this change moves p2020 machine descriptions from
ds/rdb files into p2020.c, plus copy helper functions.

Not sure what should be the best case how to do it. I did not wanted to
introduce regression in the code, so I rather did not touched non-p2020
code in ds/rdb files.

> 
> > 
> >>>
> >>> Signed-off-by: Pali Rohár <pali@kernel.org>
> >>> ---
> >>>    arch/powerpc/platforms/85xx/Makefile          |   2 +
> >>>    arch/powerpc/platforms/85xx/mpc85xx_ds.c      |  23 ---
> >>>    arch/powerpc/platforms/85xx/mpc85xx_rdb.c     |  44 ------
> >>>    .../platforms/85xx/{mpc85xx_ds.c => p2020.c}  | 134 ++++++++++++------
> >>>    4 files changed, 91 insertions(+), 112 deletions(-)
> >>>    copy arch/powerpc/platforms/85xx/{mpc85xx_ds.c => p2020.c} (65%)
> >>>
> >>> diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> >>> index 260fbad7967b..1ad261b4eeb6 100644
> >>> --- a/arch/powerpc/platforms/85xx/Makefile
> >>> +++ b/arch/powerpc/platforms/85xx/Makefile
> >>> @@ -23,6 +23,8 @@ obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
> >>>    obj-$(CONFIG_P1022_DS)    += p1022_ds.o
> >>>    obj-$(CONFIG_P1022_RDK)   += p1022_rdk.o
> >>>    obj-$(CONFIG_P1023_RDB)   += p1023_rdb.o
> >>> +obj-$(CONFIG_MPC85xx_DS)  += p2020.o
> >>> +obj-$(CONFIG_MPC85xx_RDB) += p2020.o
> >>>    obj-$(CONFIG_TWR_P102x)   += twr_p102x.o
> >>>    obj-$(CONFIG_CORENET_GENERIC)   += corenet_generic.o
> >>>    obj-$(CONFIG_FB_FSL_DIU)	+= t1042rdb_diu.o
> >>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> >>> index 9a6d637ef54a..05aac997b5ed 100644
> >>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> >>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> >>> @@ -168,7 +168,6 @@ static int __init mpc8544_ds_probe(void)
> >>>    
> >>>    machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
> >>>    machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
> >>> -machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
> >>>    
> >>>    /*
> >>>     * Called very early, device-tree isn't unflattened
> >>> @@ -178,14 +177,6 @@ static int __init mpc8572_ds_probe(void)
> >>>    	return !!of_machine_is_compatible("fsl,MPC8572DS");
> >>>    }
> >>>    
> >>> -/*
> >>> - * Called very early, device-tree isn't unflattened
> >>> - */
> >>> -static int __init p2020_ds_probe(void)
> >>> -{
> >>> -	return !!of_machine_is_compatible("fsl,P2020DS");
> >>> -}
> >>> -
> >>>    define_machine(mpc8544_ds) {
> >>>    	.name			= "MPC8544 DS",
> >>>    	.probe			= mpc8544_ds_probe,
> >>> @@ -213,17 +204,3 @@ define_machine(mpc8572_ds) {
> >>>    	.calibrate_decr		= generic_calibrate_decr,
> >>>    	.progress		= udbg_progress,
> >>>    };
> >>> -
> >>> -define_machine(p2020_ds) {
> >>> -	.name			= "P2020 DS",
> >>> -	.probe			= p2020_ds_probe,
> >>> -	.setup_arch		= mpc85xx_ds_setup_arch,
> >>> -	.init_IRQ		= mpc85xx_ds_pic_init,
> >>> -#ifdef CONFIG_PCI
> >>> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> >>> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> >>> -#endif
> >>> -	.get_irq		= mpic_get_irq,
> >>> -	.calibrate_decr		= generic_calibrate_decr,
> >>> -	.progress		= udbg_progress,
> >>> -};
> >>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> >>> index b6129c148fea..05f1ed635735 100644
> >>> --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> >>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> >>> @@ -108,8 +108,6 @@ static void __init mpc85xx_rdb_setup_arch(void)
> >>>    	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
> >>>    }
> >>>    
> >>> -machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> >>> -machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
> >>>    machine_arch_initcall(p1020_mbg_pc, mpc85xx_common_publish_devices);
> >>>    machine_arch_initcall(p1020_rdb, mpc85xx_common_publish_devices);
> >>>    machine_arch_initcall(p1020_rdb_pc, mpc85xx_common_publish_devices);
> >>> @@ -122,13 +120,6 @@ machine_arch_initcall(p1024_rdb, mpc85xx_common_publish_devices);
> >>>    /*
> >>>     * Called very early, device-tree isn't unflattened
> >>>     */
> >>> -static int __init p2020_rdb_probe(void)
> >>> -{
> >>> -	if (of_machine_is_compatible("fsl,P2020RDB"))
> >>> -		return 1;
> >>> -	return 0;
> >>> -}
> >>> -
> >>>    static int __init p1020_rdb_probe(void)
> >>>    {
> >>>    	if (of_machine_is_compatible("fsl,P1020RDB"))
> >>> @@ -153,13 +144,6 @@ static int __init p1021_rdb_pc_probe(void)
> >>>    	return 0;
> >>>    }
> >>>    
> >>> -static int __init p2020_rdb_pc_probe(void)
> >>> -{
> >>> -	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> >>> -		return 1;
> >>> -	return 0;
> >>> -}
> >>> -
> >>>    static int __init p1025_rdb_probe(void)
> >>>    {
> >>>    	return of_machine_is_compatible("fsl,P1025RDB");
> >>> @@ -180,20 +164,6 @@ static int __init p1024_rdb_probe(void)
> >>>    	return of_machine_is_compatible("fsl,P1024RDB");
> >>>    }
> >>>    
> >>> -define_machine(p2020_rdb) {
> >>> -	.name			= "P2020 RDB",
> >>> -	.probe			= p2020_rdb_probe,
> >>> -	.setup_arch		= mpc85xx_rdb_setup_arch,
> >>> -	.init_IRQ		= mpc85xx_rdb_pic_init,
> >>> -#ifdef CONFIG_PCI
> >>> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> >>> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> >>> -#endif
> >>> -	.get_irq		= mpic_get_irq,
> >>> -	.calibrate_decr		= generic_calibrate_decr,
> >>> -	.progress		= udbg_progress,
> >>> -};
> >>> -
> >>>    define_machine(p1020_rdb) {
> >>>    	.name			= "P1020 RDB",
> >>>    	.probe			= p1020_rdb_probe,
> >>> @@ -222,20 +192,6 @@ define_machine(p1021_rdb_pc) {
> >>>    	.progress		= udbg_progress,
> >>>    };
> >>>    
> >>> -define_machine(p2020_rdb_pc) {
> >>> -	.name			= "P2020RDB-PC",
> >>> -	.probe			= p2020_rdb_pc_probe,
> >>> -	.setup_arch		= mpc85xx_rdb_setup_arch,
> >>> -	.init_IRQ		= mpc85xx_rdb_pic_init,
> >>> -#ifdef CONFIG_PCI
> >>> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> >>> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> >>> -#endif
> >>> -	.get_irq		= mpic_get_irq,
> >>> -	.calibrate_decr		= generic_calibrate_decr,
> >>> -	.progress		= udbg_progress,
> >>> -};
> >>> -
> >>>    define_machine(p1025_rdb) {
> >>>    	.name			= "P1025 RDB",
> >>>    	.probe			= p1025_rdb_probe,
> >>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/p2020.c
> >>> similarity index 65%
> >>> copy from arch/powerpc/platforms/85xx/mpc85xx_ds.c
> >>> copy to arch/powerpc/platforms/85xx/p2020.c
> >>> index 9a6d637ef54a..d65d4c88ac47 100644
> >>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> >>> +++ b/arch/powerpc/platforms/85xx/p2020.c
> >>> @@ -1,11 +1,9 @@
> >>>    // SPDX-License-Identifier: GPL-2.0-or-later
> >>>    /*
> >>> - * MPC85xx DS Board Setup
> >>> + * Freescale P2020 board Setup
> >>>     *
> >>> - * Author Xianghua Xiao (x.xiao@freescale.com)
> >>> - * Roy Zang <tie-fei.zang@freescale.com>
> >>> - * 	- Add PCI/PCI Exprees support
> >>> - * Copyright 2007 Freescale Semiconductor Inc.
> >>> + * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
> >>> + * Copyright 2022 Pali Rohár <pali@kernel.org>
> >>>     */
> >>>    
> >>>    #include <linux/stddef.h>
> >>> @@ -17,6 +15,7 @@
> >>>    #include <linux/interrupt.h>
> >>>    #include <linux/of_irq.h>
> >>>    #include <linux/of_platform.h>
> >>> +#include <linux/fsl/guts.h>
> >>>    
> >>>    #include <asm/time.h>
> >>>    #include <asm/machdep.h>
> >>> @@ -27,6 +26,8 @@
> >>>    #include <asm/i8259.h>
> >>>    #include <asm/swiotlb.h>
> >>>    
> >>> +#include <soc/fsl/qe/qe.h>
> >>> +
> >>>    #include <sysdev/fsl_soc.h>
> >>>    #include <sysdev/fsl_pci.h>
> >>>    #include "smp.h"
> >>> @@ -41,6 +42,8 @@
> >>>    #define DBG(fmt, args...)
> >>>    #endif
> >>>    
> >>> +#ifdef CONFIG_MPC85xx_DS
> >>> +
> >>>    #ifdef CONFIG_PPC_I8259
> >>>    static void mpc85xx_8259_cascade(struct irq_desc *desc)
> >>>    {
> >>> @@ -62,18 +65,11 @@ static void __init mpc85xx_ds_pic_init(void)
> >>>    	struct device_node *cascade_node = NULL;
> >>>    	int cascade_irq;
> >>>    #endif
> >>> -	if (of_machine_is_compatible("fsl,MPC8572DS-CAMP")) {
> >>> -		mpic = mpic_alloc(NULL, 0,
> >>> -			MPIC_NO_RESET |
> >>> -			MPIC_BIG_ENDIAN |
> >>> -			MPIC_SINGLE_DEST_CPU,
> >>> -			0, 256, " OpenPIC  ");
> >>> -	} else {
> >>> -		mpic = mpic_alloc(NULL, 0,
> >>> -			  MPIC_BIG_ENDIAN |
> >>> -			  MPIC_SINGLE_DEST_CPU,
> >>> -			0, 256, " OpenPIC  ");
> >>> -	}
> >>> +
> >>> +	mpic = mpic_alloc(NULL, 0,
> >>> +		  MPIC_BIG_ENDIAN |
> >>> +		  MPIC_SINGLE_DEST_CPU,
> >>> +		0, 256, " OpenPIC  ");
> >>>    
> >>>    	BUG_ON(mpic == NULL);
> >>>    	mpic_init(mpic);
> >>> @@ -142,9 +138,27 @@ static void __init mpc85xx_ds_uli_init(void)
> >>>    #endif
> >>>    }
> >>>    
> >>> +#endif /* CONFIG_MPC85xx_DS */
> >>> +
> >>> +#ifdef CONFIG_MPC85xx_RDB
> >>> +static void __init mpc85xx_rdb_pic_init(void)
> >>> +{
> >>> +	struct mpic *mpic;
> >>> +
> >>> +	mpic = mpic_alloc(NULL, 0,
> >>> +	  MPIC_BIG_ENDIAN |
> >>> +	  MPIC_SINGLE_DEST_CPU,
> >>> +	  0, 256, " OpenPIC  ");
> >>> +
> >>> +	BUG_ON(mpic == NULL);
> >>> +	mpic_init(mpic);
> >>> +}
> >>> +#endif /* CONFIG_MPC85xx_RDB */
> >>> +
> >>>    /*
> >>>     * Setup the architecture
> >>>     */
> >>> +#ifdef CONFIG_MPC85xx_DS
> >>>    static void __init mpc85xx_ds_setup_arch(void)
> >>>    {
> >>>    	if (ppc_md.progress)
> >>> @@ -157,38 +171,65 @@ static void __init mpc85xx_ds_setup_arch(void)
> >>>    
> >>>    	printk("MPC85xx DS board from Freescale Semiconductor\n");
> >>>    }
> >>> +#endif /* CONFIG_MPC85xx_DS */
> >>>    
> >>> -/*
> >>> - * Called very early, device-tree isn't unflattened
> >>> - */
> >>> -static int __init mpc8544_ds_probe(void)
> >>> +#ifdef CONFIG_MPC85xx_RDB
> >>> +static void __init mpc85xx_rdb_setup_arch(void)
> >>>    {
> >>> -	return !!of_machine_is_compatible("MPC8544DS");
> >>> +	if (ppc_md.progress)
> >>> +		ppc_md.progress("mpc85xx_rdb_setup_arch()", 0);
> >>> +
> >>> +	mpc85xx_smp_init();
> >>> +
> >>> +	fsl_pci_assign_primary();
> >>> +
> >>> +#ifdef CONFIG_QUICC_ENGINE
> >>> +	mpc85xx_qe_par_io_init();
> >>> +#endif	/* CONFIG_QUICC_ENGINE */
> >>> +
> >>> +	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
> >>>    }
> >>> +#endif /* CONFIG_MPC85xx_RDB */
> >>>    
> >>> -machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
> >>> -machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
> >>> +#ifdef CONFIG_MPC85xx_DS
> >>>    machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
> >>> +#endif /* CONFIG_MPC85xx_DS */
> >>>    
> >>> -/*
> >>> - * Called very early, device-tree isn't unflattened
> >>> - */
> >>> -static int __init mpc8572_ds_probe(void)
> >>> -{
> >>> -	return !!of_machine_is_compatible("fsl,MPC8572DS");
> >>> -}
> >>> +#ifdef CONFIG_MPC85xx_RDB
> >>> +machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> >>> +machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
> >>> +#endif /* CONFIG_MPC85xx_RDB */
> >>>    
> >>>    /*
> >>>     * Called very early, device-tree isn't unflattened
> >>>     */
> >>> +#ifdef CONFIG_MPC85xx_DS
> >>>    static int __init p2020_ds_probe(void)
> >>>    {
> >>>    	return !!of_machine_is_compatible("fsl,P2020DS");
> >>>    }
> >>> +#endif /* CONFIG_MPC85xx_DS */
> >>> +
> >>> +#ifdef CONFIG_MPC85xx_RDB
> >>> +static int __init p2020_rdb_probe(void)
> >>> +{
> >>> +	if (of_machine_is_compatible("fsl,P2020RDB"))
> >>> +		return 1;
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +static int __init p2020_rdb_pc_probe(void)
> >>> +{
> >>> +	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> >>> +		return 1;
> >>> +	return 0;
> >>> +}
> >>> +#endif /* CONFIG_MPC85xx_RDB */
> >>>    
> >>> -define_machine(mpc8544_ds) {
> >>> -	.name			= "MPC8544 DS",
> >>> -	.probe			= mpc8544_ds_probe,
> >>> +#ifdef CONFIG_MPC85xx_DS
> >>> +define_machine(p2020_ds) {
> >>> +	.name			= "P2020 DS",
> >>> +	.probe			= p2020_ds_probe,
> >>>    	.setup_arch		= mpc85xx_ds_setup_arch,
> >>>    	.init_IRQ		= mpc85xx_ds_pic_init,
> >>>    #ifdef CONFIG_PCI
> >>> @@ -199,12 +240,14 @@ define_machine(mpc8544_ds) {
> >>>    	.calibrate_decr		= generic_calibrate_decr,
> >>>    	.progress		= udbg_progress,
> >>>    };
> >>> -
> >>> -define_machine(mpc8572_ds) {
> >>> -	.name			= "MPC8572 DS",
> >>> -	.probe			= mpc8572_ds_probe,
> >>> -	.setup_arch		= mpc85xx_ds_setup_arch,
> >>> -	.init_IRQ		= mpc85xx_ds_pic_init,
> >>> +#endif /* CONFIG_MPC85xx_DS */
> >>> +
> >>> +#ifdef CONFIG_MPC85xx_RDB
> >>> +define_machine(p2020_rdb) {
> >>> +	.name			= "P2020 RDB",
> >>> +	.probe			= p2020_rdb_probe,
> >>> +	.setup_arch		= mpc85xx_rdb_setup_arch,
> >>> +	.init_IRQ		= mpc85xx_rdb_pic_init,
> >>>    #ifdef CONFIG_PCI
> >>>    	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> >>>    	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> >>> @@ -214,11 +257,11 @@ define_machine(mpc8572_ds) {
> >>>    	.progress		= udbg_progress,
> >>>    };
> >>>    
> >>> -define_machine(p2020_ds) {
> >>> -	.name			= "P2020 DS",
> >>> -	.probe			= p2020_ds_probe,
> >>> -	.setup_arch		= mpc85xx_ds_setup_arch,
> >>> -	.init_IRQ		= mpc85xx_ds_pic_init,
> >>> +define_machine(p2020_rdb_pc) {
> >>> +	.name			= "P2020RDB-PC",
> >>> +	.probe			= p2020_rdb_pc_probe,
> >>> +	.setup_arch		= mpc85xx_rdb_setup_arch,
> >>> +	.init_IRQ		= mpc85xx_rdb_pic_init,
> >>>    #ifdef CONFIG_PCI
> >>>    	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
> >>>    	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
> >>> @@ -227,3 +270,4 @@ define_machine(p2020_ds) {
> >>>    	.calibrate_decr		= generic_calibrate_decr,
> >>>    	.progress		= udbg_progress,
> >>>    };
> >>> +#endif /* CONFIG_MPC85xx_RDB */

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
  2022-09-26  9:47     ` Pali Rohár
@ 2022-10-16 11:05       ` Pali Rohár
  2022-10-16 16:59         ` Christophe Leroy
  0 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-10-16 11:05 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

Hello Christophe! Do you have any other comments for this patch series?

On Monday 26 September 2022 11:47:06 Pali Rohár wrote:
> On Monday 26 September 2022 09:43:55 Christophe Leroy wrote:
> > Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> > > Function mpc85xx_ds_pic_init() is not used out of the mpc85xx_ds.c file.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > 
> > This patch should be squashed into patch 1.
> 
> No problem. Just to explain that I split those changes into different
> patches because they touch different files and different board code.
> And I thought that different things should be in different patches.
> 
> > > ---
> > >   arch/powerpc/platforms/85xx/mpc85xx_ds.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > > index f8d2c97f39bd..9a6d637ef54a 100644
> > > --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > > +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > > @@ -54,7 +54,7 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
> > >   }
> > >   #endif	/* CONFIG_PPC_I8259 */
> > >   
> > > -void __init mpc85xx_ds_pic_init(void)
> > > +static void __init mpc85xx_ds_pic_init(void)
> > >   {
> > >   	struct mpic *mpic;
> > >   #ifdef CONFIG_PPC_I8259

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
  2022-10-16 11:05       ` Pali Rohár
@ 2022-10-16 16:59         ` Christophe Leroy
  2022-11-01 23:25           ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Christophe Leroy @ 2022-10-16 16:59 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

Hello,

Le 16/10/2022 à 13:05, Pali Rohár a écrit :
> Hello Christophe! Do you have any other comments for this patch series?

I'm AFK for two weeks, but as far as I remember I don't have any more 
comments.

> 
> On Monday 26 September 2022 11:47:06 Pali Rohár wrote:
>> On Monday 26 September 2022 09:43:55 Christophe Leroy wrote:
>>> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
>>>> Function mpc85xx_ds_pic_init() is not used out of the mpc85xx_ds.c file.
>>>>
>>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>>
>>> This patch should be squashed into patch 1.
>>
>> No problem. Just to explain that I split those changes into different
>> patches because they touch different files and different board code.
>> And I thought that different things should be in different patches.
>>
>>>> ---
>>>>    arch/powerpc/platforms/85xx/mpc85xx_ds.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>> index f8d2c97f39bd..9a6d637ef54a 100644
>>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>> @@ -54,7 +54,7 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
>>>>    }
>>>>    #endif	/* CONFIG_PPC_I8259 */
>>>>    
>>>> -void __init mpc85xx_ds_pic_init(void)
>>>> +static void __init mpc85xx_ds_pic_init(void)
>>>>    {
>>>>    	struct mpic *mpic;
>>>>    #ifdef CONFIG_PPC_I8259

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
  2022-10-16 16:59         ` Christophe Leroy
@ 2022-11-01 23:25           ` Pali Rohár
  2022-11-26 16:25             ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-11-01 23:25 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

On Sunday 16 October 2022 16:59:53 Christophe Leroy wrote:
> Hello,
> 
> Le 16/10/2022 à 13:05, Pali Rohár a écrit :
> > Hello Christophe! Do you have any other comments for this patch series?
> 
> I'm AFK for two weeks, but as far as I remember I don't have any more 
> comments.

Hello! When you are back, could you look at my feedback to your comments?

> > 
> > On Monday 26 September 2022 11:47:06 Pali Rohár wrote:
> >> On Monday 26 September 2022 09:43:55 Christophe Leroy wrote:
> >>> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> >>>> Function mpc85xx_ds_pic_init() is not used out of the mpc85xx_ds.c file.
> >>>>
> >>>> Signed-off-by: Pali Rohár <pali@kernel.org>
> >>>
> >>> This patch should be squashed into patch 1.
> >>
> >> No problem. Just to explain that I split those changes into different
> >> patches because they touch different files and different board code.
> >> And I thought that different things should be in different patches.
> >>
> >>>> ---
> >>>>    arch/powerpc/platforms/85xx/mpc85xx_ds.c | 2 +-
> >>>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> >>>> index f8d2c97f39bd..9a6d637ef54a 100644
> >>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> >>>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> >>>> @@ -54,7 +54,7 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
> >>>>    }
> >>>>    #endif	/* CONFIG_PPC_I8259 */
> >>>>    
> >>>> -void __init mpc85xx_ds_pic_init(void)
> >>>> +static void __init mpc85xx_ds_pic_init(void)
> >>>>    {
> >>>>    	struct mpic *mpic;
> >>>>    #ifdef CONFIG_PPC_I8259

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
  2022-11-01 23:25           ` Pali Rohár
@ 2022-11-26 16:25             ` Pali Rohár
  2022-12-02 18:46               ` Christophe Leroy
  0 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-11-26 16:25 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

On Wednesday 02 November 2022 00:25:03 Pali Rohár wrote:
> On Sunday 16 October 2022 16:59:53 Christophe Leroy wrote:
> > Hello,
> > 
> > Le 16/10/2022 à 13:05, Pali Rohár a écrit :
> > > Hello Christophe! Do you have any other comments for this patch series?
> > 
> > I'm AFK for two weeks, but as far as I remember I don't have any more 
> > comments.
> 
> Hello! When you are back, could you look at my feedback to your comments?

PING?

> > > 
> > > On Monday 26 September 2022 11:47:06 Pali Rohár wrote:
> > >> On Monday 26 September 2022 09:43:55 Christophe Leroy wrote:
> > >>> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
> > >>>> Function mpc85xx_ds_pic_init() is not used out of the mpc85xx_ds.c file.
> > >>>>
> > >>>> Signed-off-by: Pali Rohár <pali@kernel.org>
> > >>>
> > >>> This patch should be squashed into patch 1.
> > >>
> > >> No problem. Just to explain that I split those changes into different
> > >> patches because they touch different files and different board code.
> > >> And I thought that different things should be in different patches.
> > >>
> > >>>> ---
> > >>>>    arch/powerpc/platforms/85xx/mpc85xx_ds.c | 2 +-
> > >>>>    1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > >>>> index f8d2c97f39bd..9a6d637ef54a 100644
> > >>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > >>>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > >>>> @@ -54,7 +54,7 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
> > >>>>    }
> > >>>>    #endif	/* CONFIG_PPC_I8259 */
> > >>>>    
> > >>>> -void __init mpc85xx_ds_pic_init(void)
> > >>>> +static void __init mpc85xx_ds_pic_init(void)
> > >>>>    {
> > >>>>    	struct mpic *mpic;
> > >>>>    #ifdef CONFIG_PPC_I8259

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
  2022-11-26 16:25             ` Pali Rohár
@ 2022-12-02 18:46               ` Christophe Leroy
  0 siblings, 0 replies; 30+ messages in thread
From: Christophe Leroy @ 2022-12-02 18:46 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev



Le 26/11/2022 à 17:25, Pali Rohár a écrit :
> On Wednesday 02 November 2022 00:25:03 Pali Rohár wrote:
>> On Sunday 16 October 2022 16:59:53 Christophe Leroy wrote:
>>> Hello,
>>>
>>> Le 16/10/2022 à 13:05, Pali Rohár a écrit :
>>>> Hello Christophe! Do you have any other comments for this patch series?
>>>
>>> I'm AFK for two weeks, but as far as I remember I don't have any more
>>> comments.
>>
>> Hello! When you are back, could you look at my feedback to your comments?
> 
> PING?
> 
>>>>
>>>> On Monday 26 September 2022 11:47:06 Pali Rohár wrote:
>>>>> On Monday 26 September 2022 09:43:55 Christophe Leroy wrote:
>>>>>> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
>>>>>>> Function mpc85xx_ds_pic_init() is not used out of the mpc85xx_ds.c file.
>>>>>>>
>>>>>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>>>>>
>>>>>> This patch should be squashed into patch 1.
>>>>>
>>>>> No problem. Just to explain that I split those changes into different
>>>>> patches because they touch different files and different board code.
>>>>> And I thought that different things should be in different patches.

It's fine for me if you prefer keeping them separate, up to you.

Christophe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description
  2022-08-19 19:15 [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
                   ` (7 preceding siblings ...)
  2022-09-24 12:42 ` [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
@ 2022-12-04 10:54 ` Pali Rohár
  2022-12-07 14:07   ` Christophe Leroy
  8 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2022-12-04 10:54 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev

On Friday 19 August 2022 21:15:50 Pali Rohár wrote:
> This patch series unifies all P2020 boards and machine descriptions into
> one generic unified P2020 machine description. With this generic machine
> description, kernel can boot on any P2020-based board with correct DTS
> file.
> 
> Tested on CZ.NIC Turris 1.1 board with has Freescale P2020 processor.
> Kernel during booting correctly detects P2020 and prints:
> [    0.000000] Using Freescale P2020 machine description
> 
> Pali Rohár (7):
>   powerpc/85xx: Mark mpc85xx_rdb_pic_init() as static
>   powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
>   powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
>   powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks
>   powerpc/85xx: p2020: Define just one machine description
>   powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020
>   powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string

Christophe, could you please summarize for me what is needed to change /
fix / adjust in this patch series? We had discussion about all patches
in this thread but I have not received reply for every my reaction. And
I'm not sure what to do and what not. So I can prepare a V2 version.

>  arch/powerpc/boot/dts/turris1x.dts            |   2 +-
>  arch/powerpc/platforms/85xx/Kconfig           |  22 ++-
>  arch/powerpc/platforms/85xx/Makefile          |   1 +
>  arch/powerpc/platforms/85xx/mpc85xx_ds.c      |  25 +--
>  arch/powerpc/platforms/85xx/mpc85xx_rdb.c     |  46 +-----
>  .../platforms/85xx/{mpc85xx_ds.c => p2020.c}  | 144 +++++++-----------
>  6 files changed, 75 insertions(+), 165 deletions(-)
>  copy arch/powerpc/platforms/85xx/{mpc85xx_ds.c => p2020.c} (53%)
> 
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
  2022-09-26 10:26         ` Pali Rohár
@ 2022-12-07 14:02           ` Christophe Leroy
  0 siblings, 0 replies; 30+ messages in thread
From: Christophe Leroy @ 2022-12-07 14:02 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev



Le 26/09/2022 à 12:26, Pali Rohár a écrit :
> On Monday 26 September 2022 10:17:26 Christophe Leroy wrote:
>> Le 26/09/2022 à 11:53, Pali Rohár a écrit :
>>> On Monday 26 September 2022 09:48:02 Christophe Leroy wrote:
>>>> Le 19/08/2022 à 21:15, Pali Rohár a écrit :
>>>>> This moves machine descriptions and all related code for all P2020 boards
>>>>> into new p2020.c source file. This is preparation for code deduplication
>>>>> and providing one unified machine description for all P2020 boards.
>>>>
>>>> I'm having hard time to review this patch.
>>>>
>>>> It looks like you are doing much more than just moving machine
>>>> descriptions and related code into p2020.c
>>>>
>>>> Apparently p2020.c has a lot of code that doesn't seem be move from
>>>> somewhere else.
>>>>
>>>> Maybe there is a need to tidy up in order to ease reviewing.
>>>
>>> This is probably harder to read due to how git format-patch generated
>>> this email. The important is:
>>>
>>>    copy from arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>    copy to arch/powerpc/platforms/85xx/p2020.c
>>>
>>> Which means that git thinks that my newly introduced file p2020.c is
>>> similar to old file mpc85xx_ds.c and generated diff in format which do:
>>>
>>>    1. copy mpc85xx_ds.c to p2020.c
>>>    2. apply diff on newly introduced file p2020.c
>>>
>>> Code is really moved from mpc85xx_ds.c and mpc85xx_rdb.c files into file
>>> p2020.c.
>>>
>>> File p2020.c is new in this patch.
>>
>> Well, I didn't really look in how the patch was generated, I imported
>> your series and mainly reviewed it in git directly.
>>
>> For this patch I have the following diff stat:
>>
>> $ git show --stat e2d8c39e2e32855658d1c5f042a7ce88952f488a
>> commit e2d8c39e2e32855658d1c5f042a7ce88952f488a
>> Author: Pali Rohár <pali@kernel.org>
>> Date:   Fri Aug 19 21:15:53 2022 +0200
>>
>>       powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
>>
>>       This moves machine descriptions and all related code for all P2020
>> boards
>>       into new p2020.c source file. This is preparation for code
>> deduplication
>>       and providing one unified machine description for all P2020 boards.
>>
>>       Signed-off-by: Pali Rohár <pali@kernel.org>
>>
>>    arch/powerpc/platforms/85xx/Makefile      |   2 ++
>>    arch/powerpc/platforms/85xx/mpc85xx_ds.c  |  23 --------------
>>    arch/powerpc/platforms/85xx/mpc85xx_rdb.c |  44 --------------------------
>>    arch/powerpc/platforms/85xx/p2020.c       | 273
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>    4 files changed, 275 insertions(+), 67 deletions(-)
>>
>>
>> So there is a lot more code added than deleted.
>>
>> If it was really a code move as described in the commit message, I would
>> have approximately the same number of inserts as number of deletions.
> 
> I see... The reason is that helper ds/rdb functions are copies (not
> moved) because they are needed still in ds/rdb boards. And in later
> patches in this patch series are then p2020 helper function cleaned and
> simplified.
> 
> So as I see basically this change moves p2020 machine descriptions from
> ds/rdb files into p2020.c, plus copy helper functions.
> 

If that's the case, that's fine. Please describe it like that in the 
commit message.

> Not sure what should be the best case how to do it. I did not wanted to
> introduce regression in the code, so I rather did not touched non-p2020
> code in ds/rdb files.
> 
>>
>>>
>>>>>
>>>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>>>> ---
>>>>>     arch/powerpc/platforms/85xx/Makefile          |   2 +
>>>>>     arch/powerpc/platforms/85xx/mpc85xx_ds.c      |  23 ---
>>>>>     arch/powerpc/platforms/85xx/mpc85xx_rdb.c     |  44 ------
>>>>>     .../platforms/85xx/{mpc85xx_ds.c => p2020.c}  | 134 ++++++++++++------
>>>>>     4 files changed, 91 insertions(+), 112 deletions(-)
>>>>>     copy arch/powerpc/platforms/85xx/{mpc85xx_ds.c => p2020.c} (65%)
>>>>>
>>>>> diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
>>>>> index 260fbad7967b..1ad261b4eeb6 100644
>>>>> --- a/arch/powerpc/platforms/85xx/Makefile
>>>>> +++ b/arch/powerpc/platforms/85xx/Makefile
>>>>> @@ -23,6 +23,8 @@ obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
>>>>>     obj-$(CONFIG_P1022_DS)    += p1022_ds.o
>>>>>     obj-$(CONFIG_P1022_RDK)   += p1022_rdk.o
>>>>>     obj-$(CONFIG_P1023_RDB)   += p1023_rdb.o
>>>>> +obj-$(CONFIG_MPC85xx_DS)  += p2020.o
>>>>> +obj-$(CONFIG_MPC85xx_RDB) += p2020.o
>>>>>     obj-$(CONFIG_TWR_P102x)   += twr_p102x.o
>>>>>     obj-$(CONFIG_CORENET_GENERIC)   += corenet_generic.o
>>>>>     obj-$(CONFIG_FB_FSL_DIU)	+= t1042rdb_diu.o
>>>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>>> index 9a6d637ef54a..05aac997b5ed 100644
>>>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>>> @@ -168,7 +168,6 @@ static int __init mpc8544_ds_probe(void)
>>>>>     
>>>>>     machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
>>>>>     machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
>>>>> -machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
>>>>>     
>>>>>     /*
>>>>>      * Called very early, device-tree isn't unflattened
>>>>> @@ -178,14 +177,6 @@ static int __init mpc8572_ds_probe(void)
>>>>>     	return !!of_machine_is_compatible("fsl,MPC8572DS");
>>>>>     }
>>>>>     
>>>>> -/*
>>>>> - * Called very early, device-tree isn't unflattened
>>>>> - */
>>>>> -static int __init p2020_ds_probe(void)
>>>>> -{
>>>>> -	return !!of_machine_is_compatible("fsl,P2020DS");
>>>>> -}
>>>>> -
>>>>>     define_machine(mpc8544_ds) {
>>>>>     	.name			= "MPC8544 DS",
>>>>>     	.probe			= mpc8544_ds_probe,
>>>>> @@ -213,17 +204,3 @@ define_machine(mpc8572_ds) {
>>>>>     	.calibrate_decr		= generic_calibrate_decr,
>>>>>     	.progress		= udbg_progress,
>>>>>     };
>>>>> -
>>>>> -define_machine(p2020_ds) {
>>>>> -	.name			= "P2020 DS",
>>>>> -	.probe			= p2020_ds_probe,
>>>>> -	.setup_arch		= mpc85xx_ds_setup_arch,
>>>>> -	.init_IRQ		= mpc85xx_ds_pic_init,
>>>>> -#ifdef CONFIG_PCI
>>>>> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>>>> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
>>>>> -#endif
>>>>> -	.get_irq		= mpic_get_irq,
>>>>> -	.calibrate_decr		= generic_calibrate_decr,
>>>>> -	.progress		= udbg_progress,
>>>>> -};
>>>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
>>>>> index b6129c148fea..05f1ed635735 100644
>>>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
>>>>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
>>>>> @@ -108,8 +108,6 @@ static void __init mpc85xx_rdb_setup_arch(void)
>>>>>     	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
>>>>>     }
>>>>>     
>>>>> -machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
>>>>> -machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
>>>>>     machine_arch_initcall(p1020_mbg_pc, mpc85xx_common_publish_devices);
>>>>>     machine_arch_initcall(p1020_rdb, mpc85xx_common_publish_devices);
>>>>>     machine_arch_initcall(p1020_rdb_pc, mpc85xx_common_publish_devices);
>>>>> @@ -122,13 +120,6 @@ machine_arch_initcall(p1024_rdb, mpc85xx_common_publish_devices);
>>>>>     /*
>>>>>      * Called very early, device-tree isn't unflattened
>>>>>      */
>>>>> -static int __init p2020_rdb_probe(void)
>>>>> -{
>>>>> -	if (of_machine_is_compatible("fsl,P2020RDB"))
>>>>> -		return 1;
>>>>> -	return 0;
>>>>> -}
>>>>> -
>>>>>     static int __init p1020_rdb_probe(void)
>>>>>     {
>>>>>     	if (of_machine_is_compatible("fsl,P1020RDB"))
>>>>> @@ -153,13 +144,6 @@ static int __init p1021_rdb_pc_probe(void)
>>>>>     	return 0;
>>>>>     }
>>>>>     
>>>>> -static int __init p2020_rdb_pc_probe(void)
>>>>> -{
>>>>> -	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
>>>>> -		return 1;
>>>>> -	return 0;
>>>>> -}
>>>>> -
>>>>>     static int __init p1025_rdb_probe(void)
>>>>>     {
>>>>>     	return of_machine_is_compatible("fsl,P1025RDB");
>>>>> @@ -180,20 +164,6 @@ static int __init p1024_rdb_probe(void)
>>>>>     	return of_machine_is_compatible("fsl,P1024RDB");
>>>>>     }
>>>>>     
>>>>> -define_machine(p2020_rdb) {
>>>>> -	.name			= "P2020 RDB",
>>>>> -	.probe			= p2020_rdb_probe,
>>>>> -	.setup_arch		= mpc85xx_rdb_setup_arch,
>>>>> -	.init_IRQ		= mpc85xx_rdb_pic_init,
>>>>> -#ifdef CONFIG_PCI
>>>>> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>>>> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
>>>>> -#endif
>>>>> -	.get_irq		= mpic_get_irq,
>>>>> -	.calibrate_decr		= generic_calibrate_decr,
>>>>> -	.progress		= udbg_progress,
>>>>> -};
>>>>> -
>>>>>     define_machine(p1020_rdb) {
>>>>>     	.name			= "P1020 RDB",
>>>>>     	.probe			= p1020_rdb_probe,
>>>>> @@ -222,20 +192,6 @@ define_machine(p1021_rdb_pc) {
>>>>>     	.progress		= udbg_progress,
>>>>>     };
>>>>>     
>>>>> -define_machine(p2020_rdb_pc) {
>>>>> -	.name			= "P2020RDB-PC",
>>>>> -	.probe			= p2020_rdb_pc_probe,
>>>>> -	.setup_arch		= mpc85xx_rdb_setup_arch,
>>>>> -	.init_IRQ		= mpc85xx_rdb_pic_init,
>>>>> -#ifdef CONFIG_PCI
>>>>> -	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>>>> -	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
>>>>> -#endif
>>>>> -	.get_irq		= mpic_get_irq,
>>>>> -	.calibrate_decr		= generic_calibrate_decr,
>>>>> -	.progress		= udbg_progress,
>>>>> -};
>>>>> -
>>>>>     define_machine(p1025_rdb) {
>>>>>     	.name			= "P1025 RDB",
>>>>>     	.probe			= p1025_rdb_probe,
>>>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/p2020.c
>>>>> similarity index 65%
>>>>> copy from arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>>> copy to arch/powerpc/platforms/85xx/p2020.c
>>>>> index 9a6d637ef54a..d65d4c88ac47 100644
>>>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>>> +++ b/arch/powerpc/platforms/85xx/p2020.c
>>>>> @@ -1,11 +1,9 @@
>>>>>     // SPDX-License-Identifier: GPL-2.0-or-later
>>>>>     /*
>>>>> - * MPC85xx DS Board Setup
>>>>> + * Freescale P2020 board Setup
>>>>>      *
>>>>> - * Author Xianghua Xiao (x.xiao@freescale.com)
>>>>> - * Roy Zang <tie-fei.zang@freescale.com>
>>>>> - * 	- Add PCI/PCI Exprees support
>>>>> - * Copyright 2007 Freescale Semiconductor Inc.
>>>>> + * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
>>>>> + * Copyright 2022 Pali Rohár <pali@kernel.org>
>>>>>      */
>>>>>     
>>>>>     #include <linux/stddef.h>
>>>>> @@ -17,6 +15,7 @@
>>>>>     #include <linux/interrupt.h>
>>>>>     #include <linux/of_irq.h>
>>>>>     #include <linux/of_platform.h>
>>>>> +#include <linux/fsl/guts.h>
>>>>>     
>>>>>     #include <asm/time.h>
>>>>>     #include <asm/machdep.h>
>>>>> @@ -27,6 +26,8 @@
>>>>>     #include <asm/i8259.h>
>>>>>     #include <asm/swiotlb.h>
>>>>>     
>>>>> +#include <soc/fsl/qe/qe.h>
>>>>> +
>>>>>     #include <sysdev/fsl_soc.h>
>>>>>     #include <sysdev/fsl_pci.h>
>>>>>     #include "smp.h"
>>>>> @@ -41,6 +42,8 @@
>>>>>     #define DBG(fmt, args...)
>>>>>     #endif
>>>>>     
>>>>> +#ifdef CONFIG_MPC85xx_DS
>>>>> +
>>>>>     #ifdef CONFIG_PPC_I8259
>>>>>     static void mpc85xx_8259_cascade(struct irq_desc *desc)
>>>>>     {
>>>>> @@ -62,18 +65,11 @@ static void __init mpc85xx_ds_pic_init(void)
>>>>>     	struct device_node *cascade_node = NULL;
>>>>>     	int cascade_irq;
>>>>>     #endif
>>>>> -	if (of_machine_is_compatible("fsl,MPC8572DS-CAMP")) {
>>>>> -		mpic = mpic_alloc(NULL, 0,
>>>>> -			MPIC_NO_RESET |
>>>>> -			MPIC_BIG_ENDIAN |
>>>>> -			MPIC_SINGLE_DEST_CPU,
>>>>> -			0, 256, " OpenPIC  ");
>>>>> -	} else {
>>>>> -		mpic = mpic_alloc(NULL, 0,
>>>>> -			  MPIC_BIG_ENDIAN |
>>>>> -			  MPIC_SINGLE_DEST_CPU,
>>>>> -			0, 256, " OpenPIC  ");
>>>>> -	}
>>>>> +
>>>>> +	mpic = mpic_alloc(NULL, 0,
>>>>> +		  MPIC_BIG_ENDIAN |
>>>>> +		  MPIC_SINGLE_DEST_CPU,
>>>>> +		0, 256, " OpenPIC  ");
>>>>>     
>>>>>     	BUG_ON(mpic == NULL);
>>>>>     	mpic_init(mpic);
>>>>> @@ -142,9 +138,27 @@ static void __init mpc85xx_ds_uli_init(void)
>>>>>     #endif
>>>>>     }
>>>>>     
>>>>> +#endif /* CONFIG_MPC85xx_DS */
>>>>> +
>>>>> +#ifdef CONFIG_MPC85xx_RDB
>>>>> +static void __init mpc85xx_rdb_pic_init(void)
>>>>> +{
>>>>> +	struct mpic *mpic;
>>>>> +
>>>>> +	mpic = mpic_alloc(NULL, 0,
>>>>> +	  MPIC_BIG_ENDIAN |
>>>>> +	  MPIC_SINGLE_DEST_CPU,
>>>>> +	  0, 256, " OpenPIC  ");
>>>>> +
>>>>> +	BUG_ON(mpic == NULL);
>>>>> +	mpic_init(mpic);
>>>>> +}
>>>>> +#endif /* CONFIG_MPC85xx_RDB */
>>>>> +
>>>>>     /*
>>>>>      * Setup the architecture
>>>>>      */
>>>>> +#ifdef CONFIG_MPC85xx_DS
>>>>>     static void __init mpc85xx_ds_setup_arch(void)
>>>>>     {
>>>>>     	if (ppc_md.progress)
>>>>> @@ -157,38 +171,65 @@ static void __init mpc85xx_ds_setup_arch(void)
>>>>>     
>>>>>     	printk("MPC85xx DS board from Freescale Semiconductor\n");
>>>>>     }
>>>>> +#endif /* CONFIG_MPC85xx_DS */
>>>>>     
>>>>> -/*
>>>>> - * Called very early, device-tree isn't unflattened
>>>>> - */
>>>>> -static int __init mpc8544_ds_probe(void)
>>>>> +#ifdef CONFIG_MPC85xx_RDB
>>>>> +static void __init mpc85xx_rdb_setup_arch(void)
>>>>>     {
>>>>> -	return !!of_machine_is_compatible("MPC8544DS");
>>>>> +	if (ppc_md.progress)
>>>>> +		ppc_md.progress("mpc85xx_rdb_setup_arch()", 0);
>>>>> +
>>>>> +	mpc85xx_smp_init();
>>>>> +
>>>>> +	fsl_pci_assign_primary();
>>>>> +
>>>>> +#ifdef CONFIG_QUICC_ENGINE
>>>>> +	mpc85xx_qe_par_io_init();
>>>>> +#endif	/* CONFIG_QUICC_ENGINE */
>>>>> +
>>>>> +	printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
>>>>>     }
>>>>> +#endif /* CONFIG_MPC85xx_RDB */
>>>>>     
>>>>> -machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
>>>>> -machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
>>>>> +#ifdef CONFIG_MPC85xx_DS
>>>>>     machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
>>>>> +#endif /* CONFIG_MPC85xx_DS */
>>>>>     
>>>>> -/*
>>>>> - * Called very early, device-tree isn't unflattened
>>>>> - */
>>>>> -static int __init mpc8572_ds_probe(void)
>>>>> -{
>>>>> -	return !!of_machine_is_compatible("fsl,MPC8572DS");
>>>>> -}
>>>>> +#ifdef CONFIG_MPC85xx_RDB
>>>>> +machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
>>>>> +machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
>>>>> +#endif /* CONFIG_MPC85xx_RDB */
>>>>>     
>>>>>     /*
>>>>>      * Called very early, device-tree isn't unflattened
>>>>>      */
>>>>> +#ifdef CONFIG_MPC85xx_DS
>>>>>     static int __init p2020_ds_probe(void)
>>>>>     {
>>>>>     	return !!of_machine_is_compatible("fsl,P2020DS");
>>>>>     }
>>>>> +#endif /* CONFIG_MPC85xx_DS */
>>>>> +
>>>>> +#ifdef CONFIG_MPC85xx_RDB
>>>>> +static int __init p2020_rdb_probe(void)
>>>>> +{
>>>>> +	if (of_machine_is_compatible("fsl,P2020RDB"))
>>>>> +		return 1;
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static int __init p2020_rdb_pc_probe(void)
>>>>> +{
>>>>> +	if (of_machine_is_compatible("fsl,P2020RDB-PC"))
>>>>> +		return 1;
>>>>> +	return 0;
>>>>> +}
>>>>> +#endif /* CONFIG_MPC85xx_RDB */
>>>>>     
>>>>> -define_machine(mpc8544_ds) {
>>>>> -	.name			= "MPC8544 DS",
>>>>> -	.probe			= mpc8544_ds_probe,
>>>>> +#ifdef CONFIG_MPC85xx_DS
>>>>> +define_machine(p2020_ds) {
>>>>> +	.name			= "P2020 DS",
>>>>> +	.probe			= p2020_ds_probe,
>>>>>     	.setup_arch		= mpc85xx_ds_setup_arch,
>>>>>     	.init_IRQ		= mpc85xx_ds_pic_init,
>>>>>     #ifdef CONFIG_PCI
>>>>> @@ -199,12 +240,14 @@ define_machine(mpc8544_ds) {
>>>>>     	.calibrate_decr		= generic_calibrate_decr,
>>>>>     	.progress		= udbg_progress,
>>>>>     };
>>>>> -
>>>>> -define_machine(mpc8572_ds) {
>>>>> -	.name			= "MPC8572 DS",
>>>>> -	.probe			= mpc8572_ds_probe,
>>>>> -	.setup_arch		= mpc85xx_ds_setup_arch,
>>>>> -	.init_IRQ		= mpc85xx_ds_pic_init,
>>>>> +#endif /* CONFIG_MPC85xx_DS */
>>>>> +
>>>>> +#ifdef CONFIG_MPC85xx_RDB
>>>>> +define_machine(p2020_rdb) {
>>>>> +	.name			= "P2020 RDB",
>>>>> +	.probe			= p2020_rdb_probe,
>>>>> +	.setup_arch		= mpc85xx_rdb_setup_arch,
>>>>> +	.init_IRQ		= mpc85xx_rdb_pic_init,
>>>>>     #ifdef CONFIG_PCI
>>>>>     	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>>>>     	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
>>>>> @@ -214,11 +257,11 @@ define_machine(mpc8572_ds) {
>>>>>     	.progress		= udbg_progress,
>>>>>     };
>>>>>     
>>>>> -define_machine(p2020_ds) {
>>>>> -	.name			= "P2020 DS",
>>>>> -	.probe			= p2020_ds_probe,
>>>>> -	.setup_arch		= mpc85xx_ds_setup_arch,
>>>>> -	.init_IRQ		= mpc85xx_ds_pic_init,
>>>>> +define_machine(p2020_rdb_pc) {
>>>>> +	.name			= "P2020RDB-PC",
>>>>> +	.probe			= p2020_rdb_pc_probe,
>>>>> +	.setup_arch		= mpc85xx_rdb_setup_arch,
>>>>> +	.init_IRQ		= mpc85xx_rdb_pic_init,
>>>>>     #ifdef CONFIG_PCI
>>>>>     	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
>>>>>     	.pcibios_fixup_phb      = fsl_pcibios_fixup_phb,
>>>>> @@ -227,3 +270,4 @@ define_machine(p2020_ds) {
>>>>>     	.calibrate_decr		= generic_calibrate_decr,
>>>>>     	.progress		= udbg_progress,
>>>>>     };
>>>>> +#endif /* CONFIG_MPC85xx_RDB */

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description
  2022-12-04 10:54 ` Pali Rohár
@ 2022-12-07 14:07   ` Christophe Leroy
  0 siblings, 0 replies; 30+ messages in thread
From: Christophe Leroy @ 2022-12-07 14:07 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Sinan Akman, linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev



Le 04/12/2022 à 11:54, Pali Rohár a écrit :
> On Friday 19 August 2022 21:15:50 Pali Rohár wrote:
>> This patch series unifies all P2020 boards and machine descriptions into
>> one generic unified P2020 machine description. With this generic machine
>> description, kernel can boot on any P2020-based board with correct DTS
>> file.
>>
>> Tested on CZ.NIC Turris 1.1 board with has Freescale P2020 processor.
>> Kernel during booting correctly detects P2020 and prints:
>> [    0.000000] Using Freescale P2020 machine description
>>
>> Pali Rohár (7):
>>    powerpc/85xx: Mark mpc85xx_rdb_pic_init() as static
>>    powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
>>    powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
>>    powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks
>>    powerpc/85xx: p2020: Define just one machine description
>>    powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020
>>    powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string
> 
> Christophe, could you please summarize for me what is needed to change /
> fix / adjust in this patch series? We had discussion about all patches
> in this thread but I have not received reply for every my reaction. And
> I'm not sure what to do and what not. So I can prepare a V2 version.

I've been through all comments and answers once more. If I don't reply 
to your explanation, it means I agree with it.

So I think you can proceed now with v2.

> 
>>   arch/powerpc/boot/dts/turris1x.dts            |   2 +-
>>   arch/powerpc/platforms/85xx/Kconfig           |  22 ++-
>>   arch/powerpc/platforms/85xx/Makefile          |   1 +
>>   arch/powerpc/platforms/85xx/mpc85xx_ds.c      |  25 +--
>>   arch/powerpc/platforms/85xx/mpc85xx_rdb.c     |  46 +-----
>>   .../platforms/85xx/{mpc85xx_ds.c => p2020.c}  | 144 +++++++-----------
>>   6 files changed, 75 insertions(+), 165 deletions(-)
>>   copy arch/powerpc/platforms/85xx/{mpc85xx_ds.c => p2020.c} (53%)
>>
>> -- 
>> 2.20.1
>>

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2022-12-07 14:08 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-19 19:15 [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
2022-08-19 19:15 ` [PATCH 1/7] powerpc/85xx: Mark mpc85xx_rdb_pic_init() as static Pali Rohár
2022-08-19 19:15 ` [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() " Pali Rohár
2022-09-26  9:43   ` Christophe Leroy
2022-09-26  9:47     ` Pali Rohár
2022-10-16 11:05       ` Pali Rohár
2022-10-16 16:59         ` Christophe Leroy
2022-11-01 23:25           ` Pali Rohár
2022-11-26 16:25             ` Pali Rohár
2022-12-02 18:46               ` Christophe Leroy
2022-08-19 19:15 ` [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c Pali Rohár
2022-09-26  9:48   ` Christophe Leroy
2022-09-26  9:53     ` Pali Rohár
2022-09-26 10:17       ` Christophe Leroy
2022-09-26 10:26         ` Pali Rohár
2022-12-07 14:02           ` Christophe Leroy
2022-08-19 19:15 ` [PATCH 4/7] powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks Pali Rohár
2022-09-26  9:59   ` Christophe Leroy
2022-08-19 19:15 ` [PATCH 5/7] powerpc/85xx: p2020: Define just one machine description Pali Rohár
2022-09-26 10:02   ` Christophe Leroy
2022-09-26 10:08     ` Pali Rohár
2022-08-19 19:15 ` [PATCH 6/7] powerpc/85xx: p2020: Enable boards by new config option CONFIG_P2020 Pali Rohár
2022-09-26 10:08   ` Christophe Leroy
2022-09-26 10:19     ` Pali Rohár
2022-08-19 19:15 ` [PATCH 7/7] powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string Pali Rohár
2022-09-26 10:10   ` Christophe Leroy
2022-09-26 10:21     ` Pali Rohár
2022-09-24 12:42 ` [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description Pali Rohár
2022-12-04 10:54 ` Pali Rohár
2022-12-07 14:07   ` Christophe Leroy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).