[3/6] sunxi: SPL SPI: Introduce is_new_gen_spi()

Message ID 20200106012915.2772-4-andre.przywara@arm.com
State Superseded
Headers show
Series
  • sunxi: SPL SPI booting: Enable R40 and H6 SoCs
Related show

Commit Message

Andre Przywara Jan. 6, 2020, 1:29 a.m.
So far we were using the CONFIG_SUNXI_GEN_SUN6I symbol to select between
the two SPI controller generations used on Allwinner SoCs. This is a
convenience symbol to roughly differentiate between "older" and "newer"
generation of SoCs.

The H6 SoCs is the newest SoC so far, but is sufficiently different to
not define this symbol. However it is using a SPI controller compatible
to the "new gen" SoCs.

To prepare for H6 support, we replace the check for this single symbol
with an explicit function, which can later be extended.
For now we just return CONFIG_SUNXI_GEN_SUN6I in there, so this does not
create a functional change.

Signed-off-by: Andre Przywara <andre.przywara at arm.com>
---
 arch/arm/mach-sunxi/spl_spi_sunxi.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

Comments

Jagan Teki Jan. 21, 2020, 8:20 a.m. | #1
On Mon, Jan 6, 2020 at 6:59 AM Andre Przywara <andre.przywara at arm.com> wrote:
>
> So far we were using the CONFIG_SUNXI_GEN_SUN6I symbol to select between
> the two SPI controller generations used on Allwinner SoCs. This is a
> convenience symbol to roughly differentiate between "older" and "newer"
> generation of SoCs.
>
> The H6 SoCs is the newest SoC so far, but is sufficiently different to
> not define this symbol. However it is using a SPI controller compatible
> to the "new gen" SoCs.
>
> To prepare for H6 support, we replace the check for this single symbol
> with an explicit function, which can later be extended.
> For now we just return CONFIG_SUNXI_GEN_SUN6I in there, so this does not
> create a functional change.
>
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> ---
>  arch/arm/mach-sunxi/spl_spi_sunxi.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> index 5b4598a25b..b19f1bf4af 100644
> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> @@ -100,9 +100,14 @@ static void spi0_pinmux_setup(unsigned int pin_function)
>                 sunxi_gpio_set_cfgpin(SUNXI_GPC(3), pin_function);
>  }
>
> +static bool is_new_gen_spi(void)
> +{
> +       return IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I);
> +}

Doesn't it confusing? new gen is H6, but it returns 6I?
Andre Przywara Jan. 27, 2020, 1:29 a.m. | #2
On 21/01/2020 08:20, Jagan Teki wrote:

Hi Jagan,

first: many thanks for merging those other patches of mine, much
appreciated!

> On Mon, Jan 6, 2020 at 6:59 AM Andre Przywara <andre.przywara at arm.com> wrote:
>>
>> So far we were using the CONFIG_SUNXI_GEN_SUN6I symbol to select between
>> the two SPI controller generations used on Allwinner SoCs. This is a
>> convenience symbol to roughly differentiate between "older" and "newer"
>> generation of SoCs.
>>
>> The H6 SoCs is the newest SoC so far, but is sufficiently different to
>> not define this symbol. However it is using a SPI controller compatible
>> to the "new gen" SoCs.
>>
>> To prepare for H6 support, we replace the check for this single symbol
>> with an explicit function, which can later be extended.
>> For now we just return CONFIG_SUNXI_GEN_SUN6I in there, so this does not
>> create a functional change.
>>
>> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
>> ---
>>  arch/arm/mach-sunxi/spl_spi_sunxi.c | 22 ++++++++++++++--------
>>  1 file changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c
>> index 5b4598a25b..b19f1bf4af 100644
>> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
>> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
>> @@ -100,9 +100,14 @@ static void spi0_pinmux_setup(unsigned int pin_function)
>>                 sunxi_gpio_set_cfgpin(SUNXI_GPC(3), pin_function);
>>  }
>>
>> +static bool is_new_gen_spi(void)
>> +{
>> +       return IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I);
>> +}
> 
> Doesn't it confusing? new gen is H6, but it returns 6I?

Well, naming ...
For the purpose of U-Boot there are two generations of SPI controller
*register interfaces*, the "old" one used in the older SoCs like the
A20, and the "newer" one used in everything halfway recent. The H6 uses
the same "new" generation, just at a different address. Yes, it adds
quad-SPI, but this is not relevant for this driver.
I have seen this old/new terminology at different places, so just went
with it.
I could rename it to is_spi_sun6i_gen() or something if that makes you
happy...

Cheers,
Andre
Jagan Teki Jan. 27, 2020, 1:16 p.m. | #3
On Mon, Jan 27, 2020 at 6:59 AM André Przywara <andre.przywara at arm.com> wrote:
>
> On 21/01/2020 08:20, Jagan Teki wrote:
>
> Hi Jagan,
>
> first: many thanks for merging those other patches of mine, much
> appreciated!
>
> > On Mon, Jan 6, 2020 at 6:59 AM Andre Przywara <andre.przywara at arm.com> wrote:
> >>
> >> So far we were using the CONFIG_SUNXI_GEN_SUN6I symbol to select between
> >> the two SPI controller generations used on Allwinner SoCs. This is a
> >> convenience symbol to roughly differentiate between "older" and "newer"
> >> generation of SoCs.
> >>
> >> The H6 SoCs is the newest SoC so far, but is sufficiently different to
> >> not define this symbol. However it is using a SPI controller compatible
> >> to the "new gen" SoCs.
> >>
> >> To prepare for H6 support, we replace the check for this single symbol
> >> with an explicit function, which can later be extended.
> >> For now we just return CONFIG_SUNXI_GEN_SUN6I in there, so this does not
> >> create a functional change.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> >> ---
> >>  arch/arm/mach-sunxi/spl_spi_sunxi.c | 22 ++++++++++++++--------
> >>  1 file changed, 14 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> >> index 5b4598a25b..b19f1bf4af 100644
> >> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> >> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> >> @@ -100,9 +100,14 @@ static void spi0_pinmux_setup(unsigned int pin_function)
> >>                 sunxi_gpio_set_cfgpin(SUNXI_GPC(3), pin_function);
> >>  }
> >>
> >> +static bool is_new_gen_spi(void)
> >> +{
> >> +       return IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I);
> >> +}
> >
> > Doesn't it confusing? new gen is H6, but it returns 6I?
>
> Well, naming ...
> For the purpose of U-Boot there are two generations of SPI controller
> *register interfaces*, the "old" one used in the older SoCs like the
> A20, and the "newer" one used in everything halfway recent. The H6 uses
> the same "new" generation, just at a different address. Yes, it adds
> quad-SPI, but this is not relevant for this driver.
> I have seen this old/new terminology at different places, so just went
> with it.
> I could rename it to is_spi_sun6i_gen() or something if that makes you
> happy...

Please do, would be great if you can send the new changes as soon as
possible so-that I can send PR for the rc1.

Jagan.

Patch

diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c
index 5b4598a25b..b19f1bf4af 100644
--- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
+++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
@@ -100,9 +100,14 @@  static void spi0_pinmux_setup(unsigned int pin_function)
 		sunxi_gpio_set_cfgpin(SUNXI_GPC(3), pin_function);
 }
 
+static bool is_new_gen_spi(void)
+{
+	return IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I);
+}
+
 static uintptr_t spi0_base_address(void)
 {
-	if (!IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I))
+	if (!is_new_gen_spi())
 		return 0x01C05000;
 
 	return 0x01C68000;
@@ -116,7 +121,7 @@  static void spi0_enable_clock(void)
 	uintptr_t base = spi0_base_address();
 
 	/* Deassert SPI0 reset on SUN6I */
-	if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I))
+	if (is_new_gen_spi())
 		setbits_le32(SUN6I_BUS_SOFT_RST_REG0,
 			     (1 << AHB_RESET_SPI0_SHIFT));
 
@@ -124,12 +129,12 @@  static void spi0_enable_clock(void)
 	setbits_le32(CCM_AHB_GATING0, (1 << AHB_GATE_OFFSET_SPI0));
 
 	/* Divide by 4 */
-	writel(SPI0_CLK_DIV_BY_4, base + (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I) ?
+	writel(SPI0_CLK_DIV_BY_4, base + (is_new_gen_spi() ?
 				  SUN6I_SPI0_CCTL : SUN4I_SPI0_CCTL));
 	/* 24MHz from OSC24M */
 	writel((1 << 31), CCM_SPI0_CLK);
 
-	if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I)) {
+	if (is_new_gen_spi()) {
 		/* Enable SPI in the master mode and do a soft reset */
 		setbits_le32(base + SUN6I_SPI0_GCR, SUN6I_CTL_MASTER |
 			     SUN6I_CTL_ENABLE | SUN6I_CTL_SRST);
@@ -150,7 +155,7 @@  static void spi0_disable_clock(void)
 	uintptr_t base = spi0_base_address();
 
 	/* Disable the SPI0 controller */
-	if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I))
+	if (is_new_gen_spi())
 		clrbits_le32(base + SUN6I_SPI0_GCR, SUN6I_CTL_MASTER |
 					     SUN6I_CTL_ENABLE);
 	else
@@ -164,7 +169,7 @@  static void spi0_disable_clock(void)
 	clrbits_le32(CCM_AHB_GATING0, (1 << AHB_GATE_OFFSET_SPI0));
 
 	/* Assert SPI0 reset on SUN6I */
-	if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I))
+	if (is_new_gen_spi())
 		clrbits_le32(SUN6I_BUS_SOFT_RST_REG0,
 			     (1 << AHB_RESET_SPI0_SHIFT));
 }
@@ -184,7 +189,8 @@  static void spi0_deinit(void)
 {
 	/* New SoCs can disable pins, older could only set them as input */
 	unsigned int pin_function = SUNXI_GPIO_INPUT;
-	if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I))
+
+	if (is_new_gen_spi())
 		pin_function = SUNXI_GPIO_DISABLE;
 
 	spi0_disable_clock();
@@ -245,7 +251,7 @@  static void spi0_read_data(void *buf, u32 addr, u32 len)
 		if (chunk_len > SPI_READ_MAX_SIZE)
 			chunk_len = SPI_READ_MAX_SIZE;
 
-		if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I)) {
+		if (is_new_gen_spi()) {
 			sunxi_spi0_read_data(buf8, addr, chunk_len,
 					     base + SUN6I_SPI0_TCR,
 					     SUN6I_TCR_XCH,