[U-Boot] [Patch v2] ls1088a: add VID support for QDS and RDB platforms

York Sun york.sun at nxp.com
Tue Jun 13 20:20:24 UTC 2017


On 04/27/2017 07:01 AM, Amrita Kumari wrote:
> This patch adds the support for VID on LS1088AQDS and LS1088ARDB systems.
> It reads the fusesr register and changes the VDD accordingly by adjusting the
> voltage via LTC3882 regulator.
> This patch also takes care of the special case of 0.9V VDD. In that case,
> it also changes the SERDES voltage by disabling the SERDES, changing the SVDD
> and then re-enabling SERDES.

It is not clear when 0.9V VDD is set. Please explain in the comment.

> 
> Signed-off-by: Raghav Dogra <raghav.dogra at nxp.com>
> Signed-off-by: Ashish Kumar <Ashish.Kumar at nxp.com>
> Signed-off-by: Amrita Kumari <amrita.kumari at nxp.com>
> ---
> v2:
> checkpatch errors fixed
> 
>   .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |  34 ++
>   board/freescale/ls1088a/ls1088a.c                  | 544 +++++++++++++++++++++
>   include/configs/ls1088aqds.h                       |  18 +
>   include/configs/ls1088ardb.h                       |  18 +
>   4 files changed, 614 insertions(+)
> 
> 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 e181ef2..17161a0 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> @@ -361,5 +361,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/board/freescale/ls1088a/ls1088a.c b/board/freescale/ls1088a/ls1088a.c
> index 034ef54..c57d112 100644
> --- a/board/freescale/ls1088a/ls1088a.c
> +++ b/board/freescale/ls1088a/ls1088a.c
> @@ -20,6 +20,7 @@
>   
>   #include "../common/qixis.h"
>   #include "ls1088a_qixis.h"
> +#include <fsl_immap.h>
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> @@ -292,6 +293,547 @@ void board_retimer_init(void)
>   	/*return the default channel*/
>   	select_i2c_ch_pca9547(I2C_MUX_CH_DEFAULT);
>   }
> +static int i2c_multiplexer_select_vid_channel(u8 channel)
> +{
> +	return select_i2c_ch_pca9547(channel);
> +}
> +
> +/* this function set the DDR bit in case of 0.9V VDD */
> +static void ddr_enable_0_9_vdd(void)
> +{
> +	struct ccsr_ddr *ddr1;
> +	ddr1  = (void *)(CONFIG_SYS_FSL_DDR_ADDR);
> +	u32 tmp;
> +
> +	tmp = in_le32(&ddr1->ddr_cdr1);
> +
> +	tmp |= DDR_CDR1_V0PT9_EN;
> +	out_le32(&ddr1->ddr_cdr1, tmp);
> +}
> +
> +/* read the current value of the LTC Regulator Voltage */
> +static inline int read_voltage(void)
> +{
> +	int  ret, vcode = 0;
> +
> +	/* select the PAGE 0 using PMBus commands PAGE for VDD*/
> +	ret = i2c_write(I2C_VOL_MONITOR_ADDR,
> +		PMBUS_CMD_PAGE, 1, PWM_CHANNEL0, 1);

Please align the second line to the parenthesis. Please examine the rest 
of your code for the same.

> +	if (ret) {
> +			printf("VID: failed to select VDD Page 0\n");
> +			return ret;

These two lines seem to have too deep indentation.


> +	}
> +	/*read the output voltage using PMBus command READ_VOUT*/
> +
> +	ret = i2c_read(I2C_VOL_MONITOR_ADDR,
> +		PMBUS_CMD_READ_VOUT, 1, (void *)&vcode, 2);
> +	if (ret) {
> +		printf("VID: failed to read the volatge\n");
> +		return ret;
> +	}
> +	return vcode;

Put a blank line before return. Please examine the rest of your code for 
the same.

> +}
> +
> +/* read the current value(SVDD) of the LTM Regulator Voltage */
> +static inline int read_svdd_LTM4675(void)
> +{
> +	int  ret, vcode = 0;
> +
> +	/* select the PAGE 0 using PMBus commands PAGE for VDD*/
> +	ret = i2c_write(I2C_SVDD_MONITOR_ADDR,
> +		PMBUS_CMD_PAGE, 1, PWM_CHANNEL0, 1);
> +	if (ret) {
> +			printf("VID: failed to select VDD Page 0\n");
> +			return ret;
> +	}
> +
> +	/*read the output voltage using PMBus command READ_VOUT*/
> +	ret = i2c_read(I2C_SVDD_MONITOR_ADDR,
> +		PMBUS_CMD_READ_VOUT, 1, (void *)&vcode, 2);
> +	if (ret) {
> +		printf("VID: failed to read the volatge\n");
> +		return ret;
> +	}
> +	return vcode;
> +}
> +
> +
> +/* returns the Lower byte of the vdd code */
> +static u8 get_LSB(int vdd)
> +{
> +	u8 *lower = (u8 *)&vdd;

You are presuming the endianness. Why not simply return (vdd & 0xff)?

> +	return *(lower);
> +}
> +
> +/*returns the Upper byte of the vdd code*/

Put a space after /* and before */. Please examine the rest of your code.

> +static u8 get_MSB(int vdd)
> +{
> +	u8 *lower = (u8 *)&vdd;
> +	++lower;
> +	return *(lower);
> +}
> +
> +
> +/* this function sets the 5-byte buffer which needs to be sent following the
> + * PMBus command PAGE_PLUS_WRITE
> + */
> +static void set_buffer_page_plus_write(u8 *buff, int vid)
> +{
> +	buff[0] = 0x04;
> +

Get rid of the extra white lines.

> +	buff[1] = PWM_CHANNEL0;
> +
> +	buff[2] = PMBUS_CMD_VOUT_COMMAND;
> +
> +	buff[3] = get_LSB(vid);
> +
> +	buff[4] = get_MSB(vid);
> +}
> +/* this function sets the VDD and returns the value set */
> +static int set_voltage_to_LTC(int vid)
> +{
> +	int ret, vdd_last;
> +	u8 buff[5];
> +
> +	/*number of bytes of the rest of the package*/
> +	set_buffer_page_plus_write((u8 *)&buff, vid);

Do you have to cast? It is better to use a structure to avoid writing 
over the boundary.

> +
> +	/*write the desired voltage code to the regulator*/
> +	ret = i2c_write(I2C_VOL_MONITOR_ADDR,
> +		PMBUS_CMD_PAGE_PLUS_WRITE, 1, (void *)&buff, 5);
> +	if (ret) {
> +		printf("VID: I2C failed to write to the volatge regulator\n");
> +		return -1;
> +	}
> +
> +	/*wait for the volatge to get to the desired value*/
> +
> +	do {
> +		vdd_last = read_voltage();
> +		if (vdd_last < 0) {
> +			printf("VID: Couldn't read sensor abort VID adjust\n");
> +			return -1;
> +		}
> +	} while (vdd_last != vid);

How accurate is the voltage measurement? Will the vdd_last be the exact 
the same as vid? How long do you have to wait for the voltage to be 
stable? How many times do you run this loop? Have you considered to add 
delay to reduce the polling?

> +
> +	return vdd_last;
> +}
> +#ifdef CONFIG_TARGET_LS1088AQDS
> +static int change_0_9_svddqds(int svdd)
> +{
> +	int ret, vdd_last;
> +	u8 buff[5];
> +
> +	/*number of bytes of the rest of the package*/
> +	set_buffer_page_plus_write((u8 *)&buff, svdd);
> +
> +	/*write the desired voltage code to the SVDD regulator*/
> +	ret = i2c_write(I2C_SVDD_MONITOR_ADDR,
> +		PMBUS_CMD_PAGE_PLUS_WRITE, 1, (void *)&buff, 5);
> +	if (ret) {
> +		printf("VID: I2C failed to write to the volatge regulator\n");
> +		return -1;
> +	}
> +
> +	/*wait for the volatge to get to the desired value*/
> +	do {
> +		vdd_last = read_svdd_LTM4675();
> +		if (vdd_last < 0) {
> +			printf("VID: Couldn't read sensor abort VID adjust\n");
> +			return -1;
> +		}
> +	} while (vdd_last != svdd);
> +
> +	return 1;
> +}
> +#else
> +static int change_0_9_svddrdb(int svdd)

I guess you could use the same function name for both RDB and QDS. Only 
one would be compiled according to the ifdef.

> +{
> +	int ret;
> +	u8 brdcfg4;
> +
> +	printf("SVDD changing of RDB\n");
> +
> +	/*read the BRDCFG54 via CLPD*/
> +	ret = i2c_read(CONFIG_SYS_I2C_FPGA_ADDR,
> +		QIXIS_BRDCFG4_OFFSET, 1, (void *)&brdcfg4, 1);
> +	if (ret) {
> +		printf("VID: I2C failed to read the CPLD BRDCFG4\n");
> +		return -1;
> +	}
> +
> +	brdcfg4 = brdcfg4 | 0x08;
> +
> +	/* write to the BRDCFG4 */
> +	ret = i2c_write(CONFIG_SYS_I2C_FPGA_ADDR,
> +		QIXIS_BRDCFG4_OFFSET, 1, (void *)&brdcfg4, 1);
> +	if (ret) {
> +		debug("VID: I2C failed to set the SVDD CPLD BRDCFG4\n");
> +		return -1;
> +	}
> +
> +	/*wait for the volatge to get to the desired value*/
> +	udelay(10000);
> +
> +	return 1;
> +}
> +#endif
> +
> +/* this function disables the SERDES, changes the SVDD Voltage and enables it*/
> +int switch_svdd(u32 svdd)
> +{
> +	struct ccsr_gur *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
> +	struct ccsr_serdes *serdes1_base, *serdes2_base;

We normally put __iomem for these registers.

> +	u32 cfg_rcwsrds1 = gur_in32(&gur->rcwsr[FSL_CHASSIS3_SRDS1_REGSR - 1]);
> +	u32 cfg_rcwsrds2 = gur_in32(&gur->rcwsr[FSL_CHASSIS3_SRDS2_REGSR - 1]);
> +	u32 cfg_tmp, reg = 0;
> +	int ret = 1;
> +	int i;
> +
> +	/* Only support switch SVDD to 900mV */
> +	if (svdd != 0x0E66)
> +		return -1;

What's the coding for the voltage? I failed to see how 0xe66 matches 
900mv. I saw the vdd array below, but didn't understand.

> +
> +	serdes1_base = (void *)CONFIG_SYS_FSL_LSCH3_SERDES_ADDR;
> +	serdes2_base =  (void *)(CONFIG_SYS_FSL_LSCH3_SERDES_ADDR + 0x10000);
> +
> +	/* Put the all enabled lanes in reset */
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	cfg_tmp = cfg_rcwsrds1 & FSL_CHASSIS3_SRDS1_PRTCL_MASK;
> +	cfg_tmp >>= 16;
> +
> +	for (i = 0; i < 4 && cfg_tmp & (0xf << (3 - i)); i++) {
> +		reg = in_le32(&serdes1_base->lane[i].gcr0);
> +		reg &= 0xFF9FFFFF;

Prefer to use macros for mask.

> +		out_le32(&serdes1_base->lane[i].gcr0, reg);
> +		reg = in_le32(&serdes1_base->lane[i].gcr0);
> +	}
> +#endif
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	cfg_tmp = cfg_rcwsrds2 & FSL_CHASSIS3_SRDS2_PRTCL_MASK;
> +
> +	for (i = 0; i < 4 && cfg_tmp & (0xf << (3 - i)); i++) {
> +		reg = in_le32(&serdes2_base->lane[i].gcr0);
> +		reg &= 0xFF9FFFFF;
> +		out_le32(&serdes2_base->lane[i].gcr0, reg);
> +		reg = in_le32(&serdes2_base->lane[i].gcr0);
> +	}
> +#endif
> +
> +	/* Put the all enabled PLL in reset */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> +	for (i = 0; i < 2 && !(cfg_tmp & (0x1 << (1 - i))); i++) {
> +		reg = in_le32(&serdes1_base->bank[i].rstctl);
> +		reg &= 0xFFFFFFBF;
> +		reg |= 0x10000000;

Prefer to use macros for bit setting.

> +		out_le32(&serdes1_base->bank[i].rstctl, reg);
> +	}
> +	udelay(1);
> +
> +	reg = in_le32(&serdes1_base->bank[i].rstctl);
> +	reg &= 0xFFFFFF1F;
> +	out_le32(&serdes1_base->bank[i].rstctl, reg);
> +#endif
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +
> +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> +	cfg_tmp >>= 2;
> +	for (i = 0; i < 2 && !(cfg_tmp & (0x1 << (1 - i))); i++) {
> +		reg = in_le32(&serdes2_base->bank[i].rstctl);
> +		reg &= 0xFFFFFFBF;
> +		reg |= 0x10000000;
> +		out_le32(&serdes2_base->bank[i].rstctl, reg);
> +	}
> +	udelay(1);
> +
> +	reg = in_le32(&serdes2_base->bank[i].rstctl);
> +	reg &= 0xFFFFFF1F;
> +	out_le32(&serdes2_base->bank[i].rstctl, reg);
> +
> +#endif
> +
> +	/* Put the Rx/Tx calibration into reset */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	reg = in_le32(&serdes1_base->srdstcalcr);
> +	reg &= 0xF7FFFFFF;
> +	out_le32(&serdes1_base->srdstcalcr, reg);
> +	reg = in_le32(&serdes1_base->srdsrcalcr);
> +	reg &= 0xF7FFFFFF;
> +	out_le32(&serdes1_base->srdsrcalcr, reg);
> +#endif
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	reg = in_le32(&serdes2_base->srdstcalcr);
> +	reg &= 0xF7FFFFFF;
> +	out_le32(&serdes2_base->srdstcalcr, reg);
> +	reg = in_le32(&serdes2_base->srdsrcalcr);
> +	reg &= 0xF7FFFFFF;
> +	out_le32(&serdes2_base->srdsrcalcr, reg);
> +#endif
> +
> +#ifdef CONFIG_TARGET_LS1088AQDS
> +	ret = change_0_9_svddqds(svdd);
> +	if (ret < 0) {
> +		printf("could not change SVDD\n");
> +		ret = -1;
> +	}
> +#else
> +	ret = change_0_9_svddrdb(svdd);
> +	if (ret < 0) {
> +		printf("could not change SVDD\n");
> +		ret = -1;
> +	}
> +#endif
> +
> +	/* For each PLL that’s not disabled via RCW enable the SERDES */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> +	for (i = 0; i < 2 && !(cfg_tmp & (0x1 << (1 - i))); i++) {
> +		reg = in_le32(&serdes1_base->bank[i].rstctl);
> +		reg |= 0x00000020;
> +		out_le32(&serdes1_base->bank[i].rstctl, reg);
> +		udelay(1);
> +
> +		reg = in_le32(&serdes1_base->bank[i].rstctl);
> +		reg |= 0x00000080;
> +		out_le32(&serdes1_base->bank[i].rstctl, reg);
> +		udelay(1);
> +		/* Take the Rx/Tx calibration out of reset */
> +		if (!(cfg_tmp == 0x3 && i == 1)) {
> +			udelay(1);
> +			reg = in_le32(&serdes1_base->srdstcalcr);
> +			reg |= 0x08000000;
> +			out_le32(&serdes1_base->srdstcalcr, reg);
> +			reg = in_le32(&serdes1_base->srdsrcalcr);
> +			reg |= 0x08000000;
> +			out_le32(&serdes1_base->srdsrcalcr, reg);
> +		}
> +	udelay(1);
> +	}
> +#endif
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> +	cfg_tmp >>= 2;
> +	for (i = 0; i < 2 && !(cfg_tmp & (0x1 << (1 - i))); i++) {
> +		reg = in_le32(&serdes2_base->bank[i].rstctl);
> +		reg |= 0x00000020;
> +		out_le32(&serdes2_base->bank[i].rstctl, reg);
> +		udelay(1);
> +
> +		reg = in_le32(&serdes2_base->bank[i].rstctl);
> +		reg |= 0x00000080;
> +		out_le32(&serdes2_base->bank[i].rstctl, reg);
> +		udelay(1);
> +
> +		/* Take the Rx/Tx calibration out of reset */
> +		if (!(cfg_tmp == 0x3 && i == 1)) {
> +			udelay(1);
> +			reg = in_le32(&serdes2_base->srdstcalcr);
> +			reg |= 0x08000000;
> +			out_le32(&serdes2_base->srdstcalcr, reg);
> +			reg = in_le32(&serdes2_base->srdsrcalcr);
> +			reg |= 0x08000000;
> +			out_le32(&serdes2_base->srdsrcalcr, reg);
> +		}
> +
> +		udelay(1);
> +	}
> +#endif
> +
> +	/* Wait for at lesat 625us to ensure the PLLs being reset are locked */
> +	udelay(800);
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> +	for (i = 0; i < 2 && !(cfg_tmp & (0x1 << (1 - i))); i++) {
> +		/* if the PLL is not locked, set RST_ERR */
> +		reg = in_le32(&serdes1_base->bank[i].pllcr0);
> +		if (!((reg >> 23) & 0x1)) {
> +			reg = in_le32(&serdes1_base->bank[i].rstctl);
> +			reg |= 0x20000000;
> +			out_le32(&serdes1_base->bank[i].rstctl, reg);
> +		} else {
> +			udelay(1);
> +			reg = in_le32(&serdes1_base->bank[i].rstctl);
> +			reg &= 0xFFFFFFEF;
> +			reg |= 0x00000040;
> +			out_le32(&serdes1_base->bank[i].rstctl, reg);
> +			udelay(1);
> +		}
> +	}
> +#endif
> +
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> +	cfg_tmp >>= 2;
> +
> +	for (i = 0; i < 2 && !(cfg_tmp & (0x1 << (1 - i))); i++) {
> +		reg = in_le32(&serdes2_base->bank[i].pllcr0);
> +		if (!((reg >> 23) & 0x1)) {
> +			reg = in_le32(&serdes2_base->bank[i].rstctl);
> +			reg |= 0x20000000;
> +			out_le32(&serdes2_base->bank[i].rstctl, reg);
> +		} else {
> +			udelay(1);
> +			reg = in_le32(&serdes2_base->bank[i].rstctl);
> +			reg &= 0xFFFFFFEF;
> +			reg |= 0x00000040;
> +			out_le32(&serdes2_base->bank[i].rstctl, reg);
> +			udelay(1);
> +		}
> +	}
> +#endif
> +	/* Take the all enabled lanes out of reset */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	cfg_tmp = cfg_rcwsrds1 & FSL_CHASSIS3_SRDS1_PRTCL_MASK;
> +	cfg_tmp >>= 16;
> +
> +	for (i = 0; i < 4 && cfg_tmp & (0xf << (3 - i)); i++) {
> +		reg = in_le32(&serdes1_base->lane[i].gcr0);
> +		reg |= 0x00600000;
> +		out_le32(&serdes1_base->lane[i].gcr0, reg);
> +	}
> +#endif
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	cfg_tmp = cfg_rcwsrds2 & FSL_CHASSIS3_SRDS2_PRTCL_MASK;
> +
> +	for (i = 0; i < 4 && cfg_tmp & (0xf << (3 - i)); i++) {
> +		reg = in_le32(&serdes2_base->lane[i].gcr0);
> +		reg |= 0x00600000;
> +		out_le32(&serdes2_base->lane[i].gcr0, reg);
> +	}
> +#endif
> +
> +	/* For each PLL being reset, and achieved PLL lock set RST_DONE */
> +#ifdef CONFIG_SYS_FSL_SRDS_1
> +	cfg_tmp = cfg_rcwsrds1 & 0x3;
> +	for (i = 0; i < 2; i++) {
> +		reg = in_le32(&serdes1_base->bank[i].pllcr0);
> +		if (!(cfg_tmp & (0x1 << (1 - i))) && ((reg >> 23) & 0x1)) {
> +			reg = in_le32(&serdes1_base->bank[i].rstctl);
> +			reg |= 0x40000000;
> +			out_le32(&serdes1_base->bank[i].rstctl, reg);
> +		}
> +	}
> +#endif
> +#ifdef CONFIG_SYS_FSL_SRDS_2
> +	cfg_tmp = cfg_rcwsrds1 & 0xC;
> +	cfg_tmp >>= 2;
> +
> +	for (i = 0; i < 2; i++) {
> +		reg = in_le32(&serdes2_base->bank[i].pllcr0);
> +		if (!(cfg_tmp & (0x1 << (1 - i))) && ((reg >> 23) & 0x1)) {
> +			reg = in_le32(&serdes2_base->bank[i].rstctl);
> +			reg |= 0x40000000;
> +			out_le32(&serdes2_base->bank[i].rstctl, reg);
> +		}
> +	}
> +#endif
> +
> +	return 1;
> +}
> +
> +static int adjust_vdd(ulong vdd_override)
> +{
> +	int re_enable = disable_interrupts();
> +	struct ccsr_gur *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
> +	u32 fusesr;
> +	u8 vid;
> +	int vdd_target, vdd_last;
> +	int ret;
> +	static const uint16_t vdd[32] = {
> +		0,		/* unused */
> +		0x0FCC,		/* 0.9875V */
> +		0x0F99,		/* 0.9750V */
> +		9625,
> +		0x0F33,
> +		9375,
> +		9250,
> +		9125,
> +		0x0E66,		/* 0.9000V */
> +		8875,
> +		8750,
> +		8625,
> +		8500,
> +		8375,
> +		8250,
> +		8125,
> +		0x1000,		/* 1.0000V */
> +		0x1033,		/* 1.0125V */
> +		0x1066,		/* 1.0250V */
> +		10375,
> +		10500,
> +		10625,
> +		10750,
> +		10875,
> +		11000,
> +		0,      /* reserved */

Why are you mixing decimal and hexadecimal? I don't understand the 
coding here without consulting the SoC manual. Please put some comments 
here.

> +	};
> +
> +	/*select the channel on which LTC3882 voltage regulator is present*/
> +	ret = i2c_multiplexer_select_vid_channel(I2C_MUX_CH_VOL_MONITOR);
> +	if (ret) {
> +		debug("VID: I2C failed to switch channel\n");
> +		ret = -1;
> +		goto exit;
> +	}
> +
> +	/* get the voltage ID from fuse status register */
> +	fusesr = in_le32(&gur->dcfg_fusesr);
> +	debug("FUSESR register read %x\n", fusesr);
> +	if (fusesr == 0) {
> +		ret = -1;
> +		goto exit;
> +	}
> +	/*calculate the VID */
> +	vid = (fusesr >> FSL_CHASSIS3_DCFG_FUSESR_ALTVID_SHIFT) &
> +		FSL_CHASSIS3_DCFG_FUSESR_ALTVID_MASK;
> +	if ((vid == 0) || (vid == FSL_CHASSIS3_DCFG_FUSESR_ALTVID_MASK)) {
> +		vid = (fusesr >> FSL_CHASSIS3_DCFG_FUSESR_VID_SHIFT) &
> +			FSL_CHASSIS3_DCFG_FUSESR_VID_MASK;
> +	}
> +
> +
> +	debug("VID = %d\n", vid);
> +	vdd_target = vdd[vid];
> +	debug("vdd_target read %d\n", vdd_target);
> +/*
> + * Read voltage monitor to check real voltage.
> + */

This is not a multiple line comments. Put it in a single line.

Do not post a patch to internal and upstream lists in the same email.

York


More information about the U-Boot mailing list