Hi Zhiqiang,
Sorry for late queries.
As per description of patch " Up to now, the function is_serdes_configed() doesn't check if the map of serdes protocol is initialized before accessing it. The function is_serdes_configed() will get wrong result when it was called before the serdes protocol maps initialized. As the first eliment of the map isn't used for any device, so use it as the flag to indicate if the map has been initialized."
I am just wondering the use-case/situation where this can happen. Can you please help me with understanding.
fsl_serdes_init is called from arch_early_init_r in board_r.c. As per my understanding all the driver calling is_serdes_configed (SATA, PCIe, SGMII) etc requires DDR. So are we talking about moving any driver in board_f.c.
Regards, Prabhakar
-----Original Message----- From: york sun Sent: Thursday, July 21, 2016 2:45 AM To: Zhiqiang Hou zhiqiang.hou@nxp.com; u-boot@lists.denx.de; albert.u.boot@aribaud.net; wd@denx.de; Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; alison.wang@freescale.com; Mingkai.Hu@freescale.com Cc: yao.yuan@freescale.com; Qianyu.Gong@freescale.com; bmeng.cn@gmail.com; Shengzhou Liu shengzhou.liu@nxp.com Subject: Re: [PATCH 1/5] fsl: serdes: ensure accessing the initialized maps of serdes protocol
On 07/20/2016 02:38 AM, Zhiqiang Hou wrote:
Hi York,
Thanks for your comments!
-----Original Message----- From: york sun Sent: 2016年7月19日 23:46 To: Zhiqiang Hou zhiqiang.hou@nxp.com; u-boot@lists.denx.de; albert.u.boot@aribaud.net; wd@denx.de; Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; alison.wang@freescale.com; Mingkai.Hu@freescale.com Cc: yao.yuan@freescale.com; Qianyu.Gong@freescale.com; bmeng.cn@gmail.com; Shengzhou Liu shengzhou.liu@nxp.com Subject: Re: [PATCH 1/5] fsl: serdes: ensure accessing the initialized maps of
serdes
protocol
On 07/03/2016 11:39 PM, Zhiqiang Hou wrote:
From: Hou Zhiqiang Zhiqiang.Hou@nxp.com
Up to now, the function is_serdes_configed() doesn't check if the map of serdes protocol is initialized before accessing it. The function is_serdes_configed() will get wrong result when it was called before the serdes protocol maps initialized. As the first eliment of the map
s/eliment/element
Will fix my spelling mistakes next version, thanks a lot!
isn't used for any device, so use it as the flag to indicate if the map has been initialized.
I am not sure it is safe to presume the first element is always not used. Take LS2080A as an example, the serdes map array is initialized by serdes_init(),
which
calls serdes_get_prtcl() to get the index of the array. With normal condition,
the
index shouldn't be zero. Zero is used as an error but it is not checked before
setting
serdes_prtcl_map[lane_prtcl] = 1;
The first element of enum srds_prtcl 'NONE' is aim to describe a lane isn't
assigned to
any device, and the array serdesn_prtcl_map is used to check if the specified
device
selected by the current serdes protocol, right? Nobody will check if the device
'NONE'
has been configured, right? So it can be used to indicate if the
serdes_prtcl_map has
been initialized. Don't care whether the function serdes_get_prtcl() will return zero, just focus
if the
serdes protocol map has been filled. For example, if the serdes protocol value
read from
RCW doesn't match with any entry of the serdes_cfg_tbl, then all lane will be
assigned to
'NONE'. It still means the serdes protocol map has been filled, even if there is
only the
serdesn_prtcl_map[NONE] was set to 1.
If you presumption is correct, and you want to use the first one as a flag, you probably need to check all of them to make sure errors are handled correctly, instead of setting the flag unexpected. Also it is important to document this
idea
so future platform code follows the same.
Is it necessary to add it to a document, if add a comment to the
serdesn_prtcl_map make it?
If you don't want to add a document, please at least put some comments, in case we need to change some code in the future.
York