[PATCH 0/3] ARM: sunxi: Remove A31 and A23/A33 platform SMP code

[PATCH 0/3] ARM: sunxi: Remove A31 and A23/A33 platform SMP code

[Samuel Holland]

This series is preparation for converting the PRCM MFD and legacy clock
drivers to a CCU clock driver. The platform SMP code references the PRCM
node to map its MMIO space, which will break when the PRCM node is
removed/replaced.

Since PSCI has been available for 7+ years, instead of trying to deal
with the migration, I think it's safe to just delete this code.


Samuel Holland (3):
  ARM: sunxi: Remove A31 and A23/A33 platform SMP code
  ARM: dts: sunxi: Remove obsolete CPU enable methods
  dt-bindings: arm: Remove obsolete CPU enable methods

 .../devicetree/bindings/arm/cpus.yaml         |   2 -
 arch/arm/boot/dts/sun6i-a31.dtsi              |   1 -
 arch/arm/boot/dts/sun8i-a23-a33.dtsi          |   1 -
 arch/arm/mach-sunxi/Makefile                  |   1 -
 arch/arm/mach-sunxi/platsmp.c                 | 194 ------------------
 5 files changed, 199 deletions(-)
 delete mode 100644 arch/arm/mach-sunxi/platsmp.c

-- 
2.35.1

[PATCH 1/3] ARM: sunxi: Remove A31 and A23/A33 platform SMP code

[Samuel Holland]

U-Boot has provided PSCI on Allwinner A31 and A23/A33 since May 2015,
commit 014414f53695 ("ARM: sunxi: Enable PSCI for sun8i"). Since we can
assume PSCI is available on these platforms, the custom SMP bringup code
is no longer used, and it can be removed.

The platform SMP code has a hidden dependency on the legacy PRCM
bindings, so it would be broken anyway when those are retired.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 arch/arm/mach-sunxi/Makefile  |   1 -
 arch/arm/mach-sunxi/platsmp.c | 194 ----------------------------------
 2 files changed, 195 deletions(-)
 delete mode 100644 arch/arm/mach-sunxi/platsmp.c

diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
index 146e623c54d3..e5dc8530e98c 100644
--- a/arch/arm/mach-sunxi/Makefile
+++ b/arch/arm/mach-sunxi/Makefile
@@ -3,4 +3,3 @@ CFLAGS_mc_smp.o	+= -march=armv7-a
 
 obj-$(CONFIG_ARCH_SUNXI) += sunxi.o
 obj-$(CONFIG_ARCH_SUNXI_MC_SMP) += mc_smp.o headsmp.o
-obj-$(CONFIG_SMP) += platsmp.o
diff --git a/arch/arm/mach-sunxi/platsmp.c b/arch/arm/mach-sunxi/platsmp.c
deleted file mode 100644
index 052097e78e6e..000000000000
--- a/arch/arm/mach-sunxi/platsmp.c
+++ /dev/null
@@ -1,194 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * SMP support for Allwinner SoCs
- *
- * Copyright (C) 2013 Maxime Ripard
- *
- * Maxime Ripard <maxime.ripard@free-electrons.com>
- *
- * Based on code
- *  Copyright (C) 2012-2013 Allwinner Ltd.
- *
- */
-
-#include <linux/delay.h>
-#include <linux/init.h>
-#include <linux/io.h>
-#include <linux/memory.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/smp.h>
-
-#define CPUCFG_CPU_PWR_CLAMP_STATUS_REG(cpu)	((cpu) * 0x40 + 0x64)
-#define CPUCFG_CPU_RST_CTRL_REG(cpu)		(((cpu) + 1) * 0x40)
-#define CPUCFG_CPU_CTRL_REG(cpu)		(((cpu) + 1) * 0x40 + 0x04)
-#define CPUCFG_CPU_STATUS_REG(cpu)		(((cpu) + 1) * 0x40 + 0x08)
-#define CPUCFG_GEN_CTRL_REG			0x184
-#define CPUCFG_PRIVATE0_REG			0x1a4
-#define CPUCFG_PRIVATE1_REG			0x1a8
-#define CPUCFG_DBG_CTL0_REG			0x1e0
-#define CPUCFG_DBG_CTL1_REG			0x1e4
-
-#define PRCM_CPU_PWROFF_REG			0x100
-#define PRCM_CPU_PWR_CLAMP_REG(cpu)		(((cpu) * 4) + 0x140)
-
-static void __iomem *cpucfg_membase;
-static void __iomem *prcm_membase;
-
-static DEFINE_SPINLOCK(cpu_lock);
-
-static void __init sun6i_smp_prepare_cpus(unsigned int max_cpus)
-{
-	struct device_node *node;
-
-	node = of_find_compatible_node(NULL, NULL, "allwinner,sun6i-a31-prcm");
-	if (!node) {
-		pr_err("Missing A31 PRCM node in the device tree\n");
-		return;
-	}
-
-	prcm_membase = of_iomap(node, 0);
-	of_node_put(node);
-	if (!prcm_membase) {
-		pr_err("Couldn't map A31 PRCM registers\n");
-		return;
-	}
-
-	node = of_find_compatible_node(NULL, NULL,
-				       "allwinner,sun6i-a31-cpuconfig");
-	if (!node) {
-		pr_err("Missing A31 CPU config node in the device tree\n");
-		return;
-	}
-
-	cpucfg_membase = of_iomap(node, 0);
-	of_node_put(node);
-	if (!cpucfg_membase)
-		pr_err("Couldn't map A31 CPU config registers\n");
-
-}
-
-static int sun6i_smp_boot_secondary(unsigned int cpu,
-				    struct task_struct *idle)
-{
-	u32 reg;
-	int i;
-
-	if (!(prcm_membase && cpucfg_membase))
-		return -EFAULT;
-
-	spin_lock(&cpu_lock);
-
-	/* Set CPU boot address */
-	writel(__pa_symbol(secondary_startup),
-	       cpucfg_membase + CPUCFG_PRIVATE0_REG);
-
-	/* Assert the CPU core in reset */
-	writel(0, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
-
-	/* Assert the L1 cache in reset */
-	reg = readl(cpucfg_membase + CPUCFG_GEN_CTRL_REG);
-	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_GEN_CTRL_REG);
-
-	/* Disable external debug access */
-	reg = readl(cpucfg_membase + CPUCFG_DBG_CTL1_REG);
-	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_DBG_CTL1_REG);
-
-	/* Power up the CPU */
-	for (i = 0; i <= 8; i++)
-		writel(0xff >> i, prcm_membase + PRCM_CPU_PWR_CLAMP_REG(cpu));
-	mdelay(10);
-
-	/* Clear CPU power-off gating */
-	reg = readl(prcm_membase + PRCM_CPU_PWROFF_REG);
-	writel(reg & ~BIT(cpu), prcm_membase + PRCM_CPU_PWROFF_REG);
-	mdelay(1);
-
-	/* Deassert the CPU core reset */
-	writel(3, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
-
-	/* Enable back the external debug accesses */
-	reg = readl(cpucfg_membase + CPUCFG_DBG_CTL1_REG);
-	writel(reg | BIT(cpu), cpucfg_membase + CPUCFG_DBG_CTL1_REG);
-
-	spin_unlock(&cpu_lock);
-
-	return 0;
-}
-
-static const struct smp_operations sun6i_smp_ops __initconst = {
-	.smp_prepare_cpus	= sun6i_smp_prepare_cpus,
-	.smp_boot_secondary	= sun6i_smp_boot_secondary,
-};
-CPU_METHOD_OF_DECLARE(sun6i_a31_smp, "allwinner,sun6i-a31", &sun6i_smp_ops);
-
-static void __init sun8i_smp_prepare_cpus(unsigned int max_cpus)
-{
-	struct device_node *node;
-
-	node = of_find_compatible_node(NULL, NULL, "allwinner,sun8i-a23-prcm");
-	if (!node) {
-		pr_err("Missing A23 PRCM node in the device tree\n");
-		return;
-	}
-
-	prcm_membase = of_iomap(node, 0);
-	of_node_put(node);
-	if (!prcm_membase) {
-		pr_err("Couldn't map A23 PRCM registers\n");
-		return;
-	}
-
-	node = of_find_compatible_node(NULL, NULL,
-				       "allwinner,sun8i-a23-cpuconfig");
-	if (!node) {
-		pr_err("Missing A23 CPU config node in the device tree\n");
-		return;
-	}
-
-	cpucfg_membase = of_iomap(node, 0);
-	of_node_put(node);
-	if (!cpucfg_membase)
-		pr_err("Couldn't map A23 CPU config registers\n");
-
-}
-
-static int sun8i_smp_boot_secondary(unsigned int cpu,
-				    struct task_struct *idle)
-{
-	u32 reg;
-
-	if (!(prcm_membase && cpucfg_membase))
-		return -EFAULT;
-
-	spin_lock(&cpu_lock);
-
-	/* Set CPU boot address */
-	writel(__pa_symbol(secondary_startup),
-	       cpucfg_membase + CPUCFG_PRIVATE0_REG);
-
-	/* Assert the CPU core in reset */
-	writel(0, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
-
-	/* Assert the L1 cache in reset */
-	reg = readl(cpucfg_membase + CPUCFG_GEN_CTRL_REG);
-	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_GEN_CTRL_REG);
-
-	/* Clear CPU power-off gating */
-	reg = readl(prcm_membase + PRCM_CPU_PWROFF_REG);
-	writel(reg & ~BIT(cpu), prcm_membase + PRCM_CPU_PWROFF_REG);
-	mdelay(1);
-
-	/* Deassert the CPU core reset */
-	writel(3, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
-
-	spin_unlock(&cpu_lock);
-
-	return 0;
-}
-
-static const struct smp_operations sun8i_smp_ops __initconst = {
-	.smp_prepare_cpus	= sun8i_smp_prepare_cpus,
-	.smp_boot_secondary	= sun8i_smp_boot_secondary,
-};
-CPU_METHOD_OF_DECLARE(sun8i_a23_smp, "allwinner,sun8i-a23", &sun8i_smp_ops);
-- 
2.35.1

[PATCH 2/3] ARM: dts: sunxi: Remove obsolete CPU enable methods

[Samuel Holland]

Now that the platform SMP code has been removed in favor of PSCI,
these enable methods are meaningless.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 arch/arm/boot/dts/sun6i-a31.dtsi     | 1 -
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 715d74854449..9dee04904e31 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -95,7 +95,6 @@ timer {
 	};
 
 	cpus {
-		enable-method = "allwinner,sun6i-a31";
 		#address-cells = <1>;
 		#size-cells = <0>;
 
diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 4461d5098b20..87e2d63ceb0e 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -85,7 +85,6 @@ timer {
 	};
 
 	cpus {
-		enable-method = "allwinner,sun8i-a23";
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-- 
2.35.1

[PATCH 3/3] dt-bindings: arm: Remove obsolete CPU enable methods

[Samuel Holland]

U-Boot has provided PSCI on Allwinner A31 and A23/A33 since May 2015,
commit 014414f53695 ("ARM: sunxi: Enable PSCI for sun8i"). Since we can
assume PSCI is available on these platforms, they no longer need custom
CPU enable methods.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 Documentation/devicetree/bindings/arm/cpus.yaml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml
index ed04650291a8..94527187a85a 100644
--- a/Documentation/devicetree/bindings/arm/cpus.yaml
+++ b/Documentation/devicetree/bindings/arm/cpus.yaml
@@ -195,8 +195,6 @@ properties:
       # On ARM 32-bit systems this property is optional
       - enum:
           - actions,s500-smp
-          - allwinner,sun6i-a31
-          - allwinner,sun8i-a23
           - allwinner,sun9i-a80-smp
           - allwinner,sun8i-a83t-smp
           - amlogic,meson8-smp
-- 
2.35.1

Re: [PATCH 3/3] dt-bindings: arm: Remove obsolete CPU enable methods

[Rob Herring]

On Mon, 30 May 2022 23:50:38 -0500, Samuel Holland wrote:
> U-Boot has provided PSCI on Allwinner A31 and A23/A33 since May 2015,
> commit 014414f53695 ("ARM: sunxi: Enable PSCI for sun8i"). Since we can
> assume PSCI is available on these platforms, they no longer need custom
> CPU enable methods.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
>  Documentation/devicetree/bindings/arm/cpus.yaml | 2 --
>  1 file changed, 2 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 0/3] ARM: sunxi: Remove A31 and A23/A33 platform SMP code

[Jernej Škrabec]

Dne torek, 31. maj 2022 ob 06:50:35 CEST je Samuel Holland napisal(a):
> This series is preparation for converting the PRCM MFD and legacy clock
> drivers to a CCU clock driver. The platform SMP code references the PRCM
> node to map its MMIO space, which will break when the PRCM node is
> removed/replaced.

Why can't we just leave old platform code? If older dtb file is used, it would 
still work. Actually, isn't trivial to support new CCU binding too, just by 
including new CCU compatible string? IIUC new CCU node will have same address 
as current PRCM node.

Best regards,
Jernej

> 
> Since PSCI has been available for 7+ years, instead of trying to deal
> with the migration, I think it's safe to just delete this code.
> 
> 
> Samuel Holland (3):
>   ARM: sunxi: Remove A31 and A23/A33 platform SMP code
>   ARM: dts: sunxi: Remove obsolete CPU enable methods
>   dt-bindings: arm: Remove obsolete CPU enable methods
> 
>  .../devicetree/bindings/arm/cpus.yaml         |   2 -
>  arch/arm/boot/dts/sun6i-a31.dtsi              |   1 -
>  arch/arm/boot/dts/sun8i-a23-a33.dtsi          |   1 -
>  arch/arm/mach-sunxi/Makefile                  |   1 -
>  arch/arm/mach-sunxi/platsmp.c                 | 194 ------------------
>  5 files changed, 199 deletions(-)
>  delete mode 100644 arch/arm/mach-sunxi/platsmp.c
> 
> -- 
> 2.35.1
> 
> 

Re: [PATCH 0/3] ARM: sunxi: Remove A31 and A23/A33 platform SMP code

[Andre Przywara]

On Sun, 12 Jun 2022 23:09:07 +0200
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

Hi Samuel,

> Dne torek, 31. maj 2022 ob 06:50:35 CEST je Samuel Holland napisal(a):
> > This series is preparation for converting the PRCM MFD and legacy clock
> > drivers to a CCU clock driver.

May I ask what the purpose of this exercise is? So if I understand
correctly, then it's about to convert the sun8i-a23-prcm MFD driver and
its children to a single "modern style" CCU clock driver, with its opaque
DT node?
If that changes the compatible strings or the references to the clock
providers (and I guess it would need to?), then this would mean an
incompatible change. Which also means we would need to keep the old code
around, to maintain compatibility with "old" DTs? So what is the win then?
Now we have *two* clock drivers, for the same device, which need
maintenance and testing.

So can you confirm that this will be a breaking change?

> The platform SMP code references the PRCM
> > node to map its MMIO space, which will break when the PRCM node is
> > removed/replaced.  
> 
> Why can't we just leave old platform code? If older dtb file is used, it would 
> still work. Actually, isn't trivial to support new CCU binding too, just by 
> including new CCU compatible string? IIUC new CCU node will have same address 
> as current PRCM node.

This aims for a similar direction, though in this case the alternative
(PSCI) predates the sunxi specific method in the kernel support. Can we
just deprecate this code, maybe issue a warning, with the hint to update
the bootloader (which might not be possible for some devices)?

Cheers,
Andre

> Best regards,
> Jernej
> 
> > 
> > Since PSCI has been available for 7+ years, instead of trying to deal
> > with the migration, I think it's safe to just delete this code.
> > 
> > 
> > Samuel Holland (3):
> >   ARM: sunxi: Remove A31 and A23/A33 platform SMP code
> >   ARM: dts: sunxi: Remove obsolete CPU enable methods
> >   dt-bindings: arm: Remove obsolete CPU enable methods
> > 
> >  .../devicetree/bindings/arm/cpus.yaml         |   2 -
> >  arch/arm/boot/dts/sun6i-a31.dtsi              |   1 -
> >  arch/arm/boot/dts/sun8i-a23-a33.dtsi          |   1 -
> >  arch/arm/mach-sunxi/Makefile                  |   1 -
> >  arch/arm/mach-sunxi/platsmp.c                 | 194 ------------------
> >  5 files changed, 199 deletions(-)
> >  delete mode 100644 arch/arm/mach-sunxi/platsmp.c
> > 
> > -- 
> > 2.35.1
> > 
> >   
> 
> 
> 

Re: [PATCH 0/3] ARM: sunxi: Remove A31 and A23/A33 platform SMP code

[Samuel Holland]

[+ clock maintainers]

On 6/13/22 6:17 AM, Andre Przywara wrote:
> On Sun, 12 Jun 2022 23:09:07 +0200
> Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> 
> Hi Samuel,
> 
>> Dne torek, 31. maj 2022 ob 06:50:35 CEST je Samuel Holland napisal(a):
>>> This series is preparation for converting the PRCM MFD and legacy clock
>>> drivers to a CCU clock driver.
> 
> May I ask what the purpose of this exercise is? So if I understand
> correctly, then it's about to convert the sun8i-a23-prcm MFD driver and
> its children to a single "modern style" CCU clock driver, with its opaque
> DT node?

Yes, the purpose is to finish the conversion that was started six years
ago[1][2], and eventually delete drivers/clk/sunxi.

[1]: https://git.kernel.org/torvalds/c/78a9f0dbcd60
[2]: https://git.kernel.org/torvalds/c/2c89ce4f4b19

> If that changes the compatible strings or the references to the clock
> providers (and I guess it would need to?), then this would mean an
> incompatible change. Which also means we would need to keep the old code
> around, to maintain compatibility with "old" DTs? So what is the win then?
> Now we have *two* clock drivers, for the same device, which need
> maintenance and testing.

We already have two drivers for the PRCM. ccu-sun8i-r, which is already built in
to all MACH_SUN8I kernels, was originally intended to support A31/A23/A33, but
nobody ever implemented it. See the comment in the binding header:

    /* 8 is reserved for CLK_APB0_W1 on A31 */

The benefits of conversion are:
 - U-Boot does not have to add support for an already-deprecated clock framework.
 - Users who know they have a recent devicetree can disable CLK_SUNXI.
 - Eventually we can disable CLK_SUNXI and delete drivers/clk/sunxi.

Yes, we need to keep the old code for several years to support existing
devicetrees. That is exactly why I want to do the conversion as soon as
possible: to get that clock started now.

> So can you confirm that this will be a breaking change?

Yes, just like the other half of the CCU conversion was.

>> The platform SMP code references the PRCM
>>> node to map its MMIO space, which will break when the PRCM node is
>>> removed/replaced.  
>>
>> Why can't we just leave old platform code? If older dtb file is used, it would 
>> still work. Actually, isn't trivial to support new CCU binding too, just by 
>> including new CCU compatible string? IIUC new CCU node will have same address 
>> as current PRCM node.

Yes, I could add the new compatible. I was trying to avoid adding more code and
more complexity to code that appeared to be unused in the first place.

> This aims for a similar direction, though in this case the alternative
> (PSCI) predates the sunxi specific method in the kernel support. Can we
> just deprecate this code, maybe issue a warning, with the hint to update
> the bootloader (which might not be possible for some devices)?

Since mainline U-Boot provides PSCI, I assume the purpose of this code was to
support the vendor bootloader blob. And the threads adding this code[3][4]
confirm that. In that case, users would have to be using a DTB shipped with the
kernel, not the one provided by the bootloader. So if we change the devicetree,
we have to change the code, too.

Regards,
Samuel

[3]: https://lore.kernel.org/lkml/20131110100312.GI26440@lukather/
[4]: https://lore.kernel.org/lkml/1426649042-30547-1-git-send-email-wens@csie.org/

> Cheers,
> Andre
> 
>> Best regards,
>> Jernej
>>
>>>
>>> Since PSCI has been available for 7+ years, instead of trying to deal
>>> with the migration, I think it's safe to just delete this code.
>>>
>>>
>>> Samuel Holland (3):
>>>   ARM: sunxi: Remove A31 and A23/A33 platform SMP code
>>>   ARM: dts: sunxi: Remove obsolete CPU enable methods
>>>   dt-bindings: arm: Remove obsolete CPU enable methods
>>>
>>>  .../devicetree/bindings/arm/cpus.yaml         |   2 -
>>>  arch/arm/boot/dts/sun6i-a31.dtsi              |   1 -
>>>  arch/arm/boot/dts/sun8i-a23-a33.dtsi          |   1 -
>>>  arch/arm/mach-sunxi/Makefile                  |   1 -
>>>  arch/arm/mach-sunxi/platsmp.c                 | 194 ------------------
>>>  5 files changed, 199 deletions(-)
>>>  delete mode 100644 arch/arm/mach-sunxi/platsmp.c
>>>
>>> -- 
>>> 2.35.1