From mboxrd@z Thu Jan 1 00:00:00 1970 From: Li Ye-B37916 Date: Wed, 5 Nov 2014 17:34:50 +0800 Subject: [U-Boot] [PATCH 4/4] imx: mx6: Set Pfuze mode to decrease power number for DSM In-Reply-To: <5458F755.7060008@samsung.com> References: <1410340097-19804-1-git-send-email-B37916@freescale.com> <1410340097-19804-4-git-send-email-B37916@freescale.com> <5458F755.7060008@samsung.com> Message-ID: <5459EF3A.6080008@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Przemyslaw, On 11/4/2014 11:57 PM, Przemyslaw Marczak wrote: > Hello Ye Li, > > On 09/10/2014 11:08 AM, Ye.Li wrote: >> Set all switches APS mode in normal and PFM mode in standby. So when >> mx6 entering DSM mode, the power number can be decreased. There is >> no impact for mx6 in run mode. >> >> Changes for boards: >> -mx6 sabreauto >> -mx6 sabresd >> -mx6slevk >> -mx6sxsabresd >> >> Signed-off-by: Ye.Li >> --- >> board/freescale/mx6qsabreauto/mx6qsabreauto.c | 36 +++++++++++++++++++++++++ >> board/freescale/mx6sabresd/mx6sabresd.c | 36 +++++++++++++++++++++++++ >> board/freescale/mx6slevk/mx6slevk.c | 36 +++++++++++++++++++++++++ >> board/freescale/mx6sxsabresd/mx6sxsabresd.c | 36 +++++++++++++++++++++++++ >> 4 files changed, 144 insertions(+), 0 deletions(-) >> >> diff --git a/board/freescale/mx6qsabreauto/mx6qsabreauto.c b/board/freescale/mx6qsabreauto/mx6qsabreauto.c >> index 76b024b..9e79915 100644 >> --- a/board/freescale/mx6qsabreauto/mx6qsabreauto.c >> +++ b/board/freescale/mx6qsabreauto/mx6qsabreauto.c >> @@ -263,6 +263,37 @@ int board_init(void) >> return 0; >> } >> >> +/* set all switches APS in normal and PFM mode in standby */ > > I think that the "chip" is quite misleading. > Just use chipid and check the real value instead of using it as bool. > And the PF100 DEVICEID should be defined in pf100 header. > Accept. Will change it in next version. >> +static int pfuze_setup_mode(struct pmic *p, int chip) >> +{ >> + unsigned char offset, i, switch_num, value; >> + >> + if (!chip) { >> + /* pfuze100 */ >> + switch_num = 6; >> + offset = 0x31; >> + } else { >> + /* pfuze200 */ >> + switch_num = 4; >> + offset = 0x38; >> + } >> + >> + value = 0xc; > > If you change "magic" values (e.g. 0xc) with proper defines in pmic header, then it could be reused in the future and the code will be more readable. > Ok. >> + if (pmic_reg_write(p, 0x23, value)) { >> + printf("Set SW1AB mode error!\n"); >> + return -1; >> + } >> + >> + for (i = 0; i < switch_num - 1; i++) { >> + if (pmic_reg_write(p, offset + i * 7, value)) { >> + printf("Set switch%x mode error!\n", offset); >> + return -1; >> + } >> + } >> + >> + return 0; >> +} > > The function above is duplicated few times in each board code. > Why not make it common and just one? > I already have made it as a common function in v4. The patch you reviewed seems not the latest one. I will address your other comments and send out v5. >> + >> static int pfuze_init(void) >> { >> struct pmic *p; >> @@ -281,6 +312,11 @@ static int pfuze_init(void) >> pmic_reg_read(p, PFUZE100_DEVICEID, ®); >> printf("PMIC: PFUZE100 ID=0x%02x\n", reg); >> >> + if (pfuze_setup_mode(p, (reg & 0xf))) { >> + printf("setup pfuze mode error!\n"); >> + return -1; >> + } >> + >> /* Set SW1AB stanby volage to 0.975V */ >> pmic_reg_read(p, PFUZE100_SW1ABSTBY, ®); >> reg &= ~0x3f; >> diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c >> index 72d6562..810fe13 100644 >> --- a/board/freescale/mx6sabresd/mx6sabresd.c >> +++ b/board/freescale/mx6sabresd/mx6sabresd.c >> @@ -456,6 +456,37 @@ int board_init(void) >> return 0; >> } >> >> +/* set all switches APS in normal and PFM mode in standby */ >> +static int pfuze_setup_mode(struct pmic *p, int chip) >> +{ >> + unsigned char offset, i, switch_num, value; >> + >> + if (!chip) { >> + /* pfuze100 */ >> + switch_num = 6; >> + offset = 0x31; >> + } else { >> + /* pfuze200 */ >> + switch_num = 4; >> + offset = 0x38; >> + } >> + >> + value = 0xc; >> + if (pmic_reg_write(p, 0x23, value)) { >> + printf("Set SW1AB mode error!\n"); >> + return -1; >> + } >> + >> + for (i = 0; i < switch_num - 1; i++) { >> + if (pmic_reg_write(p, offset + i * 7, value)) { >> + printf("Set switch%x mode error!\n", offset); >> + return -1; >> + } >> + } >> + >> + return 0; >> +} >> + >> static int pfuze_init(void) >> { >> struct pmic *p; >> @@ -475,6 +506,11 @@ static int pfuze_init(void) >> printf("PMIC: PFUZE%s ID=0x%02x\n", >> ((reg & 0xf) == 0) ? "100" : "200", reg); >> >> + if (pfuze_setup_mode(p, (reg & 0xf))) { >> + printf("setup pfuze mode error!\n"); >> + return -1; >> + } >> + >> /* Increase VGEN3 from 2.5 to 2.8V */ >> pmic_reg_read(p, PFUZE100_VGEN3VOL, ®); >> reg &= ~0xf; >> diff --git a/board/freescale/mx6slevk/mx6slevk.c b/board/freescale/mx6slevk/mx6slevk.c >> index 8b6a79c..fe5e37d 100644 >> --- a/board/freescale/mx6slevk/mx6slevk.c >> +++ b/board/freescale/mx6slevk/mx6slevk.c >> @@ -195,6 +195,37 @@ int board_init(void) >> return 0; >> } >> >> +/* set all switches APS in normal and PFM mode in standby */ >> +static int pfuze_setup_mode(struct pmic *p, int chip) >> +{ >> + unsigned char offset, i, switch_num, value; >> + >> + if (!chip) { >> + /* pfuze100 */ >> + switch_num = 6; >> + offset = 0x31; >> + } else { >> + /* pfuze200 */ >> + switch_num = 4; >> + offset = 0x38; >> + } >> + >> + value = 0xc; >> + if (pmic_reg_write(p, 0x23, value)) { >> + printf("Set SW1AB mode error!\n"); >> + return -1; >> + } >> + >> + for (i = 0; i < switch_num - 1; i++) { >> + if (pmic_reg_write(p, offset + i * 7, value)) { >> + printf("Set switch%x mode error!\n", offset); >> + return -1; >> + } >> + } >> + >> + return 0; >> +} >> + >> static int pfuze_init(void) >> { >> struct pmic *p; >> @@ -214,6 +245,11 @@ static int pfuze_init(void) >> printf("PMIC: PFUZE%s ID=0x%02x\n", >> ((reg & 0xf) == 0) ? "100" : "200", reg); >> >> + if (pfuze_setup_mode(p, (reg & 0xf))) { >> + printf("setup pfuze mode error!\n"); >> + return -1; >> + } >> + >> /* Set SW1AB stanby volage to 0.975V */ >> pmic_reg_read(p, PFUZE100_SW1ABSTBY, ®); >> reg &= ~0x3f; >> diff --git a/board/freescale/mx6sxsabresd/mx6sxsabresd.c b/board/freescale/mx6sxsabresd/mx6sxsabresd.c >> index 80d2d99..d6a33cd 100644 >> --- a/board/freescale/mx6sxsabresd/mx6sxsabresd.c >> +++ b/board/freescale/mx6sxsabresd/mx6sxsabresd.c >> @@ -170,6 +170,37 @@ struct i2c_pads_info i2c_pad_info1 = { >> }, >> }; >> >> +/* set all switches APS in normal and PFM mode in standby */ >> +static int pfuze_setup_mode(struct pmic *p, int chip) >> +{ >> + unsigned char offset, i, switch_num, value; >> + >> + if (!chip) { >> + /* pfuze100 */ >> + switch_num = 6; >> + offset = 0x31; >> + } else { >> + /* pfuze200 */ >> + switch_num = 4; >> + offset = 0x38; >> + } >> + >> + value = 0xc; >> + if (pmic_reg_write(p, 0x23, value)) { >> + printf("Set SW1AB mode error!\n"); >> + return -1; >> + } >> + >> + for (i = 0; i < switch_num - 1; i++) { >> + if (pmic_reg_write(p, offset + i * 7, value)) { >> + printf("Set switch%x mode error!\n", offset); >> + return -1; >> + } >> + } >> + >> + return 0; >> +} >> + >> static int pfuze_init(void) >> { >> struct pmic *p; >> @@ -188,6 +219,11 @@ static int pfuze_init(void) >> pmic_reg_read(p, PFUZE100_DEVICEID, ®); >> printf("PMIC: PFUZE100 ID=0x%02x\n", reg); >> >> + if (pfuze_setup_mode(p, (reg & 0xf))) { >> + printf("setup pfuze mode error!\n"); >> + return -1; >> + } >> + >> /* Set SW1AB standby voltage to 0.975V */ >> pmic_reg_read(p, PFUZE100_SW1ABSTBY, ®); >> reg &= ~0x3f; >> > > And as Fabio wrote, please return real error values or use errno instead of "-1". > Have fixed it in v4 patch. > Best regards, Best regards, Ye Li