Hi Lukasz Majewski, Thank you for the coments.
On Tue, May 22, 2012 at 12:20 PM, Lukasz Majewski <l.majew...@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.akh...@samsung.com> >> Signed-off-by: Rajeshwari Shinde <rajeshwar...@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.akh...@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.akh...@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@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot Regards Rajeshwari Shinde. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot