> -----Original Message----- > From: York Sun > Sent: Tuesday, November 14, 2017 10:27 PM > To: Rajesh Bhagat <rajesh.bha...@nxp.com>; u-boot@lists.denx.de > Cc: Prabhakar Kushwaha <prabhakar.kushw...@nxp.com>; Priyanka Jain > <priyanka.j...@nxp.com>; Ashish Kumar <ashish.ku...@nxp.com> > Subject: Re: [PATCH v6 1/7] armv8: lsch3: Add serdes and DDR voltage setup > > On 11/13/2017 11:06 PM, Rajesh Bhagat wrote: > > Adds SERDES voltage and reset SERDES lanes API and makes > > enable/disable DDR controller support 0.9V API common. > > > > Signed-off-by: Ashish Kumar <ashish.ku...@nxp.com> > > Signed-off-by: Rajesh Bhagat <rajesh.bha...@nxp.com> > > --- > > Changes in v6: > > - Corrected indentation/alignment issues at various places > > - Changed NULL ENTRY in srds_prctl_info array to id as zero > > - Corrected the PLL Reset logic, moved code inside for loop > > - Used error code(-EINVAL) in setup_serdes_volt API > > > > Changes in v5: > > - Moved local macros to static functions > > - Used array to handle PRCTL mask and shift operations > > > > Changes in v4: > > - Added local macros instead of magical numbers > > - Created macros to remove duplicate code > > > > Changes in v3: > > Restructured LS1088A VID support to use common VID driver Cosmetic > > review comments fixed Added __iomem for accessing registers > > > > Changes in v2: > > Checkpatch errors fixed > > > > .../cpu/armv8/fsl-layerscape/fsl_lsch3_serdes.c | 312 > +++++++++++++++++++++ > > arch/arm/cpu/armv8/fsl-layerscape/soc.c | 34 +-- > > .../include/asm/arch-fsl-layerscape/fsl_serdes.h | 2 +- > > .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 34 +++ > > arch/arm/include/asm/arch-fsl-layerscape/soc.h | 1 + > > 5 files changed, 365 insertions(+), 18 deletions(-) > > > > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch3_serdes.c > > b/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch3_serdes.c > > index 179cac6..3e4b0bc 100644 > > --- a/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch3_serdes.c > > +++ b/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch3_serdes.c > > @@ -158,6 +158,318 @@ void serdes_init(u32 sd, u32 sd_addr, u32 rcwsr, u32 > sd_prctl_mask, > > serdes_prtcl_map[NONE] = 1; > > } > > > > +__weak int get_serdes_volt(void) > > +{ > > + return -1; > > +} > > + > > +__weak int set_serdes_volt(int svdd) > > +{ > > + return -1; > > +} > > + > > +#define LNAGCR0_RESET_MASK 0xFF9FFFFF > > +#define LNAGCR0_RT_RSTB 0x00600000 > > +#define RSTCTL_RESET_MASK_1 0xFFFFFFBF > > +#define RSTCTL_RESET_MASK_2 0xFFFFFF1F > > +#define RSTCTL_RESET_MASK_3 0xFFFFFFEF > > +#define RSTCTL_RSTREQ 0x10000000 > > +#define RSTCTL_RSTERR 0x20000000 > > +#define RSTCTL_SDEN 0x00000020 > > +#define RSTCTL_SDRST_B 0x00000040 > > +#define RSTCTL_PLLRST_B 0x00000080 > > +#define RSTCTL_RST_DONE 0x40000000 > > +#define TCALCR_RESET_MASK 0xF7FFFFFF > > +#define TCALCR_CALRST_B 0x08000000 > > + > > +struct serdes_prctl_info { > > + u32 id; > > + u32 mask; > > + u32 shift; > > +}; > > + > > +struct serdes_prctl_info srds_prctl_info[] = { #ifdef > > +CONFIG_SYS_FSL_SRDS_1 > > + {.id = 1, > > + .mask = FSL_CHASSIS3_SRDS1_PRTCL_MASK, > > + .shift = FSL_CHASSIS3_SRDS1_PRTCL_SHIFT > > + }, > > + > > +#endif > > +#ifdef CONFIG_SYS_FSL_SRDS_2 > > + {.id = 2, > > + .mask = FSL_CHASSIS3_SRDS2_PRTCL_MASK, > > + .shift = FSL_CHASSIS3_SRDS2_PRTCL_SHIFT > > + }, > > +#endif > > + {.id = 0, > > + .mask = 0, > > + .shift = 0 > > + } /* NULL ENTRY */ > > +}; > > A simple {} will do the same.
Will take care in v7. > > > + > > +static int get_serdes_prctl_info_idx(u32 serdes_id) { > > + int pos = 0; > > + struct serdes_prctl_info *srds_info; > > + > > + /* loop until NULL ENTRY defined by .id=0 */ > > + for (srds_info = srds_prctl_info; srds_info->id != 0; > > + srds_info++, pos++) { > > + if (srds_info->id == serdes_id) > > + return pos; > > + } > > + > > + return -1; > > +} > > + > > +static void do_enabled_lanes_reset(u32 serdes_id, u32 cfg, > > + struct ccsr_serdes __iomem *serdes_base, > > + bool cmplt) > > +{ > > + int i, pos; > > + u32 cfg_tmp, reg = 0; > > + > > + pos = get_serdes_prctl_info_idx(serdes_id); > > + if (pos == -1) { > > + printf("invalid serdes_id %d\n", serdes_id); > > + return; > > + } > > + > > + cfg_tmp = cfg & srds_prctl_info[pos].mask; > > + cfg_tmp >>= srds_prctl_info[pos].shift; > > + > > + for (i = 0; i < 4 && cfg_tmp & (0xf << (3 - i)); i++) { > > + reg = in_le32(&serdes_base->lane[i].gcr0); > > + reg = (cmplt ? reg | LNAGCR0_RT_RSTB : > > + reg & LNAGCR0_RESET_MASK); > > + out_le32(&serdes_base->lane[i].gcr0, reg); > > + } > > +} > > Have you considered to use clrbits_le32(), setbits_le32(), > clrsetbits_le32() here and below as I suggested on previous review? > I missed the comment, Let me update the patch with these APIs in v7. - Rajesh > York _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot