[U-Boot] [PATCH] PMIC: MAX77686: Add support for MAX77686

Rajeshwari Birje rajeshwari.birje at gmail.com
Wed May 23 07:27:42 CEST 2012


Hi Lukasz Majewski,

Thank you for the coments.

On Tue, May 22, 2012 at 12:20 PM, Lukasz Majewski
<l.majewski at samsung.com> wrote:
> Hi Rajeshwari,
>
>> This patch adds driver and register definitions for PMIC chip
>> MAX77686.
>
> Why don't use the PMIC framework? Please look into drivers/misc/pmic_*
> for examples.
Will implement the same and resend the patch.
>
> For now there is a support for MAX899{7|8} chips for Samsung platforms.
>
>>
>> Signed-off-by: Alim Akhtar <alim.akhtar at samsung.com>
>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s at samsung.com>
>> ---
>>  drivers/power/Makefile   |    1 +
>>  drivers/power/max77686.c |  225
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> include/max77686.h       |  115 +++++++++++++++++++++++ 3 files
>> changed, 341 insertions(+), 0 deletions(-) create mode 100644
>> drivers/power/max77686.c create mode 100644 include/max77686.h
>>
>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
>> index 6bf388c..b4ffc1d 100644
>> --- a/drivers/power/Makefile
>> +++ b/drivers/power/Makefile
>> @@ -26,6 +26,7 @@ include $(TOPDIR)/config.mk
>>  LIB  := $(obj)libpower.o
>>
>>  COBJS-$(CONFIG_FTPMU010_POWER)       += ftpmu010.o
>> +COBJS-$(CONFIG_MAX77686_POWER)       += max77686.o
>>  COBJS-$(CONFIG_TPS6586X_POWER)       += tps6586x.o
>>  COBJS-$(CONFIG_TWL4030_POWER)        += twl4030.o
>>  COBJS-$(CONFIG_TWL6030_POWER)        += twl6030.o
>> diff --git a/drivers/power/max77686.c b/drivers/power/max77686.c
>> new file mode 100644
>> index 0000000..cf58611
>> --- /dev/null
>> +++ b/drivers/power/max77686.c
>> @@ -0,0 +1,225 @@
>> +/*
>> + * Copyright (C) 2012 Samsung Electronics
>> + * Alim Akhtar <alim.akhtar at samsung.com>
>> + *
>> + * See file CREDITS for list of people who contributed to this
>> + * project.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +
>> +#include <common.h>
>> +#include <i2c.h>
>> +#include <max77686.h>
>> +
>> +/*
>> + * Max77686 parameters values
>> + * see max77686.h for parameters details
>> + */
>> +struct max77686_para max77686_param[] = {/*{regnum, vol_addr,
>> vol_bitpos,
>> +     vol_bitmask, reg_enaddr, reg_enbitpos, reg_enbitmask,
>> reg_enbiton,
>> +     reg_enbitoff, vol_min, vol_div}*/
>> +     {PMIC_BUCK1, 0x11, 0x0, 0x3F, 0x10, 0x0, 0x3, 0x3, 0x0, 750,
>> 50000},
>> +     {PMIC_BUCK2, 0x14, 0x0, 0xFF, 0x12, 0x4, 0x3, 0x1, 0x0, 600,
>> 12500},
>> +     {PMIC_BUCK3, 0x1E, 0x0, 0xFF, 0x1C, 0x4, 0x3, 0x1, 0x0, 600,
>> 12500},
>> +     {PMIC_BUCK4, 0x28, 0x0, 0xFF, 0x26, 0x4, 0x3, 0x1, 0x0, 600,
>> 12500},
>> +     {PMIC_BUCK5, 0x31, 0x0, 0x3F, 0x30, 0x0, 0x3, 0x3, 0x0, 750,
>> 50000},
>> +     {PMIC_BUCK6, 0x33, 0x0, 0x3F, 0x32, 0x0, 0x3, 0x3, 0x0, 750,
>> 50000},
>> +     {PMIC_BUCK7, 0x35, 0x0, 0x3F, 0x34, 0x0, 0x3, 0x3, 0x0, 750,
>> 50000},
>> +     {PMIC_BUCK8, 0x37, 0x0, 0x3F, 0x36, 0x0, 0x3, 0x3, 0x0, 750,
>> 50000},
>> +     {PMIC_BUCK9, 0x39, 0x0, 0x3F, 0x38, 0x0, 0x3, 0x3, 0x0, 750,
>> 50000},
>> +     {PMIC_LDO1,  0x40, 0x0, 0x3F, 0x40, 0x6, 0x3, 0x3, 0x0, 800,
>> 25000},
>> +     {PMIC_LDO2,  0x41, 0x0, 0x3F, 0x41, 0x6, 0x3, 0x1, 0x0, 800,
>> 25000},
>> +     {PMIC_LDO3,  0x42, 0x0, 0x3F, 0x42, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO4,  0x43, 0x0, 0x3F, 0x43, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO5,  0x44, 0x0, 0x3F, 0x44, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO6,  0x45, 0x0, 0x3F, 0x45, 0x6, 0x3, 0x1, 0x0, 800,
>> 25000},
>> +     {PMIC_LDO7,  0x46, 0x0, 0x3F, 0x46, 0x6, 0x3, 0x1, 0x0, 800,
>> 25000},
>> +     {PMIC_LDO8,  0x47, 0x0, 0x3F, 0x47, 0x6, 0x3, 0x1, 0x0, 800,
>> 25000},
>> +     {PMIC_LDO9,  0x48, 0x0, 0x3F, 0x48, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO10, 0x49, 0x0, 0x3F, 0x49, 0x6, 0x3, 0x1, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO11, 0x4A, 0x0, 0x3F, 0x4A, 0x6, 0x3, 0x1, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO12, 0x4B, 0x0, 0x3F, 0x4B, 0x6, 0x3, 0x1, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO13, 0x4C, 0x0, 0x3F, 0x4C, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO14, 0x4D, 0x0, 0x3F, 0x4D, 0x6, 0x3, 0x1, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO15, 0x4E, 0x0, 0x3F, 0x4E, 0x6, 0x3, 0x1, 0x0, 800,
>> 25000},
>> +     {PMIC_LDO16, 0x4F, 0x0, 0x3F, 0x4F, 0x6, 0x3, 0x1, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO17, 0x50, 0x0, 0x3F, 0x50, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO18, 0x51, 0x0, 0x3F, 0x51, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO19, 0x52, 0x0, 0x3F, 0x52, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO20, 0x53, 0x0, 0x3F, 0x53, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO21, 0x54, 0x0, 0x3F, 0x54, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO22, 0x55, 0x0, 0x3F, 0x55, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO23, 0x56, 0x0, 0x3F, 0x56, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO24, 0x57, 0x0, 0x3F, 0x57, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO25, 0x58, 0x0, 0x3F, 0x58, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_LDO26, 0x59, 0x0, 0x3F, 0x59, 0x6, 0x3, 0x3, 0x0, 800,
>> 50000},
>> +     {PMIC_EN32KHZ_CP, 0x0, 0x0, 0x0, 0x7F, 0x1, 0x1, 0x1, 0x0,
>> 0x0, 0x0}, +};
>
> Frankly speaking it looks a bit unreadable (especially without specs
> attached).
>
- ok
>> +
>> +/*
>> + * Write a value to a register
>> + *
>> + * @param chip_addr  i2c addr for max77686
>> + * @param reg                reg number to write
>> + * @param val                value to be written
>> + *
>> + */
>> +static inline int max77686_i2c_write(unsigned char chip_addr,
>> +                                     unsigned int reg, unsigned
>> char val) +{
>> +     return i2c_write(chip_addr, reg, 1, &val, 1);
>> +}
>
> This "write register" is also available in the PMIC framework.
- will reuse the same.
>
>> +
>> +/*
>> + * Read a value from a register
>> + *
>> + * @param chip_addr  i2c addr for max77686
>> + * @param reg                reg number to write
>> + * @param val                value to be written
>> + *
>> + */
>> +static inline int max77686_i2c_read(unsigned char chip_addr,
>> +                                     unsigned int reg, unsigned
>
> It is also available on the PMIC framework.
>
>> char *val) +{
>> +     return i2c_read(chip_addr, reg, 1, val, 1);
>> +}
>> +
>> +/*
>> + * Enable the max77686 register
>> + *
>> + * @param reg                register number of buck/ldo to be
>> enabled
>> + * @param enable     enable or disable bit
>> + *
>> + *                   REG_DISABLE = 0,
>> +                     needed to set the buck/ldo enable bit OFF
>> + * @return           Return 0 if ok, else -1
>> + */
>> +static int max77686_enablereg(enum max77686_regnum reg, int enable)
>> +{
>> +     struct max77686_para *pmic;
>> +     unsigned char read_data;
>> +     int ret;
>> +
>> +     pmic = &max77686_param[reg];
>> +
>> +     ret = max77686_i2c_read(MAX77686_I2C_ADDR, pmic->vol_addr,
>> &read_data);
>> +     if (ret != 0) {
>> +             debug("max77686 i2c read failed.\n");
>> +             return -1;
>> +     }
>> +
>> +     if (enable == REG_DISABLE) {
>> +             clrbits_8(&read_data,
>> +                             pmic->reg_enbitmask <<
>> pmic->reg_enbitpos);
>> +     } else {
>> +             clrsetbits_8(&read_data,
>> +                             pmic->reg_enbitmask <<
>> pmic->reg_enbitpos,
>> +                             pmic->reg_enbiton <<
>> pmic->reg_enbitpos);
>> +     }
>> +
>> +     ret = max77686_i2c_write(MAX77686_I2C_ADDR,
>> +                              pmic->reg_enaddr, read_data);
>> +     if (ret != 0) {
>> +             debug("max77686 i2c write failed.\n");
>> +             return -1;
>> +     }
>> +
>> +     return 0;
>> +}
>
> Please look into the PMIC section of ./board/samsung/trats/trats.c for
> reference. Moreover changing/enabling regulator function is already
> available at pmic_core.c
- Ok
>
>> +
>> +/* Set the required voltage level of pmic
>> + *
>> + * @param reg                register number of buck/ldo to be set
>> + * @param volt               voltage level to be set
>> + * @param enable     enable or disable bit
>> + *
>> + * @return           Return 0 if ok, else -1
>> + */
>> +static int max77686_volsetting(enum max77686_regnum reg, unsigned
>> int volt,
>> +                            int enable)
>> +{
>> +     struct max77686_para *pmic;
>> +     unsigned char read_data;
>> +     unsigned int vol_level = 0;
>> +     int ret;
>> +
>> +     pmic = &max77686_param[reg];
>> +
>> +     if (pmic->vol_addr == 0) {
>> +             debug("not a voltage register.\n");
>> +             return -1;
>> +     }
>> +
>> +     ret = max77686_i2c_read(MAX77686_I2C_ADDR, pmic->vol_addr,
>> &read_data);
>> +     if (ret != 0) {
>> +             debug("max77686 i2c read failed.\n");
>> +             return -1;
>> +     }
>> +
>> +     if (volt - pmic->vol_min > 0)
>> +             vol_level = ((volt - pmic->vol_min) * 1000) /
>> pmic->vol_div; +
>> +     clrsetbits_8(&read_data, pmic->vol_bitmask <<
>> pmic->vol_bitpos,
>> +                     vol_level << pmic->vol_bitpos);
>> +
>> +     ret = max77686_i2c_write(MAX77686_I2C_ADDR, pmic->vol_addr,
>> read_data);
>> +     if (ret != 0) {
>> +             debug("max77686 i2c write failed.\n");
>> +             return -1;
>> +     }
>> +
>> +     ret = max77686_enablereg(reg, enable);
>> +     if (ret != 0) {
>> +             debug("Failed to enable buck/ldo.\n");
>> +             return -1;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>
> I believe,that it would be better to further develop PMIC
> framework, than writing your own "special" driver.
Will rework on the same.
>
>
>> +int max77686_enable_32khz_cp(void)
>> +{
>> +     return max77686_enablereg(PMIC_EN32KHZ_CP, REG_ENABLE);
>> +}
>> +
>> +/**
>> + * Initialize the pmic voltages to power up the system
>> + * This also calls i2c_init so that we can program the pmic
>> + *
>> + * REG_ENABLE = 0, needed to set the buck/ldo enable bit ON
>> + *
>> + * @return   Return 0 if ok, else -1
>> + */
>> +int power_init(void)
>> +{
>> +     int error = 0;
>> +
>> +     /* init the i2c so that we can program pmic chip */
>> +     i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>> +
>> +     error = max77686_volsetting(PMIC_BUCK2, CONFIG_VDD_ARM,
>> REG_ENABLE);
>> +     error |= max77686_volsetting(PMIC_BUCK3, CONFIG_VDD_INT,
>> REG_ENABLE);
>> +     error |= max77686_volsetting(PMIC_BUCK1, CONFIG_VDD_MIF,
>> REG_ENABLE);
>> +     error |= max77686_volsetting(PMIC_LDO2, CONFIG_VDD_LDO2,
>> REG_ENABLE);
>> +     error |= max77686_volsetting(PMIC_LDO3, CONFIG_VDD_LDO3,
>> REG_ENABLE);
>> +     error |= max77686_volsetting(PMIC_LDO5, CONFIG_VDD_LDO5,
>> REG_ENABLE);
>> +     error |= max77686_volsetting(PMIC_LDO10, CONFIG_VDD_LDO10,
>> REG_ENABLE);
>> +     if (error != 0)
>> +             debug("power init failed\n");
>> +
>> +     return error;
>> +}
>> diff --git a/include/max77686.h b/include/max77686.h
>> new file mode 100644
>> index 0000000..e630ed8
>> --- /dev/null
>> +++ b/include/max77686.h
>> @@ -0,0 +1,115 @@
>> +/*
>> + *  Copyright (C) 2012 Samsung Electronics
>> + *  Alim Akhtar <alim.akhtar at samsung.com>
>> + *
>> + * See file CREDITS for list of people who contributed to this
>> + * project.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +
>> +#ifndef __MAX77686_H_
>> +#define __MAX77686_H_
>> +
>> +enum max77686_regnum {
>> +     PMIC_BUCK1 = 0,
>> +     PMIC_BUCK2,
>> +     PMIC_BUCK3,
>> +     PMIC_BUCK4,
>> +     PMIC_BUCK5,
>> +     PMIC_BUCK6,
>> +     PMIC_BUCK7,
>> +     PMIC_BUCK8,
>> +     PMIC_BUCK9,
>> +     PMIC_LDO1,
>> +     PMIC_LDO2,
>> +     PMIC_LDO3,
>> +     PMIC_LDO4,
>> +     PMIC_LDO5,
>> +     PMIC_LDO6,
>> +     PMIC_LDO7,
>> +     PMIC_LDO8,
>> +     PMIC_LDO9,
>> +     PMIC_LDO10,
>> +     PMIC_LDO11,
>> +     PMIC_LDO12,
>> +     PMIC_LDO13,
>> +     PMIC_LDO14,
>> +     PMIC_LDO15,
>> +     PMIC_LDO16,
>> +     PMIC_LDO17,
>> +     PMIC_LDO18,
>> +     PMIC_LDO19,
>> +     PMIC_LDO20,
>> +     PMIC_LDO21,
>> +     PMIC_LDO22,
>> +     PMIC_LDO23,
>> +     PMIC_LDO24,
>> +     PMIC_LDO25,
>> +     PMIC_LDO26,
>> +     PMIC_EN32KHZ_CP
>> +};
>> +
>> +/**
>> + * struct max77686_para - max77686 register parameters
>> + * @param vol_addr   i2c address of the given buck/ldo register
>> + * @param vol_bitpos bit position to be set or clear within
>> register
>> + * @param vol_bitmask        bit mask value
>> + * @param reg_enaddr control register address, which enable
>> the given
>> + *                   given buck/ldo.
>> + * @param reg_enbitpos       bit position to be enabled
>> + * @param reg_enbiton        value to be written to buck/ldo to make
>> it ON
>> + * @param reg_enbitoff       value to be written to buck/ldo to
>> make it OFF
>> + * @param vol_min    minimum voltage level supported by given
>> buck/ldo
>> + * @param vol_div    voltage division value of given buck/ldo
>> + */
>> +struct max77686_para {
>> +     enum max77686_regnum regnum;
>> +     u8      vol_addr;
>> +     u8      vol_bitpos;
>> +     u8      vol_bitmask;
>> +     u8      reg_enaddr;
>> +     u8      reg_enbitpos;
>> +     u8      reg_enbitmask;
>> +     u8      reg_enbiton;
>> +     u8      reg_enbitoff;
>> +     u32     vol_min;
>> +     u32     vol_div;
>> +};
>> +
>> +/* I2C device address for pmic max77686 */
>> +#define MAX77686_I2C_ADDR (0x12 >> 1)
>> +
>> +enum {
>> +     REG_DISABLE = 0,
>> +     REG_ENABLE
>> +};
>> +
>> +/**
>> + * This function enables the 32KHz coprocessor clock.
>> + *
>> + * Return 0 if ok, else -1
>> + */
>> +int max77686_enable_32khz_cp(void);
>> +
>> +/**
>> + * This function sets the BUCK's/LDO's voltages of pmic
>> + *
>> + * Return 0 if ok, else -1
>> + */
>> +int power_init(void);
>> +
>> +#endif /* __MAX77686_PMIC_H_ */
>
> It looks like the max77686.h file is already written in a similar way
> to e.g. max8998.h file and stick to the PMIC framework.
>
> I'd really encourage you to look into the PMIC driver and adjust your
> driver to the PMIC framework.
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung Poland R&D Center | Linux Platform Group
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Regards
Rajeshwari Shinde.


More information about the U-Boot mailing list