Skip to contentSkip to conversation

mach-hisi: Make Hi3620 explicit, remove wildcard

PATCH New Marty Plummer 3 participants 8 messages Aug 30, 2016 – Sep 5, 2016

Subsystems (7)
Device tree bindings Arm (32-bit) Arm sub-architectures Common clk +3 more subsystems
lore
Share & tools
lore

Marty Plummer

author
#0 lore raw

PATCH 1/1 mach-hisi: Make Hi3620 explicit, remove wildcard

Signed-off-by Marty Plummer <[email protected]>
---
 arch/arm/Kconfig.debug              |  2 +-
 arch/arm/boot/dts/Makefile          |  2 +-
 arch/arm/configs/hisi_defconfig     |  2 +-
 arch/arm/configs/multi_v7_defconfig |  2 +-
 arch/arm/mach-hisi/Kconfig          |  6 +++---
 arch/arm/mach-hisi/core.h           | 10 +++++-----
 arch/arm/mach-hisi/hisilicon.c      |  4 ++--
 arch/arm/mach-hisi/hotplug.c        | 16 ++++++++--------
 arch/arm/mach-hisi/platsmp.c        | 24 ++++++++++++------------
 drivers/clk/hisilicon/Makefile      |  2 +-
 drivers/dma/Kconfig                 |  2 +-
 11 files changed, 36 insertions(+), 36 deletions(-)
arch/arm/Kconfig.debug +1 -1
index a9693b6..9094ca6 100644
@@ -280,7 +280,7 @@ choice
 
 	config DEBUG_HI3620_UART
 		bool "Hisilicon HI3620 Debug UART"
-		depends on ARCH_HI3xxx
+		depends on ARCH_HI3620
 		select DEBUG_UART_PL01X
 		help
 		  Say Y here if you want kernel low-level debugging support
arch/arm/boot/dts/Makefile +1 -1
index faacd52..7ffd3a3 100644
@@ -154,7 +154,7 @@ dtb-$(CONFIG_ARCH_EXYNOS5) += \
 	exynos5440-sd5v1.dtb \
 	exynos5440-ssdk5440.dtb \
 	exynos5800-peach-pi.dtb
-dtb-$(CONFIG_ARCH_HI3xxx) += \
+dtb-$(CONFIG_ARCH_HI3620) += \
 	hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += \
 	highbank.dtb \
arch/arm/configs/hisi_defconfig +1 -1
index b2e340b..14c8da6 100644
@@ -4,7 +4,7 @@ CONFIG_HIGH_RES_TIMERS=y
 CONFIG_BLK_DEV_INITRD=y
 CONFIG_RD_LZMA=y
 CONFIG_ARCH_HISI=y
-CONFIG_ARCH_HI3xxx=y
+CONFIG_ARCH_HI3620=y
 CONFIG_PARTITION_ADVANCED=y
 CONFIG_CMDLINE_PARTITION=y
 CONFIG_ARCH_HIX5HD2=y
arch/arm/configs/multi_v7_defconfig +1 -1
index 2c8665c..a63865e 100644
@@ -45,7 +45,7 @@ CONFIG_MACH_BERLIN_BG2Q=y
 CONFIG_ARCH_DIGICOLOR=y
 CONFIG_ARCH_HIGHBANK=y
 CONFIG_ARCH_HISI=y
-CONFIG_ARCH_HI3xxx=y
+CONFIG_ARCH_HI3620=y
 CONFIG_ARCH_HIX5HD2=y
 CONFIG_ARCH_HIP01=y
 CONFIG_ARCH_HIP04=y
arch/arm/mach-hisi/Kconfig +3 -3
index a3b091a..08871e0 100644
@@ -12,8 +12,8 @@ if ARCH_HISI
 
 menu "Hisilicon platform type"
 
-config ARCH_HI3xxx
-	bool "Hisilicon Hi36xx family"
+config ARCH_HI3620
+	bool "Hisilicon Hi3620 family"
 	depends on ARCH_MULTI_V7
 	select CACHE_L2X0
 	select HAVE_ARM_SCU if SMP

@@ -21,7 +21,7 @@ config ARCH_HI3xxx
 	select PINCTRL
 	select PINCTRL_SINGLE
 	help
-	  Support for Hisilicon Hi36xx SoC family
+	  Support for Hisilicon Hi3620 SoC family
 
 config ARCH_HIP01
        bool "Hisilicon HIP01 family"
arch/arm/mach-hisi/core.h +5 -5
index e883583..cb7bc84 100644
@@ -3,13 +3,13 @@
 
 #include <linux/reboot.h>
 
-extern void hi3xxx_set_cpu_jump(int cpu, void *jump_addr);
-extern int hi3xxx_get_cpu_jump(int cpu);
+extern void hi3620_set_cpu_jump(int cpu, void *jump_addr);
+extern int hi3620_get_cpu_jump(int cpu);
 extern void secondary_startup(void);
 
-extern void hi3xxx_cpu_die(unsigned int cpu);
-extern int hi3xxx_cpu_kill(unsigned int cpu);
-extern void hi3xxx_set_cpu(int cpu, bool enable);
+extern void hi3620_cpu_die(unsigned int cpu);
+extern int hi3620_cpu_kill(unsigned int cpu);
+extern void hi3620_set_cpu(int cpu, bool enable);
 
 extern void hix5hd2_set_cpu(int cpu, bool enable);
 extern void hix5hd2_cpu_die(unsigned int cpu);
arch/arm/mach-hisi/hisilicon.c +2 -2
index c08c44e..0aea7e9 100644
@@ -44,12 +44,12 @@ static void __init hi3620_map_io(void)
 	iotable_init(hi3620_io_desc, ARRAY_SIZE(hi3620_io_desc));
 }
 
-static const char *const hi3xxx_compat[] __initconst = {
+static const char *const hi3620_compat[] __initconst = {
 	"hisilicon,hi3620-hi4511",
 	NULL,
 };
 
 DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
 	.map_io		= hi3620_map_io,
-	.dt_compat	= hi3xxx_compat,
+	.dt_compat	= hi3620_compat,
 MACHINE_END
arch/arm/mach-hisi/hotplug.c +8 -8
index a129aae..f06b428 100644
@@ -143,7 +143,7 @@ static void set_cpu_hi3620(int cpu, bool enable)
 	}
 }
 
-static int hi3xxx_hotplug_init(void)
+static int hi3620_hotplug_init(void)
 {
 	struct device_node *node;
 

@@ -157,10 +157,10 @@ static int hi3xxx_hotplug_init(void)
 	return -ENOENT;
 }
 
-void hi3xxx_set_cpu(int cpu, bool enable)
+void hi3620_set_cpu(int cpu, bool enable)
 {
 	if (!ctrl_base) {
-		if (hi3xxx_hotplug_init() < 0)
+		if (hi3620_hotplug_init() < 0)
 			return;
 	}
 

@@ -262,24 +262,24 @@ static inline void cpu_enter_lowpower(void)
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
-void hi3xxx_cpu_die(unsigned int cpu)
+void hi3620_cpu_die(unsigned int cpu)
 {
 	cpu_enter_lowpower();
-	hi3xxx_set_cpu_jump(cpu, phys_to_virt(0));
+	hi3620_set_cpu_jump(cpu, phys_to_virt(0));
 	cpu_do_idle();
 
 	/* We should have never returned from idle */
 	panic("cpu %d unexpectedly exit from shutdown\n", cpu);
 }
 
-int hi3xxx_cpu_kill(unsigned int cpu)
+int hi3620_cpu_kill(unsigned int cpu)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(50);
 
-	while (hi3xxx_get_cpu_jump(cpu))
+	while (hi3620_get_cpu_jump(cpu))
 		if (time_after(jiffies, timeout))
 			return 0;
-	hi3xxx_set_cpu(cpu, false);
+	hi3620_set_cpu(cpu, false);
 	return 1;
 }
arch/arm/mach-hisi/platsmp.c +12 -12
index e1d6764..e36783e 100644
@@ -23,7 +23,7 @@
 
 static void __iomem *ctrl_base;
 
-void hi3xxx_set_cpu_jump(int cpu, void *jump_addr)
+void hi3620_set_cpu_jump(int cpu, void *jump_addr)
 {
 	cpu = cpu_logical_map(cpu);
 	if (!cpu || !ctrl_base)

@@ -31,7 +31,7 @@ void hi3xxx_set_cpu_jump(int cpu, void *jump_addr)
 	writel_relaxed(virt_to_phys(jump_addr), ctrl_base + ((cpu - 1) << 2));
 }
 
-int hi3xxx_get_cpu_jump(int cpu)
+int hi3620_get_cpu_jump(int cpu)
 {
 	cpu = cpu_logical_map(cpu);
 	if (!cpu || !ctrl_base)

@@ -56,7 +56,7 @@ static void __init hisi_enable_scu_a9(void)
 	}
 }
 
-static void __init hi3xxx_smp_prepare_cpus(unsigned int max_cpus)
+static void __init hi3620_smp_prepare_cpus(unsigned int max_cpus)
 {
 	struct device_node *np = NULL;
 	u32 offset = 0;

@@ -81,20 +81,20 @@ static void __init hi3xxx_smp_prepare_cpus(unsigned int max_cpus)
 	}
 }
 
-static int hi3xxx_boot_secondary(unsigned int cpu, struct task_struct *idle)
+static int hi3620_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
-	hi3xxx_set_cpu(cpu, true);
-	hi3xxx_set_cpu_jump(cpu, secondary_startup);
+	hi3620_set_cpu(cpu, true);
+	hi3620_set_cpu_jump(cpu, secondary_startup);
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 	return 0;
 }
 
-static const struct smp_operations hi3xxx_smp_ops __initconst = {
-	.smp_prepare_cpus	= hi3xxx_smp_prepare_cpus,
-	.smp_boot_secondary	= hi3xxx_boot_secondary,
+static const struct smp_operations hi3620_smp_ops __initconst = {
+	.smp_prepare_cpus	= hi3620_smp_prepare_cpus,
+	.smp_boot_secondary	= hi3620_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
-	.cpu_die		= hi3xxx_cpu_die,
-	.cpu_kill		= hi3xxx_cpu_kill,
+	.cpu_die		= hi3620_cpu_die,
+	.cpu_kill		= hi3620_cpu_kill,
 #endif
 };
 

@@ -181,6 +181,6 @@ static const struct smp_operations hip01_smp_ops __initconst = {
 	.smp_boot_secondary     = hip01_boot_secondary,
 };
 
-CPU_METHOD_OF_DECLARE(hi3xxx_smp, "hisilicon,hi3620-smp", &hi3xxx_smp_ops);
+CPU_METHOD_OF_DECLARE(hi3620_smp, "hisilicon,hi3620-smp", &hi3620_smp_ops);
 CPU_METHOD_OF_DECLARE(hix5hd2_smp, "hisilicon,hix5hd2-smp", &hix5hd2_smp_ops);
 CPU_METHOD_OF_DECLARE(hip01_smp, "hisilicon,hip01-smp", &hip01_smp_ops);
drivers/clk/hisilicon/Makefile +1 -1
index e169ec7..2edf3b5 100644
@@ -4,7 +4,7 @@
 
 obj-y	+= clk.o clkgate-separated.o clkdivider-hi6220.o
 
-obj-$(CONFIG_ARCH_HI3xxx)	+= clk-hi3620.o
+obj-$(CONFIG_ARCH_HI3620)	+= clk-hi3620.o
 obj-$(CONFIG_ARCH_HIP04)	+= clk-hip04.o
 obj-$(CONFIG_ARCH_HIX5HD2)	+= clk-hix5hd2.o
 obj-$(CONFIG_COMMON_CLK_HI3519)	+= clk-hi3519.o
drivers/dma/Kconfig +1 -1
index 739f797..74bab9d0 100644
@@ -279,7 +279,7 @@ config INTEL_MIC_X100_DMA
 
 config K3_DMA
 	tristate "Hisilicon K3 DMA support"
-	depends on ARCH_HI3xxx
+	depends on ARCH_HI3620
 	select DMA_ENGINE
 	select DMA_VIRTUAL_CHANNELS
 	help
Signature · 1 line
2.9.3
#1 lore raw

PATCH 1/1 mach-hisi: Make Hi3620 explicit, remove wildcard

Hi Marty,

Not bad for a first patch. :-)

Please Cc folks who were in the earlier conversations as well. It's a lot easier to see it amongst 50-100 email/day vice 800-1500. I added Arnd and Russell.

On Tue, Aug 30, 2016 at 12:38:27PM -0500, Marty Plummer wrote:

Please put an explanation in the commit log that gives future readers (and reviewers) the "why".

e.g. "This is a preparatory series for adding the ARMv5 hisi35xx SoCs. Assumptions were made when adding hisi 36xx that don't hold water in light of adding support for the ARMv5 SoC. Fix the issue by renaming config options and other namespaces to avoid collisions with the new work.

Only internal APIs are modified with this series."

Or something like that.

Quoted 2 lines
Signed-off-by Marty Plummer <[email protected]>

---

This needs to be split up into a series of patches:

Quoted 14 lines
 arch/arm/Kconfig.debug              |  2 +-
 arch/arm/boot/dts/Makefile          |  2 +-
 arch/arm/configs/hisi_defconfig     |  2 +-
 arch/arm/configs/multi_v7_defconfig |  2 +-
 arch/arm/mach-hisi/Kconfig          |  6 +++---
 arch/arm/mach-hisi/core.h           | 10 +++++-----
 arch/arm/mach-hisi/hisilicon.c      |  4 ++--
 arch/arm/mach-hisi/hotplug.c        | 16 ++++++++--------
 arch/arm/mach-hisi/platsmp.c        | 24 ++++++++++++------------
Quoted 3 lines
 drivers/clk/hisilicon/Makefile      |  2 +-
 drivers/dma/Kconfig                 |  2 +-

Most likely, the subsystem maintainers will Ack the relevant patch and then they'll go in together to avoid bisection issues.

arch/arm/Kconfig.debug choice · line 283

Quoted 12 lines
 11 files changed, 36 insertions(+), 36 deletions(-)
arch/arm/Kconfig.debug +1 -1
index a9693b6..9094ca6 100644
@@ -280,7 +280,7 @@ choice
 
 	config DEBUG_HI3620_UART
 		bool "Hisilicon HI3620 Debug UART"
-		depends on ARCH_HI3xxx
+		depends on ARCH_HI3620

Is there a general rule like

	ARCH_HI36xx = ARMv7
	ARCH_HI35xx = ARMv5

?

If so, that might make a more sensible config option.

Quoted 47 lines
arch/arm/Kconfig.debug +0 -0
 		select DEBUG_UART_PL01X
 		help
 		  Say Y here if you want kernel low-level debugging support
arch/arm/boot/dts/Makefile +1 -1
index faacd52..7ffd3a3 100644
@@ -154,7 +154,7 @@ dtb-$(CONFIG_ARCH_EXYNOS5) += \
 	exynos5440-sd5v1.dtb \
 	exynos5440-ssdk5440.dtb \
 	exynos5800-peach-pi.dtb
-dtb-$(CONFIG_ARCH_HI3xxx) += \
+dtb-$(CONFIG_ARCH_HI3620) += \
 	hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += \
 	highbank.dtb \
arch/arm/configs/hisi_defconfig +1 -1
index b2e340b..14c8da6 100644
@@ -4,7 +4,7 @@ CONFIG_HIGH_RES_TIMERS=y
 CONFIG_BLK_DEV_INITRD=y
 CONFIG_RD_LZMA=y
 CONFIG_ARCH_HISI=y
-CONFIG_ARCH_HI3xxx=y
+CONFIG_ARCH_HI3620=y
 CONFIG_PARTITION_ADVANCED=y
 CONFIG_CMDLINE_PARTITION=y
 CONFIG_ARCH_HIX5HD2=y
arch/arm/configs/multi_v7_defconfig +1 -1
index 2c8665c..a63865e 100644
@@ -45,7 +45,7 @@ CONFIG_MACH_BERLIN_BG2Q=y
 CONFIG_ARCH_DIGICOLOR=y
 CONFIG_ARCH_HIGHBANK=y
 CONFIG_ARCH_HISI=y
-CONFIG_ARCH_HI3xxx=y
+CONFIG_ARCH_HI3620=y
 CONFIG_ARCH_HIX5HD2=y
 CONFIG_ARCH_HIP01=y
 CONFIG_ARCH_HIP04=y
arch/arm/mach-hisi/Kconfig +0 -0
index a3b091a..08871e0 100644

arch/arm/mach-hisi/Kconfig if ARCH_HISI · line 16

Quoted 7 lines
arch/arm/mach-hisi/Kconfig +2 -2
@@ -12,8 +12,8 @@ if ARCH_HISI
 
 menu "Hisilicon platform type"
 
-config ARCH_HI3xxx
-	bool "Hisilicon Hi36xx family"
+config ARCH_HI3620
+	bool "Hisilicon Hi3620 family"

Which would then help here.

thx,

Jason.

Quoted 213 lines
arch/arm/mach-hisi/Kconfig +1 -1
 	depends on ARCH_MULTI_V7
 	select CACHE_L2X0
 	select HAVE_ARM_SCU if SMP

@@ -21,7 +21,7 @@ config ARCH_HI3xxx
 	select PINCTRL
 	select PINCTRL_SINGLE
 	help
-	  Support for Hisilicon Hi36xx SoC family
+	  Support for Hisilicon Hi3620 SoC family
 
 config ARCH_HIP01
        bool "Hisilicon HIP01 family"
arch/arm/mach-hisi/core.h +5 -5
index e883583..cb7bc84 100644
@@ -3,13 +3,13 @@
 
 #include <linux/reboot.h>
 
-extern void hi3xxx_set_cpu_jump(int cpu, void *jump_addr);
-extern int hi3xxx_get_cpu_jump(int cpu);
+extern void hi3620_set_cpu_jump(int cpu, void *jump_addr);
+extern int hi3620_get_cpu_jump(int cpu);
 extern void secondary_startup(void);
 
-extern void hi3xxx_cpu_die(unsigned int cpu);
-extern int hi3xxx_cpu_kill(unsigned int cpu);
-extern void hi3xxx_set_cpu(int cpu, bool enable);
+extern void hi3620_cpu_die(unsigned int cpu);
+extern int hi3620_cpu_kill(unsigned int cpu);
+extern void hi3620_set_cpu(int cpu, bool enable);
 
 extern void hix5hd2_set_cpu(int cpu, bool enable);
 extern void hix5hd2_cpu_die(unsigned int cpu);
arch/arm/mach-hisi/hisilicon.c +2 -2
index c08c44e..0aea7e9 100644
@@ -44,12 +44,12 @@ static void __init hi3620_map_io(void)
 	iotable_init(hi3620_io_desc, ARRAY_SIZE(hi3620_io_desc));
 }
 
-static const char *const hi3xxx_compat[] __initconst = {
+static const char *const hi3620_compat[] __initconst = {
 	"hisilicon,hi3620-hi4511",
 	NULL,
 };
 
 DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
 	.map_io		= hi3620_map_io,
-	.dt_compat	= hi3xxx_compat,
+	.dt_compat	= hi3620_compat,
 MACHINE_END
arch/arm/mach-hisi/hotplug.c +8 -8
index a129aae..f06b428 100644
@@ -143,7 +143,7 @@ static void set_cpu_hi3620(int cpu, bool enable)
 	}
 }
 
-static int hi3xxx_hotplug_init(void)
+static int hi3620_hotplug_init(void)
 {
 	struct device_node *node;
 

@@ -157,10 +157,10 @@ static int hi3xxx_hotplug_init(void)
 	return -ENOENT;
 }
 
-void hi3xxx_set_cpu(int cpu, bool enable)
+void hi3620_set_cpu(int cpu, bool enable)
 {
 	if (!ctrl_base) {
-		if (hi3xxx_hotplug_init() < 0)
+		if (hi3620_hotplug_init() < 0)
 			return;
 	}
 

@@ -262,24 +262,24 @@ static inline void cpu_enter_lowpower(void)
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
-void hi3xxx_cpu_die(unsigned int cpu)
+void hi3620_cpu_die(unsigned int cpu)
 {
 	cpu_enter_lowpower();
-	hi3xxx_set_cpu_jump(cpu, phys_to_virt(0));
+	hi3620_set_cpu_jump(cpu, phys_to_virt(0));
 	cpu_do_idle();
 
 	/* We should have never returned from idle */
 	panic("cpu %d unexpectedly exit from shutdown\n", cpu);
 }
 
-int hi3xxx_cpu_kill(unsigned int cpu)
+int hi3620_cpu_kill(unsigned int cpu)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(50);
 
-	while (hi3xxx_get_cpu_jump(cpu))
+	while (hi3620_get_cpu_jump(cpu))
 		if (time_after(jiffies, timeout))
 			return 0;
-	hi3xxx_set_cpu(cpu, false);
+	hi3620_set_cpu(cpu, false);
 	return 1;
 }
arch/arm/mach-hisi/platsmp.c +12 -12
index e1d6764..e36783e 100644
@@ -23,7 +23,7 @@
 
 static void __iomem *ctrl_base;
 
-void hi3xxx_set_cpu_jump(int cpu, void *jump_addr)
+void hi3620_set_cpu_jump(int cpu, void *jump_addr)
 {
 	cpu = cpu_logical_map(cpu);
 	if (!cpu || !ctrl_base)

@@ -31,7 +31,7 @@ void hi3xxx_set_cpu_jump(int cpu, void *jump_addr)
 	writel_relaxed(virt_to_phys(jump_addr), ctrl_base + ((cpu - 1) << 2));
 }
 
-int hi3xxx_get_cpu_jump(int cpu)
+int hi3620_get_cpu_jump(int cpu)
 {
 	cpu = cpu_logical_map(cpu);
 	if (!cpu || !ctrl_base)

@@ -56,7 +56,7 @@ static void __init hisi_enable_scu_a9(void)
 	}
 }
 
-static void __init hi3xxx_smp_prepare_cpus(unsigned int max_cpus)
+static void __init hi3620_smp_prepare_cpus(unsigned int max_cpus)
 {
 	struct device_node *np = NULL;
 	u32 offset = 0;

@@ -81,20 +81,20 @@ static void __init hi3xxx_smp_prepare_cpus(unsigned int max_cpus)
 	}
 }
 
-static int hi3xxx_boot_secondary(unsigned int cpu, struct task_struct *idle)
+static int hi3620_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
-	hi3xxx_set_cpu(cpu, true);
-	hi3xxx_set_cpu_jump(cpu, secondary_startup);
+	hi3620_set_cpu(cpu, true);
+	hi3620_set_cpu_jump(cpu, secondary_startup);
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 	return 0;
 }
 
-static const struct smp_operations hi3xxx_smp_ops __initconst = {
-	.smp_prepare_cpus	= hi3xxx_smp_prepare_cpus,
-	.smp_boot_secondary	= hi3xxx_boot_secondary,
+static const struct smp_operations hi3620_smp_ops __initconst = {
+	.smp_prepare_cpus	= hi3620_smp_prepare_cpus,
+	.smp_boot_secondary	= hi3620_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
-	.cpu_die		= hi3xxx_cpu_die,
-	.cpu_kill		= hi3xxx_cpu_kill,
+	.cpu_die		= hi3620_cpu_die,
+	.cpu_kill		= hi3620_cpu_kill,
 #endif
 };
 

@@ -181,6 +181,6 @@ static const struct smp_operations hip01_smp_ops __initconst = {
 	.smp_boot_secondary     = hip01_boot_secondary,
 };
 
-CPU_METHOD_OF_DECLARE(hi3xxx_smp, "hisilicon,hi3620-smp", &hi3xxx_smp_ops);
+CPU_METHOD_OF_DECLARE(hi3620_smp, "hisilicon,hi3620-smp", &hi3620_smp_ops);
 CPU_METHOD_OF_DECLARE(hix5hd2_smp, "hisilicon,hix5hd2-smp", &hix5hd2_smp_ops);
 CPU_METHOD_OF_DECLARE(hip01_smp, "hisilicon,hip01-smp", &hip01_smp_ops);
drivers/clk/hisilicon/Makefile +1 -1
index e169ec7..2edf3b5 100644
@@ -4,7 +4,7 @@
 
 obj-y	+= clk.o clkgate-separated.o clkdivider-hi6220.o
 
-obj-$(CONFIG_ARCH_HI3xxx)	+= clk-hi3620.o
+obj-$(CONFIG_ARCH_HI3620)	+= clk-hi3620.o
 obj-$(CONFIG_ARCH_HIP04)	+= clk-hip04.o
 obj-$(CONFIG_ARCH_HIX5HD2)	+= clk-hix5hd2.o
 obj-$(CONFIG_COMMON_CLK_HI3519)	+= clk-hi3519.o
drivers/dma/Kconfig +1 -1
index 739f797..74bab9d0 100644
@@ -279,7 +279,7 @@ config INTEL_MIC_X100_DMA
 
 config K3_DMA
 	tristate "Hisilicon K3 DMA support"
-	depends on ARCH_HI3xxx
+	depends on ARCH_HI3620
 	select DMA_ENGINE
 	select DMA_VIRTUAL_CHANNELS
 	help
Signature · 8 lines
2.9.3



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#2 lore raw

PATCH 1/1 mach-hisi: Make Hi3620 explicit, remove wildcard

Quoted 19 lines · Jason Cooper
Quoted 2 lines
Signed-off-by Marty Plummer <[email protected]>

---

This needs to be split up into a series of patches:

Quoted 13 lines
 arch/arm/Kconfig.debug              |  2 +-
 arch/arm/boot/dts/Makefile          |  2 +-
 arch/arm/configs/hisi_defconfig     |  2 +-
 arch/arm/configs/multi_v7_defconfig |  2 +-
 arch/arm/mach-hisi/Kconfig          |  6 +++---
 arch/arm/mach-hisi/core.h           | 10 +++++-----
 arch/arm/mach-hisi/hisilicon.c      |  4 ++--
 arch/arm/mach-hisi/hotplug.c        | 16 ++++++++--------
 arch/arm/mach-hisi/platsmp.c        | 24 ++++++++++++------------
Quoted 6 lines
Quoted 3 lines
 drivers/clk/hisilicon/Makefile      |  2 +-
 drivers/dma/Kconfig                 |  2 +-

Most likely, the subsystem maintainers will Ack the relevant patch and then they'll go in together to avoid bisection issues.

Crap. I forgot to add, use scripts/get_maintainer.pl on the patches to figure out who to Cc for each patch. That way, the DMA maintainer should get the coverletter (explaining the big picture for the series), and the DMA patch.

hth,

Jason.

author
#3 lore raw

PATCH 1/1 mach-hisi: Make Hi3620 explicit, remove wildcard

In reply to Jason Cooper
Quoted 19 lines · Jason Cooper

Hi Marty,

Not bad for a first patch. :-)

Please Cc folks who were in the earlier conversations as well. It's a lot easier to see it amongst 50-100 email/day vice 800-1500. I added Arnd and Russell.

On Tue, Aug 30, 2016 at 12:38:27PM -0500, Marty Plummer wrote:

Please put an explanation in the commit log that gives future readers (and reviewers) the "why".

e.g. "This is a preparatory series for adding the ARMv5 hisi35xx SoCs. Assumptions were made when adding hisi 36xx that don't hold water in light of adding support for the ARMv5 SoC. Fix the issue by renaming config options and other namespaces to avoid collisions with the new work.

Quoted 4 lines

Only internal APIs are modified with this series."

Or something like that.

Ah, well since it pretty much only consists of renames and no actual changes in behavior I though it fell under the category of a trivial patch.

Quoted 19 lines
Quoted 2 lines
Signed-off-by Marty Plummer <[email protected]>

---

This needs to be split up into a series of patches:

Quoted 13 lines
 arch/arm/Kconfig.debug              |  2 +-
 arch/arm/boot/dts/Makefile          |  2 +-
 arch/arm/configs/hisi_defconfig     |  2 +-
 arch/arm/configs/multi_v7_defconfig |  2 +-
 arch/arm/mach-hisi/Kconfig          |  6 +++---
 arch/arm/mach-hisi/core.h           | 10 +++++-----
 arch/arm/mach-hisi/hisilicon.c      |  4 ++--
 arch/arm/mach-hisi/hotplug.c        | 16 ++++++++--------
 arch/arm/mach-hisi/platsmp.c        | 24 ++++++++++++------------
Quoted 7 lines
Quoted 3 lines
 drivers/clk/hisilicon/Makefile      |  2 +-
 drivers/dma/Kconfig                 |  2 +-

Most likely, the subsystem maintainers will Ack the relevant patch and then they'll go in together to avoid bisection issues.

Ah, ok. Once again, I thought since it made little in the way of 'real' changes it was more or less irrelevant. I used `git format-patch' to make the patch, and it was all one commit. should I instead then break it down into multiple commits, then?

Quoted 15 lines

arch/arm/Kconfig.debug choice · line 283

Quoted 12 lines
 11 files changed, 36 insertions(+), 36 deletions(-)
arch/arm/Kconfig.debug +1 -1
index a9693b6..9094ca6 100644
@@ -280,7 +280,7 @@ choice
 
 	config DEBUG_HI3620_UART
 		bool "Hisilicon HI3620 Debug UART"
-		depends on ARCH_HI3xxx
+		depends on ARCH_HI3620

Is there a general rule like

Quoted 4 lines
	ARCH_HI36xx = ARMv7
	ARCH_HI35xx = ARMv5

?

Unsure about all Hi35xx chipsets, I've not had reason to look into those yet for lack of hardware. However, ARCH_HI3520 is ARMv6/ARMv5, it has two cores, arm1176/arm926

Quoted 273 lines

If so, that might make a more sensible config option.

Quoted 47 lines
arch/arm/Kconfig.debug +0 -0
 		select DEBUG_UART_PL01X
 		help
 		  Say Y here if you want kernel low-level debugging support
arch/arm/boot/dts/Makefile +1 -1
index faacd52..7ffd3a3 100644
@@ -154,7 +154,7 @@ dtb-$(CONFIG_ARCH_EXYNOS5) += \
 	exynos5440-sd5v1.dtb \
 	exynos5440-ssdk5440.dtb \
 	exynos5800-peach-pi.dtb
-dtb-$(CONFIG_ARCH_HI3xxx) += \
+dtb-$(CONFIG_ARCH_HI3620) += \
 	hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += \
 	highbank.dtb \
arch/arm/configs/hisi_defconfig +1 -1
index b2e340b..14c8da6 100644
@@ -4,7 +4,7 @@ CONFIG_HIGH_RES_TIMERS=y
 CONFIG_BLK_DEV_INITRD=y
 CONFIG_RD_LZMA=y
 CONFIG_ARCH_HISI=y
-CONFIG_ARCH_HI3xxx=y
+CONFIG_ARCH_HI3620=y
 CONFIG_PARTITION_ADVANCED=y
 CONFIG_CMDLINE_PARTITION=y
 CONFIG_ARCH_HIX5HD2=y
arch/arm/configs/multi_v7_defconfig +1 -1
index 2c8665c..a63865e 100644
@@ -45,7 +45,7 @@ CONFIG_MACH_BERLIN_BG2Q=y
 CONFIG_ARCH_DIGICOLOR=y
 CONFIG_ARCH_HIGHBANK=y
 CONFIG_ARCH_HISI=y
-CONFIG_ARCH_HI3xxx=y
+CONFIG_ARCH_HI3620=y
 CONFIG_ARCH_HIX5HD2=y
 CONFIG_ARCH_HIP01=y
 CONFIG_ARCH_HIP04=y
arch/arm/mach-hisi/Kconfig +0 -0
index a3b091a..08871e0 100644

arch/arm/mach-hisi/Kconfig if ARCH_HISI · line 16

Quoted 7 lines
arch/arm/mach-hisi/Kconfig +2 -2
@@ -12,8 +12,8 @@ if ARCH_HISI
 
 menu "Hisilicon platform type"
 
-config ARCH_HI3xxx
-	bool "Hisilicon Hi36xx family"
+config ARCH_HI3620
+	bool "Hisilicon Hi3620 family"

Which would then help here.

thx,

Jason.

Quoted 209 lines
arch/arm/mach-hisi/Kconfig +1 -1
 	depends on ARCH_MULTI_V7
 	select CACHE_L2X0
 	select HAVE_ARM_SCU if SMP

@@ -21,7 +21,7 @@ config ARCH_HI3xxx
 	select PINCTRL
 	select PINCTRL_SINGLE
 	help
-	  Support for Hisilicon Hi36xx SoC family
+	  Support for Hisilicon Hi3620 SoC family
 
 config ARCH_HIP01
        bool "Hisilicon HIP01 family"
arch/arm/mach-hisi/core.h +5 -5
index e883583..cb7bc84 100644
@@ -3,13 +3,13 @@
 
 #include <linux/reboot.h>
 
-extern void hi3xxx_set_cpu_jump(int cpu, void *jump_addr);
-extern int hi3xxx_get_cpu_jump(int cpu);
+extern void hi3620_set_cpu_jump(int cpu, void *jump_addr);
+extern int hi3620_get_cpu_jump(int cpu);
 extern void secondary_startup(void);
 
-extern void hi3xxx_cpu_die(unsigned int cpu);
-extern int hi3xxx_cpu_kill(unsigned int cpu);
-extern void hi3xxx_set_cpu(int cpu, bool enable);
+extern void hi3620_cpu_die(unsigned int cpu);
+extern int hi3620_cpu_kill(unsigned int cpu);
+extern void hi3620_set_cpu(int cpu, bool enable);
 
 extern void hix5hd2_set_cpu(int cpu, bool enable);
 extern void hix5hd2_cpu_die(unsigned int cpu);
arch/arm/mach-hisi/hisilicon.c +2 -2
index c08c44e..0aea7e9 100644
@@ -44,12 +44,12 @@ static void __init hi3620_map_io(void)
 	iotable_init(hi3620_io_desc, ARRAY_SIZE(hi3620_io_desc));
 }
 
-static const char *const hi3xxx_compat[] __initconst = {
+static const char *const hi3620_compat[] __initconst = {
 	"hisilicon,hi3620-hi4511",
 	NULL,
 };
 
 DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
 	.map_io		= hi3620_map_io,
-	.dt_compat	= hi3xxx_compat,
+	.dt_compat	= hi3620_compat,
 MACHINE_END
arch/arm/mach-hisi/hotplug.c +8 -8
index a129aae..f06b428 100644
@@ -143,7 +143,7 @@ static void set_cpu_hi3620(int cpu, bool enable)
 	}
 }
 
-static int hi3xxx_hotplug_init(void)
+static int hi3620_hotplug_init(void)
 {
 	struct device_node *node;
 

@@ -157,10 +157,10 @@ static int hi3xxx_hotplug_init(void)
 	return -ENOENT;
 }
 
-void hi3xxx_set_cpu(int cpu, bool enable)
+void hi3620_set_cpu(int cpu, bool enable)
 {
 	if (!ctrl_base) {
-		if (hi3xxx_hotplug_init() < 0)
+		if (hi3620_hotplug_init() < 0)
 			return;
 	}
 

@@ -262,24 +262,24 @@ static inline void cpu_enter_lowpower(void)
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
-void hi3xxx_cpu_die(unsigned int cpu)
+void hi3620_cpu_die(unsigned int cpu)
 {
 	cpu_enter_lowpower();
-	hi3xxx_set_cpu_jump(cpu, phys_to_virt(0));
+	hi3620_set_cpu_jump(cpu, phys_to_virt(0));
 	cpu_do_idle();
 
 	/* We should have never returned from idle */
 	panic("cpu %d unexpectedly exit from shutdown\n", cpu);
 }
 
-int hi3xxx_cpu_kill(unsigned int cpu)
+int hi3620_cpu_kill(unsigned int cpu)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(50);
 
-	while (hi3xxx_get_cpu_jump(cpu))
+	while (hi3620_get_cpu_jump(cpu))
 		if (time_after(jiffies, timeout))
 			return 0;
-	hi3xxx_set_cpu(cpu, false);
+	hi3620_set_cpu(cpu, false);
 	return 1;
 }
arch/arm/mach-hisi/platsmp.c +12 -12
index e1d6764..e36783e 100644
@@ -23,7 +23,7 @@
 
 static void __iomem *ctrl_base;
 
-void hi3xxx_set_cpu_jump(int cpu, void *jump_addr)
+void hi3620_set_cpu_jump(int cpu, void *jump_addr)
 {
 	cpu = cpu_logical_map(cpu);
 	if (!cpu || !ctrl_base)

@@ -31,7 +31,7 @@ void hi3xxx_set_cpu_jump(int cpu, void *jump_addr)
 	writel_relaxed(virt_to_phys(jump_addr), ctrl_base + ((cpu - 1) << 2));
 }
 
-int hi3xxx_get_cpu_jump(int cpu)
+int hi3620_get_cpu_jump(int cpu)
 {
 	cpu = cpu_logical_map(cpu);
 	if (!cpu || !ctrl_base)

@@ -56,7 +56,7 @@ static void __init hisi_enable_scu_a9(void)
 	}
 }
 
-static void __init hi3xxx_smp_prepare_cpus(unsigned int max_cpus)
+static void __init hi3620_smp_prepare_cpus(unsigned int max_cpus)
 {
 	struct device_node *np = NULL;
 	u32 offset = 0;

@@ -81,20 +81,20 @@ static void __init hi3xxx_smp_prepare_cpus(unsigned int max_cpus)
 	}
 }
 
-static int hi3xxx_boot_secondary(unsigned int cpu, struct task_struct *idle)
+static int hi3620_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
-	hi3xxx_set_cpu(cpu, true);
-	hi3xxx_set_cpu_jump(cpu, secondary_startup);
+	hi3620_set_cpu(cpu, true);
+	hi3620_set_cpu_jump(cpu, secondary_startup);
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 	return 0;
 }
 
-static const struct smp_operations hi3xxx_smp_ops __initconst = {
-	.smp_prepare_cpus	= hi3xxx_smp_prepare_cpus,
-	.smp_boot_secondary	= hi3xxx_boot_secondary,
+static const struct smp_operations hi3620_smp_ops __initconst = {
+	.smp_prepare_cpus	= hi3620_smp_prepare_cpus,
+	.smp_boot_secondary	= hi3620_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
-	.cpu_die		= hi3xxx_cpu_die,
-	.cpu_kill		= hi3xxx_cpu_kill,
+	.cpu_die		= hi3620_cpu_die,
+	.cpu_kill		= hi3620_cpu_kill,
 #endif
 };
 

@@ -181,6 +181,6 @@ static const struct smp_operations hip01_smp_ops __initconst = {
 	.smp_boot_secondary     = hip01_boot_secondary,
 };
 
-CPU_METHOD_OF_DECLARE(hi3xxx_smp, "hisilicon,hi3620-smp", &hi3xxx_smp_ops);
+CPU_METHOD_OF_DECLARE(hi3620_smp, "hisilicon,hi3620-smp", &hi3620_smp_ops);
 CPU_METHOD_OF_DECLARE(hix5hd2_smp, "hisilicon,hix5hd2-smp", &hix5hd2_smp_ops);
 CPU_METHOD_OF_DECLARE(hip01_smp, "hisilicon,hip01-smp", &hip01_smp_ops);
drivers/clk/hisilicon/Makefile +1 -1
index e169ec7..2edf3b5 100644
@@ -4,7 +4,7 @@
 
 obj-y	+= clk.o clkgate-separated.o clkdivider-hi6220.o
 
-obj-$(CONFIG_ARCH_HI3xxx)	+= clk-hi3620.o
+obj-$(CONFIG_ARCH_HI3620)	+= clk-hi3620.o
 obj-$(CONFIG_ARCH_HIP04)	+= clk-hip04.o
 obj-$(CONFIG_ARCH_HIX5HD2)	+= clk-hix5hd2.o
 obj-$(CONFIG_COMMON_CLK_HI3519)	+= clk-hi3519.o
drivers/dma/Kconfig +1 -1
index 739f797..74bab9d0 100644
@@ -279,7 +279,7 @@ config INTEL_MIC_X100_DMA
 
 config K3_DMA
 	tristate "Hisilicon K3 DMA support"
-	depends on ARCH_HI3xxx
+	depends on ARCH_HI3620
 	select DMA_ENGINE
 	select DMA_VIRTUAL_CHANNELS
 	help
Signature · 1 line
2.9.3
Quoted 4 lines
Quoted 4 lines

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

I'm always willing to learn more, but as you mentioned, first time doing this sort of thing.

Also, there is one area I'd simply like to reorder some struct initialization so it matches the definition, iirc it was the struct map_desc.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160830/808ac90f/attachment.sig>

#4 lore raw

PATCH 1/1 mach-hisi: Make Hi3620 explicit, remove wildcard

Quoted 12 lines · Marty Plummer
Quoted 11 lines · Jason Cooper

On Tue, Aug 30, 2016 at 12:38:27PM -0500, Marty Plummer wrote:

Please put an explanation in the commit log that gives future readers (and reviewers) the "why".

e.g. "This is a preparatory series for adding the ARMv5 hisi35xx SoCs. Assumptions were made when adding hisi 36xx that don't hold water in light of adding support for the ARMv5 SoC. Fix the issue by renaming config options and other namespaces to avoid collisions with the new work.

Quoted 7 lines
Quoted 4 lines

Only internal APIs are modified with this series."

Or something like that.

Ah, well since it pretty much only consists of renames and no actual changes in behavior I though it fell under the category of a trivial patch.

Right, we try not to add useless dogma, but this helps reviewers quickly assess the magnitude of the patch and why it exists. The easier we can make it for them to go "Uh-huh... Got it, Acked-by ---" the better off we are.

Quoted 23 lines
Quoted 23 lines
Quoted 2 lines
Signed-off-by Marty Plummer <[email protected]>

---

This needs to be split up into a series of patches:

Quoted 17 lines
 arch/arm/Kconfig.debug              |  2 +-
 arch/arm/boot/dts/Makefile          |  2 +-
 arch/arm/configs/hisi_defconfig     |  2 +-
 arch/arm/configs/multi_v7_defconfig |  2 +-
 arch/arm/mach-hisi/Kconfig          |  6 +++---
 arch/arm/mach-hisi/core.h           | 10 +++++-----
 arch/arm/mach-hisi/hisilicon.c      |  4 ++--
 arch/arm/mach-hisi/hotplug.c        | 16 ++++++++--------
 arch/arm/mach-hisi/platsmp.c        | 24 ++++++++++++------------
 drivers/clk/hisilicon/Makefile      |  2 +-
 drivers/dma/Kconfig                 |  2 +-
Quoted 7 lines
Quoted 3 lines

Most likely, the subsystem maintainers will Ack the relevant patch and then they'll go in together to avoid bisection issues.

Ah, ok. Once again, I thought since it made little in the way of 'real' changes it was more or less irrelevant. I used `git format-patch' to make the patch, and it was all one commit. should I instead then break it down into multiple commits, then?

Please do.

Quoted 15 lines
Quoted 15 lines

arch/arm/Kconfig.debug choice · line 283

Quoted 12 lines
 11 files changed, 36 insertions(+), 36 deletions(-)
arch/arm/Kconfig.debug +1 -1
index a9693b6..9094ca6 100644
@@ -280,7 +280,7 @@ choice
 
 	config DEBUG_HI3620_UART
 		bool "Hisilicon HI3620 Debug UART"
-		depends on ARCH_HI3xxx
+		depends on ARCH_HI3620

Is there a general rule like

Quoted 7 lines
Quoted 4 lines
	ARCH_HI36xx = ARMv7
	ARCH_HI35xx = ARMv5

?

Unsure about all Hi35xx chipsets, I've not had reason to look into those yet for lack of hardware. However, ARCH_HI3520 is ARMv6/ARMv5, it has two cores, arm1176/arm926

Ok, I'll defer to Arnd on this one. I'd be inclined to do the above, or _HI36xx, _HI3520 at least. Since you run separate kernels on each core, it's probably best to treat them like two different boards. Perhaps we need _HI3520_ARM1176, _HI3520_ARM926?

Quoted 3 lines

Also, there is one area I'd simply like to reorder some struct initialization so it matches the definition, iirc it was the struct map_desc.

Let's get the 'trivial' stuff in first. There may be a good reason for the ordering.

thx,

Jason.

author
#5 lore raw

PATCH 1/1 mach-hisi: Make Hi3620 explicit, remove wildcard

Quoted 17 lines · Jason Cooper
Quoted 16 lines · Marty Plummer
Quoted 15 lines · Jason Cooper

On Tue, Aug 30, 2016 at 12:38:27PM -0500, Marty Plummer wrote:

Please put an explanation in the commit log that gives future readers (and reviewers) the "why".

e.g. "This is a preparatory series for adding the ARMv5 hisi35xx SoCs. Assumptions were made when adding hisi 36xx that don't hold water in light of adding support for the ARMv5 SoC. Fix the issue by renaming config options and other namespaces to avoid collisions with the new work.

Only internal APIs are modified with this series."

Or something like that.

Quoted 9 lines
Quoted 3 lines

Ah, well since it pretty much only consists of renames and no actual changes in behavior I though it fell under the category of a trivial patch.

Right, we try not to add useless dogma, but this helps reviewers quickly assess the magnitude of the patch and why it exists. The easier we can make it for them to go "Uh-huh... Got it, Acked-by ---" the better off we are.

A gotcha. Basically a "Hey, this patch is nothing really too much to worry about, just some renames".

Also, 'commit log' is what, here?

Quoted 52 lines
Quoted 23 lines
Quoted 23 lines
Quoted 2 lines
Signed-off-by Marty Plummer <[email protected]>

---

This needs to be split up into a series of patches:

Quoted 17 lines
 arch/arm/Kconfig.debug              |  2 +-
 arch/arm/boot/dts/Makefile          |  2 +-
 arch/arm/configs/hisi_defconfig     |  2 +-
 arch/arm/configs/multi_v7_defconfig |  2 +-
 arch/arm/mach-hisi/Kconfig          |  6 +++---
 arch/arm/mach-hisi/core.h           | 10 +++++-----
 arch/arm/mach-hisi/hisilicon.c      |  4 ++--
 arch/arm/mach-hisi/hotplug.c        | 16 ++++++++--------
 arch/arm/mach-hisi/platsmp.c        | 24 ++++++++++++------------
 drivers/clk/hisilicon/Makefile      |  2 +-
 drivers/dma/Kconfig                 |  2 +-
Quoted 7 lines
Quoted 3 lines

Most likely, the subsystem maintainers will Ack the relevant patch and then they'll go in together to avoid bisection issues.

Ah, ok. Once again, I thought since it made little in the way of 'real' changes it was more or less irrelevant. I used `git format-patch' to make the patch, and it was all one commit. should I instead then break it down into multiple commits, then?

Please do.

Quoted 19 lines
Quoted 19 lines

arch/arm/Kconfig.debug choice · line 283

Quoted 12 lines
 11 files changed, 36 insertions(+), 36 deletions(-)
arch/arm/Kconfig.debug +1 -1
index a9693b6..9094ca6 100644
@@ -280,7 +280,7 @@ choice
 
 	config DEBUG_HI3620_UART
 		bool "Hisilicon HI3620 Debug UART"
-		depends on ARCH_HI3xxx
+		depends on ARCH_HI3620

Is there a general rule like

	ARCH_HI36xx = ARMv7
	ARCH_HI35xx = ARMv5

?

Quoted 9 lines
Quoted 3 lines

Unsure about all Hi35xx chipsets, I've not had reason to look into those yet for lack of hardware. However, ARCH_HI3520 is ARMv6/ARMv5, it has two cores, arm1176/arm926

Ok, I'll defer to Arnd on this one. I'd be inclined to do the above, or _HI36xx, _HI3520 at least. Since you run separate kernels on each core, it's probably best to treat them like two different boards. Perhaps we need _HI3520_ARM1176, _HI3520_ARM926?

Yeah. I was going to go with HI3520, HI3520_SLAVE, but I suppose a more explicit naming would be better in this instance.

Quoted 11 lines
Quoted 3 lines

Also, there is one area I'd simply like to reorder some struct initialization so it matches the definition, iirc it was the struct map_desc.

Let's get the 'trivial' stuff in first. There may be a good reason for the ordering.

thx,

Jason.

Alrigty... funny how getting a patch submitted right is far harder than making one :P

#6 lore raw

PATCH 1/1 mach-hisi: Make Hi3620 explicit, remove wildcard

Quoted 27 lines · Marty Plummer
Quoted 17 lines · Jason Cooper
Quoted 16 lines · Marty Plummer
Quoted 15 lines · Jason Cooper

On Tue, Aug 30, 2016 at 12:38:27PM -0500, Marty Plummer wrote:

Please put an explanation in the commit log that gives future readers (and reviewers) the "why".

e.g. "This is a preparatory series for adding the ARMv5 hisi35xx SoCs. Assumptions were made when adding hisi 36xx that don't hold water in light of adding support for the ARMv5 SoC. Fix the issue by renaming config options and other namespaces to avoid collisions with the new work.

Only internal APIs are modified with this series."

Or something like that.

Quoted 9 lines
Quoted 3 lines

Ah, well since it pretty much only consists of renames and no actual changes in behavior I though it fell under the category of a trivial patch.

Right, we try not to add useless dogma, but this helps reviewers quickly assess the magnitude of the patch and why it exists. The easier we can make it for them to go "Uh-huh... Got it, Acked-by ---" the better off we are.

Quoted 4 lines

A gotcha. Basically a "Hey, this patch is nothing really too much to worry about, just some renames".

Also, 'commit log' is what, here?

When you run 'git commit ...' without '-m', the first line becomes the subject line of the patch. Then you put an empty line, then the commit body. That's where the explaination goes. The end of the commit body has the S-o-b.

Quoted 56 lines
Quoted 56 lines
Quoted 23 lines
Quoted 23 lines
Quoted 2 lines
Signed-off-by Marty Plummer <[email protected]>

---

This needs to be split up into a series of patches:

Quoted 17 lines
 arch/arm/Kconfig.debug              |  2 +-
 arch/arm/boot/dts/Makefile          |  2 +-
 arch/arm/configs/hisi_defconfig     |  2 +-
 arch/arm/configs/multi_v7_defconfig |  2 +-
 arch/arm/mach-hisi/Kconfig          |  6 +++---
 arch/arm/mach-hisi/core.h           | 10 +++++-----
 arch/arm/mach-hisi/hisilicon.c      |  4 ++--
 arch/arm/mach-hisi/hotplug.c        | 16 ++++++++--------
 arch/arm/mach-hisi/platsmp.c        | 24 ++++++++++++------------
 drivers/clk/hisilicon/Makefile      |  2 +-
 drivers/dma/Kconfig                 |  2 +-
Quoted 7 lines
Quoted 3 lines

Most likely, the subsystem maintainers will Ack the relevant patch and then they'll go in together to avoid bisection issues.

Ah, ok. Once again, I thought since it made little in the way of 'real' changes it was more or less irrelevant. I used `git format-patch' to make the patch, and it was all one commit. should I instead then break it down into multiple commits, then?

Please do.

Quoted 22 lines
Quoted 15 lines

arch/arm/Kconfig.debug choice · line 283

Quoted 12 lines
 11 files changed, 36 insertions(+), 36 deletions(-)
arch/arm/Kconfig.debug +1 -1
index a9693b6..9094ca6 100644
@@ -280,7 +280,7 @@ choice
 
 	config DEBUG_HI3620_UART
 		bool "Hisilicon HI3620 Debug UART"
-		depends on ARCH_HI3xxx
+		depends on ARCH_HI3620

Is there a general rule like

Quoted 4 lines
	ARCH_HI36xx = ARMv7
	ARCH_HI35xx = ARMv5

?

Unsure about all Hi35xx chipsets, I've not had reason to look into those yet for lack of hardware. However, ARCH_HI3520 is ARMv6/ARMv5, it has two cores, arm1176/arm926

Quoted 7 lines
Quoted 5 lines

Ok, I'll defer to Arnd on this one. I'd be inclined to do the above, or _HI36xx, _HI3520 at least. Since you run separate kernels on each core, it's probably best to treat them like two different boards. Perhaps we need _HI3520_ARM1176, _HI3520_ARM926?

Yeah. I was going to go with HI3520, HI3520_SLAVE, but I suppose a more explicit naming would be better in this instance.

iirc, the mv78xx0 platform did something similar, although the cores were identical. There were never too many users of it with mainline Linux. Debian's ARM build servers used to run on it, but then they got a donation :-) So there aren't too many examples that I'm aware of in mainline.

thx,

Jason.

maintainer
#7 lore raw

PATCH 1/1 mach-hisi: Make Hi3620 explicit, remove wildcard

In reply to Marty Plummer

Hi Marty,

Quoted 13 lines · Marty Plummer
Signed-off-by Marty Plummer <[email protected]>
---
 arch/arm/Kconfig.debug              |  2 +-
 arch/arm/boot/dts/Makefile          |  2 +-
 arch/arm/configs/hisi_defconfig     |  2 +-
 arch/arm/configs/multi_v7_defconfig |  2 +-
 arch/arm/mach-hisi/Kconfig          |  6 +++---
 arch/arm/mach-hisi/core.h           | 10 +++++-----
 arch/arm/mach-hisi/hisilicon.c      |  4 ++--
 arch/arm/mach-hisi/hotplug.c        | 16 ++++++++--------
 arch/arm/mach-hisi/platsmp.c        | 24 ++++++++++++------------
Quoted 2 lines
 drivers/clk/hisilicon/Makefile      |  2 +-
 drivers/dma/Kconfig                 |  2 +-

Same as Jason, please break down.

Quoted 263 lines
 11 files changed, 36 insertions(+), 36 deletions(-)
arch/arm/Kconfig.debug +1 -1
index a9693b6..9094ca6 100644
@@ -280,7 +280,7 @@ choice
 
 	config DEBUG_HI3620_UART
 		bool "Hisilicon HI3620 Debug UART"
-		depends on ARCH_HI3xxx
+		depends on ARCH_HI3620
 		select DEBUG_UART_PL01X
 		help
 		  Say Y here if you want kernel low-level debugging support
arch/arm/boot/dts/Makefile +1 -1
index faacd52..7ffd3a3 100644
@@ -154,7 +154,7 @@ dtb-$(CONFIG_ARCH_EXYNOS5) += \
 	exynos5440-sd5v1.dtb \
 	exynos5440-ssdk5440.dtb \
 	exynos5800-peach-pi.dtb
-dtb-$(CONFIG_ARCH_HI3xxx) += \
+dtb-$(CONFIG_ARCH_HI3620) += \
 	hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += \
 	highbank.dtb \
arch/arm/configs/hisi_defconfig +1 -1
index b2e340b..14c8da6 100644
@@ -4,7 +4,7 @@ CONFIG_HIGH_RES_TIMERS=y
 CONFIG_BLK_DEV_INITRD=y
 CONFIG_RD_LZMA=y
 CONFIG_ARCH_HISI=y
-CONFIG_ARCH_HI3xxx=y
+CONFIG_ARCH_HI3620=y
 CONFIG_PARTITION_ADVANCED=y
 CONFIG_CMDLINE_PARTITION=y
 CONFIG_ARCH_HIX5HD2=y
arch/arm/configs/multi_v7_defconfig +1 -1
index 2c8665c..a63865e 100644
@@ -45,7 +45,7 @@ CONFIG_MACH_BERLIN_BG2Q=y
 CONFIG_ARCH_DIGICOLOR=y
 CONFIG_ARCH_HIGHBANK=y
 CONFIG_ARCH_HISI=y
-CONFIG_ARCH_HI3xxx=y
+CONFIG_ARCH_HI3620=y
 CONFIG_ARCH_HIX5HD2=y
 CONFIG_ARCH_HIP01=y
 CONFIG_ARCH_HIP04=y
arch/arm/mach-hisi/Kconfig +3 -3
index a3b091a..08871e0 100644
@@ -12,8 +12,8 @@ if ARCH_HISI
 
 menu "Hisilicon platform type"
 
-config ARCH_HI3xxx
-	bool "Hisilicon Hi36xx family"
+config ARCH_HI3620
+	bool "Hisilicon Hi3620 family"
 	depends on ARCH_MULTI_V7
 	select CACHE_L2X0
 	select HAVE_ARM_SCU if SMP

@@ -21,7 +21,7 @@ config ARCH_HI3xxx
 	select PINCTRL
 	select PINCTRL_SINGLE
 	help
-	  Support for Hisilicon Hi36xx SoC family
+	  Support for Hisilicon Hi3620 SoC family
 
 config ARCH_HIP01
        bool "Hisilicon HIP01 family"
arch/arm/mach-hisi/core.h +5 -5
index e883583..cb7bc84 100644
@@ -3,13 +3,13 @@
 
 #include <linux/reboot.h>
 
-extern void hi3xxx_set_cpu_jump(int cpu, void *jump_addr);
-extern int hi3xxx_get_cpu_jump(int cpu);
+extern void hi3620_set_cpu_jump(int cpu, void *jump_addr);
+extern int hi3620_get_cpu_jump(int cpu);
 extern void secondary_startup(void);
 
-extern void hi3xxx_cpu_die(unsigned int cpu);
-extern int hi3xxx_cpu_kill(unsigned int cpu);
-extern void hi3xxx_set_cpu(int cpu, bool enable);
+extern void hi3620_cpu_die(unsigned int cpu);
+extern int hi3620_cpu_kill(unsigned int cpu);
+extern void hi3620_set_cpu(int cpu, bool enable);
 
 extern void hix5hd2_set_cpu(int cpu, bool enable);
 extern void hix5hd2_cpu_die(unsigned int cpu);
arch/arm/mach-hisi/hisilicon.c +2 -2
index c08c44e..0aea7e9 100644
@@ -44,12 +44,12 @@ static void __init hi3620_map_io(void)
 	iotable_init(hi3620_io_desc, ARRAY_SIZE(hi3620_io_desc));
 }
 
-static const char *const hi3xxx_compat[] __initconst = {
+static const char *const hi3620_compat[] __initconst = {
 	"hisilicon,hi3620-hi4511",
 	NULL,
 };
 
 DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
 	.map_io		= hi3620_map_io,
-	.dt_compat	= hi3xxx_compat,
+	.dt_compat	= hi3620_compat,
 MACHINE_END
arch/arm/mach-hisi/hotplug.c +8 -8
index a129aae..f06b428 100644
@@ -143,7 +143,7 @@ static void set_cpu_hi3620(int cpu, bool enable)
 	}
 }
 
-static int hi3xxx_hotplug_init(void)
+static int hi3620_hotplug_init(void)
 {
 	struct device_node *node;
 

@@ -157,10 +157,10 @@ static int hi3xxx_hotplug_init(void)
 	return -ENOENT;
 }
 
-void hi3xxx_set_cpu(int cpu, bool enable)
+void hi3620_set_cpu(int cpu, bool enable)
 {
 	if (!ctrl_base) {
-		if (hi3xxx_hotplug_init() < 0)
+		if (hi3620_hotplug_init() < 0)
 			return;
 	}
 

@@ -262,24 +262,24 @@ static inline void cpu_enter_lowpower(void)
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
-void hi3xxx_cpu_die(unsigned int cpu)
+void hi3620_cpu_die(unsigned int cpu)
 {
 	cpu_enter_lowpower();
-	hi3xxx_set_cpu_jump(cpu, phys_to_virt(0));
+	hi3620_set_cpu_jump(cpu, phys_to_virt(0));
 	cpu_do_idle();
 
 	/* We should have never returned from idle */
 	panic("cpu %d unexpectedly exit from shutdown\n", cpu);
 }
 
-int hi3xxx_cpu_kill(unsigned int cpu)
+int hi3620_cpu_kill(unsigned int cpu)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(50);
 
-	while (hi3xxx_get_cpu_jump(cpu))
+	while (hi3620_get_cpu_jump(cpu))
 		if (time_after(jiffies, timeout))
 			return 0;
-	hi3xxx_set_cpu(cpu, false);
+	hi3620_set_cpu(cpu, false);
 	return 1;
 }
arch/arm/mach-hisi/platsmp.c +12 -12
index e1d6764..e36783e 100644
@@ -23,7 +23,7 @@
 
 static void __iomem *ctrl_base;
 
-void hi3xxx_set_cpu_jump(int cpu, void *jump_addr)
+void hi3620_set_cpu_jump(int cpu, void *jump_addr)
 {
 	cpu = cpu_logical_map(cpu);
 	if (!cpu || !ctrl_base)

@@ -31,7 +31,7 @@ void hi3xxx_set_cpu_jump(int cpu, void *jump_addr)
 	writel_relaxed(virt_to_phys(jump_addr), ctrl_base + ((cpu - 1) << 2));
 }
 
-int hi3xxx_get_cpu_jump(int cpu)
+int hi3620_get_cpu_jump(int cpu)
 {
 	cpu = cpu_logical_map(cpu);
 	if (!cpu || !ctrl_base)

@@ -56,7 +56,7 @@ static void __init hisi_enable_scu_a9(void)
 	}
 }
 
-static void __init hi3xxx_smp_prepare_cpus(unsigned int max_cpus)
+static void __init hi3620_smp_prepare_cpus(unsigned int max_cpus)
 {
 	struct device_node *np = NULL;
 	u32 offset = 0;

@@ -81,20 +81,20 @@ static void __init hi3xxx_smp_prepare_cpus(unsigned int max_cpus)
 	}
 }
 
-static int hi3xxx_boot_secondary(unsigned int cpu, struct task_struct *idle)
+static int hi3620_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
-	hi3xxx_set_cpu(cpu, true);
-	hi3xxx_set_cpu_jump(cpu, secondary_startup);
+	hi3620_set_cpu(cpu, true);
+	hi3620_set_cpu_jump(cpu, secondary_startup);
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 	return 0;
 }
 
-static const struct smp_operations hi3xxx_smp_ops __initconst = {
-	.smp_prepare_cpus	= hi3xxx_smp_prepare_cpus,
-	.smp_boot_secondary	= hi3xxx_boot_secondary,
+static const struct smp_operations hi3620_smp_ops __initconst = {
+	.smp_prepare_cpus	= hi3620_smp_prepare_cpus,
+	.smp_boot_secondary	= hi3620_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
-	.cpu_die		= hi3xxx_cpu_die,
-	.cpu_kill		= hi3xxx_cpu_kill,
+	.cpu_die		= hi3620_cpu_die,
+	.cpu_kill		= hi3620_cpu_kill,
 #endif
 };
 

@@ -181,6 +181,6 @@ static const struct smp_operations hip01_smp_ops __initconst = {
 	.smp_boot_secondary     = hip01_boot_secondary,
 };
 
-CPU_METHOD_OF_DECLARE(hi3xxx_smp, "hisilicon,hi3620-smp", &hi3xxx_smp_ops);
+CPU_METHOD_OF_DECLARE(hi3620_smp, "hisilicon,hi3620-smp", &hi3620_smp_ops);
 CPU_METHOD_OF_DECLARE(hix5hd2_smp, "hisilicon,hix5hd2-smp", &hix5hd2_smp_ops);
 CPU_METHOD_OF_DECLARE(hip01_smp, "hisilicon,hip01-smp", &hip01_smp_ops);
drivers/clk/hisilicon/Makefile +1 -1
index e169ec7..2edf3b5 100644
@@ -4,7 +4,7 @@
 
 obj-y	+= clk.o clkgate-separated.o clkdivider-hi6220.o
 
-obj-$(CONFIG_ARCH_HI3xxx)	+= clk-hi3620.o
+obj-$(CONFIG_ARCH_HI3620)	+= clk-hi3620.o
 obj-$(CONFIG_ARCH_HIP04)	+= clk-hip04.o
 obj-$(CONFIG_ARCH_HIX5HD2)	+= clk-hix5hd2.o
 obj-$(CONFIG_COMMON_CLK_HI3519)	+= clk-hi3519.o
drivers/dma/Kconfig +0 -0
index 739f797..74bab9d0 100644

drivers/dma/Kconfig config INTEL_MIC_X100_DMA · line 285

Quoted 8 lines
drivers/dma/Kconfig +1 -1
@@ -279,7 +279,7 @@ config INTEL_MIC_X100_DMA
 
 config K3_DMA
 	tristate "Hisilicon K3 DMA support"
-	depends on ARCH_HI3xxx
+	depends on ARCH_HI3620
 	select DMA_ENGINE
 	select DMA_VIRTUAL_CHANNELS
 	help

Please check with "scripts/checkpatch.pl" for the format as well. Others is fine to me.
Thanks!

Best Regards,
Wei