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....@nxp.com>; u-boot@lists.denx.de; >> albert.u.b...@aribaud.net; w...@denx.de; Prabhakar Kushwaha >> <prabhakar.kushw...@nxp.com>; alison.w...@freescale.com; >> mingkai...@freescale.com >> Cc: yao.y...@freescale.com; qianyu.g...@freescale.com; >> bmeng...@gmail.com; Shengzhou Liu <shengzhou....@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....@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
