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 C7571C4332F for ; Thu, 10 Feb 2022 10:52:16 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id C91F68142D; Thu, 10 Feb 2022 11:52:13 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 87B5982014; Thu, 10 Feb 2022 11:52:11 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id 04ECB81277 for ; Thu, 10 Feb 2022 11:52:07 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6272CED1; Thu, 10 Feb 2022 02:52:07 -0800 (PST) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 165CE3F73B; Thu, 10 Feb 2022 02:52:03 -0800 (PST) Date: Thu, 10 Feb 2022 10:52:01 +0000 From: Andre Przywara To: Jesse Taube Cc: u-boot@lists.denx.de, jagan@amarulasolutions.com, hdegoede@redhat.com, sjg@chromium.org, icenowy@aosc.io, marek.behun@nic.cz, festevam@denx.de, narmstrong@baylibre.com, tharvey@gateworks.com, christianshewitt@gmail.com, pbrobinson@gmail.com, jernej.skrabec@gmail.com, hs@denx.de, samuel@sholland.org, arnaud.ferraris@gmail.com, giulio.benetti@benettiengineering.com, thirtythreeforty@gmail.com Subject: Re: [PATCH v1 2/3] mach-sunxi: Add spi boot for SUNIV Message-ID: <20220210105201.7aed71ab@donnerap.cambridge.arm.com> In-Reply-To: <20220210043438.1706939-3-Mr.Bossman075@gmail.com> References: <20220210043438.1706939-1-Mr.Bossman075@gmail.com> <20220210043438.1706939-3-Mr.Bossman075@gmail.com> Organization: ARM X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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.5 at phobos.denx.de X-Virus-Status: Clean On Wed, 9 Feb 2022 23:34:37 -0500 Jesse Taube wrote: Hi Jesse, many thanks for sending this, I guess this makes those little boards much more useful. > Add support for the spi boot in spl on suniv architecture. A more elaborate commit message would be welcomed. Please mention the F1C100s, to give some more context. Also briefly mention the differences, I think Icenowy summarised this quite well in her version of the patch (06/27): The suniv SoC come with a sun6i-style SPI controller at the base address of sun4i SPI controller. The module clock of the SPI controller is also missing. You could add: "... is also missing, which leaves us running directly from the AHB clock, set to 200 MHz." > > Signed-off-by: Jesse Taube > --- > arch/arm/include/asm/arch-sunxi/gpio.h | 1 + > arch/arm/mach-sunxi/spl_spi_sunxi.c | 26 +++++++++++++++++++------- > 2 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h > index 7f7eb0517c..edd0fbf49f 100644 > --- a/arch/arm/include/asm/arch-sunxi/gpio.h > +++ b/arch/arm/include/asm/arch-sunxi/gpio.h > @@ -160,6 +160,7 @@ enum sunxi_gpio_number { > #define SUNXI_GPC_SDC2 3 > #define SUN6I_GPC_SDC3 4 > #define SUN50I_GPC_SPI0 4 > +#define SUNIV_GPC_SPI0 2 > > #define SUNXI_GPD_LCD0 2 > #define SUNXI_GPD_LVDS0 3 > diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c > index 910e805016..9a3666a2d7 100644 > --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c > +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c > @@ -90,6 +90,7 @@ > > #define SPI0_CLK_DIV_BY_2 0x1000 > #define SPI0_CLK_DIV_BY_4 0x1001 > +#define SPI0_CLK_DIV_BY_32 0x100f > > /*****************************************************************************/ > > @@ -132,7 +133,8 @@ static uintptr_t spi0_base_address(void) > if (IS_ENABLED(CONFIG_MACH_SUN50I_H6)) > return 0x05010000; > > - if (!is_sun6i_gen_spi()) > + if (!is_sun6i_gen_spi() || > + IS_ENABLED(CONFIG_MACH_SUNIV)) > return 0x01C05000; > > return 0x01C68000; > @@ -156,11 +158,17 @@ static void spi0_enable_clock(void) > if (!IS_ENABLED(CONFIG_MACH_SUN50I_H6)) > setbits_le32(CCM_AHB_GATING0, (1 << AHB_GATE_OFFSET_SPI0)); > > - /* Divide by 4 */ > - writel(SPI0_CLK_DIV_BY_4, base + (is_sun6i_gen_spi() ? > - SUN6I_SPI0_CCTL : SUN4I_SPI0_CCTL)); > - /* 24MHz from OSC24M */ > - writel((1 << 31), CCM_SPI0_CLK); > + if (IS_ENABLED(CONFIG_MACH_SUNIV)) { > + /* Divide by 32, clock source is AHB clock 200MHz */ > + writel(SPI0_CLK_DIV_BY_32, base + (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I) ? This seems pointless and redundant, since we exactly know the register when MACH_SUNIV is selected. > + SUN6I_SPI0_CCTL : SUN4I_SPI0_CCTL)); > + } else { > + /* Divide by 4 */ > + writel(SPI0_CLK_DIV_BY_4, base + (is_sun6i_gen_spi() ? > + SUN6I_SPI0_CCTL : SUN4I_SPI0_CCTL)); > + /* 24MHz from OSC24M */ > + writel((1 << 31), CCM_SPI0_CLK); > + } > > if (is_sun6i_gen_spi()) { > /* Enable SPI in the master mode and do a soft reset */ > @@ -191,7 +199,8 @@ static void spi0_disable_clock(void) > SUN4I_CTL_ENABLE); > > /* Disable the SPI0 clock */ > - writel(0, CCM_SPI0_CLK); > + if (!IS_ENABLED(CONFIG_MACH_SUNIV)) > + writel(0, CCM_SPI0_CLK); > > /* Close the SPI0 gate */ > if (!IS_ENABLED(CONFIG_MACH_SUN50I_H6)) > @@ -213,6 +222,9 @@ static void spi0_init(void) > IS_ENABLED(CONFIG_MACH_SUN50I_H6)) > pin_function = SUN50I_GPC_SPI0; > > + if (IS_ENABLED(CONFIG_MACH_SUNIV)) > + pin_function = SUNIV_GPC_SPI0; > + It looks a bit better to tie connect this with an "else if" to the previous comparison, since there is only one choice. Cheers, Andre > spi0_pinmux_setup(pin_function); > spi0_enable_clock(); > }