2022-08-19 19:21:10

by Pali Rohár

[permalink] [raw]
Subject: [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description

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


2022-08-19 19:31:02

by Pali Rohár

[permalink] [raw]
Subject: [PATCH 5/7] powerpc/85xx: p2020: Define just one machine description

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 <[email protected]>
---
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

2022-08-19 19:40:31

by Pali Rohár

[permalink] [raw]
Subject: [PATCH 3/7] 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 <[email protected]>
---
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 ([email protected])
- * Roy Zang <[email protected]>
- * - Add PCI/PCI Exprees support
- * Copyright 2007 Freescale Semiconductor Inc.
+ * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
+ * Copyright 2022 Pali Rohár <[email protected]>
*/

#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

2022-08-19 20:14:41

by Pali Rohár

[permalink] [raw]
Subject: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static

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

Signed-off-by: Pali Rohár <[email protected]>
---
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

2022-09-24 12:53:45

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description

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
>

2022-09-26 10:16:00

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c



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 <[email protected]>
> ---
> 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 ([email protected])
> - * Roy Zang <[email protected]>
> - * - Add PCI/PCI Exprees support
> - * Copyright 2007 Freescale Semiconductor Inc.
> + * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
> + * Copyright 2022 Pali Rohár <[email protected]>
> */
>
> #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 */

2022-09-26 10:16:12

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c

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 <[email protected]>
> > ---
> > 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 ([email protected])
> > - * Roy Zang <[email protected]>
> > - * - Add PCI/PCI Exprees support
> > - * Copyright 2007 Freescale Semiconductor Inc.
> > + * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
> > + * Copyright 2022 Pali Rohár <[email protected]>
> > */
> >
> > #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 */

2022-09-26 10:17:51

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static



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 <[email protected]>

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

2022-09-26 11:07:40

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static

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 <[email protected]>
>
> 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

2022-09-26 11:24:46

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c

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 <[email protected]>
> 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 <[email protected]>
>
> 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 <[email protected]>
> >>> ---
> >>> 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 ([email protected])
> >>> - * Roy Zang <[email protected]>
> >>> - * - Add PCI/PCI Exprees support
> >>> - * Copyright 2007 Freescale Semiconductor Inc.
> >>> + * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
> >>> + * Copyright 2022 Pali Rohár <[email protected]>
> >>> */
> >>>
> >>> #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 */

2022-09-26 12:36:50

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 5/7] powerpc/85xx: p2020: Define just one machine description

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.

2022-09-26 12:59:07

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c



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 <[email protected]>
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 <[email protected]>

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 <[email protected]>
>>> ---
>>> 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 ([email protected])
>>> - * Roy Zang <[email protected]>
>>> - * - Add PCI/PCI Exprees support
>>> - * Copyright 2007 Freescale Semiconductor Inc.
>>> + * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
>>> + * Copyright 2022 Pali Rohár <[email protected]>
>>> */
>>>
>>> #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 */

2022-09-26 13:25:13

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 5/7] powerpc/85xx: p2020: Define just one machine description



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 <[email protected]>
> ---
> 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 */

2022-10-16 11:25:25

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static

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 <[email protected]>
> >
> > 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

2022-10-16 17:18:30

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static

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 <[email protected]>
>>>
>>> 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

2022-11-01 23:43:16

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static

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 <[email protected]>
> >>>
> >>> 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

2022-11-26 17:20:32

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static

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 <[email protected]>
> > >>>
> > >>> 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

2022-12-02 18:57:14

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 2/7] powerpc/85xx: Mark mpc85xx_ds_pic_init() as static



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 <[email protected]>
>>>>>>
>>>>>> 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

2022-12-04 11:28:42

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description

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
>

2022-12-07 14:23:42

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 3/7] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c



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 <[email protected]>
>> 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 <[email protected]>
>>
>> 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 <[email protected]>
>>>>> ---
>>>>> 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 ([email protected])
>>>>> - * Roy Zang <[email protected]>
>>>>> - * - Add PCI/PCI Exprees support
>>>>> - * Copyright 2007 Freescale Semiconductor Inc.
>>>>> + * Copyright 2007,2009,2012-2013 Freescale Semiconductor Inc.
>>>>> + * Copyright 2022 Pali Rohár <[email protected]>
>>>>> */
>>>>>
>>>>> #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 */

2022-12-07 15:17:30

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 0/7] powerpc/85xx: p2020: Create one unified machine description



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
>>