Hi York, > -----Original Message----- > From: York Sun > Sent: 2017年8月9日 0:17 > To: Z.q. Hou <zhiqiang....@nxp.com>; u-boot@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....@nxp.com>; u-boot@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....@nxp.com> > >>> > >>> The implementation of workaround A-010315 is wrong, it updated other > >>> IP's CSU registers. > >>> > >>> Signed-off-by: Hou Zhiqiang <zhiqiang....@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot