Dear Piotr Wilczek,
On 21 May 2013 21:54, Piotr Wilczek <p.wilc...@samsung.com> wrote:
> This patch add new functions to pmic max77686 to set voltage and mode.
>
> Signed-off-by: Piotr Wilczek <p.wilc...@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> CC: Minkyu Kang <mk7.k...@samsung.com>
> CC: Rajeshwari Shinde <rajeshwar...@samsung.com>
>
> Acked-by: Rajeshwari Shinde <rajeshwar...@samsung.com>
> ---
> 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
