diff mbox series

[U-Boot,v5,1/7] armv8: lsch3: Add serdes and DDR voltage setup

Message ID 1510551016-12234-2-git-send-email-rajesh.bhagat@nxp.com
State Superseded
Delegated to: York Sun
Headers show
Series Add VID support for QDS and RDB platforms | expand

Commit Message

Rajesh Bhagat Nov. 13, 2017, 5:30 a.m. UTC
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.Kumar@nxp.com>
Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
---
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    | 306 +++++++++++++++++++++
 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, 359 insertions(+), 18 deletions(-)

Comments

York Sun Nov. 13, 2017, 5:03 p.m. UTC | #1
On 11/12/2017 09:30 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.Kumar@nxp.com>
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> ---
> 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    | 306 +++++++++++++++++++++
>  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, 359 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..6a87350 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,312 @@ 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		0x80000000
> +#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},
> +

This is much better. Please align each line. Even checkpatch doesn't
give out warning, it is still a good practice to keep every assignment
in individual line, and match parenthesis.

> +#endif
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	{.id = 2, .mask = FSL_CHASSIS3_SRDS2_PRTCL_MASK,
> +		.shift = FSL_CHASSIS3_SRDS2_PRTCL_SHIFT},
> +#endif
> +	{.id = 0xf, .mask = 0, .shift = 0} /* NULL ENTRY */
> +};

Can you use all zeros for the last line, including "id"?

> +
> +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=0xf */
> +	for (srds_info = srds_prctl_info; srds_info->id != 0xf;
> +		srds_info++, pos++) {

Please fix the alignment.

> +		if (srds_info->id == serdes_id)
> +			return pos;
> +	}
> +	return -1;

Put a blank line above return.

> +}
> +
> +static void do_enabled_lanes_reset(u32 serdes_id, u32 cfg,
> +			    struct ccsr_serdes __iomem *serdes_base,
> +			    bool cmplt)

Alignment doesn't match parenthesis. You have several of same issue below.

> +{
> +	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);
> +	}
> +}
> +
> +static void do_pll_reset(u32 cfg,
> +		  struct ccsr_serdes __iomem *serdes_base)
> +{
> +	int i;
> +	u32 reg = 0;
> +
> +	for (i = 0; i < 2 && !(cfg & (0x1 << (1 - i))); i++) {
> +		reg = in_le32(&serdes_base->bank[i].rstctl);
> +		reg &= RSTCTL_RESET_MASK_1;
> +		reg |= RSTCTL_RSTREQ;
> +		out_le32(&serdes_base->bank[i].rstctl, reg);
> +	}
> +	udelay(1);
> +
> +	reg = in_le32(&serdes_base->bank[i].rstctl);
> +	reg &= RSTCTL_RESET_MASK_2;
> +	out_le32(&serdes_base->bank[i].rstctl, reg);

The above three lines are suspicious as the variable "i" is used out of
the loop.

The steps above are different from the protocol of "PLL Reset and
Reconfiguration", described in LS2088A reference manual. Please explain
if you are following different protocol.

> +}
> +
> +static void do_rx_tx_cal_reset(struct ccsr_serdes __iomem *serdes_base)
> +{
> +	u32 reg = 0;
> +
> +	reg = in_le32(&serdes_base->srdstcalcr);
> +	reg &= TCALCR_RESET_MASK;
> +	out_le32(&serdes_base->srdstcalcr, reg);
> +	reg = in_le32(&serdes_base->srdsrcalcr);
> +	reg &= TCALCR_RESET_MASK;
> +	out_le32(&serdes_base->srdsrcalcr, reg);
> +}

Please check if you can use clrbits_le32(), setbits_le32(),
clrsetbits_le32() throughout your patch.


<snip>

> +
> +int setup_serdes_volt(u32 svdd)
> +{
> +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
> +	struct ccsr_serdes __iomem *serdes1_base;
> +	u32 cfg_rcwsrds1 = gur_in32(&gur->rcwsr[FSL_CHASSIS3_SRDS1_REGSR - 1]);
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	struct ccsr_serdes __iomem *serdes2_base;
> +	u32 cfg_rcwsrds2 = gur_in32(&gur->rcwsr[FSL_CHASSIS3_SRDS2_REGSR - 1]);
> +#endif
> +	u32 cfg_tmp;
> +	int svdd_cur, svdd_tar;
> +	int ret = 1;
> +
> +	/* Only support switch SVDD to 900mV */
> +	if (svdd != 900)
> +		return -1;

Shouldn't this be -EINVAL?

> +
> +	/* Scale up to the LTC resolution is 1/4096V */
> +	svdd = (svdd * 4096) / 1000;
> +
> +	svdd_tar = svdd;
> +	svdd_cur = get_serdes_volt();
> +	if (svdd_cur < 0)
> +		return -EINVAL;
> +
> +	debug("%s: current SVDD: %x; target SVDD: %x\n",
> +	      __func__, svdd_cur, svdd_tar);
> +	if (svdd_cur == svdd_tar)
> +		return 0;
> +
> +	serdes1_base = (void *)CONFIG_SYS_FSL_LSCH3_SERDES_ADDR;
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	serdes2_base =  (void *)(CONFIG_SYS_FSL_LSCH3_SERDES_ADDR + 0x10000);
> +#endif

To simplify the code, you can move the assignment of these variables to
the beginning of this function, where you declare the variables.

> +
> +	/* Put the all enabled lanes in reset */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	do_enabled_lanes_reset(1, cfg_rcwsrds1, serdes1_base, false);
> +#endif
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	do_enabled_lanes_reset(2, cfg_rcwsrds2, serdes2_base, false);
> +#endif
> +
> +	/* Put the all enabled PLL in reset */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> +	do_pll_reset(cfg_tmp, serdes1_base);
> +#endif
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> +	cfg_tmp >>= 2;
> +	do_pll_reset(cfg_tmp, serdes2_base);
> +#endif
> +
> +	/* Put the Rx/Tx calibration into reset */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	do_rx_tx_cal_reset(serdes1_base);
> +#endif
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	do_rx_tx_cal_reset(serdes2_base);
> +#endif
> +
> +	ret = set_serdes_volt(svdd);
> +	if (ret < 0) {
> +		printf("could not change SVDD\n");
> +		ret = -1;
> +	}
> +
> +	/* For each PLL that’s not disabled via RCW enable the SERDES */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> +	do_serdes_enable(cfg_tmp, serdes1_base);
> +#endif
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> +	cfg_tmp >>= 2;
> +	do_serdes_enable(cfg_tmp, serdes2_base);
> +#endif
> +
> +	/* Wait for at atleast 625us, ensure the PLLs being reset are locked */

s/atleast/at least/

For my education, where can I find the document describing this procedure?

> +	udelay(800);
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> +	do_pll_lock(cfg_tmp, serdes1_base);
> +#endif
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> +	cfg_tmp >>= 2;
> +	do_pll_lock(cfg_tmp, serdes2_base);
> +#endif
> +	/* Take the all enabled lanes out of reset */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	do_enabled_lanes_reset(1, cfg_rcwsrds1, serdes1_base, true);
> +#endif
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	do_enabled_lanes_reset(2, cfg_rcwsrds2, serdes2_base, true);
> +#endif
> +
> +	/* For each PLL being reset, and achieved PLL lock set RST_DONE */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> +	do_pll_reset_done(cfg_tmp, serdes1_base);
> +#endif
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> +	cfg_tmp >>= 2;
> +	do_pll_reset_done(cfg_tmp, serdes2_base);
> +#endif
> +	return ret;
> +}
> +
>  void fsl_serdes_init(void)
>  {
>  #if defined(CONFIG_FSL_MC_ENET) && !defined(CONFIG_SPL_BUILD)
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> index 497a4b5..47d89b2 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> @@ -513,23 +513,6 @@ static int setup_core_volt(u32 vdd)
>  	return board_setup_core_volt(vdd);
>  }
>  
> -#ifdef CONFIG_SYS_FSL_DDR
> -static void ddr_enable_0v9_volt(bool en)
> -{
> -	struct ccsr_ddr __iomem *ddr = (void *)CONFIG_SYS_FSL_DDR_ADDR;
> -	u32 tmp;
> -
> -	tmp = ddr_in32(&ddr->ddr_cdr1);
> -
> -	if (en)
> -		tmp |= DDR_CDR1_V0PT9_EN;
> -	else
> -		tmp &= ~DDR_CDR1_V0PT9_EN;
> -
> -	ddr_out32(&ddr->ddr_cdr1, tmp);
> -}
> -#endif
> -
>  int setup_chip_volt(void)
>  {
>  	int vdd;
> @@ -598,6 +581,23 @@ void fsl_lsch2_early_init_f(void)
>  }
>  #endif
>  
> +#ifdef CONFIG_SYS_FSL_DDR
> +void ddr_enable_0v9_volt(bool en)
> +{
> +	struct ccsr_ddr __iomem *ddr = (void *)CONFIG_SYS_FSL_DDR_ADDR;
> +	u32 tmp;
> +
> +	tmp = ddr_in32(&ddr->ddr_cdr1);
> +
> +	if (en)
> +		tmp |= DDR_CDR1_V0PT9_EN;
> +	else
> +		tmp &= ~DDR_CDR1_V0PT9_EN;
> +
> +	ddr_out32(&ddr->ddr_cdr1, tmp);
> +}
> +#endif

While you are on this file, can you explain why not set cdr1 in board
ddr.c file? It seems too late to change here. You may consult Zhiqiang
Hou. He added the original code.

York
Rajesh Bhagat Nov. 14, 2017, 3:48 a.m. UTC | #2
> -----Original Message-----
> From: York Sun
> Sent: Monday, November 13, 2017 10:33 PM
> To: Rajesh Bhagat <rajesh.bhagat@nxp.com>; u-boot@lists.denx.de
> Cc: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; Priyanka Jain
> <priyanka.jain@nxp.com>; Ashish Kumar <ashish.kumar@nxp.com>
> Subject: Re: [PATCH v5 1/7] armv8: lsch3: Add serdes and DDR voltage setup
> 
> On 11/12/2017 09:30 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.Kumar@nxp.com>
> > Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > ---
> > 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    | 306
> +++++++++++++++++++++
> >  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, 359 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..6a87350 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,312 @@ 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		0x80000000
> > +#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},
> > +
> 
> This is much better. Please align each line. Even checkpatch doesn't give out warning,
> it is still a good practice to keep every assignment in individual line, and match
> parenthesis.
> 

Will change in v6.

> > +#endif
> > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > +	{.id = 2, .mask = FSL_CHASSIS3_SRDS2_PRTCL_MASK,
> > +		.shift = FSL_CHASSIS3_SRDS2_PRTCL_SHIFT}, #endif
> > +	{.id = 0xf, .mask = 0, .shift = 0} /* NULL ENTRY */ };
> 
> Can you use all zeros for the last line, including "id"?
> 

Will change in v6.

> > +
> > +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=0xf */
> > +	for (srds_info = srds_prctl_info; srds_info->id != 0xf;
> > +		srds_info++, pos++) {
> 
> Please fix the alignment.

Will change in v6.

> 
> > +		if (srds_info->id == serdes_id)
> > +			return pos;
> > +	}
> > +	return -1;
> 
> Put a blank line above return.
> 

Will change in v6.

> > +}
> > +
> > +static void do_enabled_lanes_reset(u32 serdes_id, u32 cfg,
> > +			    struct ccsr_serdes __iomem *serdes_base,
> > +			    bool cmplt)
> 
> Alignment doesn't match parenthesis. You have several of same issue below.
> 

Will change in v6.

> > +{
> > +	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);
> > +	}
> > +}
> > +
> > +static void do_pll_reset(u32 cfg,
> > +		  struct ccsr_serdes __iomem *serdes_base) {
> > +	int i;
> > +	u32 reg = 0;
> > +
> > +	for (i = 0; i < 2 && !(cfg & (0x1 << (1 - i))); i++) {
> > +		reg = in_le32(&serdes_base->bank[i].rstctl);
> > +		reg &= RSTCTL_RESET_MASK_1;
> > +		reg |= RSTCTL_RSTREQ;
> > +		out_le32(&serdes_base->bank[i].rstctl, reg);
> > +	}
> > +	udelay(1);
> > +
> > +	reg = in_le32(&serdes_base->bank[i].rstctl);
> > +	reg &= RSTCTL_RESET_MASK_2;
> > +	out_le32(&serdes_base->bank[i].rstctl, reg);
> 
> The above three lines are suspicious as the variable "i" is used out of the loop.
> 
> The steps above are different from the protocol of "PLL Reset and Reconfiguration",
> described in LS2088A reference manual. Please explain if you are following different
> protocol.
> 

This code was implemented as part of LS1088A, Let me check the steps in "PLL Reset and Reconfiguration"
section and update this code. 

> > +}
> > +
> > +static void do_rx_tx_cal_reset(struct ccsr_serdes __iomem
> > +*serdes_base) {
> > +	u32 reg = 0;
> > +
> > +	reg = in_le32(&serdes_base->srdstcalcr);
> > +	reg &= TCALCR_RESET_MASK;
> > +	out_le32(&serdes_base->srdstcalcr, reg);
> > +	reg = in_le32(&serdes_base->srdsrcalcr);
> > +	reg &= TCALCR_RESET_MASK;
> > +	out_le32(&serdes_base->srdsrcalcr, reg); }
> 
> Please check if you can use clrbits_le32(), setbits_le32(),
> clrsetbits_le32() throughout your patch.
> 
> 
> <snip>
> 
> > +
> > +int setup_serdes_volt(u32 svdd)
> > +{
> > +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
> > +	struct ccsr_serdes __iomem *serdes1_base;
> > +	u32 cfg_rcwsrds1 = gur_in32(&gur->rcwsr[FSL_CHASSIS3_SRDS1_REGSR -
> > +1]); #ifdef CONFIG_SYS_FSL_SRDS_2
> > +	struct ccsr_serdes __iomem *serdes2_base;
> > +	u32 cfg_rcwsrds2 = gur_in32(&gur->rcwsr[FSL_CHASSIS3_SRDS2_REGSR -
> > +1]); #endif
> > +	u32 cfg_tmp;
> > +	int svdd_cur, svdd_tar;
> > +	int ret = 1;
> > +
> > +	/* Only support switch SVDD to 900mV */
> > +	if (svdd != 900)
> > +		return -1;
> 
> Shouldn't this be -EINVAL?
> 

Will take care v6

> > +
> > +	/* Scale up to the LTC resolution is 1/4096V */
> > +	svdd = (svdd * 4096) / 1000;
> > +
> > +	svdd_tar = svdd;
> > +	svdd_cur = get_serdes_volt();
> > +	if (svdd_cur < 0)
> > +		return -EINVAL;
> > +
> > +	debug("%s: current SVDD: %x; target SVDD: %x\n",
> > +	      __func__, svdd_cur, svdd_tar);
> > +	if (svdd_cur == svdd_tar)
> > +		return 0;
> > +
> > +	serdes1_base = (void *)CONFIG_SYS_FSL_LSCH3_SERDES_ADDR;
> > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > +	serdes2_base =  (void *)(CONFIG_SYS_FSL_LSCH3_SERDES_ADDR +
> > +0x10000); #endif
> 
> To simplify the code, you can move the assignment of these variables to the beginning
> of this function, where you declare the variables.
> 
> > +
> > +	/* Put the all enabled lanes in reset */ #ifdef
> > +CONFIG_SYS_FSL_SRDS_1
> > +	do_enabled_lanes_reset(1, cfg_rcwsrds1, serdes1_base, false); #endif
> > +
> > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > +	do_enabled_lanes_reset(2, cfg_rcwsrds2, serdes2_base, false); #endif
> > +
> > +	/* Put the all enabled PLL in reset */ #ifdef CONFIG_SYS_FSL_SRDS_1
> > +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> > +	do_pll_reset(cfg_tmp, serdes1_base); #endif
> > +
> > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> > +	cfg_tmp >>= 2;
> > +	do_pll_reset(cfg_tmp, serdes2_base); #endif
> > +
> > +	/* Put the Rx/Tx calibration into reset */ #ifdef
> > +CONFIG_SYS_FSL_SRDS_1
> > +	do_rx_tx_cal_reset(serdes1_base);
> > +#endif
> > +
> > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > +	do_rx_tx_cal_reset(serdes2_base);
> > +#endif
> > +
> > +	ret = set_serdes_volt(svdd);
> > +	if (ret < 0) {
> > +		printf("could not change SVDD\n");
> > +		ret = -1;
> > +	}
> > +
> > +	/* For each PLL that's not disabled via RCW enable the SERDES */
> > +#ifdef CONFIG_SYS_FSL_SRDS_1
> > +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> > +	do_serdes_enable(cfg_tmp, serdes1_base); #endif #ifdef
> > +CONFIG_SYS_FSL_SRDS_2
> > +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> > +	cfg_tmp >>= 2;
> > +	do_serdes_enable(cfg_tmp, serdes2_base); #endif
> > +
> > +	/* Wait for at atleast 625us, ensure the PLLs being reset are locked
> > +*/
> 
> s/atleast/at least/
> 
Will take care v6

> For my education, where can I find the document describing this procedure?
> 
> > +	udelay(800);
> > +
> > +#ifdef CONFIG_SYS_FSL_SRDS_1
> > +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> > +	do_pll_lock(cfg_tmp, serdes1_base);
> > +#endif
> > +
> > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> > +	cfg_tmp >>= 2;
> > +	do_pll_lock(cfg_tmp, serdes2_base);
> > +#endif
> > +	/* Take the all enabled lanes out of reset */ #ifdef
> > +CONFIG_SYS_FSL_SRDS_1
> > +	do_enabled_lanes_reset(1, cfg_rcwsrds1, serdes1_base, true); #endif
> > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > +	do_enabled_lanes_reset(2, cfg_rcwsrds2, serdes2_base, true); #endif
> > +
> > +	/* For each PLL being reset, and achieved PLL lock set RST_DONE */
> > +#ifdef CONFIG_SYS_FSL_SRDS_1
> > +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> > +	do_pll_reset_done(cfg_tmp, serdes1_base); #endif #ifdef
> > +CONFIG_SYS_FSL_SRDS_2
> > +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> > +	cfg_tmp >>= 2;
> > +	do_pll_reset_done(cfg_tmp, serdes2_base); #endif
> > +	return ret;
> > +}
> > +
> >  void fsl_serdes_init(void)
> >  {
> >  #if defined(CONFIG_FSL_MC_ENET) && !defined(CONFIG_SPL_BUILD) diff
> > --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > index 497a4b5..47d89b2 100644
> > --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > @@ -513,23 +513,6 @@ static int setup_core_volt(u32 vdd)
> >  	return board_setup_core_volt(vdd);
> >  }
> >
> > -#ifdef CONFIG_SYS_FSL_DDR
> > -static void ddr_enable_0v9_volt(bool en) -{
> > -	struct ccsr_ddr __iomem *ddr = (void *)CONFIG_SYS_FSL_DDR_ADDR;
> > -	u32 tmp;
> > -
> > -	tmp = ddr_in32(&ddr->ddr_cdr1);
> > -
> > -	if (en)
> > -		tmp |= DDR_CDR1_V0PT9_EN;
> > -	else
> > -		tmp &= ~DDR_CDR1_V0PT9_EN;
> > -
> > -	ddr_out32(&ddr->ddr_cdr1, tmp);
> > -}
> > -#endif
> > -
> >  int setup_chip_volt(void)
> >  {
> >  	int vdd;
> > @@ -598,6 +581,23 @@ void fsl_lsch2_early_init_f(void)  }  #endif
> >
> > +#ifdef CONFIG_SYS_FSL_DDR
> > +void ddr_enable_0v9_volt(bool en)
> > +{
> > +	struct ccsr_ddr __iomem *ddr = (void *)CONFIG_SYS_FSL_DDR_ADDR;
> > +	u32 tmp;
> > +
> > +	tmp = ddr_in32(&ddr->ddr_cdr1);
> > +
> > +	if (en)
> > +		tmp |= DDR_CDR1_V0PT9_EN;
> > +	else
> > +		tmp &= ~DDR_CDR1_V0PT9_EN;
> > +
> > +	ddr_out32(&ddr->ddr_cdr1, tmp);
> > +}
> > +#endif
> 
> While you are on this file, can you explain why not set cdr1 in board ddr.c file? It
> seems too late to change here. You may consult Zhiqiang Hou. He added the original
> code.
> 

Ok, Let me consult with him.

- Rajesh 

> York
Rajesh Bhagat Nov. 14, 2017, 6:08 a.m. UTC | #3
> -----Original Message-----
> From: Rajesh Bhagat
> Sent: Tuesday, November 14, 2017 9:18 AM
> To: York Sun <york.sun@nxp.com>; u-boot@lists.denx.de
> Cc: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; Priyanka Jain
> <priyanka.jain@nxp.com>; Ashish Kumar <ashish.kumar@nxp.com>
> Subject: RE: [PATCH v5 1/7] armv8: lsch3: Add serdes and DDR voltage setup
> 
> 
> 
> > -----Original Message-----
> > From: York Sun
> > Sent: Monday, November 13, 2017 10:33 PM
> > To: Rajesh Bhagat <rajesh.bhagat@nxp.com>; u-boot@lists.denx.de
> > Cc: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; Priyanka Jain
> > <priyanka.jain@nxp.com>; Ashish Kumar <ashish.kumar@nxp.com>
> > Subject: Re: [PATCH v5 1/7] armv8: lsch3: Add serdes and DDR voltage
> > setup
> >
> > On 11/12/2017 09:30 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.Kumar@nxp.com>
> > > Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > > ---
> > > 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    | 306
> > +++++++++++++++++++++
> > >  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, 359 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..6a87350 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,312 @@ 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		0x80000000
> > > +#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},
> > > +
> >
> > This is much better. Please align each line. Even checkpatch doesn't
> > give out warning, it is still a good practice to keep every assignment
> > in individual line, and match parenthesis.
> >
> 
> Will change in v6.
> 
> > > +#endif
> > > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > > +	{.id = 2, .mask = FSL_CHASSIS3_SRDS2_PRTCL_MASK,
> > > +		.shift = FSL_CHASSIS3_SRDS2_PRTCL_SHIFT}, #endif
> > > +	{.id = 0xf, .mask = 0, .shift = 0} /* NULL ENTRY */ };
> >
> > Can you use all zeros for the last line, including "id"?
> >
> 
> Will change in v6.
> 
> > > +
> > > +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=0xf */
> > > +	for (srds_info = srds_prctl_info; srds_info->id != 0xf;
> > > +		srds_info++, pos++) {
> >
> > Please fix the alignment.
> 
> Will change in v6.
> 
> >
> > > +		if (srds_info->id == serdes_id)
> > > +			return pos;
> > > +	}
> > > +	return -1;
> >
> > Put a blank line above return.
> >
> 
> Will change in v6.
> 
> > > +}
> > > +
> > > +static void do_enabled_lanes_reset(u32 serdes_id, u32 cfg,
> > > +			    struct ccsr_serdes __iomem *serdes_base,
> > > +			    bool cmplt)
> >
> > Alignment doesn't match parenthesis. You have several of same issue below.
> >
> 
> Will change in v6.
> 
> > > +{
> > > +	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);
> > > +	}
> > > +}
> > > +
> > > +static void do_pll_reset(u32 cfg,
> > > +		  struct ccsr_serdes __iomem *serdes_base) {
> > > +	int i;
> > > +	u32 reg = 0;
> > > +
> > > +	for (i = 0; i < 2 && !(cfg & (0x1 << (1 - i))); i++) {
> > > +		reg = in_le32(&serdes_base->bank[i].rstctl);
> > > +		reg &= RSTCTL_RESET_MASK_1;
> > > +		reg |= RSTCTL_RSTREQ;
> > > +		out_le32(&serdes_base->bank[i].rstctl, reg);
> > > +	}
> > > +	udelay(1);
> > > +
> > > +	reg = in_le32(&serdes_base->bank[i].rstctl);
> > > +	reg &= RSTCTL_RESET_MASK_2;
> > > +	out_le32(&serdes_base->bank[i].rstctl, reg);
> >
> > The above three lines are suspicious as the variable "i" is used out of the loop.
> >
> > The steps above are different from the protocol of "PLL Reset and
> > Reconfiguration", described in LS2088A reference manual. Please
> > explain if you are following different protocol.
> >
> 
> This code was implemented as part of LS1088A, Let me check the steps in "PLL Reset
> and Reconfiguration"
> section and update this code.
> 
> > > +}
> > > +
> > > +static void do_rx_tx_cal_reset(struct ccsr_serdes __iomem
> > > +*serdes_base) {
> > > +	u32 reg = 0;
> > > +
> > > +	reg = in_le32(&serdes_base->srdstcalcr);
> > > +	reg &= TCALCR_RESET_MASK;
> > > +	out_le32(&serdes_base->srdstcalcr, reg);
> > > +	reg = in_le32(&serdes_base->srdsrcalcr);
> > > +	reg &= TCALCR_RESET_MASK;
> > > +	out_le32(&serdes_base->srdsrcalcr, reg); }
> >
> > Please check if you can use clrbits_le32(), setbits_le32(),
> > clrsetbits_le32() throughout your patch.
> >
> >
> > <snip>
> >
> > > +
> > > +int setup_serdes_volt(u32 svdd)
> > > +{
> > > +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
> > > +	struct ccsr_serdes __iomem *serdes1_base;
> > > +	u32 cfg_rcwsrds1 = gur_in32(&gur->rcwsr[FSL_CHASSIS3_SRDS1_REGSR -
> > > +1]); #ifdef CONFIG_SYS_FSL_SRDS_2
> > > +	struct ccsr_serdes __iomem *serdes2_base;
> > > +	u32 cfg_rcwsrds2 = gur_in32(&gur->rcwsr[FSL_CHASSIS3_SRDS2_REGSR -
> > > +1]); #endif
> > > +	u32 cfg_tmp;
> > > +	int svdd_cur, svdd_tar;
> > > +	int ret = 1;
> > > +
> > > +	/* Only support switch SVDD to 900mV */
> > > +	if (svdd != 900)
> > > +		return -1;
> >
> > Shouldn't this be -EINVAL?
> >
> 
> Will take care v6
> 
> > > +
> > > +	/* Scale up to the LTC resolution is 1/4096V */
> > > +	svdd = (svdd * 4096) / 1000;
> > > +
> > > +	svdd_tar = svdd;
> > > +	svdd_cur = get_serdes_volt();
> > > +	if (svdd_cur < 0)
> > > +		return -EINVAL;
> > > +
> > > +	debug("%s: current SVDD: %x; target SVDD: %x\n",
> > > +	      __func__, svdd_cur, svdd_tar);
> > > +	if (svdd_cur == svdd_tar)
> > > +		return 0;
> > > +
> > > +	serdes1_base = (void *)CONFIG_SYS_FSL_LSCH3_SERDES_ADDR;
> > > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > > +	serdes2_base =  (void *)(CONFIG_SYS_FSL_LSCH3_SERDES_ADDR +
> > > +0x10000); #endif
> >
> > To simplify the code, you can move the assignment of these variables
> > to the beginning of this function, where you declare the variables.
> >
> > > +
> > > +	/* Put the all enabled lanes in reset */ #ifdef
> > > +CONFIG_SYS_FSL_SRDS_1
> > > +	do_enabled_lanes_reset(1, cfg_rcwsrds1, serdes1_base, false);
> > > +#endif
> > > +
> > > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > > +	do_enabled_lanes_reset(2, cfg_rcwsrds2, serdes2_base, false);
> > > +#endif
> > > +
> > > +	/* Put the all enabled PLL in reset */ #ifdef CONFIG_SYS_FSL_SRDS_1
> > > +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> > > +	do_pll_reset(cfg_tmp, serdes1_base); #endif
> > > +
> > > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > > +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> > > +	cfg_tmp >>= 2;
> > > +	do_pll_reset(cfg_tmp, serdes2_base); #endif
> > > +
> > > +	/* Put the Rx/Tx calibration into reset */ #ifdef
> > > +CONFIG_SYS_FSL_SRDS_1
> > > +	do_rx_tx_cal_reset(serdes1_base);
> > > +#endif
> > > +
> > > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > > +	do_rx_tx_cal_reset(serdes2_base);
> > > +#endif
> > > +
> > > +	ret = set_serdes_volt(svdd);
> > > +	if (ret < 0) {
> > > +		printf("could not change SVDD\n");
> > > +		ret = -1;
> > > +	}
> > > +
> > > +	/* For each PLL that's not disabled via RCW enable the SERDES */
> > > +#ifdef CONFIG_SYS_FSL_SRDS_1
> > > +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> > > +	do_serdes_enable(cfg_tmp, serdes1_base); #endif #ifdef
> > > +CONFIG_SYS_FSL_SRDS_2
> > > +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> > > +	cfg_tmp >>= 2;
> > > +	do_serdes_enable(cfg_tmp, serdes2_base); #endif
> > > +
> > > +	/* Wait for at atleast 625us, ensure the PLLs being reset are
> > > +locked */
> >
> > s/atleast/at least/
> >
> Will take care v6
> 
> > For my education, where can I find the document describing this procedure?
> >
> > > +	udelay(800);
> > > +
> > > +#ifdef CONFIG_SYS_FSL_SRDS_1
> > > +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> > > +	do_pll_lock(cfg_tmp, serdes1_base); #endif
> > > +
> > > +#ifdef CONFIG_SYS_FSL_SRDS_2
> > > +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> > > +	cfg_tmp >>= 2;
> > > +	do_pll_lock(cfg_tmp, serdes2_base); #endif
> > > +	/* Take the all enabled lanes out of reset */ #ifdef
> > > +CONFIG_SYS_FSL_SRDS_1
> > > +	do_enabled_lanes_reset(1, cfg_rcwsrds1, serdes1_base, true);
> > > +#endif #ifdef CONFIG_SYS_FSL_SRDS_2
> > > +	do_enabled_lanes_reset(2, cfg_rcwsrds2, serdes2_base, true);
> > > +#endif
> > > +
> > > +	/* For each PLL being reset, and achieved PLL lock set RST_DONE */
> > > +#ifdef CONFIG_SYS_FSL_SRDS_1
> > > +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> > > +	do_pll_reset_done(cfg_tmp, serdes1_base); #endif #ifdef
> > > +CONFIG_SYS_FSL_SRDS_2
> > > +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> > > +	cfg_tmp >>= 2;
> > > +	do_pll_reset_done(cfg_tmp, serdes2_base); #endif
> > > +	return ret;
> > > +}
> > > +
> > >  void fsl_serdes_init(void)
> > >  {
> > >  #if defined(CONFIG_FSL_MC_ENET) && !defined(CONFIG_SPL_BUILD) diff
> > > --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > > b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > > index 497a4b5..47d89b2 100644
> > > --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > > +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> > > @@ -513,23 +513,6 @@ static int setup_core_volt(u32 vdd)
> > >  	return board_setup_core_volt(vdd);  }
> > >
> > > -#ifdef CONFIG_SYS_FSL_DDR
> > > -static void ddr_enable_0v9_volt(bool en) -{
> > > -	struct ccsr_ddr __iomem *ddr = (void *)CONFIG_SYS_FSL_DDR_ADDR;
> > > -	u32 tmp;
> > > -
> > > -	tmp = ddr_in32(&ddr->ddr_cdr1);
> > > -
> > > -	if (en)
> > > -		tmp |= DDR_CDR1_V0PT9_EN;
> > > -	else
> > > -		tmp &= ~DDR_CDR1_V0PT9_EN;
> > > -
> > > -	ddr_out32(&ddr->ddr_cdr1, tmp);
> > > -}
> > > -#endif
> > > -
> > >  int setup_chip_volt(void)
> > >  {
> > >  	int vdd;
> > > @@ -598,6 +581,23 @@ void fsl_lsch2_early_init_f(void)  }  #endif
> > >
> > > +#ifdef CONFIG_SYS_FSL_DDR
> > > +void ddr_enable_0v9_volt(bool en)
> > > +{
> > > +	struct ccsr_ddr __iomem *ddr = (void *)CONFIG_SYS_FSL_DDR_ADDR;
> > > +	u32 tmp;
> > > +
> > > +	tmp = ddr_in32(&ddr->ddr_cdr1);
> > > +
> > > +	if (en)
> > > +		tmp |= DDR_CDR1_V0PT9_EN;
> > > +	else
> > > +		tmp &= ~DDR_CDR1_V0PT9_EN;
> > > +
> > > +	ddr_out32(&ddr->ddr_cdr1, tmp);
> > > +}
> > > +#endif
> >
> > While you are on this file, can you explain why not set cdr1 in board
> > ddr.c file? It seems too late to change here. You may consult Zhiqiang
> > Hou. He added the original code.
> >
> 
> Ok, Let me consult with him.
> 

Checked with Zhiqiang Hou, he added the code so that it can be re-used by other boards.

IMO, this is required for special handling of 0.9v to readjust DDR controller to operate at 0.9v,
Hence not part of board ddr.c. 

> - Rajesh
> 
> > York
diff mbox series

Patch

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..6a87350 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,312 @@  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		0x80000000
+#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 = 0xf, .mask = 0, .shift = 0} /* NULL ENTRY */
+};
+
+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=0xf */
+	for (srds_info = srds_prctl_info; srds_info->id != 0xf;
+		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);
+	}
+}
+
+static void do_pll_reset(u32 cfg,
+		  struct ccsr_serdes __iomem *serdes_base)
+{
+	int i;
+	u32 reg = 0;
+
+	for (i = 0; i < 2 && !(cfg & (0x1 << (1 - i))); i++) {
+		reg = in_le32(&serdes_base->bank[i].rstctl);
+		reg &= RSTCTL_RESET_MASK_1;
+		reg |= RSTCTL_RSTREQ;
+		out_le32(&serdes_base->bank[i].rstctl, reg);
+	}
+	udelay(1);
+
+	reg = in_le32(&serdes_base->bank[i].rstctl);
+	reg &= RSTCTL_RESET_MASK_2;
+	out_le32(&serdes_base->bank[i].rstctl, reg);
+}
+
+static void do_rx_tx_cal_reset(struct ccsr_serdes __iomem *serdes_base)
+{
+	u32 reg = 0;
+
+	reg = in_le32(&serdes_base->srdstcalcr);
+	reg &= TCALCR_RESET_MASK;
+	out_le32(&serdes_base->srdstcalcr, reg);
+	reg = in_le32(&serdes_base->srdsrcalcr);
+	reg &= TCALCR_RESET_MASK;
+	out_le32(&serdes_base->srdsrcalcr, reg);
+}
+
+static void do_rx_tx_cal_reset_comp(u32 cfg, int i,
+			     struct ccsr_serdes __iomem *serdes_base)
+{
+	u32 reg = 0;
+
+	if (!(cfg == 0x3 && i == 1)) {
+		udelay(1);
+		reg = in_le32(&serdes_base->srdstcalcr);
+		reg |= TCALCR_CALRST_B;
+		out_le32(&serdes_base->srdstcalcr, reg);
+		reg = in_le32(&serdes_base->srdsrcalcr);
+		reg |= TCALCR_CALRST_B;
+		out_le32(&serdes_base->srdsrcalcr, reg);
+	}
+	udelay(1);
+}
+
+static void do_pll_reset_done(u32 cfg,
+		       struct ccsr_serdes __iomem *serdes_base)
+{
+	int i;
+	u32 reg = 0;
+
+	for (i = 0; i < 2; i++) {
+		reg = in_le32(&serdes_base->bank[i].pllcr0);
+		if (!(cfg & (0x1 << (1 - i))) && ((reg >> 23) & 0x1)) {
+			reg = in_le32(&serdes_base->bank[i].rstctl);
+			reg |= RSTCTL_RST_DONE;
+			out_le32(&serdes_base->bank[i].rstctl, reg);
+		}
+	}
+}
+
+static void do_serdes_enable(u32 cfg,
+		      struct ccsr_serdes __iomem *serdes_base)
+{
+	int i;
+	u32 reg = 0;
+
+	for (i = 0; i < 2 && !(cfg & (0x1 << (1 - i))); i++) {
+		reg = in_le32(&serdes_base->bank[i].rstctl);
+		reg |= RSTCTL_SDEN;
+		out_le32(&serdes_base->bank[i].rstctl, reg);
+		udelay(1);
+
+		reg = in_le32(&serdes_base->bank[i].rstctl);
+		reg |= RSTCTL_PLLRST_B;
+		out_le32(&serdes_base->bank[i].rstctl, reg);
+		udelay(1);
+		/* Take the Rx/Tx calibration out of reset */
+		do_rx_tx_cal_reset_comp(cfg, i, serdes_base);
+	}
+}
+
+static void do_pll_lock(u32 cfg,
+		 struct ccsr_serdes __iomem *serdes_base)
+{
+	int i;
+	u32 reg = 0;
+
+	for (i = 0; i < 2 && !(cfg & (0x1 << (1 - i))); i++) {
+		/* if the PLL is not locked, set RST_ERR */
+		reg = in_le32(&serdes_base->bank[i].pllcr0);
+		if (!((reg >> 23) & 0x1)) {
+			reg = in_le32(&serdes_base->bank[i].rstctl);
+			reg |= RSTCTL_RSTERR;
+			out_le32(&serdes_base->bank[i].rstctl, reg);
+		} else {
+			udelay(1);
+			reg = in_le32(&serdes_base->bank[i].rstctl);
+			reg &= RSTCTL_RESET_MASK_3;
+			reg |= RSTCTL_SDRST_B;
+			out_le32(&serdes_base->bank[i].rstctl, reg);
+			udelay(1);
+		}
+	}
+}
+
+int setup_serdes_volt(u32 svdd)
+{
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+	struct ccsr_serdes __iomem *serdes1_base;
+	u32 cfg_rcwsrds1 = gur_in32(&gur->rcwsr[FSL_CHASSIS3_SRDS1_REGSR - 1]);
+#ifdef CONFIG_SYS_FSL_SRDS_2
+	struct ccsr_serdes __iomem *serdes2_base;
+	u32 cfg_rcwsrds2 = gur_in32(&gur->rcwsr[FSL_CHASSIS3_SRDS2_REGSR - 1]);
+#endif
+	u32 cfg_tmp;
+	int svdd_cur, svdd_tar;
+	int ret = 1;
+
+	/* Only support switch SVDD to 900mV */
+	if (svdd != 900)
+		return -1;
+
+	/* Scale up to the LTC resolution is 1/4096V */
+	svdd = (svdd * 4096) / 1000;
+
+	svdd_tar = svdd;
+	svdd_cur = get_serdes_volt();
+	if (svdd_cur < 0)
+		return -EINVAL;
+
+	debug("%s: current SVDD: %x; target SVDD: %x\n",
+	      __func__, svdd_cur, svdd_tar);
+	if (svdd_cur == svdd_tar)
+		return 0;
+
+	serdes1_base = (void *)CONFIG_SYS_FSL_LSCH3_SERDES_ADDR;
+#ifdef CONFIG_SYS_FSL_SRDS_2
+	serdes2_base =  (void *)(CONFIG_SYS_FSL_LSCH3_SERDES_ADDR + 0x10000);
+#endif
+
+	/* Put the all enabled lanes in reset */
+#ifdef CONFIG_SYS_FSL_SRDS_1
+	do_enabled_lanes_reset(1, cfg_rcwsrds1, serdes1_base, false);
+#endif
+
+#ifdef CONFIG_SYS_FSL_SRDS_2
+	do_enabled_lanes_reset(2, cfg_rcwsrds2, serdes2_base, false);
+#endif
+
+	/* Put the all enabled PLL in reset */
+#ifdef CONFIG_SYS_FSL_SRDS_1
+	cfg_tmp = cfg_rcwsrds1 & 0x3;
+	do_pll_reset(cfg_tmp, serdes1_base);
+#endif
+
+#ifdef CONFIG_SYS_FSL_SRDS_2
+	cfg_tmp = cfg_rcwsrds1 & 0xC;
+	cfg_tmp >>= 2;
+	do_pll_reset(cfg_tmp, serdes2_base);
+#endif
+
+	/* Put the Rx/Tx calibration into reset */
+#ifdef CONFIG_SYS_FSL_SRDS_1
+	do_rx_tx_cal_reset(serdes1_base);
+#endif
+
+#ifdef CONFIG_SYS_FSL_SRDS_2
+	do_rx_tx_cal_reset(serdes2_base);
+#endif
+
+	ret = set_serdes_volt(svdd);
+	if (ret < 0) {
+		printf("could not change SVDD\n");
+		ret = -1;
+	}
+
+	/* For each PLL that’s not disabled via RCW enable the SERDES */
+#ifdef CONFIG_SYS_FSL_SRDS_1
+	cfg_tmp = cfg_rcwsrds1 & 0x3;
+	do_serdes_enable(cfg_tmp, serdes1_base);
+#endif
+#ifdef CONFIG_SYS_FSL_SRDS_2
+	cfg_tmp = cfg_rcwsrds1 & 0xC;
+	cfg_tmp >>= 2;
+	do_serdes_enable(cfg_tmp, serdes2_base);
+#endif
+
+	/* Wait for at atleast 625us, ensure the PLLs being reset are locked */
+	udelay(800);
+
+#ifdef CONFIG_SYS_FSL_SRDS_1
+	cfg_tmp = cfg_rcwsrds1 & 0x3;
+	do_pll_lock(cfg_tmp, serdes1_base);
+#endif
+
+#ifdef CONFIG_SYS_FSL_SRDS_2
+	cfg_tmp = cfg_rcwsrds1 & 0xC;
+	cfg_tmp >>= 2;
+	do_pll_lock(cfg_tmp, serdes2_base);
+#endif
+	/* Take the all enabled lanes out of reset */
+#ifdef CONFIG_SYS_FSL_SRDS_1
+	do_enabled_lanes_reset(1, cfg_rcwsrds1, serdes1_base, true);
+#endif
+#ifdef CONFIG_SYS_FSL_SRDS_2
+	do_enabled_lanes_reset(2, cfg_rcwsrds2, serdes2_base, true);
+#endif
+
+	/* For each PLL being reset, and achieved PLL lock set RST_DONE */
+#ifdef CONFIG_SYS_FSL_SRDS_1
+	cfg_tmp = cfg_rcwsrds1 & 0x3;
+	do_pll_reset_done(cfg_tmp, serdes1_base);
+#endif
+#ifdef CONFIG_SYS_FSL_SRDS_2
+	cfg_tmp = cfg_rcwsrds1 & 0xC;
+	cfg_tmp >>= 2;
+	do_pll_reset_done(cfg_tmp, serdes2_base);
+#endif
+	return ret;
+}
+
 void fsl_serdes_init(void)
 {
 #if defined(CONFIG_FSL_MC_ENET) && !defined(CONFIG_SPL_BUILD)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
index 497a4b5..47d89b2 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
@@ -513,23 +513,6 @@  static int setup_core_volt(u32 vdd)
 	return board_setup_core_volt(vdd);
 }
 
-#ifdef CONFIG_SYS_FSL_DDR
-static void ddr_enable_0v9_volt(bool en)
-{
-	struct ccsr_ddr __iomem *ddr = (void *)CONFIG_SYS_FSL_DDR_ADDR;
-	u32 tmp;
-
-	tmp = ddr_in32(&ddr->ddr_cdr1);
-
-	if (en)
-		tmp |= DDR_CDR1_V0PT9_EN;
-	else
-		tmp &= ~DDR_CDR1_V0PT9_EN;
-
-	ddr_out32(&ddr->ddr_cdr1, tmp);
-}
-#endif
-
 int setup_chip_volt(void)
 {
 	int vdd;
@@ -598,6 +581,23 @@  void fsl_lsch2_early_init_f(void)
 }
 #endif
 
+#ifdef CONFIG_SYS_FSL_DDR
+void ddr_enable_0v9_volt(bool en)
+{
+	struct ccsr_ddr __iomem *ddr = (void *)CONFIG_SYS_FSL_DDR_ADDR;
+	u32 tmp;
+
+	tmp = ddr_in32(&ddr->ddr_cdr1);
+
+	if (en)
+		tmp |= DDR_CDR1_V0PT9_EN;
+	else
+		tmp &= ~DDR_CDR1_V0PT9_EN;
+
+	ddr_out32(&ddr->ddr_cdr1, tmp);
+}
+#endif
+
 #ifdef CONFIG_QSPI_AHB_INIT
 /* Enable 4bytes address support and fast read */
 int qspi_ahb_init(void)
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/fsl_serdes.h b/arch/arm/include/asm/arch-fsl-layerscape/fsl_serdes.h
index 12fd6b8..9becdf3 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/fsl_serdes.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/fsl_serdes.h
@@ -164,6 +164,7 @@  void fsl_rgmii_init(void);
 #ifdef CONFIG_FSL_LSCH2
 const char *serdes_clock_to_string(u32 clock);
 int get_serdes_protocol(void);
+#endif
 #ifdef CONFIG_SYS_HAS_SERDES
 /* Get the volt of SVDD in unit mV */
 int get_serdes_volt(void);
@@ -172,6 +173,5 @@  int set_serdes_volt(int svdd);
 /* The target volt of SVDD in unit mV */
 int setup_serdes_volt(u32 svdd);
 #endif
-#endif
 
 #endif /* __FSL_SERDES_H__ */
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
index 957e23b..47e8b5a 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
@@ -387,5 +387,39 @@  struct ccsr_reset {
 	u32 ip_rev2;			/* 0xbfc */
 };
 
+struct ccsr_serdes {
+	struct {
+		u32     rstctl; /* Reset Control Register */
+		u32     pllcr0; /* PLL Control Register 0 */
+		u32     pllcr1; /* PLL Control Register 1 */
+		u32     pllcr2; /* PLL Control Register 2 */
+		u32     pllcr3; /* PLL Control Register 3 */
+		u32     pllcr4; /* PLL Control Register 4 */
+		u32     pllcr5; /* PLL Control Register 5 */
+		u8      res[0x20 - 0x1c];
+	} bank[2];
+	u8      res1[0x90 - 0x40];
+	u32     srdstcalcr;     /* TX Calibration Control */
+	u32     srdstcalcr1;    /* TX Calibration Control1 */
+	u8      res2[0xa0 - 0x98];
+	u32     srdsrcalcr;     /* RX Calibration Control */
+	u32     srdsrcalcr1;    /* RX Calibration Control1 */
+	u8      res3[0xb0 - 0xa8];
+	u32     srdsgr0;        /* General Register 0 */
+	u8      res4[0x800 - 0xb4];
+	struct serdes_lane {
+		u32     gcr0;   /* General Control Register 0 */
+		u32     gcr1;   /* General Control Register 1 */
+		u32     gcr2;   /* General Control Register 2 */
+		u32     ssc0;   /* Speed Switch Control 0 */
+		u32     rec0;   /* Receive Equalization Control 0 */
+		u32     rec1;   /* Receive Equalization Control 1 */
+		u32     tec0;   /* Transmit Equalization Control 0 */
+		u32     ssc1;   /* Speed Switch Control 1 */
+		u8      res1[0x840 - 0x820];
+	} lane[8];
+	u8 res5[0x19fc - 0xa00];
+};
+
 #endif /*__ASSEMBLY__*/
 #endif /* __ARCH_FSL_LSCH3_IMMAP_H_ */
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/soc.h b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
index 247f09e..8c242c0 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
@@ -125,6 +125,7 @@  int setup_chip_volt(void);
 /* Setup core vdd in unit mV */
 int board_setup_core_volt(u32 vdd);
 #endif
+void ddr_enable_0v9_volt(bool en);
 
 void cpu_name(char *name);
 #ifdef CONFIG_SYS_FSL_ERRATUM_A009635