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

[Samuel Holland <samuel@sholland.org>]

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