[U-Boot] ddr: fsl: Impl. Erratum A008109
diff mbox series

Message ID 20191120160734.11300-1-joakim.tjernlund@infinera.com
State New
Delegated to: Priyanka Jain
Headers show
Series
  • [U-Boot] ddr: fsl: Impl. Erratum A008109
Related show

Commit Message

Joakim Tjernlund Nov. 20, 2019, 4:07 p.m. UTC
Impl. erratum as descibed in errata doc.
Enable A008109 for T1040 and T1024

Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
---
 arch/powerpc/cpu/mpc85xx/Kconfig | 2 ++
 drivers/ddr/fsl/Kconfig          | 3 +++
 drivers/ddr/fsl/ctrl_regs.c      | 6 ++++++
 3 files changed, 11 insertions(+)

Comments

Joakim Tjernlund Nov. 25, 2019, 8:36 a.m. UTC | #1
On Wed, 2019-11-20 at 17:07 +0100, Joakim Tjernlund wrote:
> Impl. erratum as descibed in errata doc.
> Enable A008109 for T1040 and T1024
> 
> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> ---
>  arch/powerpc/cpu/mpc85xx/Kconfig | 2 ++
>  drivers/ddr/fsl/Kconfig          | 3 +++
>  drivers/ddr/fsl/ctrl_regs.c      | 6 ++++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
> index 8cc82f80b4..6caf5c8389 100644
> --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> @@ -1038,6 +1038,7 @@ config ARCH_T1040
>  	select SYS_FSL_DDR_VER_50
>  	select SYS_FSL_ERRATUM_A008044
>  	select SYS_FSL_ERRATUM_A008378
> +	select SYS_FSL_ERRATUM_A008109
>  	select SYS_FSL_ERRATUM_A009663
>  	select SYS_FSL_ERRATUM_A009942
>  	select SYS_FSL_ERRATUM_ESDHC111
> @@ -1061,6 +1062,7 @@ config ARCH_T1042
>  	select SYS_FSL_DDR_VER_50
>  	select SYS_FSL_ERRATUM_A008044
>  	select SYS_FSL_ERRATUM_A008378
> +	select SYS_FSL_ERRATUM_A008109
>  	select SYS_FSL_ERRATUM_A009663
>  	select SYS_FSL_ERRATUM_A009942
>  	select SYS_FSL_ERRATUM_ESDHC111
> diff --git a/drivers/ddr/fsl/Kconfig b/drivers/ddr/fsl/Kconfig
> index 1b73df82de..f75d97b15c 100644
> --- a/drivers/ddr/fsl/Kconfig
> +++ b/drivers/ddr/fsl/Kconfig
> @@ -151,6 +151,9 @@ endmenu
>  config SYS_FSL_ERRATUM_A008378
>  	bool
>  
> +config SYS_FSL_ERRATUM_A008109
> +	bool
> +
>  config SYS_FSL_ERRATUM_A008511
>  	bool
>  
> diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
> index 98ccbb70de..d467160d0c 100644
> --- a/drivers/ddr/fsl/ctrl_regs.c
> +++ b/drivers/ddr/fsl/ctrl_regs.c
> @@ -2626,6 +2626,12 @@ compute_fsl_memctl_config_regs(const unsigned int ctrl_num,
>  	}
>  #endif
>  
> +#ifdef CONFIG_SYS_FSL_ERRATUM_A008109
> +	ddr->ddr_sdram_cfg_2 = ddr_in32(&ddr->ddr_sdram_cfg_2) | 0x800; /* DDR_SLOW */
> +	ddr->debug[18] = ddr_in32(&ddrc->debug[18]) | 0x2;
> +	ddr->debug[28] = 0x30000000;
> +#endif
> +

I just noticed something odd(buggy?). Each errata reads from DDR controller bug saves the new
value in a RAM variable(ddr->debug[X]) but does not write values back to HW.

As several erratas update the same register, this looses the previous data.
Also, are theses errtas/debug stateless? Now the code tries gather all writes into a RAM variable and
write the total change in one go later on, this will only work if all errata fixes are stateless.

>  #ifdef CONFIG_SYS_FSL_ERRATUM_A009942
>  	ddr_freq = get_ddr_freq(ctrl_num) / 1000000;
>  	ddr->debug[28] |= ddr_in32(&ddrc->debug[28]);
Priyanka Jain Dec. 17, 2019, 8:55 a.m. UTC | #2
>-----Original Message-----
>From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
>Sent: Monday, November 25, 2019 2:06 PM
>To: Priyanka Jain <priyanka.jain@nxp.com>; u-boot@lists.denx.de
>Cc: York Sun <york.sun@nxp.com>
>Subject: Re: [PATCH] ddr: fsl: Impl. Erratum A008109
>
>On Wed, 2019-11-20 at 17:07 +0100, Joakim Tjernlund wrote:
>> Impl. erratum as descibed in errata doc.
>> Enable A008109 for T1040 and T1024
>>
>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
>> ---
>>  arch/powerpc/cpu/mpc85xx/Kconfig | 2 ++
>>  drivers/ddr/fsl/Kconfig          | 3 +++
>>  drivers/ddr/fsl/ctrl_regs.c      | 6 ++++++
>>  3 files changed, 11 insertions(+)
>>
>> diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig
>> b/arch/powerpc/cpu/mpc85xx/Kconfig
>> index 8cc82f80b4..6caf5c8389 100644
>> --- a/arch/powerpc/cpu/mpc85xx/Kconfig
>> +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
>> @@ -1038,6 +1038,7 @@ config ARCH_T1040
>>  	select SYS_FSL_DDR_VER_50
>>  	select SYS_FSL_ERRATUM_A008044
>>  	select SYS_FSL_ERRATUM_A008378
>> +	select SYS_FSL_ERRATUM_A008109
>>  	select SYS_FSL_ERRATUM_A009663
>>  	select SYS_FSL_ERRATUM_A009942
>>  	select SYS_FSL_ERRATUM_ESDHC111
>> @@ -1061,6 +1062,7 @@ config ARCH_T1042
>>  	select SYS_FSL_DDR_VER_50
>>  	select SYS_FSL_ERRATUM_A008044
>>  	select SYS_FSL_ERRATUM_A008378
>> +	select SYS_FSL_ERRATUM_A008109
>>  	select SYS_FSL_ERRATUM_A009663
>>  	select SYS_FSL_ERRATUM_A009942
>>  	select SYS_FSL_ERRATUM_ESDHC111
>> diff --git a/drivers/ddr/fsl/Kconfig b/drivers/ddr/fsl/Kconfig index
>> 1b73df82de..f75d97b15c 100644
>> --- a/drivers/ddr/fsl/Kconfig
>> +++ b/drivers/ddr/fsl/Kconfig
>> @@ -151,6 +151,9 @@ endmenu
>>  config SYS_FSL_ERRATUM_A008378
>>  	bool
>>
>> +config SYS_FSL_ERRATUM_A008109
>> +	bool
>> +
>>  config SYS_FSL_ERRATUM_A008511
>>  	bool
>>
>> diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
>> index 98ccbb70de..d467160d0c 100644
>> --- a/drivers/ddr/fsl/ctrl_regs.c
>> +++ b/drivers/ddr/fsl/ctrl_regs.c
>> @@ -2626,6 +2626,12 @@ compute_fsl_memctl_config_regs(const unsigned
>int ctrl_num,
>>  	}
>>  #endif
>>
>> +#ifdef CONFIG_SYS_FSL_ERRATUM_A008109
>> +	ddr->ddr_sdram_cfg_2 = ddr_in32(&ddr->ddr_sdram_cfg_2) | 0x800;
>/* DDR_SLOW */
>> +	ddr->debug[18] = ddr_in32(&ddrc->debug[18]) | 0x2;
>> +	ddr->debug[28] = 0x30000000;
>> +#endif
>> +
>
>I just noticed something odd(buggy?). Each errata reads from DDR controller
>bug saves the new value in a RAM variable(ddr->debug[X]) but does not write
>values back to HW.
>
>As several erratas update the same register, this looses the previous data.
>Also, are theses errtas/debug stateless? Now the code tries gather all writes
>into a RAM variable and write the total change in one go later on, this will
>only work if all errata fixes are stateless.
>
It seems Errata fixes are not stateless.
We need to program HW register separately for each errata workaround.
In SoC chip errata document: section A-008109, 
order of errata workaround implementation is mentioned
as first implement A-008378, then A-008109 and then A-009942.
Can you please check if things works fine at your end with this order.
Meanwhile I will also check within NXP.
>>  #ifdef CONFIG_SYS_FSL_ERRATUM_A009942
>>  	ddr_freq = get_ddr_freq(ctrl_num) / 1000000;
>>  	ddr->debug[28] |= ddr_in32(&ddrc->debug[28]);
Priyanka
Joakim Tjernlund Dec. 17, 2019, 9:35 a.m. UTC | #3
On Tue, 2019-12-17 at 08:55 +0000, Priyanka Jain wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> > -----Original Message-----
> > From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> > Sent: Monday, November 25, 2019 2:06 PM
> > To: Priyanka Jain <priyanka.jain@nxp.com>; u-boot@lists.denx.de
> > Cc: York Sun <york.sun@nxp.com>
> > Subject: Re: [PATCH] ddr: fsl: Impl. Erratum A008109
> > 
> > On Wed, 2019-11-20 at 17:07 +0100, Joakim Tjernlund wrote:
> > > Impl. erratum as descibed in errata doc.
> > > Enable A008109 for T1040 and T1024
> > > 
> > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > ---
> > >  arch/powerpc/cpu/mpc85xx/Kconfig | 2 ++
> > >  drivers/ddr/fsl/Kconfig          | 3 +++
> > >  drivers/ddr/fsl/ctrl_regs.c      | 6 ++++++
> > >  3 files changed, 11 insertions(+)
> > > 
> > > diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig
> > > b/arch/powerpc/cpu/mpc85xx/Kconfig
> > > index 8cc82f80b4..6caf5c8389 100644
> > > --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> > > +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> > > @@ -1038,6 +1038,7 @@ config ARCH_T1040
> > >      select SYS_FSL_DDR_VER_50
> > >      select SYS_FSL_ERRATUM_A008044
> > >      select SYS_FSL_ERRATUM_A008378
> > > +    select SYS_FSL_ERRATUM_A008109
> > >      select SYS_FSL_ERRATUM_A009663
> > >      select SYS_FSL_ERRATUM_A009942
> > >      select SYS_FSL_ERRATUM_ESDHC111
> > > @@ -1061,6 +1062,7 @@ config ARCH_T1042
> > >      select SYS_FSL_DDR_VER_50
> > >      select SYS_FSL_ERRATUM_A008044
> > >      select SYS_FSL_ERRATUM_A008378
> > > +    select SYS_FSL_ERRATUM_A008109
> > >      select SYS_FSL_ERRATUM_A009663
> > >      select SYS_FSL_ERRATUM_A009942
> > >      select SYS_FSL_ERRATUM_ESDHC111
> > > diff --git a/drivers/ddr/fsl/Kconfig b/drivers/ddr/fsl/Kconfig index
> > > 1b73df82de..f75d97b15c 100644
> > > --- a/drivers/ddr/fsl/Kconfig
> > > +++ b/drivers/ddr/fsl/Kconfig
> > > @@ -151,6 +151,9 @@ endmenu
> > >  config SYS_FSL_ERRATUM_A008378
> > >      bool
> > > 
> > > +config SYS_FSL_ERRATUM_A008109
> > > +    bool
> > > +
> > >  config SYS_FSL_ERRATUM_A008511
> > >      bool
> > > 
> > > diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
> > > index 98ccbb70de..d467160d0c 100644
> > > --- a/drivers/ddr/fsl/ctrl_regs.c
> > > +++ b/drivers/ddr/fsl/ctrl_regs.c
> > > @@ -2626,6 +2626,12 @@ compute_fsl_memctl_config_regs(const unsigned
> > int ctrl_num,
> > >      }
> > >  #endif
> > > 
> > > +#ifdef CONFIG_SYS_FSL_ERRATUM_A008109
> > > +    ddr->ddr_sdram_cfg_2 = ddr_in32(&ddr->ddr_sdram_cfg_2) | 0x800;
> > /* DDR_SLOW */
> > > +    ddr->debug[18] = ddr_in32(&ddrc->debug[18]) | 0x2;
> > > +    ddr->debug[28] = 0x30000000;
> > > +#endif
> > > +
> > 
> > I just noticed something odd(buggy?). Each errata reads from DDR controller
> > bug saves the new value in a RAM variable(ddr->debug[X]) but does not write
> > values back to HW.
> > 
> > As several erratas update the same register, this looses the previous data.
> > Also, are theses errtas/debug stateless? Now the code tries gather all writes
> > into a RAM variable and write the total change in one go later on, this will
> > only work if all errata fixes are stateless.
> > 
> It seems Errata fixes are not stateless.
> We need to program HW register separately for each errata workaround.
> In SoC chip errata document: section A-008109,
> order of errata workaround implementation is mentioned
> as first implement A-008378, then A-008109 and then A-009942.
> Can you please check if things works fine at your end with this order.
> Meanwhile I will also check within NXP.
> > >  #ifdef CONFIG_SYS_FSL_ERRATUM_A009942
> > >      ddr_freq = get_ddr_freq(ctrl_num) / 1000000;
> > >      ddr->debug[28] |= ddr_in32(&ddrc->debug[28]);
> Priyanka

Hi Priyanka

I have given up on this errata, any impl. attempt didn't make a difference and I could
not get any details from NXP w.r.t this errata.

The thing that did fix the problem was the patch: "mpc85xx: ddr: Always start DDR RAM in Self Refresh mode"
NXP did confirm that starting in SR should have no ill effects but could not confirm if it fixed any HW bug
in the controller. Starting in SR mode provides a softer transition from configuration to full operation though.

I cannot help thinking that errata A008109 and my SR fix is related but I think you will have to
sort that out.

   Jocke

Patch
diff mbox series

diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
index 8cc82f80b4..6caf5c8389 100644
--- a/arch/powerpc/cpu/mpc85xx/Kconfig
+++ b/arch/powerpc/cpu/mpc85xx/Kconfig
@@ -1038,6 +1038,7 @@  config ARCH_T1040
 	select SYS_FSL_DDR_VER_50
 	select SYS_FSL_ERRATUM_A008044
 	select SYS_FSL_ERRATUM_A008378
+	select SYS_FSL_ERRATUM_A008109
 	select SYS_FSL_ERRATUM_A009663
 	select SYS_FSL_ERRATUM_A009942
 	select SYS_FSL_ERRATUM_ESDHC111
@@ -1061,6 +1062,7 @@  config ARCH_T1042
 	select SYS_FSL_DDR_VER_50
 	select SYS_FSL_ERRATUM_A008044
 	select SYS_FSL_ERRATUM_A008378
+	select SYS_FSL_ERRATUM_A008109
 	select SYS_FSL_ERRATUM_A009663
 	select SYS_FSL_ERRATUM_A009942
 	select SYS_FSL_ERRATUM_ESDHC111
diff --git a/drivers/ddr/fsl/Kconfig b/drivers/ddr/fsl/Kconfig
index 1b73df82de..f75d97b15c 100644
--- a/drivers/ddr/fsl/Kconfig
+++ b/drivers/ddr/fsl/Kconfig
@@ -151,6 +151,9 @@  endmenu
 config SYS_FSL_ERRATUM_A008378
 	bool
 
+config SYS_FSL_ERRATUM_A008109
+	bool
+
 config SYS_FSL_ERRATUM_A008511
 	bool
 
diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
index 98ccbb70de..d467160d0c 100644
--- a/drivers/ddr/fsl/ctrl_regs.c
+++ b/drivers/ddr/fsl/ctrl_regs.c
@@ -2626,6 +2626,12 @@  compute_fsl_memctl_config_regs(const unsigned int ctrl_num,
 	}
 #endif
 
+#ifdef CONFIG_SYS_FSL_ERRATUM_A008109
+	ddr->ddr_sdram_cfg_2 = ddr_in32(&ddr->ddr_sdram_cfg_2) | 0x800; /* DDR_SLOW */
+	ddr->debug[18] = ddr_in32(&ddrc->debug[18]) | 0x2;
+	ddr->debug[28] = 0x30000000;
+#endif
+
 #ifdef CONFIG_SYS_FSL_ERRATUM_A009942
 	ddr_freq = get_ddr_freq(ctrl_num) / 1000000;
 	ddr->debug[28] |= ddr_in32(&ddrc->debug[28]);