On Tue, Sep 20, 2022 at 9:22 PM Pali Rohár <p...@kernel.org> wrote: > > On Tuesday 20 September 2022 20:31:52 Chris Packham wrote: > > Add support for the Allecat5/Alleycat5X SoC. These are L3 switches with > > an integrated CPU (referred to as the CnM block in Marvell's > > documentation). These have dual ARMv8.2 CPUs (Cortex-A55). This support > > has been ported from Marvell's SDK which is based on a much older > > version of U-Boot. > > > > Signed-off-by: Chris Packham <judge.pack...@gmail.com> > > --- > > > > (no changes since v1) > > > > arch/arm/dts/ac5-98dx25xx.dtsi | 292 +++++++++++++++++++++++ > > arch/arm/dts/ac5-98dx35xx.dtsi | 17 ++ > > arch/arm/mach-mvebu/Kconfig | 5 + > > arch/arm/mach-mvebu/Makefile | 1 + > > arch/arm/mach-mvebu/alleycat5/Makefile | 9 + > > arch/arm/mach-mvebu/alleycat5/clock.c | 49 ++++ > > arch/arm/mach-mvebu/alleycat5/cpu.c | 129 ++++++++++ > > arch/arm/mach-mvebu/alleycat5/soc.c | 229 ++++++++++++++++++ > > arch/arm/mach-mvebu/arm64-common.c | 15 ++ > > arch/arm/mach-mvebu/include/mach/clock.h | 11 + > > arch/arm/mach-mvebu/include/mach/cpu.h | 4 + > > arch/arm/mach-mvebu/include/mach/soc.h | 4 + > > 12 files changed, 765 insertions(+) > > create mode 100644 arch/arm/dts/ac5-98dx25xx.dtsi > > create mode 100644 arch/arm/dts/ac5-98dx35xx.dtsi > > create mode 100644 arch/arm/mach-mvebu/alleycat5/Makefile > > create mode 100644 arch/arm/mach-mvebu/alleycat5/clock.c > > create mode 100644 arch/arm/mach-mvebu/alleycat5/cpu.c > > create mode 100644 arch/arm/mach-mvebu/alleycat5/soc.c > > create mode 100644 arch/arm/mach-mvebu/include/mach/clock.h > > > ... > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig > > index a81b8e2b0d..400a308985 100644 > > --- a/arch/arm/mach-mvebu/Kconfig > > +++ b/arch/arm/mach-mvebu/Kconfig > > @@ -49,6 +49,11 @@ config ARMADA_8K > > bool > > select ARM64 > > > > +config ALLEYCAT_5 > > + bool > > + select ARM64 > > + select HAVE_MVEBU_EFUSE > > Hello! You are not adding any efuse implementation for AC5 platform, > so selecting this symbol seems to be wrong. > > Or are you reusing A3720 or A38x OTP implementation for AC5? This is not > clear from the patch. >
The original code did have some EFUSE stuff in it but I've dropped most of it out. I think this can go. > > + > > # Armada PLL frequency (used for NAND clock generation) > > config SYS_MVEBU_PLL_CLOCK > > int > ... > > diff --git a/arch/arm/mach-mvebu/arm64-common.c > > b/arch/arm/mach-mvebu/arm64-common.c > > index 238edbe6ba..c9b6f16c63 100644 > > --- a/arch/arm/mach-mvebu/arm64-common.c > > +++ b/arch/arm/mach-mvebu/arm64-common.c > > @@ -53,6 +53,8 @@ __weak int dram_init_banksize(void) > > return a8k_dram_init_banksize(); > > else if (CONFIG_IS_ENABLED(ARMADA_3700)) > > return a3700_dram_init_banksize(); > > + else if (CONFIG_IS_ENABLED(ALLEYCAT_5)) > > + return alleycat5_dram_init_banksize(); > > else > > return fdtdec_setup_memory_banksize(); > > } > > @@ -68,6 +70,9 @@ __weak int dram_init(void) > > if (CONFIG_IS_ENABLED(ARMADA_3700)) > > return a3700_dram_init(); > > > > + if (CONFIG_IS_ENABLED(ALLEYCAT_5)) > > + return alleycat5_dram_init(); > > + > > if (fdtdec_setup_mem_size_base() != 0) > > return -EINVAL; > > > > @@ -100,6 +105,16 @@ int arch_early_init_r(void) > > break; > > } > > > > + i = 0; > > + while (1) { > > + /* Call the pinctrl code via the PINCTRL uclass driver */ > > + ret = uclass_get_device(UCLASS_PINCTRL, i++, &dev); > > + > > + /* We're done, once no further CP110 device is found */ > > + if (ret) > > + break; > > + } > > This code is unconditionally called for all 64-bit mvebu platforms, not > only for new AC5. So if this is some fix, it should be in separate > commit. If not then it should be marked AC5 specific and explained why. > Yeah I can't see why it's needed. The pinctrl device still gets probed without it so I suspect it's just old cruft left over from a time that wasn't the case. > > + > > /* Cause the SATA device to do its early init */ > > uclass_first_device(UCLASS_AHCI, &dev); > > > > diff --git a/arch/arm/mach-mvebu/include/mach/clock.h > > b/arch/arm/mach-mvebu/include/mach/clock.h > > new file mode 100644 > > index 0000000000..93d965ad5a > > --- /dev/null > > +++ b/arch/arm/mach-mvebu/include/mach/clock.h > > @@ -0,0 +1,11 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > +/* > > + * Copyright (C) 2018 Marvell International Ltd. > > + */ > > + > > +#ifndef _MVEBU_CLOCK_H_ > > +#define _MVEBU_CLOCK_H_ > > + > > +void soc_print_clock_info(void); > > I'm not sure if this function needs to be exported. You are using it > only in AC5 cpu.c file, in function print_cpuinfo(). So probably you can > move soc_print_clock_info() into that file and then global mvebu clock.h > would not be needed at all. > I'd need to fold all of the stuff in alleycat5/clock.c into alleycat5/cpu.c but it should be doable. Alternatively I could keep it separate but have a local .h file for this (and soc.h) to avoid polluting the mvebu generic files. > > + > > +#endif /* _MVEBU_CLOCK_H_ */ > > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h > > b/arch/arm/mach-mvebu/include/mach/cpu.h > > index b127fce865..c17c2440f1 100644 > > --- a/arch/arm/mach-mvebu/include/mach/cpu.h > > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h > > @@ -174,6 +174,10 @@ int a3700_dram_init_banksize(void); > > /* A3700 PCIe regions fixer for device tree */ > > int a3700_fdt_fix_pcie_regions(void *blob); > > > > +/* Alleycat5 dram functions */ > > +int alleycat5_dram_init(void); > > +int alleycat5_dram_init_banksize(void); > > + > > /* > > * get_ref_clk > > * > > diff --git a/arch/arm/mach-mvebu/include/mach/soc.h > > b/arch/arm/mach-mvebu/include/mach/soc.h > > index 3b9618852c..b9312e37dc 100644 > > --- a/arch/arm/mach-mvebu/include/mach/soc.h > > +++ b/arch/arm/mach-mvebu/include/mach/soc.h > > @@ -212,4 +212,8 @@ > > #define CONFIG_SYS_TCLK 250000000 /* 250MHz */ > > #endif > > > > +#ifndef __ASSEMBLY__ > > +void soc_print_device_info(void); > > +int soc_early_init_f(void); > > These two functions are not available on existing mvebu platforms, > so I think functions should not be declared in global mvebu soc.h file. > This one is a bit weird. There is a weak version defined in the board.c (same for the octeontx2_cn913x, possibly copied from similar vendor code). But the actual implementation that we need is in arch/arm/mach-mvebu/alleycat5/soc.c so my attempt to put a definition in soc.h was to make this less mysterious but I forgot to remove the weak definition. The strong implementation is the thing that triggers the init for the mvebu_sar driver from the previous patch. So I think I need to unpick the SAR driver and figure out how to do it "properly" (addressing the issues you've pointed out). What I intend to do is send a v3 series without addressing this specific concern and then try and figure out how much (or how little) of the SAR code I really need. > > +#endif /* __ASSEMBLY__ */ > > #endif /* _MVEBU_SOC_H */ > > -- > > 2.37.3 > >