From mboxrd@z Thu Jan 1 00:00:00 1970 From: Piotr Wilczek Date: Mon, 24 Jun 2013 10:44:16 +0200 Subject: [U-Boot] [PATCH V2] drivers:power:max77686: add function to set voltage and mode In-Reply-To: References: <1369140859-22548-1-git-send-email-p.wilczek@samsung.com> Message-ID: <000301ce70b7$033b7f40$09b27dc0$%wilczek@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Minkyu Kang, Thank you for comments. I agree with all of them. Best regards, Piotr Wilczek From: Minkyu Kang [mailto:promsoft at gmail.com] Sent: Friday, June 21, 2013 6:24 PM To: Piotr Wilczek Cc: u-boot at lists.denx.de; Kyungmin Park; Rajeshwari Shinde Subject: Re: [U-Boot] [PATCH V2] drivers:power:max77686: add function to set voltage and mode Dear Piotr Wilczek, On 21 May 2013 21:54, Piotr Wilczek wrote: This patch add new functions to pmic max77686 to set voltage and mode. Signed-off-by: Piotr Wilczek Signed-off-by: Kyungmin Park CC: Minkyu Kang CC: Rajeshwari Shinde Acked-by: Rajeshwari Shinde --- Changes in v2: - changed printf to debug drivers/power/pmic/pmic_max77686.c | 186 ++++++++++++++++++++++++++++++++++++ include/power/max77686_pmic.h | 11 +++ 2 files changed, 197 insertions(+) diff --git a/drivers/power/pmic/pmic_max77686.c b/drivers/power/pmic/pmic_max77686.c index 7fcb4c0..dabd6b6 100644 --- a/drivers/power/pmic/pmic_max77686.c +++ b/drivers/power/pmic/pmic_max77686.c @@ -30,6 +30,192 @@ DECLARE_GLOBAL_DATA_PTR; +static const char max77686_buck_addr[] = { + 0xff, 0x10, 0x12, 0x1c, 0x26, 0x30, 0x32, 0x34, 0x36, 0x38 +}; + +static unsigned int max77686_ldo_volt2hex(int ldo, ulong uV) +{ + unsigned int hex = 0; + const unsigned int max_hex = 0x3f; Is it constant? then please define it. + + switch (ldo) { + case 1: + case 2: + case 6: + case 7: + case 8: + case 15: + hex = (uV - 800000) / 25000; + break; + default: + hex = (uV - 800000) / 50000; + } + + if (0 <= hex && hex <= max_hex) I think, hex >= 0 looks more comfortable. + return hex; + + debug("%s: %ld is wrong voltage value for LDO%d\n", __func__, uV, ldo); + return 0; +} + +int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) +{ + unsigned int val, ret, hex, adr, mask; + + if (ldo < 1 && 26 < ldo) { ditto. + printf("%s: %d is wrong ldo number\n", __func__, ldo); + return -1; + } + + adr = MAX77686_REG_PMIC_LDO1CTRL1 + ldo - 1; + mask = 0x3f; + hex = max77686_ldo_volt2hex(ldo, uV); + + if (!hex) + return -1; + + ret = pmic_reg_read(p, adr, &val); I think.. if you got error while read the register then please return error without writing. + val &= ~mask; + val |= hex; + ret |= pmic_reg_write(p, adr, val); + + return ret; +} + +int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode) +{ + unsigned int val, ret, mask, adr, mode; + + if (ldo < 1 && 26 < ldo) { + printf("%s: %d is wrong ldo number\n", __func__, ldo); + return -1; + } + + adr = MAX77686_REG_PMIC_LDO1CTRL1 + ldo - 1; + + /* mask */ + mask = 0xc0; Is it constant? then please define it. + + /* mode */ + if (opmode == OPMODE_OFF) { + mode = 0x00; + } else if (opmode == OPMODE_STANDBY) { + switch (ldo) { + case 2: + case 6: + case 7: + case 8: + case 10: + case 11: + case 12: + case 14: + case 15: + case 16: + mode = 0x40; + break; + default: + mode = 0xff; + } + } else if (opmode == OPMODE_LPM) { + mode = 0x80; + } else if (opmode == OPMODE_ON) { + mode = 0xc0; + } else { + mode = 0xff; + } switch case? What means 0x40 and 0x80, 0xc0? There are so many magic values in this patch. I'll not mentioned about it anymore. Please fix it globally. + + if (mode == 0xff) { + printf("%s: %d is not supported on LDO%d\n", + __func__, opmode, ldo); + return -1; + } + + ret = pmic_reg_read(p, adr, &val); ditto. + val &= ~mask; + val |= mode; + ret |= pmic_reg_write(p, adr, val); + + return ret; +} + +int max77686_set_buck_mode(struct pmic *p, int buck, char opmode) +{ + unsigned int val, ret, mask, adr, size, mode; + + size = sizeof(max77686_buck_addr) / sizeof(*max77686_buck_addr); ARRAY_SIZE()? + if (buck >= size) { + printf("%s: %d is wrong buck number\n", __func__, buck); + return -1; + } + + adr = max77686_buck_addr[buck]; + + /* mask */ + switch (buck) { + case 2: + case 3: + case 4: + mask = 0x30; + break; + default: + mask = 0x03; + } + + /* mode */ + if (opmode == OPMODE_OFF) { + mode = 0x00; + } else if (opmode == OPMODE_STANDBY) { + switch (buck) { + case 1: + mode = 0x01; + break; + case 2: + case 3: + case 4: + mode = 0x10; + break; + default: + mode = 0xff; + } + } else if (opmode == OPMODE_LPM) { + switch (buck) { + case 2: + case 3: + case 4: + mode = 0x20; + break; + default: + mode = 0xff; + } + } else if (opmode == OPMODE_ON) { + switch (buck) { + case 2: + case 3: + case 4: + mode = 0x30; + break; + default: + mode = 0x03; + } + } else { + mode = 0xff; + } + + if (mode == 0xff) { + printf("%s: %d is not supported on BUCK%d\n", + __func__, opmode, buck); + return -1; + } + + ret = pmic_reg_read(p, adr, &val); + val &= ~mask; + val |= mode; + ret |= pmic_reg_write(p, adr, val); + + return ret; +} + int pmic_init(unsigned char bus) { static const char name[] = "MAX77686_PMIC"; diff --git a/include/power/max77686_pmic.h b/include/power/max77686_pmic.h index fdc7ca9..ce58c02 100644 --- a/include/power/max77686_pmic.h +++ b/include/power/max77686_pmic.h @@ -155,6 +155,17 @@ enum { EN_LDO = (0x3 << 6), }; +enum { + OPMODE_OFF = 0, + OPMODE_STANDBY, + OPMODE_LPM, + OPMODE_ON, +}; + +int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV); +int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode); +int max77686_set_buck_mode(struct pmic *p, int buck, char opmode); + /* Buck1 1 volt value */ #define MAX77686_BUCK1OUT_1V 0x5 #define MAX77686_BUCK1CTRL_EN (3 << 0) -- 1.7.9.5 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot Thanks, Minkyu Kang. -- from. prom. www.promsoft.net