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.

York

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to