[U-Boot] [PATCH] fsl-lsch2: csu: correct the workaround A-010315

[Z.q. Hou]

Hi York,

> -----Original Message-----
> From: York Sun
> Sent: 2017年8月9日 0:17
> To: Z.q. Hou <zhiqiang.hou at nxp.com>; u-boot at lists.denx.de
> Subject: Re: [PATCH] fsl-lsch2: csu: correct the workaround A-010315
> 
> On 08/07/2017 11:31 PM, Z.q. Hou wrote:
> > Hi York,
> >
> > Thanks a lot for your comments!
> >
> >> -----Original Message-----
> >> From: York Sun
> >> Sent: 2017年8月8日 5:18
> >> To: Z.q. Hou <zhiqiang.hou at nxp.com>; u-boot at lists.denx.de
> >> Subject: Re: [PATCH] fsl-lsch2: csu: correct the workaround A-010315
> >>
> >> On 07/03/2017 03:07 AM, Zhiqiang Hou wrote:
> >>> From: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> >>>
> >>> The implementation of workaround A-010315 is wrong, it updated other
> >>> IP's CSU registers.
> >>>
> >>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> >>> ---
> >>>    board/freescale/common/ns_access.c | 20 ++++++++++----------
> >>>    include/fsl_csu.h                  |  2 +-
> >>>    2 files changed, 11 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/board/freescale/common/ns_access.c
> >>> b/board/freescale/common/ns_access.c
> >>> index 1c2287d..0c3a54c 100644
> >>> --- a/board/freescale/common/ns_access.c
> >>> +++ b/board/freescale/common/ns_access.c
> >>> @@ -10,15 +10,15 @@
> >>>    #include <asm/arch/ns_access.h>
> >>>    #include <asm/arch/fsl_serdes.h>
> >>>
> >>> -void set_devices_ns_access(struct csu_ns_dev *ns_dev, u16 val)
> >>> +void set_devices_ns_access(unsigned long index, u16 val)
> >>>    {
> >>>    	u32 *base = (u32 *)CONFIG_SYS_FSL_CSU_ADDR;
> >>>    	u32 *reg;
> >>>    	uint32_t tmp;
> >>>
> >>> -	reg = base + ns_dev->ind / 2;
> >>> +	reg = base + index / 2;
> >>>    	tmp = in_be32(reg);
> >>> -	if (ns_dev->ind % 2 == 0) {
> >>> +	if (index % 2 == 0) {
> >>>    		tmp &= 0x0000ffff;
> >>>    		tmp |= val << 16;
> >>>    	} else {
> >>> @@ -34,7 +34,7 @@ static void enable_devices_ns_access(struct
> >> csu_ns_dev *ns_dev, uint32_t num)
> >>>    	int i;
> >>>
> >>>    	for (i = 0; i < num; i++)
> >>> -		set_devices_ns_access(ns_dev + i, ns_dev[i].val);
> >>> +		set_devices_ns_access(ns_dev[i].ind, ns_dev[i].val);
> >>>    }
> >>>
> >>>    void enable_layerscape_ns_access(void) @@ -50,20 +50,20 @@ void
> >>> set_pcie_ns_access(int pcie, u16 val)
> >>>    	switch (pcie) {
> >>>    #ifdef CONFIG_PCIE1
> >>>    	case PCIE1:
> >>> -		set_devices_ns_access(&ns_dev[CSU_CSLX_PCIE1], val);
> >>> -		set_devices_ns_access(&ns_dev[CSU_CSLX_PCIE1_IO], val);
> >>> +		set_devices_ns_access(CSU_CSLX_PCIE1, val);
> >>> +		set_devices_ns_access(CSU_CSLX_PCIE1_IO, val);
> >>>    		return;
> >>>    #endif
> >>>    #ifdef CONFIG_PCIE2
> >>>    	case PCIE2:
> >>> -		set_devices_ns_access(&ns_dev[CSU_CSLX_PCIE2], val);
> >>> -		set_devices_ns_access(&ns_dev[CSU_CSLX_PCIE2_IO], val);
> >>> +		set_devices_ns_access(CSU_CSLX_PCIE2, val);
> >>> +		set_devices_ns_access(CSU_CSLX_PCIE2_IO, val);
> >>>    		return;
> >>>    #endif
> >>>    #ifdef CONFIG_PCIE3
> >>>    	case PCIE3:
> >>> -		set_devices_ns_access(&ns_dev[CSU_CSLX_PCIE3], val);
> >>> -		set_devices_ns_access(&ns_dev[CSU_CSLX_PCIE3_IO], val);
> >>> +		set_devices_ns_access(CSU_CSLX_PCIE3, val);
> >>> +		set_devices_ns_access(CSU_CSLX_PCIE3_IO, val);
> >>>    		return;
> >>>    #endif
> >>>    	default:
> >>> diff --git a/include/fsl_csu.h b/include/fsl_csu.h index
> >>> 8582ac0..027a811 100644
> >>> --- a/include/fsl_csu.h
> >>> +++ b/include/fsl_csu.h
> >>> @@ -30,7 +30,7 @@ struct csu_ns_dev {
> >>>    };
> >>>
> >>>    void enable_layerscape_ns_access(void);
> >>> -void set_devices_ns_access(struct csu_ns_dev *ns_dev, u16 val);
> >>> +void set_devices_ns_access(unsigned long, u16 val);
> >>>    void set_pcie_ns_access(int pcie, u16 val);
> >>>
> >>>    #endif
> >>>
> >>
> >> Zhiqiang,
> >>
> >> Your subject and commit message both say fixing the workaround for
> >> A010315 but the change is for non-secure access. Did you mismatch them?
> >
> > No, this patch is to fix a error of workaround A-010315.
> > The function set_pcie_ns_access() is wrongly implemented during add
> wordaround A-010315.
> > The structure array ns_dev has a member 'ind' which is initialized by
> CSU_CSLX_*, the mistake is I use its member 'ind' as the index of array ns_dev,
> actually it should use the 'ind' directly to address the PCIe's CSL register
> (CSL_base + CSU_CSLX_PCIE*).
> > With the current argument list of set_pcie_ns_access(struct csu_ns_dev
> *ns_dev, u16 val), it hard to find and set the specified IP block's non-secure
> access, so this patch also changed the first argument to the 'ind' of structure
> csu_ns_dev.
> >
> 
> Zhiqiang,
> 
> Thanks for the explanation. I will try to rephase the commit message with the
> information you provided.

Ok, thanks a lot!

- Zhiqiang