All of lore.kernel.org
 help / color / mirror / Atom feed
From: York Sun <york.sun@nxp.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] fsl-lsch2: csu: correct the workaround A-010315
Date: Tue, 8 Aug 2017 16:17:01 +0000	[thread overview]
Message-ID: <VI1PR04MB2078981C944AA56140F1B29C9A8A0@VI1PR04MB2078.eurprd04.prod.outlook.com> (raw)
In-Reply-To: AM5PR0402MB277145FE0F2D3EFEB1F48B00848A0@AM5PR0402MB2771.eurprd04.prod.outlook.com

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@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@nxp.com>
>>>
>>> The implementation of workaround A-010315 is wrong, it updated other
>>> IP's CSU registers.
>>>
>>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@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

  reply	other threads:[~2017-08-08 16:17 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-03  9:51 [U-Boot] [PATCH] fsl-lsch2: csu: correct the workaround A-010315 Zhiqiang Hou
2017-08-07 21:18 ` York Sun
2017-08-08  6:31   ` Z.q. Hou
2017-08-08 16:17     ` York Sun [this message]
2017-08-09  2:47       ` Z.q. Hou
2017-08-10 15:48 ` York Sun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR04MB2078981C944AA56140F1B29C9A8A0@VI1PR04MB2078.eurprd04.prod.outlook.com \
    --to=york.sun@nxp.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.