| Message ID | 20161227083305.13907-1-jh80.chung@samsung.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Minkyu Kang |
| Headers | show |
Hi, On 27 December 2016 at 17:33, Jaehoon Chung <jh80.chung@samsung.com> wrote: > Remove the "ifndef CONFIG_DM_I2C". > Instead, use the driver model for max8998. > > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> > --- > board/samsung/goni/goni.c | 61 ++++++++++++++++++++++++------ > ----------------- > 1 file changed, 31 insertions(+), 30 deletions(-) > > diff --git a/board/samsung/goni/goni.c b/board/samsung/goni/goni.c > index b066832..c1d7438 100644 > --- a/board/samsung/goni/goni.c > +++ b/board/samsung/goni/goni.c > @@ -9,6 +9,7 @@ > #include <common.h> > #include <asm/gpio.h> > #include <asm/arch/mmc.h> > +#include <dm.h> > #include <power/pmic.h> > #include <usb/dwc2_udc.h> > #include <asm/arch/cpu.h> > @@ -43,19 +44,6 @@ void i2c_init_board(void) > } > #endif > > -int power_init_board(void) > -{ > -#ifndef CONFIG_DM_I2C /* TODO(maintainer): Convert to driver model */ > - /* > - * For PMIC the I2C bus is named as I2C5, but it is connected > - * to logical I2C adapter 0 > - */ > - return pmic_init(I2C_0); > -#else > - return 0; > -#endif > -} > - > int dram_init(void) > { > gd->ram_size = PHYS_SDRAM_1_SIZE + PHYS_SDRAM_2_SIZE + > @@ -146,34 +134,47 @@ int board_mmc_init(bd_t *bis) > #ifdef CONFIG_USB_GADGET > static int s5pc1xx_phy_control(int on) > { > -#ifndef CONFIG_DM_I2C /* TODO(maintainer): Convert to driver model */ > - int ret; > +#ifdef CONFIG_DM_PMIC_MAX8998 > I think, we don't need it. What do you think? + struct udevice *dev; > static int status; > - struct pmic *p = pmic_get("MAX8998_PMIC"); > - if (!p) > - return -ENODEV; > + int reg, ret; > > - if (pmic_probe(p)) > - return -1; > + ret = pmic_get("max8998_pmix", &dev); > pmix -> typo? > + if (ret) > + return ret; > > if (on && !status) { > - ret = pmic_set_output(p, MAX8998_REG_ONOFF1, > - MAX8998_LDO3, LDO_ON); > - ret = pmic_set_output(p, MAX8998_REG_ONOFF2, > - MAX8998_LDO8, LDO_ON); > + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF1); > + reg |= MAX8998_LDO3; > + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF1, reg); > if (ret) { > puts("MAX8998 LDO setting error!\n"); > - return -1; > + return -EINVAL; > + } > + > + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF2); > + reg |= MAX8998_LDO8; > + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF2, reg); > + if (ret) { > + puts("MAX8998 LDO setting error!\n"); > + return -EINVAL; > } > status = 1; > } else if (!on && status) { > - ret = pmic_set_output(p, MAX8998_REG_ONOFF1, > - MAX8998_LDO3, LDO_OFF); > - ret = pmic_set_output(p, MAX8998_REG_ONOFF2, > - MAX8998_LDO8, LDO_OFF); > + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF1); > + reg &= ~MAX8998_LDO3; > + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF1, reg); > + if (ret) { > + puts("MAX8998 LDO setting error!\n"); > + return -EINVAL; > + } > + > + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF2); > + reg &= ~MAX8998_LDO8; > + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF2, reg); > if (ret) { > puts("MAX8998 LDO setting error!\n"); > - return -1; > + return -EINVAL; > } > status = 0; > } > -- > 2.10.2 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > Thanks,
Hi, On 12/27/2016 11:31 PM, Minkyu Kang wrote: > Hi, > > On 27 December 2016 at 17:33, Jaehoon Chung <jh80.chung@samsung.com> wrote: > >> Remove the "ifndef CONFIG_DM_I2C". >> Instead, use the driver model for max8998. >> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >> --- >> board/samsung/goni/goni.c | 61 ++++++++++++++++++++++++------ >> ----------------- >> 1 file changed, 31 insertions(+), 30 deletions(-) >> >> diff --git a/board/samsung/goni/goni.c b/board/samsung/goni/goni.c >> index b066832..c1d7438 100644 >> --- a/board/samsung/goni/goni.c >> +++ b/board/samsung/goni/goni.c >> @@ -9,6 +9,7 @@ >> #include <common.h> >> #include <asm/gpio.h> >> #include <asm/arch/mmc.h> >> +#include <dm.h> >> #include <power/pmic.h> >> #include <usb/dwc2_udc.h> >> #include <asm/arch/cpu.h> >> @@ -43,19 +44,6 @@ void i2c_init_board(void) >> } >> #endif >> >> -int power_init_board(void) >> -{ >> -#ifndef CONFIG_DM_I2C /* TODO(maintainer): Convert to driver model */ >> - /* >> - * For PMIC the I2C bus is named as I2C5, but it is connected >> - * to logical I2C adapter 0 >> - */ >> - return pmic_init(I2C_0); >> -#else >> - return 0; >> -#endif >> -} >> - >> int dram_init(void) >> { >> gd->ram_size = PHYS_SDRAM_1_SIZE + PHYS_SDRAM_2_SIZE + >> @@ -146,34 +134,47 @@ int board_mmc_init(bd_t *bis) >> #ifdef CONFIG_USB_GADGET >> static int s5pc1xx_phy_control(int on) >> { >> -#ifndef CONFIG_DM_I2C /* TODO(maintainer): Convert to driver model */ >> - int ret; >> +#ifdef CONFIG_DM_PMIC_MAX8998 >> > > I think, we don't need it. > What do you think? Yes, we can remove it. Will remove. > > + struct udevice *dev; >> static int status; >> - struct pmic *p = pmic_get("MAX8998_PMIC"); >> - if (!p) >> - return -ENODEV; >> + int reg, ret; >> >> - if (pmic_probe(p)) >> - return -1; >> + ret = pmic_get("max8998_pmix", &dev); >> > > pmix -> typo? Sorry. "max8998_pmic" is right. Will fix. Best Regards, Jaehoon Chung > > >> + if (ret) >> + return ret; >> >> if (on && !status) { >> - ret = pmic_set_output(p, MAX8998_REG_ONOFF1, >> - MAX8998_LDO3, LDO_ON); >> - ret = pmic_set_output(p, MAX8998_REG_ONOFF2, >> - MAX8998_LDO8, LDO_ON); >> + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF1); >> + reg |= MAX8998_LDO3; >> + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF1, reg); >> if (ret) { >> puts("MAX8998 LDO setting error!\n"); >> - return -1; >> + return -EINVAL; >> + } >> + >> + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF2); >> + reg |= MAX8998_LDO8; >> + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF2, reg); >> + if (ret) { >> + puts("MAX8998 LDO setting error!\n"); >> + return -EINVAL; >> } >> status = 1; >> } else if (!on && status) { >> - ret = pmic_set_output(p, MAX8998_REG_ONOFF1, >> - MAX8998_LDO3, LDO_OFF); >> - ret = pmic_set_output(p, MAX8998_REG_ONOFF2, >> - MAX8998_LDO8, LDO_OFF); >> + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF1); >> + reg &= ~MAX8998_LDO3; >> + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF1, reg); >> + if (ret) { >> + puts("MAX8998 LDO setting error!\n"); >> + return -EINVAL; >> + } >> + >> + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF2); >> + reg &= ~MAX8998_LDO8; >> + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF2, reg); >> if (ret) { >> puts("MAX8998 LDO setting error!\n"); >> - return -1; >> + return -EINVAL; >> } >> status = 0; >> } >> -- >> 2.10.2 >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot >> > > > Thanks, >
Hi Jaehoon, On 27 December 2016 at 15:35, Jaehoon Chung <jh80.chung@samsung.com> wrote: > Hi, > > On 12/27/2016 11:31 PM, Minkyu Kang wrote: >> Hi, >> >> On 27 December 2016 at 17:33, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> >>> Remove the "ifndef CONFIG_DM_I2C". >>> Instead, use the driver model for max8998. >>> >>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >>> --- >>> board/samsung/goni/goni.c | 61 ++++++++++++++++++++++++------ >>> ----------------- >>> 1 file changed, 31 insertions(+), 30 deletions(-) This looks good to me. A further step would be to add a regulator driver, rather than poking the pmic registers directly. But that could come later. Regards, Simon
Hi Simon, On 01/12/2017 02:07 PM, Simon Glass wrote: > Hi Jaehoon, > > On 27 December 2016 at 15:35, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> Hi, >> >> On 12/27/2016 11:31 PM, Minkyu Kang wrote: >>> Hi, >>> >>> On 27 December 2016 at 17:33, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>> >>>> Remove the "ifndef CONFIG_DM_I2C". >>>> Instead, use the driver model for max8998. >>>> >>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >>>> --- >>>> board/samsung/goni/goni.c | 61 ++++++++++++++++++++++++------ >>>> ----------------- >>>> 1 file changed, 31 insertions(+), 30 deletions(-) > > This looks good to me. A further step would be to add a regulator > driver, rather than poking the pmic registers directly. But that could > come later. Yes, I will add a regulator driver for Samsung boards. Before going next step, i'm doing to convert the driver model(I2C, PMIC) for all Samsung SoC. There are some boards what don't use DM. goni/universal_c210/trat.. I have already completed the converting DM for those boards, and next step will be the using regulator driver. (I will send the patches..it needs to apply the previous patches...) My plan is 1. Convert DM_PMIC and DM_I2C. 2. Use the regulator driver. 3. Remove the unnecessary legacy drivers After that, i will consider for UCLASS_CHARGER and UCLASS_MUIC. (As i checked, it leaves a matter in future work, right?) Best Regards, Jaehoon Chung > > Regards, > Simon > > >
diff --git a/board/samsung/goni/goni.c b/board/samsung/goni/goni.c index b066832..c1d7438 100644 --- a/board/samsung/goni/goni.c +++ b/board/samsung/goni/goni.c @@ -9,6 +9,7 @@ #include <common.h> #include <asm/gpio.h> #include <asm/arch/mmc.h> +#include <dm.h> #include <power/pmic.h> #include <usb/dwc2_udc.h> #include <asm/arch/cpu.h> @@ -43,19 +44,6 @@ void i2c_init_board(void) } #endif -int power_init_board(void) -{ -#ifndef CONFIG_DM_I2C /* TODO(maintainer): Convert to driver model */ - /* - * For PMIC the I2C bus is named as I2C5, but it is connected - * to logical I2C adapter 0 - */ - return pmic_init(I2C_0); -#else - return 0; -#endif -} - int dram_init(void) { gd->ram_size = PHYS_SDRAM_1_SIZE + PHYS_SDRAM_2_SIZE + @@ -146,34 +134,47 @@ int board_mmc_init(bd_t *bis) #ifdef CONFIG_USB_GADGET static int s5pc1xx_phy_control(int on) { -#ifndef CONFIG_DM_I2C /* TODO(maintainer): Convert to driver model */ - int ret; +#ifdef CONFIG_DM_PMIC_MAX8998 + struct udevice *dev; static int status; - struct pmic *p = pmic_get("MAX8998_PMIC"); - if (!p) - return -ENODEV; + int reg, ret; - if (pmic_probe(p)) - return -1; + ret = pmic_get("max8998_pmix", &dev); + if (ret) + return ret; if (on && !status) { - ret = pmic_set_output(p, MAX8998_REG_ONOFF1, - MAX8998_LDO3, LDO_ON); - ret = pmic_set_output(p, MAX8998_REG_ONOFF2, - MAX8998_LDO8, LDO_ON); + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF1); + reg |= MAX8998_LDO3; + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF1, reg); if (ret) { puts("MAX8998 LDO setting error!\n"); - return -1; + return -EINVAL; + } + + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF2); + reg |= MAX8998_LDO8; + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF2, reg); + if (ret) { + puts("MAX8998 LDO setting error!\n"); + return -EINVAL; } status = 1; } else if (!on && status) { - ret = pmic_set_output(p, MAX8998_REG_ONOFF1, - MAX8998_LDO3, LDO_OFF); - ret = pmic_set_output(p, MAX8998_REG_ONOFF2, - MAX8998_LDO8, LDO_OFF); + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF1); + reg &= ~MAX8998_LDO3; + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF1, reg); + if (ret) { + puts("MAX8998 LDO setting error!\n"); + return -EINVAL; + } + + reg = pmic_reg_read(dev, MAX8998_REG_ONOFF2); + reg &= ~MAX8998_LDO8; + ret = pmic_reg_write(dev, MAX8998_REG_ONOFF2, reg); if (ret) { puts("MAX8998 LDO setting error!\n"); - return -1; + return -EINVAL; } status = 0; }
Remove the "ifndef CONFIG_DM_I2C". Instead, use the driver model for max8998. Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> --- board/samsung/goni/goni.c | 61 ++++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 30 deletions(-)