diff mbox

[U-Boot] armv8: QSPI: Add AHB bus 16MB+ size support

Message ID 1477447038-23803-1-git-send-email-yao.yuan@freescale.com
State Superseded
Delegated to: York Sun
Headers show

Commit Message

Yao Yuan Oct. 26, 2016, 1:57 a.m. UTC
From: Yuan Yao <yao.yuan@nxp.com>

The default configuration for QSPI AHB bus can't support 16MB+.
But some flash on NXP layerscape board are more than 16MB.

Signed-off-by: Yuan Yao <yao.yuan@nxp.com>
---
 arch/arm/cpu/armv8/fsl-layerscape/soc.c            | 37 ++++++++++++++++++++++
 .../include/asm/arch-fsl-layerscape/immap_lsch2.h  |  1 +
 .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |  1 +
 include/configs/ls1012a_common.h                   |  1 +
 include/configs/ls1046ardb.h                       |  1 +
 5 files changed, 41 insertions(+)

Comments

York Sun Nov. 7, 2016, 7:07 p.m. UTC | #1
On 10/25/2016 07:10 PM, Yuan Yao wrote:
> From: Yuan Yao <yao.yuan@nxp.com>
>
> The default configuration for QSPI AHB bus can't support 16MB+.
> But some flash on NXP layerscape board are more than 16MB.

So what do you do?

Is this an erratum workaround? If yes, please refer the erratum number.

>
> Signed-off-by: Yuan Yao <yao.yuan@nxp.com>
> ---
>  arch/arm/cpu/armv8/fsl-layerscape/soc.c            | 37 ++++++++++++++++++++++
>  .../include/asm/arch-fsl-layerscape/immap_lsch2.h  |  1 +
>  .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |  1 +
>  include/configs/ls1012a_common.h                   |  1 +
>  include/configs/ls1046ardb.h                       |  1 +
>  5 files changed, 41 insertions(+)
>
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> index d68eeba..18d753e 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> @@ -370,6 +370,40 @@ void fsl_lsch2_early_init_f(void)
>  }
>  #endif
>
> +#ifdef CONFIG_QSPI_AHB_INIT
> +/* Enable 4bytes address support and fast read */
> +int qspi_ahb_init(void)
> +{
> +	u32 *qspi_lut, lut_key, *qspi_key;
> +
> +	qspi_key = (void *)CONFIG_SYS_QSPI_ADDR + 0x300;
> +	qspi_lut = (void *)CONFIG_SYS_QSPI_ADDR + 0x310;
> +
> +	lut_key = in_be32(qspi_key);
> +
> +	if (lut_key == 0x5af05af0) {
> +		/* That means the register is BE */
> +		out_be32(qspi_key, 0x5af05af0);
> +		out_be32(qspi_key + 1, 0x00000002);
> +		out_be32(qspi_lut, 0x0820040c);
> +		out_be32(qspi_lut + 1, 0x1c080c08);
> +		out_be32(qspi_lut + 2, 0x00002400);
> +		out_be32(qspi_key, 0x5af05af0);
> +		out_be32(qspi_key + 1, 0x00000001);
> +	} else {
> +		/* That means the register is LE */
> +		out_le32(qspi_key, 0x5af05af0);
> +		out_le32(qspi_key + 1, 0x00000002);
> +		out_le32(qspi_lut, 0x0820040c);
> +		out_le32(qspi_lut + 1, 0x1c080c08);
> +		out_le32(qspi_lut + 2, 0x00002400);
> +		out_le32(qspi_key, 0x5af05af0);
> +		out_le32(qspi_key + 1, 0x00000001);
> +	}

What do these sequences do?

Put a blank line before return.

> +	return 0;
> +}
> +#endif
> +
>  #ifdef CONFIG_BOARD_LATE_INIT
>  int board_late_init(void)
>  {
> @@ -379,6 +413,9 @@ int board_late_init(void)
>  #ifdef CONFIG_CHAIN_OF_TRUST
>  	fsl_setenv_chain_of_trust();
>  #endif
> +#ifdef CONFIG_QSPI_AHB_INIT
> +	qspi_ahb_init();
> +#endif
>
>  	return 0;
>  }
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> index d88543d..a28b1fd 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> @@ -18,6 +18,7 @@
>  #define CONFIG_SYS_CCI400_ADDR			(CONFIG_SYS_IMMR + 0x00180000)
>  #define CONFIG_SYS_GIC400_ADDR			(CONFIG_SYS_IMMR + 0x00400000)
>  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR + 0x00530000)
> +#define CONFIG_SYS_QSPI_ADDR			(CONFIG_SYS_IMMR + 0x00550000)
>  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR + 0x00560000)
>  #define CONFIG_SYS_FSL_CSU_ADDR			(CONFIG_SYS_IMMR + 0x00510000)
>  #define CONFIG_SYS_FSL_GUTS_ADDR		(CONFIG_SYS_IMMR + 0x00ee0000)
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> index 7acba27..8aefc76 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> @@ -19,6 +19,7 @@
>  #define CONFIG_SYS_FSL_CH3_CLK_GRPA_ADDR	(CONFIG_SYS_IMMR + 0x00300000)
>  #define CONFIG_SYS_FSL_CH3_CLK_GRPB_ADDR	(CONFIG_SYS_IMMR + 0x00310000)
>  #define CONFIG_SYS_FSL_CH3_CLK_CTRL_ADDR	(CONFIG_SYS_IMMR + 0x00370000)
> +#define CONFIG_SYS_QSPI_ADDR			(CONFIG_SYS_IMMR + 0x010c0000)
>  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR + 0x01140000)
>  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR + 0x01240000)
>  #define CONFIG_SYS_NS16550_COM1			(CONFIG_SYS_IMMR + 0x011C0500)
> diff --git a/include/configs/ls1012a_common.h b/include/configs/ls1012a_common.h
> index 80603c9..c1e1102 100644
> --- a/include/configs/ls1012a_common.h
> +++ b/include/configs/ls1012a_common.h
> @@ -61,6 +61,7 @@
>
>  #define FSL_QSPI_FLASH_SIZE		(1 << 24)
>  #define FSL_QSPI_FLASH_NUM		2
> +#define CONFIG_QSPI_AHB_INIT
>
>  /*
>   * Environment
> diff --git a/include/configs/ls1046ardb.h b/include/configs/ls1046ardb.h
> index 2fe8fc1..662ecb1 100644
> --- a/include/configs/ls1046ardb.h
> +++ b/include/configs/ls1046ardb.h
> @@ -209,6 +209,7 @@
>  #define FSL_QSPI_FLASH_SIZE		(1 << 26)
>  #define FSL_QSPI_FLASH_NUM		2
>  #define CONFIG_SPI_FLASH_BAR
> +#define CONFIG_QSPI_AHB_INIT

Is it really a config option? Is it really board-dependent? Does it 
compile on the latest master branch?

York
yao yuan Nov. 8, 2016, 6:03 a.m. UTC | #2
On 11/08/2016 02:27 AM, York Sun wrote:
> On 10/25/2016 07:10 PM, Yuan Yao wrote:
> > From: Yuan Yao <yao.yuan@nxp.com>
> >
> > The default configuration for QSPI AHB bus can't support 16MB+.
> > But some flash on NXP layerscape board are more than 16MB.
> 
> So what do you do?
> 
> Is this an erratum workaround? If yes, please refer the erratum number.

Hi York,

I think It's not an erratum maybe it's better to call it new feature.

As a default configuration for QSPI AHB, the address size is 3-bytes.
It has a good compatibility for QSPI boot for different SPI-NOR flash.

But if the address size is only 3-bytes, the QSPI can't access to the data that more than 16M+.

So we can update the default configuration for QSPI AHB in uboot to use 4-bytes address.
So that QSPI can access to 16M+ size by AHB bus.

Thanks.
> 
> >
> > Signed-off-by: Yuan Yao <yao.yuan@nxp.com>
> > ---
> >  arch/arm/cpu/armv8/fsl-layerscape/soc.c            | 37
> ++++++++++++++++++++++
> >  .../include/asm/arch-fsl-layerscape/immap_lsch2.h  |  1 +
> > .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |  1 +
> >  include/configs/ls1012a_common.h                   |  1 +
> >  include/configs/ls1046ardb.h                       |  1 +
> >  5 files changed, 41 insertions(+)
> >
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > index d68eeba..18d753e 100644
> > --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > @@ -370,6 +370,40 @@ void fsl_lsch2_early_init_f(void)  }  #endif
> >
> > +#ifdef CONFIG_QSPI_AHB_INIT
> > +/* Enable 4bytes address support and fast read */ int
> > +qspi_ahb_init(void) {
> > +	u32 *qspi_lut, lut_key, *qspi_key;
> > +
> > +	qspi_key = (void *)CONFIG_SYS_QSPI_ADDR + 0x300;
> > +	qspi_lut = (void *)CONFIG_SYS_QSPI_ADDR + 0x310;
> > +
> > +	lut_key = in_be32(qspi_key);
> > +
> > +	if (lut_key == 0x5af05af0) {
> > +		/* That means the register is BE */
> > +		out_be32(qspi_key, 0x5af05af0);
> > +		out_be32(qspi_key + 1, 0x00000002);
> > +		out_be32(qspi_lut, 0x0820040c);
> > +		out_be32(qspi_lut + 1, 0x1c080c08);
> > +		out_be32(qspi_lut + 2, 0x00002400);
> > +		out_be32(qspi_key, 0x5af05af0);
> > +		out_be32(qspi_key + 1, 0x00000001);
> > +	} else {
> > +		/* That means the register is LE */
> > +		out_le32(qspi_key, 0x5af05af0);
> > +		out_le32(qspi_key + 1, 0x00000002);
> > +		out_le32(qspi_lut, 0x0820040c);
> > +		out_le32(qspi_lut + 1, 0x1c080c08);
> > +		out_le32(qspi_lut + 2, 0x00002400);
> > +		out_le32(qspi_key, 0x5af05af0);
> > +		out_le32(qspi_key + 1, 0x00000001);
> > +	}
> 
> What do these sequences do?

It used to set the AHB bus to use 4-bytes command and the corresponding sequence.
So that QSPI can access to 16M+ size by AHB bus.

> 
> Put a blank line before return.
> 
> > +	return 0;
> > +}
> > +#endif
> > +
> >  #ifdef CONFIG_BOARD_LATE_INIT
> >  int board_late_init(void)
> >  {
> > @@ -379,6 +413,9 @@ int board_late_init(void)  #ifdef
> > CONFIG_CHAIN_OF_TRUST
> >  	fsl_setenv_chain_of_trust();
> >  #endif
> > +#ifdef CONFIG_QSPI_AHB_INIT
> > +	qspi_ahb_init();
> > +#endif
> >
> >  	return 0;
> >  }
> > diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> > b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> > index d88543d..a28b1fd 100644
> > --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> > +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> > @@ -18,6 +18,7 @@
> >  #define CONFIG_SYS_CCI400_ADDR			(CONFIG_SYS_IMMR +
> 0x00180000)
> >  #define CONFIG_SYS_GIC400_ADDR			(CONFIG_SYS_IMMR +
> 0x00400000)
> >  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR +
> 0x00530000)
> > +#define CONFIG_SYS_QSPI_ADDR			(CONFIG_SYS_IMMR +
> 0x00550000)
> >  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR +
> 0x00560000)
> >  #define CONFIG_SYS_FSL_CSU_ADDR
> 	(CONFIG_SYS_IMMR + 0x00510000)
> >  #define CONFIG_SYS_FSL_GUTS_ADDR		(CONFIG_SYS_IMMR +
> 0x00ee0000)
> > diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> > b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> > index 7acba27..8aefc76 100644
> > --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> > +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> > @@ -19,6 +19,7 @@
> >  #define CONFIG_SYS_FSL_CH3_CLK_GRPA_ADDR	(CONFIG_SYS_IMMR +
> 0x00300000)
> >  #define CONFIG_SYS_FSL_CH3_CLK_GRPB_ADDR	(CONFIG_SYS_IMMR +
> 0x00310000)
> >  #define CONFIG_SYS_FSL_CH3_CLK_CTRL_ADDR	(CONFIG_SYS_IMMR +
> 0x00370000)
> > +#define CONFIG_SYS_QSPI_ADDR			(CONFIG_SYS_IMMR +
> 0x010c0000)
> >  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR +
> 0x01140000)
> >  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR +
> 0x01240000)
> >  #define CONFIG_SYS_NS16550_COM1
> 	(CONFIG_SYS_IMMR + 0x011C0500)
> > diff --git a/include/configs/ls1012a_common.h
> > b/include/configs/ls1012a_common.h
> > index 80603c9..c1e1102 100644
> > --- a/include/configs/ls1012a_common.h
> > +++ b/include/configs/ls1012a_common.h
> > @@ -61,6 +61,7 @@
> >
> >  #define FSL_QSPI_FLASH_SIZE		(1 << 24)
> >  #define FSL_QSPI_FLASH_NUM		2
> > +#define CONFIG_QSPI_AHB_INIT
> >
> >  /*
> >   * Environment
> > diff --git a/include/configs/ls1046ardb.h
> > b/include/configs/ls1046ardb.h index 2fe8fc1..662ecb1 100644
> > --- a/include/configs/ls1046ardb.h
> > +++ b/include/configs/ls1046ardb.h
> > @@ -209,6 +209,7 @@
> >  #define FSL_QSPI_FLASH_SIZE		(1 << 26)
> >  #define FSL_QSPI_FLASH_NUM		2
> >  #define CONFIG_SPI_FLASH_BAR
> > +#define CONFIG_QSPI_AHB_INIT
> 
> Is it really a config option? Is it really board-dependent? Does it compile on the
> latest master branch?
> 

Yes, I think it's a config option.
1, whether it supports 16M+ or not will not break the existing functionality.
2, Some flash is just only 16M size. (the SPI flash on LS1043A)

Yes, It's board-dependent.
It's depend the flash size on the board whether it's more than 16M.
For example:
1, LS1046ARDB, LS1012ARDB, LS1012AQDS need
2, LS1043 don't need. 

I have tested it on LS1046ARDB and LS1012ARDB.

Thanks.
York Sun Nov. 8, 2016, 6:11 p.m. UTC | #3
On 11/07/2016 10:03 PM, Yao Yuan wrote:
> On 11/08/2016 02:27 AM, York Sun wrote:
>> On 10/25/2016 07:10 PM, Yuan Yao wrote:
>>> From: Yuan Yao <yao.yuan@nxp.com>
>>>
>>> The default configuration for QSPI AHB bus can't support 16MB+.
>>> But some flash on NXP layerscape board are more than 16MB.
>>
>> So what do you do?
>>
>> Is this an erratum workaround? If yes, please refer the erratum number.
>
> Hi York,
>
> I think It's not an erratum maybe it's better to call it new feature.
>
> As a default configuration for QSPI AHB, the address size is 3-bytes.
> It has a good compatibility for QSPI boot for different SPI-NOR flash.
>
> But if the address size is only 3-bytes, the QSPI can't access to the data that more than 16M+.
>
> So we can update the default configuration for QSPI AHB in uboot to use 4-bytes address.
> So that QSPI can access to 16M+ size by AHB bus.
>

I cannot figure this out from the change you proposed, or from the 
commit message. Maybe adding some comments next to the code? Those magic 
numbers are better explained.

York
York Sun Nov. 14, 2016, 5:46 p.m. UTC | #4
On 11/07/2016 10:03 PM, Yao Yuan wrote:
> On 11/08/2016 02:27 AM, York Sun wrote:
>> On 10/25/2016 07:10 PM, Yuan Yao wrote:
>>> From: Yuan Yao <yao.yuan@nxp.com>
>>>
>>> The default configuration for QSPI AHB bus can't support 16MB+.
>>> But some flash on NXP layerscape board are more than 16MB.
>>
>> So what do you do?
>>
>> Is this an erratum workaround? If yes, please refer the erratum number.
>
> Hi York,
>
> I think It's not an erratum maybe it's better to call it new feature.
>
> As a default configuration for QSPI AHB, the address size is 3-bytes.
> It has a good compatibility for QSPI boot for different SPI-NOR flash.
>
> But if the address size is only 3-bytes, the QSPI can't access to the data that more than 16M+.
>
> So we can update the default configuration for QSPI AHB in uboot to use 4-bytes address.
> So that QSPI can access to 16M+ size by AHB bus.

Let me try to understand this. With your change, 4-byte addressing is 
supported. Do all flash chips support 4-byte addressing?


>
> Thanks.
>>
>>>
>>> Signed-off-by: Yuan Yao <yao.yuan@nxp.com>
>>> ---
>>>  arch/arm/cpu/armv8/fsl-layerscape/soc.c            | 37
>> ++++++++++++++++++++++
>>>  .../include/asm/arch-fsl-layerscape/immap_lsch2.h  |  1 +
>>> .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |  1 +
>>>  include/configs/ls1012a_common.h                   |  1 +
>>>  include/configs/ls1046ardb.h                       |  1 +
>>>  5 files changed, 41 insertions(+)
>>>
>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> index d68eeba..18d753e 100644
>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> @@ -370,6 +370,40 @@ void fsl_lsch2_early_init_f(void)  }  #endif
>>>
>>> +#ifdef CONFIG_QSPI_AHB_INIT
>>> +/* Enable 4bytes address support and fast read */ int
>>> +qspi_ahb_init(void) {
>>> +	u32 *qspi_lut, lut_key, *qspi_key;
>>> +
>>> +	qspi_key = (void *)CONFIG_SYS_QSPI_ADDR + 0x300;
>>> +	qspi_lut = (void *)CONFIG_SYS_QSPI_ADDR + 0x310;
>>> +
>>> +	lut_key = in_be32(qspi_key);
>>> +
>>> +	if (lut_key == 0x5af05af0) {
>>> +		/* That means the register is BE */
>>> +		out_be32(qspi_key, 0x5af05af0);
>>> +		out_be32(qspi_key + 1, 0x00000002);
>>> +		out_be32(qspi_lut, 0x0820040c);
>>> +		out_be32(qspi_lut + 1, 0x1c080c08);
>>> +		out_be32(qspi_lut + 2, 0x00002400);
>>> +		out_be32(qspi_key, 0x5af05af0);
>>> +		out_be32(qspi_key + 1, 0x00000001);
>>> +	} else {
>>> +		/* That means the register is LE */
>>> +		out_le32(qspi_key, 0x5af05af0);
>>> +		out_le32(qspi_key + 1, 0x00000002);
>>> +		out_le32(qspi_lut, 0x0820040c);
>>> +		out_le32(qspi_lut + 1, 0x1c080c08);
>>> +		out_le32(qspi_lut + 2, 0x00002400);
>>> +		out_le32(qspi_key, 0x5af05af0);
>>> +		out_le32(qspi_key + 1, 0x00000001);
>>> +	}
>>
>> What do these sequences do?
>
> It used to set the AHB bus to use 4-bytes command and the corresponding sequence.
> So that QSPI can access to 16M+ size by AHB bus.
>
>>
>> Put a blank line before return.

You need a blank line here.

>>
>>> +	return 0;
>>> +}
>>> +#endif
>>> +
>>>  #ifdef CONFIG_BOARD_LATE_INIT
>>>  int board_late_init(void)
>>>  {
>>> @@ -379,6 +413,9 @@ int board_late_init(void)  #ifdef
>>> CONFIG_CHAIN_OF_TRUST
>>>  	fsl_setenv_chain_of_trust();
>>>  #endif
>>> +#ifdef CONFIG_QSPI_AHB_INIT
>>> +	qspi_ahb_init();
>>> +#endif
>>>
>>>  	return 0;
>>>  }
>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>> index d88543d..a28b1fd 100644
>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>> @@ -18,6 +18,7 @@
>>>  #define CONFIG_SYS_CCI400_ADDR			(CONFIG_SYS_IMMR +
>> 0x00180000)
>>>  #define CONFIG_SYS_GIC400_ADDR			(CONFIG_SYS_IMMR +
>> 0x00400000)
>>>  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR +
>> 0x00530000)
>>> +#define CONFIG_SYS_QSPI_ADDR			(CONFIG_SYS_IMMR +
>> 0x00550000)
>>>  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR +
>> 0x00560000)
>>>  #define CONFIG_SYS_FSL_CSU_ADDR
>> 	(CONFIG_SYS_IMMR + 0x00510000)
>>>  #define CONFIG_SYS_FSL_GUTS_ADDR		(CONFIG_SYS_IMMR +
>> 0x00ee0000)
>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> index 7acba27..8aefc76 100644
>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> @@ -19,6 +19,7 @@
>>>  #define CONFIG_SYS_FSL_CH3_CLK_GRPA_ADDR	(CONFIG_SYS_IMMR +
>> 0x00300000)
>>>  #define CONFIG_SYS_FSL_CH3_CLK_GRPB_ADDR	(CONFIG_SYS_IMMR +
>> 0x00310000)
>>>  #define CONFIG_SYS_FSL_CH3_CLK_CTRL_ADDR	(CONFIG_SYS_IMMR +
>> 0x00370000)
>>> +#define CONFIG_SYS_QSPI_ADDR			(CONFIG_SYS_IMMR +
>> 0x010c0000)
>>>  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR +
>> 0x01140000)
>>>  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR +
>> 0x01240000)
>>>  #define CONFIG_SYS_NS16550_COM1
>> 	(CONFIG_SYS_IMMR + 0x011C0500)
>>> diff --git a/include/configs/ls1012a_common.h
>>> b/include/configs/ls1012a_common.h
>>> index 80603c9..c1e1102 100644
>>> --- a/include/configs/ls1012a_common.h
>>> +++ b/include/configs/ls1012a_common.h
>>> @@ -61,6 +61,7 @@
>>>
>>>  #define FSL_QSPI_FLASH_SIZE		(1 << 24)
>>>  #define FSL_QSPI_FLASH_NUM		2
>>> +#define CONFIG_QSPI_AHB_INIT

New CONFIG_* macros are not accepted. Please use Kconfig instead.

>>>
>>>  /*
>>>   * Environment
>>> diff --git a/include/configs/ls1046ardb.h
>>> b/include/configs/ls1046ardb.h index 2fe8fc1..662ecb1 100644
>>> --- a/include/configs/ls1046ardb.h
>>> +++ b/include/configs/ls1046ardb.h
>>> @@ -209,6 +209,7 @@
>>>  #define FSL_QSPI_FLASH_SIZE		(1 << 26)
>>>  #define FSL_QSPI_FLASH_NUM		2
>>>  #define CONFIG_SPI_FLASH_BAR
>>> +#define CONFIG_QSPI_AHB_INIT
>>
>> Is it really a config option? Is it really board-dependent? Does it compile on the
>> latest master branch?
>>
>
> Yes, I think it's a config option.
> 1, whether it supports 16M+ or not will not break the existing functionality.

What do you mean exactly? With this patch, old flash chips continue to 
work even you set it to 4-byte addressing? If true, why don't you enable 
this feature for all and get rid of the macro?

> 2, Some flash is just only 16M size. (the SPI flash on LS1043A)

This is not an option. It is a requirement.

>
> Yes, It's board-dependent.
> It's depend the flash size on the board whether it's more than 16M.
> For example:
> 1, LS1046ARDB, LS1012ARDB, LS1012AQDS need
> 2, LS1043 don't need.

Do LS1043A boards still work with this feature on?

>
> I have tested it on LS1046ARDB and LS1012ARDB.

The bottom of line is, if all existing and future boards work fine with 
this feature, you don't need the condition macro. If some board/flash 
cannot work without this feature, enable it in Kconfig.

York
yao yuan Nov. 15, 2016, 9:38 a.m. UTC | #5
On 11/15/2016 01:47 AM, York Sun wrote:
> On 11/07/2016 10:03 PM, Yao Yuan wrote:
> > On 11/08/2016 02:27 AM, York Sun wrote:
> >> On 10/25/2016 07:10 PM, Yuan Yao wrote:
> >>> From: Yuan Yao <yao.yuan@nxp.com>
> >>>
> >>> The default configuration for QSPI AHB bus can't support 16MB+.
> >>> But some flash on NXP layerscape board are more than 16MB.
> >>
> >> So what do you do?
> >>
> >> Is this an erratum workaround? If yes, please refer the erratum number.
> >
> > Hi York,
> >
> > I think It's not an erratum maybe it's better to call it new feature.
> >
> > As a default configuration for QSPI AHB, the address size is 3-bytes.
> > It has a good compatibility for QSPI boot for different SPI-NOR flash.
> >
> > But if the address size is only 3-bytes, the QSPI can't access to the data that
> more than 16M+.
> >
> > So we can update the default configuration for QSPI AHB in uboot to use 4-
> bytes address.
> > So that QSPI can access to 16M+ size by AHB bus.
> 
> Let me try to understand this. With your change, 4-byte addressing is
> supported. Do all flash chips support 4-byte addressing?
> 
> 
No, not all the flash chips support 4-byte addressing.
But the flash chips on LS1012ARDB, LS1046ARDB can support 4-byte addressing.

> >
> > Thanks.
> >>
> >>>
> >>> Signed-off-by: Yuan Yao <yao.yuan@nxp.com>
> >>> ---
> >>>  arch/arm/cpu/armv8/fsl-layerscape/soc.c            | 37
> >> ++++++++++++++++++++++
> >>>  .../include/asm/arch-fsl-layerscape/immap_lsch2.h  |  1 +
> >>> .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |  1 +
> >>>  include/configs/ls1012a_common.h                   |  1 +
> >>>  include/configs/ls1046ardb.h                       |  1 +
> >>>  5 files changed, 41 insertions(+)
> >>>
> >>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> >>> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> >>> index d68eeba..18d753e 100644
> >>> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> >>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> >>> @@ -370,6 +370,40 @@ void fsl_lsch2_early_init_f(void)  }  #endif
> >>>
> >>> +#ifdef CONFIG_QSPI_AHB_INIT
> >>> +/* Enable 4bytes address support and fast read */ int
> >>> +qspi_ahb_init(void) {
> >>> +	u32 *qspi_lut, lut_key, *qspi_key;
> >>> +
> >>> +	qspi_key = (void *)CONFIG_SYS_QSPI_ADDR + 0x300;
> >>> +	qspi_lut = (void *)CONFIG_SYS_QSPI_ADDR + 0x310;
> >>> +
> >>> +	lut_key = in_be32(qspi_key);
> >>> +
> >>> +	if (lut_key == 0x5af05af0) {
> >>> +		/* That means the register is BE */
> >>> +		out_be32(qspi_key, 0x5af05af0);
> >>> +		out_be32(qspi_key + 1, 0x00000002);
> >>> +		out_be32(qspi_lut, 0x0820040c);
> >>> +		out_be32(qspi_lut + 1, 0x1c080c08);
> >>> +		out_be32(qspi_lut + 2, 0x00002400);
> >>> +		out_be32(qspi_key, 0x5af05af0);
> >>> +		out_be32(qspi_key + 1, 0x00000001);
> >>> +	} else {
> >>> +		/* That means the register is LE */
> >>> +		out_le32(qspi_key, 0x5af05af0);
> >>> +		out_le32(qspi_key + 1, 0x00000002);
> >>> +		out_le32(qspi_lut, 0x0820040c);
> >>> +		out_le32(qspi_lut + 1, 0x1c080c08);
> >>> +		out_le32(qspi_lut + 2, 0x00002400);
> >>> +		out_le32(qspi_key, 0x5af05af0);
> >>> +		out_le32(qspi_key + 1, 0x00000001);
> >>> +	}
> >>
> >> What do these sequences do?
> >
> > It used to set the AHB bus to use 4-bytes command and the corresponding
> sequence.
> > So that QSPI can access to 16M+ size by AHB bus.
> >
> >>
> >> Put a blank line before return.
> 
> You need a blank line here.

OK, I will.

> 
> >>
> >>> +	return 0;
> >>> +}
> >>> +#endif
> >>> +
> >>>  #ifdef CONFIG_BOARD_LATE_INIT
> >>>  int board_late_init(void)
> >>>  {
> >>> @@ -379,6 +413,9 @@ int board_late_init(void)  #ifdef
> >>> CONFIG_CHAIN_OF_TRUST
> >>>  	fsl_setenv_chain_of_trust();
> >>>  #endif
> >>> +#ifdef CONFIG_QSPI_AHB_INIT
> >>> +	qspi_ahb_init();
> >>> +#endif
> >>>
> >>>  	return 0;
> >>>  }
> >>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> >>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> >>> index d88543d..a28b1fd 100644
> >>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> >>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> >>> @@ -18,6 +18,7 @@
> >>>  #define CONFIG_SYS_CCI400_ADDR
> 	(CONFIG_SYS_IMMR +
> >> 0x00180000)
> >>>  #define CONFIG_SYS_GIC400_ADDR
> 	(CONFIG_SYS_IMMR +
> >> 0x00400000)
> >>>  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR +
> >> 0x00530000)
> >>> +#define CONFIG_SYS_QSPI_ADDR
> 	(CONFIG_SYS_IMMR +
> >> 0x00550000)
> >>>  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR +
> >> 0x00560000)
> >>>  #define CONFIG_SYS_FSL_CSU_ADDR
> >> 	(CONFIG_SYS_IMMR + 0x00510000)
> >>>  #define CONFIG_SYS_FSL_GUTS_ADDR		(CONFIG_SYS_IMMR +
> >> 0x00ee0000)
> >>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >>> index 7acba27..8aefc76 100644
> >>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >>> @@ -19,6 +19,7 @@
> >>>  #define CONFIG_SYS_FSL_CH3_CLK_GRPA_ADDR
> 	(CONFIG_SYS_IMMR +
> >> 0x00300000)
> >>>  #define CONFIG_SYS_FSL_CH3_CLK_GRPB_ADDR	(CONFIG_SYS_IMMR +
> >> 0x00310000)
> >>>  #define CONFIG_SYS_FSL_CH3_CLK_CTRL_ADDR	(CONFIG_SYS_IMMR +
> >> 0x00370000)
> >>> +#define CONFIG_SYS_QSPI_ADDR
> 	(CONFIG_SYS_IMMR +
> >> 0x010c0000)
> >>>  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR +
> >> 0x01140000)
> >>>  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR +
> >> 0x01240000)
> >>>  #define CONFIG_SYS_NS16550_COM1
> >> 	(CONFIG_SYS_IMMR + 0x011C0500)
> >>> diff --git a/include/configs/ls1012a_common.h
> >>> b/include/configs/ls1012a_common.h
> >>> index 80603c9..c1e1102 100644
> >>> --- a/include/configs/ls1012a_common.h
> >>> +++ b/include/configs/ls1012a_common.h
> >>> @@ -61,6 +61,7 @@
> >>>
> >>>  #define FSL_QSPI_FLASH_SIZE		(1 << 24)
> >>>  #define FSL_QSPI_FLASH_NUM		2
> >>> +#define CONFIG_QSPI_AHB_INIT
> 
> New CONFIG_* macros are not accepted. Please use Kconfig instead.
> 
> >>>
> >>>  /*
> >>>   * Environment
> >>> diff --git a/include/configs/ls1046ardb.h
> >>> b/include/configs/ls1046ardb.h index 2fe8fc1..662ecb1 100644
> >>> --- a/include/configs/ls1046ardb.h
> >>> +++ b/include/configs/ls1046ardb.h
> >>> @@ -209,6 +209,7 @@
> >>>  #define FSL_QSPI_FLASH_SIZE		(1 << 26)
> >>>  #define FSL_QSPI_FLASH_NUM		2
> >>>  #define CONFIG_SPI_FLASH_BAR
> >>> +#define CONFIG_QSPI_AHB_INIT
> >>
> >> Is it really a config option? Is it really board-dependent? Does it compile on
> the
> >> latest master branch?
> >>
> >
> > Yes, I think it's a config option.
> > 1, whether it supports 16M+ or not will not break the existing functionality.
> 
> What do you mean exactly? With this patch, old flash chips continue to
> work even you set it to 4-byte addressing? If true, why don't you enable
> this feature for all and get rid of the macro?
> 

No, we can't use the macro for the flash which not support 4-byte addressing 
So, we have to use the macro for the board which need support 16M+ size.

Which my means is that, without the macro all the board can still working but below 16M.
This patch is just to add a new 16M+ support for some new flash.

> > 2, Some flash is just only 16M size. (the SPI flash on LS1043A)
> 
> This is not an option. It is a requirement.
 
1, new flash with the macro: working
2, new flash without the macro: working, we still can use QSPI driver to access the 16M+size.
3, old flash without the macro: working
4, old flash with the macro: not working, that's why I need this macro.

> 
> >
> > Yes, It's board-dependent.
> > It's depend the flash size on the board whether it's more than 16M.
> > For example:
> > 1, LS1046ARDB, LS1012ARDB, LS1012AQDS need
> > 2, LS1043 don't need.
> 
> Do LS1043A boards still work with this feature on?

No, the flash on LS1043AQDS and LS1043ARDB can't support 4-bytes addressing.
So, LS1043A boards can't work with this feature on.
 
> 
> >
> > I have tested it on LS1046ARDB and LS1012ARDB.
> 
> The bottom of line is, if all existing and future boards work fine with
> this feature, you don't need the condition macro. If some board/flash
> cannot work without this feature, enable it in Kconfig.
> 
> York

So I think I will enable it in Kconfig for which board can work with this feature.

I will send v2 soon,

Thanks for your review.

Yao.
yao yuan Nov. 30, 2016, 3:15 a.m. UTC | #6
On 11/15/2016 02:27 AM, York Sun wrote:
> On 11/07/2016 10:03 PM, Yao Yuan wrote:
> > On 11/08/2016 02:27 AM, York Sun wrote:
> >> On 10/25/2016 07:10 PM, Yuan Yao wrote:
> >>> From: Yuan Yao <yao.yuan@nxp.com>
> >>>
> >>> The default configuration for QSPI AHB bus can't support 16MB+.
> >>> But some flash on NXP layerscape board are more than 16MB.
> >>
> >> So what do you do?
> >>
> >> Is this an erratum workaround? If yes, please refer the erratum number.
> >
> > Hi York,
> >
> > I think It's not an erratum maybe it's better to call it new feature.
> >
> > As a default configuration for QSPI AHB, the address size is 3-bytes.
> > It has a good compatibility for QSPI boot for different SPI-NOR flash.
> >
> > But if the address size is only 3-bytes, the QSPI can't access to the data that
> more than 16M+.
> >
> > So we can update the default configuration for QSPI AHB in uboot to use 4-
> bytes address.
> > So that QSPI can access to 16M+ size by AHB bus.
> 
> Let me try to understand this. With your change, 4-byte addressing is
> supported. Do all flash chips support 4-byte addressing?

No, Not all the flash.
 
> 
> >
> > Thanks.
> >>
> >>>
> >>> Signed-off-by: Yuan Yao <yao.yuan@nxp.com>
> >>> ---
> >>>  arch/arm/cpu/armv8/fsl-layerscape/soc.c            | 37
> >> ++++++++++++++++++++++
> >>>  .../include/asm/arch-fsl-layerscape/immap_lsch2.h  |  1 +
> >>> .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |  1 +
> >>>  include/configs/ls1012a_common.h                   |  1 +
> >>>  include/configs/ls1046ardb.h                       |  1 +
> >>>  5 files changed, 41 insertions(+)
> >>>
> >>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> >>> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> >>> index d68eeba..18d753e 100644
> >>> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> >>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> >>> @@ -370,6 +370,40 @@ void fsl_lsch2_early_init_f(void)  }  #endif
> >>>
> >>> +#ifdef CONFIG_QSPI_AHB_INIT
> >>> +/* Enable 4bytes address support and fast read */ int
> >>> +qspi_ahb_init(void) {
> >>> +	u32 *qspi_lut, lut_key, *qspi_key;
> >>> +
> >>> +	qspi_key = (void *)CONFIG_SYS_QSPI_ADDR + 0x300;
> >>> +	qspi_lut = (void *)CONFIG_SYS_QSPI_ADDR + 0x310;
> >>> +
> >>> +	lut_key = in_be32(qspi_key);
> >>> +
> >>> +	if (lut_key == 0x5af05af0) {
> >>> +		/* That means the register is BE */
> >>> +		out_be32(qspi_key, 0x5af05af0);
> >>> +		out_be32(qspi_key + 1, 0x00000002);
> >>> +		out_be32(qspi_lut, 0x0820040c);
> >>> +		out_be32(qspi_lut + 1, 0x1c080c08);
> >>> +		out_be32(qspi_lut + 2, 0x00002400);
> >>> +		out_be32(qspi_key, 0x5af05af0);
> >>> +		out_be32(qspi_key + 1, 0x00000001);
> >>> +	} else {
> >>> +		/* That means the register is LE */
> >>> +		out_le32(qspi_key, 0x5af05af0);
> >>> +		out_le32(qspi_key + 1, 0x00000002);
> >>> +		out_le32(qspi_lut, 0x0820040c);
> >>> +		out_le32(qspi_lut + 1, 0x1c080c08);
> >>> +		out_le32(qspi_lut + 2, 0x00002400);
> >>> +		out_le32(qspi_key, 0x5af05af0);
> >>> +		out_le32(qspi_key + 1, 0x00000001);
> >>> +	}
> >>
> >> What do these sequences do?
> >
> > It used to set the AHB bus to use 4-bytes command and the corresponding
> sequence.
> > So that QSPI can access to 16M+ size by AHB bus.
> >
> >>
> >> Put a blank line before return.
> 
> You need a blank line here.

OK, get it.


> 
> >>
> >>> +	return 0;
> >>> +}
> >>> +#endif
> >>> +
> >>>  #ifdef CONFIG_BOARD_LATE_INIT
> >>>  int board_late_init(void)
> >>>  {
> >>> @@ -379,6 +413,9 @@ int board_late_init(void)  #ifdef
> >>> CONFIG_CHAIN_OF_TRUST
> >>>  	fsl_setenv_chain_of_trust();
> >>>  #endif
> >>> +#ifdef CONFIG_QSPI_AHB_INIT
> >>> +	qspi_ahb_init();
> >>> +#endif
> >>>
> >>>  	return 0;
> >>>  }
> >>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> >>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> >>> index d88543d..a28b1fd 100644
> >>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> >>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
> >>> @@ -18,6 +18,7 @@
> >>>  #define CONFIG_SYS_CCI400_ADDR
> 	(CONFIG_SYS_IMMR +
> >> 0x00180000)
> >>>  #define CONFIG_SYS_GIC400_ADDR
> 	(CONFIG_SYS_IMMR +
> >> 0x00400000)
> >>>  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR +
> >> 0x00530000)
> >>> +#define CONFIG_SYS_QSPI_ADDR
> 	(CONFIG_SYS_IMMR +
> >> 0x00550000)
> >>>  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR +
> >> 0x00560000)
> >>>  #define CONFIG_SYS_FSL_CSU_ADDR
> >> 	(CONFIG_SYS_IMMR + 0x00510000)
> >>>  #define CONFIG_SYS_FSL_GUTS_ADDR		(CONFIG_SYS_IMMR +
> >> 0x00ee0000)
> >>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >>> index 7acba27..8aefc76 100644
> >>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >>> @@ -19,6 +19,7 @@
> >>>  #define CONFIG_SYS_FSL_CH3_CLK_GRPA_ADDR
> 	(CONFIG_SYS_IMMR +
> >> 0x00300000)
> >>>  #define CONFIG_SYS_FSL_CH3_CLK_GRPB_ADDR	(CONFIG_SYS_IMMR +
> >> 0x00310000)
> >>>  #define CONFIG_SYS_FSL_CH3_CLK_CTRL_ADDR	(CONFIG_SYS_IMMR +
> >> 0x00370000)
> >>> +#define CONFIG_SYS_QSPI_ADDR
> 	(CONFIG_SYS_IMMR +
> >> 0x010c0000)
> >>>  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR +
> >> 0x01140000)
> >>>  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR +
> >> 0x01240000)
> >>>  #define CONFIG_SYS_NS16550_COM1
> >> 	(CONFIG_SYS_IMMR + 0x011C0500)
> >>> diff --git a/include/configs/ls1012a_common.h
> >>> b/include/configs/ls1012a_common.h
> >>> index 80603c9..c1e1102 100644
> >>> --- a/include/configs/ls1012a_common.h
> >>> +++ b/include/configs/ls1012a_common.h
> >>> @@ -61,6 +61,7 @@
> >>>
> >>>  #define FSL_QSPI_FLASH_SIZE		(1 << 24)
> >>>  #define FSL_QSPI_FLASH_NUM		2
> >>> +#define CONFIG_QSPI_AHB_INIT
> 
> New CONFIG_* macros are not accepted. Please use Kconfig instead.
> 

OK, get it.

> >>>
> >>>  /*
> >>>   * Environment
> >>> diff --git a/include/configs/ls1046ardb.h
> >>> b/include/configs/ls1046ardb.h index 2fe8fc1..662ecb1 100644
> >>> --- a/include/configs/ls1046ardb.h
> >>> +++ b/include/configs/ls1046ardb.h
> >>> @@ -209,6 +209,7 @@
> >>>  #define FSL_QSPI_FLASH_SIZE		(1 << 26)
> >>>  #define FSL_QSPI_FLASH_NUM		2
> >>>  #define CONFIG_SPI_FLASH_BAR
> >>> +#define CONFIG_QSPI_AHB_INIT
> >>
> >> Is it really a config option? Is it really board-dependent? Does it compile on
> the
> >> latest master branch?
> >>
> >
> > Yes, I think it's a config option.
> > 1, whether it supports 16M+ or not will not break the existing functionality.
> 
> What do you mean exactly? With this patch, old flash chips continue to
> work even you set it to 4-byte addressing? If true, why don't you enable
> this feature for all and get rid of the macro?

No, the old flash can't work with set it to 4-byte addressing.
That's why we need a macro to control.

> > 2, Some flash is just only 16M size. (the SPI flash on LS1043A)
> 
> This is not an option. It is a requirement.

Yes, 


> >
> > Yes, It's board-dependent.
> > It's depend the flash size on the board whether it's more than 16M.
> > For example:
> > 1, LS1046ARDB, LS1012ARDB, LS1012AQDS need
> > 2, LS1043 don't need.
> 
> Do LS1043A boards still work with this feature on?

No, it can't


> >
> > I have tested it on LS1046ARDB and LS1012ARDB.
> 
> The bottom of line is, if all existing and future boards work fine with
> this feature, you don't need the condition macro. If some board/flash
> cannot work without this feature, enable it in Kconfig.
> 

OK, get it.
I will send v2 soon.
Mike Looijmans Nov. 30, 2016, 6:17 a.m. UTC | #7
On 14-11-16 18:46, york sun wrote:
> On 11/07/2016 10:03 PM, Yao Yuan wrote:
>> On 11/08/2016 02:27 AM, York Sun wrote:
>>> On 10/25/2016 07:10 PM, Yuan Yao wrote:
>>>> From: Yuan Yao <yao.yuan@nxp.com>
>>>>
>>>> The default configuration for QSPI AHB bus can't support 16MB+.
>>>> But some flash on NXP layerscape board are more than 16MB.
>>>
>>> So what do you do?
>>>
>>> Is this an erratum workaround? If yes, please refer the erratum number.
>>
>> Hi York,
>>
>> I think It's not an erratum maybe it's better to call it new feature.
>>
>> As a default configuration for QSPI AHB, the address size is 3-bytes.
>> It has a good compatibility for QSPI boot for different SPI-NOR flash.
>>
>> But if the address size is only 3-bytes, the QSPI can't access to the data that more than 16M+.
>>
>> So we can update the default configuration for QSPI AHB in uboot to use 4-bytes address.
>> So that QSPI can access to 16M+ size by AHB bus.
>
> Let me try to understand this. With your change, 4-byte addressing is
> supported. Do all flash chips support 4-byte addressing?

Nope.

And an extra word of warning: Not all QSPI host controllers support 4 byte 
addressing, so BOTH the chip and the controller must have that capability.

Note that the 3-byte address can be combined with a page-select register, 
effectively making the whole 4GB address space available.

>>
>> Thanks.
>>>
>>>>
>>>> Signed-off-by: Yuan Yao <yao.yuan@nxp.com>
>>>> ---
>>>>  arch/arm/cpu/armv8/fsl-layerscape/soc.c            | 37
>>> ++++++++++++++++++++++
>>>>  .../include/asm/arch-fsl-layerscape/immap_lsch2.h  |  1 +
>>>> .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |  1 +
>>>>  include/configs/ls1012a_common.h                   |  1 +
>>>>  include/configs/ls1046ardb.h                       |  1 +
>>>>  5 files changed, 41 insertions(+)
>>>>
>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>>> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>>> index d68eeba..18d753e 100644
>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>>> @@ -370,6 +370,40 @@ void fsl_lsch2_early_init_f(void)  }  #endif
>>>>
>>>> +#ifdef CONFIG_QSPI_AHB_INIT
>>>> +/* Enable 4bytes address support and fast read */ int
>>>> +qspi_ahb_init(void) {
>>>> +	u32 *qspi_lut, lut_key, *qspi_key;
>>>> +
>>>> +	qspi_key = (void *)CONFIG_SYS_QSPI_ADDR + 0x300;
>>>> +	qspi_lut = (void *)CONFIG_SYS_QSPI_ADDR + 0x310;
>>>> +
>>>> +	lut_key = in_be32(qspi_key);
>>>> +
>>>> +	if (lut_key == 0x5af05af0) {
>>>> +		/* That means the register is BE */
>>>> +		out_be32(qspi_key, 0x5af05af0);
>>>> +		out_be32(qspi_key + 1, 0x00000002);
>>>> +		out_be32(qspi_lut, 0x0820040c);
>>>> +		out_be32(qspi_lut + 1, 0x1c080c08);
>>>> +		out_be32(qspi_lut + 2, 0x00002400);
>>>> +		out_be32(qspi_key, 0x5af05af0);
>>>> +		out_be32(qspi_key + 1, 0x00000001);
>>>> +	} else {
>>>> +		/* That means the register is LE */
>>>> +		out_le32(qspi_key, 0x5af05af0);
>>>> +		out_le32(qspi_key + 1, 0x00000002);
>>>> +		out_le32(qspi_lut, 0x0820040c);
>>>> +		out_le32(qspi_lut + 1, 0x1c080c08);
>>>> +		out_le32(qspi_lut + 2, 0x00002400);
>>>> +		out_le32(qspi_key, 0x5af05af0);
>>>> +		out_le32(qspi_key + 1, 0x00000001);
>>>> +	}
>>>
>>> What do these sequences do?
>>
>> It used to set the AHB bus to use 4-bytes command and the corresponding sequence.
>> So that QSPI can access to 16M+ size by AHB bus.
>>
>>>
>>> Put a blank line before return.
>
> You need a blank line here.
>
>>>
>>>> +	return 0;
>>>> +}
>>>> +#endif
>>>> +
>>>>  #ifdef CONFIG_BOARD_LATE_INIT
>>>>  int board_late_init(void)
>>>>  {
>>>> @@ -379,6 +413,9 @@ int board_late_init(void)  #ifdef
>>>> CONFIG_CHAIN_OF_TRUST
>>>>  	fsl_setenv_chain_of_trust();
>>>>  #endif
>>>> +#ifdef CONFIG_QSPI_AHB_INIT
>>>> +	qspi_ahb_init();
>>>> +#endif
>>>>
>>>>  	return 0;
>>>>  }
>>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> index d88543d..a28b1fd 100644
>>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> @@ -18,6 +18,7 @@
>>>>  #define CONFIG_SYS_CCI400_ADDR			(CONFIG_SYS_IMMR +
>>> 0x00180000)
>>>>  #define CONFIG_SYS_GIC400_ADDR			(CONFIG_SYS_IMMR +
>>> 0x00400000)
>>>>  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR +
>>> 0x00530000)
>>>> +#define CONFIG_SYS_QSPI_ADDR			(CONFIG_SYS_IMMR +
>>> 0x00550000)
>>>>  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR +
>>> 0x00560000)
>>>>  #define CONFIG_SYS_FSL_CSU_ADDR
>>> 	(CONFIG_SYS_IMMR + 0x00510000)
>>>>  #define CONFIG_SYS_FSL_GUTS_ADDR		(CONFIG_SYS_IMMR +
>>> 0x00ee0000)
>>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> index 7acba27..8aefc76 100644
>>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> @@ -19,6 +19,7 @@
>>>>  #define CONFIG_SYS_FSL_CH3_CLK_GRPA_ADDR	(CONFIG_SYS_IMMR +
>>> 0x00300000)
>>>>  #define CONFIG_SYS_FSL_CH3_CLK_GRPB_ADDR	(CONFIG_SYS_IMMR +
>>> 0x00310000)
>>>>  #define CONFIG_SYS_FSL_CH3_CLK_CTRL_ADDR	(CONFIG_SYS_IMMR +
>>> 0x00370000)
>>>> +#define CONFIG_SYS_QSPI_ADDR			(CONFIG_SYS_IMMR +
>>> 0x010c0000)
>>>>  #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR +
>>> 0x01140000)
>>>>  #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR +
>>> 0x01240000)
>>>>  #define CONFIG_SYS_NS16550_COM1
>>> 	(CONFIG_SYS_IMMR + 0x011C0500)
>>>> diff --git a/include/configs/ls1012a_common.h
>>>> b/include/configs/ls1012a_common.h
>>>> index 80603c9..c1e1102 100644
>>>> --- a/include/configs/ls1012a_common.h
>>>> +++ b/include/configs/ls1012a_common.h
>>>> @@ -61,6 +61,7 @@
>>>>
>>>>  #define FSL_QSPI_FLASH_SIZE		(1 << 24)
>>>>  #define FSL_QSPI_FLASH_NUM		2
>>>> +#define CONFIG_QSPI_AHB_INIT
>
> New CONFIG_* macros are not accepted. Please use Kconfig instead.
>
>>>>
>>>>  /*
>>>>   * Environment
>>>> diff --git a/include/configs/ls1046ardb.h
>>>> b/include/configs/ls1046ardb.h index 2fe8fc1..662ecb1 100644
>>>> --- a/include/configs/ls1046ardb.h
>>>> +++ b/include/configs/ls1046ardb.h
>>>> @@ -209,6 +209,7 @@
>>>>  #define FSL_QSPI_FLASH_SIZE		(1 << 26)
>>>>  #define FSL_QSPI_FLASH_NUM		2
>>>>  #define CONFIG_SPI_FLASH_BAR
>>>> +#define CONFIG_QSPI_AHB_INIT
>>>
>>> Is it really a config option? Is it really board-dependent? Does it compile on the
>>> latest master branch?
>>>
>>
>> Yes, I think it's a config option.
>> 1, whether it supports 16M+ or not will not break the existing functionality.
>
> What do you mean exactly? With this patch, old flash chips continue to
> work even you set it to 4-byte addressing? If true, why don't you enable
> this feature for all and get rid of the macro?
>
>> 2, Some flash is just only 16M size. (the SPI flash on LS1043A)
>
> This is not an option. It is a requirement.
>
>>
>> Yes, It's board-dependent.
>> It's depend the flash size on the board whether it's more than 16M.
>> For example:
>> 1, LS1046ARDB, LS1012ARDB, LS1012AQDS need
>> 2, LS1043 don't need.
>
> Do LS1043A boards still work with this feature on?
>
>>
>> I have tested it on LS1046ARDB and LS1012ARDB.
>
> The bottom of line is, if all existing and future boards work fine with
> this feature, you don't need the condition macro. If some board/flash
> cannot work without this feature, enable it in Kconfig.
>
> York
>
> 

Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail
diff mbox

Patch

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
index d68eeba..18d753e 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
@@ -370,6 +370,40 @@  void fsl_lsch2_early_init_f(void)
 }
 #endif
 
+#ifdef CONFIG_QSPI_AHB_INIT
+/* Enable 4bytes address support and fast read */
+int qspi_ahb_init(void)
+{
+	u32 *qspi_lut, lut_key, *qspi_key;
+
+	qspi_key = (void *)CONFIG_SYS_QSPI_ADDR + 0x300;
+	qspi_lut = (void *)CONFIG_SYS_QSPI_ADDR + 0x310;
+
+	lut_key = in_be32(qspi_key);
+
+	if (lut_key == 0x5af05af0) {
+		/* That means the register is BE */
+		out_be32(qspi_key, 0x5af05af0);
+		out_be32(qspi_key + 1, 0x00000002);
+		out_be32(qspi_lut, 0x0820040c);
+		out_be32(qspi_lut + 1, 0x1c080c08);
+		out_be32(qspi_lut + 2, 0x00002400);
+		out_be32(qspi_key, 0x5af05af0);
+		out_be32(qspi_key + 1, 0x00000001);
+	} else {
+		/* That means the register is LE */
+		out_le32(qspi_key, 0x5af05af0);
+		out_le32(qspi_key + 1, 0x00000002);
+		out_le32(qspi_lut, 0x0820040c);
+		out_le32(qspi_lut + 1, 0x1c080c08);
+		out_le32(qspi_lut + 2, 0x00002400);
+		out_le32(qspi_key, 0x5af05af0);
+		out_le32(qspi_key + 1, 0x00000001);
+	}
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_BOARD_LATE_INIT
 int board_late_init(void)
 {
@@ -379,6 +413,9 @@  int board_late_init(void)
 #ifdef CONFIG_CHAIN_OF_TRUST
 	fsl_setenv_chain_of_trust();
 #endif
+#ifdef CONFIG_QSPI_AHB_INIT
+	qspi_ahb_init();
+#endif
 
 	return 0;
 }
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
index d88543d..a28b1fd 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
@@ -18,6 +18,7 @@ 
 #define CONFIG_SYS_CCI400_ADDR			(CONFIG_SYS_IMMR + 0x00180000)
 #define CONFIG_SYS_GIC400_ADDR			(CONFIG_SYS_IMMR + 0x00400000)
 #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR + 0x00530000)
+#define CONFIG_SYS_QSPI_ADDR			(CONFIG_SYS_IMMR + 0x00550000)
 #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR + 0x00560000)
 #define CONFIG_SYS_FSL_CSU_ADDR			(CONFIG_SYS_IMMR + 0x00510000)
 #define CONFIG_SYS_FSL_GUTS_ADDR		(CONFIG_SYS_IMMR + 0x00ee0000)
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
index 7acba27..8aefc76 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
@@ -19,6 +19,7 @@ 
 #define CONFIG_SYS_FSL_CH3_CLK_GRPA_ADDR	(CONFIG_SYS_IMMR + 0x00300000)
 #define CONFIG_SYS_FSL_CH3_CLK_GRPB_ADDR	(CONFIG_SYS_IMMR + 0x00310000)
 #define CONFIG_SYS_FSL_CH3_CLK_CTRL_ADDR	(CONFIG_SYS_IMMR + 0x00370000)
+#define CONFIG_SYS_QSPI_ADDR			(CONFIG_SYS_IMMR + 0x010c0000)
 #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR + 0x01140000)
 #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR + 0x01240000)
 #define CONFIG_SYS_NS16550_COM1			(CONFIG_SYS_IMMR + 0x011C0500)
diff --git a/include/configs/ls1012a_common.h b/include/configs/ls1012a_common.h
index 80603c9..c1e1102 100644
--- a/include/configs/ls1012a_common.h
+++ b/include/configs/ls1012a_common.h
@@ -61,6 +61,7 @@ 
 
 #define FSL_QSPI_FLASH_SIZE		(1 << 24)
 #define FSL_QSPI_FLASH_NUM		2
+#define CONFIG_QSPI_AHB_INIT
 
 /*
  * Environment
diff --git a/include/configs/ls1046ardb.h b/include/configs/ls1046ardb.h
index 2fe8fc1..662ecb1 100644
--- a/include/configs/ls1046ardb.h
+++ b/include/configs/ls1046ardb.h
@@ -209,6 +209,7 @@ 
 #define FSL_QSPI_FLASH_SIZE		(1 << 26)
 #define FSL_QSPI_FLASH_NUM		2
 #define CONFIG_SPI_FLASH_BAR
+#define CONFIG_QSPI_AHB_INIT
 #endif
 
 /* SATA */