From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6309BC32771 for ; Wed, 21 Sep 2022 03:55:13 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3730984951; Wed, 21 Sep 2022 05:55:10 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="nW9O3+4v"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9D62284B4D; Wed, 21 Sep 2022 05:55:08 +0200 (CEST) Received: from mail-ej1-x634.google.com (mail-ej1-x634.google.com [IPv6:2a00:1450:4864:20::634]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id D626D84951 for ; Wed, 21 Sep 2022 05:55:05 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=judge.packham@gmail.com Received: by mail-ej1-x634.google.com with SMTP id lh5so10715977ejb.10 for ; Tue, 20 Sep 2022 20:55:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date; bh=0Pswnc9YqicUAWdgunPWIfpj5kJW1O+h3Fvc4/J0HT0=; b=nW9O3+4vu4+jwrcFDvwcmrNK3nrK5H8SN8mMQcI1Bk2Xz80YCmvaNwEqr0i4begsW3 m8L7sHGK7UcS6xTJvJOImQ4lfyq7ybB+3KA1OK2x1nFxALwb2DdtsfGLVTyzVNW1mspP ngaRPKWcnhALakuMG5rfKyxEHAS53K0neO005A+WTosUru+ccztuDgmyxs2i4fDJpse9 +Aaxls2aC7f8pFZyB+phz9CU7xG839F5NKCyKcjBmQKmJranrqPoz5+p/KpXIPlr86Aq TifsfSL6yCe1rT5VJCa367RCWqEm7dIuKvpSzavb0fPGc5q65fZpSB6DYLC0FSZH/Fe+ 7sfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date; bh=0Pswnc9YqicUAWdgunPWIfpj5kJW1O+h3Fvc4/J0HT0=; b=6cS1WRonmR/91aFZIl5kD0tcTT9PreBboZS6GO15Ua0F6/idhbz5zobWtlwjF6sy2d 6/OEE9rve2Zsga49au717xZjiJz023dx8Qh5VMRZr3BclTL6Vxa5R+L16H2ta/kNRSH/ mlMGIdHvo9FKr0SNepYjI3uXpdtTO6sLVIMs2jCgBZIMnHUlfaeV+K/FdMu+DVzDp4nZ 8g/6YW1LAusZuLdUbUNLVEp95XsZq/0vEdmgKaFjn52rwX7G8ogLpWEHGxR7eYPT5o4V A0DLQYy5PehEyQTAIrE5/GHm/f3l4Stb8IIcx/75XLYAuT56QmuOaD98YdtFB3qitUj8 VNzw== X-Gm-Message-State: ACrzQf3k7DO/ulsHikee01toYRZQjCg6Dcq/aHfU3ouN6HAGqfgTQX8N AzysNiH7Vmh5N2GooTsa3WLtgZWKY2Wp82nWDRM= X-Google-Smtp-Source: AMsMyM7tYj+tjZxtntEJ6fNcNvV4pFareosAZ+wt+8Ca9ijmkdMpGCglnaIIJxN3jePKGiwQlr3DXnlLHcFibHaoApE= X-Received: by 2002:a17:907:d07:b0:72e:ec79:ad0f with SMTP id gn7-20020a1709070d0700b0072eec79ad0fmr19574963ejc.296.1663732505292; Tue, 20 Sep 2022 20:55:05 -0700 (PDT) MIME-Version: 1.0 References: <20220920083153.319370-1-judge.packham@gmail.com> <20220920083153.319370-6-judge.packham@gmail.com> <20220920092250.o3nyqy4r2p7axai5@pali> In-Reply-To: <20220920092250.o3nyqy4r2p7axai5@pali> From: Chris Packham Date: Wed, 21 Sep 2022 15:54:52 +1200 Message-ID: Subject: Re: [PATCH v2 5/6] arm: mvebu: Support for 98DX25xx/98DX35xx SoC To: =?UTF-8?Q?Pali_Roh=C3=A1r?= Cc: Stefan Roese , Elad Nachman , Vadym Kochan , =?UTF-8?B?TWFyZWsgQmVow7pu?= , Tom Rini , u-boot Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Tue, Sep 20, 2022 at 9:22 PM Pali Roh=C3=A1r 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 > > --- > > > > (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/a= rm64-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() !=3D 0) > > return -EINVAL; > > > > @@ -100,6 +105,16 @@ int arch_early_init_r(void) > > break; > > } > > > > + i =3D 0; > > + while (1) { > > + /* Call the pinctrl code via the PINCTRL uclass driver */ > > + ret =3D 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-m= vebu/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-mve= bu/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-mve= bu/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 > >