[U-Boot] [PATCH v5 1/7] armv8: lsch3: Add serdes and DDR voltage setup
York Sun
york.sun at nxp.com
Mon Nov 13 17:03:28 UTC 2017
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 at nxp.com>
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat at 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
More information about the U-Boot
mailing list