Message ID | 1425399883-14053-11-git-send-email-p.marczak@samsung.com |
---|---|
State | Changes Requested |
Delegated to: | Simon Glass |
Headers | show |
Hi Przemyslaw, On 3 March 2015 at 09:24, Przemyslaw Marczak <p.marczak@samsung.com> wrote: > This commit change the old pmic framework calls with the new ones. > > Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com> > --- > Changes v2: > - remove board_init_i2c() call > - update regulator calls > - update headers > - samsung/misc.c: include required header > --- > board/samsung/common/misc.c | 1 + > board/samsung/odroid/odroid.c | 52 ++++++++++++++++++++++++++----------------- > 2 files changed, 33 insertions(+), 20 deletions(-) > > diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c > index 4538ac7..18d71e8 100644 > --- a/board/samsung/common/misc.c > +++ b/board/samsung/common/misc.c > @@ -16,6 +16,7 @@ > #include <asm/arch/cpu.h> > #include <asm/gpio.h> > #include <linux/input.h> > +#include <dm.h> > #include <power/pmic.h> > #include <mmc.h> > > diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c > index bff6ac9..2448cde 100644 > --- a/board/samsung/odroid/odroid.c > +++ b/board/samsung/odroid/odroid.c > @@ -12,7 +12,9 @@ > #include <asm/arch/gpio.h> > #include <asm/gpio.h> > #include <asm/arch/cpu.h> > +#include <dm.h> > #include <power/pmic.h> > +#include <power/regulator.h> > #include <power/max77686_pmic.h> > #include <errno.h> > #include <usb.h> > @@ -402,15 +404,23 @@ static void board_gpio_init(void) > > static int pmic_init_max77686(void) > { > - struct pmic *p = pmic_get("MAX77686_PMIC"); > + struct udevice *reg; > + int type; > > - if (pmic_probe(p)) > + if (regulator_get("max77686", ®)) { > + error("Regulator get error\n"); > return -ENODEV; > + } > > /* Set LDO Voltage */ > - max77686_set_ldo_voltage(p, 20, 1800000); /* LDO20 eMMC */ > - max77686_set_ldo_voltage(p, 21, 2800000); /* LDO21 SD */ > - max77686_set_ldo_voltage(p, 22, 2800000); /* LDO22 eMMC */ > + type = REGULATOR_TYPE_LDO; > + regulator_set_value(reg, type, 20, 1800000); /* LDO20 eMMC */ > + regulator_set_value(reg, type, 21, 2800000); /* LDO21 SD */ > + regulator_set_value(reg, type, 22, 2800000); /* LDO22 eMMC */ > + > + regulator_set_mode(reg, type, 20, OPMODE_ON); > + regulator_set_mode(reg, type, 21, OPMODE_ON); > + regulator_set_mode(reg, type, 22, OPMODE_ON); These 20, 21, 22 values seem bad to me. Do we not have names? Also I see in the device tree that these regulators have the same min and max voltage, so perhaps you can use that voltage automatically? I think when you change the LDOs to devices you might end up with: struct udevice *ldo; ret = regulator_get("VCCQ_MMC2_2.8V", &ldo); // use the regulator device name which comes from regulator-name property. if (ret) { debug(...) goto err; } ret = regulator_set_voltage(ldo, 1800000); ...error check ret = regulator_set_mode(ldo, OPMODE_ON); ...error check > > return 0; > } > @@ -435,7 +445,6 @@ int exynos_init(void) > > int exynos_power_init(void) > { > - pmic_init(0); > pmic_init_max77686(); > > return 0; > @@ -444,19 +453,21 @@ int exynos_power_init(void) > #ifdef CONFIG_USB_GADGET > static int s5pc210_phy_control(int on) > { > - struct pmic *p_pmic; > + struct udevice *reg; > + int type; > > - p_pmic = pmic_get("MAX77686_PMIC"); > - if (!p_pmic) > + if (regulator_get("max77686", ®)) { > + error("Regulator get error\n"); > return -ENODEV; > + } > > - if (pmic_probe(p_pmic)) > - return -1; > + type = REGULATOR_TYPE_LDO; > > if (on) > - return max77686_set_ldo_mode(p_pmic, 12, OPMODE_ON); > + return regulator_set_mode(reg, type, 12, OPMODE_ON); > else > - return max77686_set_ldo_mode(p_pmic, 12, OPMODE_LPM); > + return regulator_set_mode(reg, type, 12, OPMODE_LPM); > + > } > > struct s3c_plat_otg_data s5pc210_otg_data = { > @@ -473,7 +484,8 @@ struct s3c_plat_otg_data s5pc210_otg_data = { > int board_usb_init(int index, enum usb_init_type init) > { > #ifdef CONFIG_CMD_USB > - struct pmic *p_pmic; > + struct udevice *reg; > + int type, ret; > > /* Set Ref freq 0 => 24MHz, 1 => 26MHz*/ > /* Odroid Us have it at 24MHz, Odroid Xs at 26MHz */ > @@ -491,14 +503,14 @@ int board_usb_init(int index, enum usb_init_type init) > /* Power off and on BUCK8 for LAN9730 */ > debug("LAN9730 - Turning power buck 8 OFF and ON.\n"); > > - p_pmic = pmic_get("MAX77686_PMIC"); > - if (p_pmic && !pmic_probe(p_pmic)) { > - max77686_set_buck_voltage(p_pmic, 8, 750000); > - max77686_set_buck_voltage(p_pmic, 8, 3300000); > + if (regulator_get("max77686", ®)) { > + type = REGULATOR_TYPE_BUCK; > + ret = regulator_set_value(reg, type, 8, 750000); > + ret |= regulator_set_value(reg, type, 8, 3300000); > + if (ret) > + error("Can't set regulator\n"); We should return a proper error number. Also print it here as it might provide clues. > } > - > #endif > - > debug("USB_udc_probe\n"); > return s3c_udc_probe(&s5pc210_otg_data); > } > -- > 1.9.1 > Regards, Simon
Hello, On 03/06/2015 03:14 PM, Simon Glass wrote: > Hi Przemyslaw, > > On 3 March 2015 at 09:24, Przemyslaw Marczak <p.marczak@samsung.com> wrote: >> This commit change the old pmic framework calls with the new ones. >> >> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com> >> --- >> Changes v2: >> - remove board_init_i2c() call >> - update regulator calls >> - update headers >> - samsung/misc.c: include required header >> --- >> board/samsung/common/misc.c | 1 + >> board/samsung/odroid/odroid.c | 52 ++++++++++++++++++++++++++----------------- >> 2 files changed, 33 insertions(+), 20 deletions(-) >> >> diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c >> index 4538ac7..18d71e8 100644 >> --- a/board/samsung/common/misc.c >> +++ b/board/samsung/common/misc.c >> @@ -16,6 +16,7 @@ >> #include <asm/arch/cpu.h> >> #include <asm/gpio.h> >> #include <linux/input.h> >> +#include <dm.h> >> #include <power/pmic.h> >> #include <mmc.h> >> >> diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c >> index bff6ac9..2448cde 100644 >> --- a/board/samsung/odroid/odroid.c >> +++ b/board/samsung/odroid/odroid.c >> @@ -12,7 +12,9 @@ >> #include <asm/arch/gpio.h> >> #include <asm/gpio.h> >> #include <asm/arch/cpu.h> >> +#include <dm.h> >> #include <power/pmic.h> >> +#include <power/regulator.h> >> #include <power/max77686_pmic.h> >> #include <errno.h> >> #include <usb.h> >> @@ -402,15 +404,23 @@ static void board_gpio_init(void) >> >> static int pmic_init_max77686(void) >> { >> - struct pmic *p = pmic_get("MAX77686_PMIC"); >> + struct udevice *reg; >> + int type; >> >> - if (pmic_probe(p)) >> + if (regulator_get("max77686", ®)) { >> + error("Regulator get error\n"); >> return -ENODEV; >> + } >> >> /* Set LDO Voltage */ >> - max77686_set_ldo_voltage(p, 20, 1800000); /* LDO20 eMMC */ >> - max77686_set_ldo_voltage(p, 21, 2800000); /* LDO21 SD */ >> - max77686_set_ldo_voltage(p, 22, 2800000); /* LDO22 eMMC */ >> + type = REGULATOR_TYPE_LDO; >> + regulator_set_value(reg, type, 20, 1800000); /* LDO20 eMMC */ >> + regulator_set_value(reg, type, 21, 2800000); /* LDO21 SD */ >> + regulator_set_value(reg, type, 22, 2800000); /* LDO22 eMMC */ >> + >> + regulator_set_mode(reg, type, 20, OPMODE_ON); >> + regulator_set_mode(reg, type, 21, OPMODE_ON); >> + regulator_set_mode(reg, type, 22, OPMODE_ON); > > These 20, 21, 22 values seem bad to me. Do we not have names? Also I > see in the device tree that these regulators have the same min and max > voltage, so perhaps you can use that voltage automatically? > > I think when you change the LDOs to devices you might end up with: > > struct udevice *ldo; > > ret = regulator_get("VCCQ_MMC2_2.8V", &ldo); // use the regulator > device name which comes from regulator-name property. > if (ret) { > debug(...) > goto err; > } > ret = regulator_set_voltage(ldo, 1800000); > ...error check > ret = regulator_set_mode(ldo, OPMODE_ON); > ...error check > The above suggestion is implemented in V3. >> >> return 0; >> } >> @@ -435,7 +445,6 @@ int exynos_init(void) >> >> int exynos_power_init(void) >> { >> - pmic_init(0); >> pmic_init_max77686(); >> >> return 0; >> @@ -444,19 +453,21 @@ int exynos_power_init(void) >> #ifdef CONFIG_USB_GADGET >> static int s5pc210_phy_control(int on) >> { >> - struct pmic *p_pmic; >> + struct udevice *reg; >> + int type; >> >> - p_pmic = pmic_get("MAX77686_PMIC"); >> - if (!p_pmic) >> + if (regulator_get("max77686", ®)) { >> + error("Regulator get error\n"); >> return -ENODEV; >> + } >> >> - if (pmic_probe(p_pmic)) >> - return -1; >> + type = REGULATOR_TYPE_LDO; >> >> if (on) >> - return max77686_set_ldo_mode(p_pmic, 12, OPMODE_ON); >> + return regulator_set_mode(reg, type, 12, OPMODE_ON); >> else >> - return max77686_set_ldo_mode(p_pmic, 12, OPMODE_LPM); >> + return regulator_set_mode(reg, type, 12, OPMODE_LPM); >> + >> } >> >> struct s3c_plat_otg_data s5pc210_otg_data = { >> @@ -473,7 +484,8 @@ struct s3c_plat_otg_data s5pc210_otg_data = { >> int board_usb_init(int index, enum usb_init_type init) >> { >> #ifdef CONFIG_CMD_USB >> - struct pmic *p_pmic; >> + struct udevice *reg; >> + int type, ret; >> >> /* Set Ref freq 0 => 24MHz, 1 => 26MHz*/ >> /* Odroid Us have it at 24MHz, Odroid Xs at 26MHz */ >> @@ -491,14 +503,14 @@ int board_usb_init(int index, enum usb_init_type init) >> /* Power off and on BUCK8 for LAN9730 */ >> debug("LAN9730 - Turning power buck 8 OFF and ON.\n"); >> >> - p_pmic = pmic_get("MAX77686_PMIC"); >> - if (p_pmic && !pmic_probe(p_pmic)) { >> - max77686_set_buck_voltage(p_pmic, 8, 750000); >> - max77686_set_buck_voltage(p_pmic, 8, 3300000); >> + if (regulator_get("max77686", ®)) { >> + type = REGULATOR_TYPE_BUCK; >> + ret = regulator_set_value(reg, type, 8, 750000); >> + ret |= regulator_set_value(reg, type, 8, 3300000); >> + if (ret) >> + error("Can't set regulator\n"); > > We should return a proper error number. Also print it here as it might > provide clues. > >> } >> - >> #endif >> - >> debug("USB_udc_probe\n"); >> return s3c_udc_probe(&s5pc210_otg_data); >> } >> -- >> 1.9.1 >> > > Regards, > Simon > Thanks,
diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c index 4538ac7..18d71e8 100644 --- a/board/samsung/common/misc.c +++ b/board/samsung/common/misc.c @@ -16,6 +16,7 @@ #include <asm/arch/cpu.h> #include <asm/gpio.h> #include <linux/input.h> +#include <dm.h> #include <power/pmic.h> #include <mmc.h> diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c index bff6ac9..2448cde 100644 --- a/board/samsung/odroid/odroid.c +++ b/board/samsung/odroid/odroid.c @@ -12,7 +12,9 @@ #include <asm/arch/gpio.h> #include <asm/gpio.h> #include <asm/arch/cpu.h> +#include <dm.h> #include <power/pmic.h> +#include <power/regulator.h> #include <power/max77686_pmic.h> #include <errno.h> #include <usb.h> @@ -402,15 +404,23 @@ static void board_gpio_init(void) static int pmic_init_max77686(void) { - struct pmic *p = pmic_get("MAX77686_PMIC"); + struct udevice *reg; + int type; - if (pmic_probe(p)) + if (regulator_get("max77686", ®)) { + error("Regulator get error\n"); return -ENODEV; + } /* Set LDO Voltage */ - max77686_set_ldo_voltage(p, 20, 1800000); /* LDO20 eMMC */ - max77686_set_ldo_voltage(p, 21, 2800000); /* LDO21 SD */ - max77686_set_ldo_voltage(p, 22, 2800000); /* LDO22 eMMC */ + type = REGULATOR_TYPE_LDO; + regulator_set_value(reg, type, 20, 1800000); /* LDO20 eMMC */ + regulator_set_value(reg, type, 21, 2800000); /* LDO21 SD */ + regulator_set_value(reg, type, 22, 2800000); /* LDO22 eMMC */ + + regulator_set_mode(reg, type, 20, OPMODE_ON); + regulator_set_mode(reg, type, 21, OPMODE_ON); + regulator_set_mode(reg, type, 22, OPMODE_ON); return 0; } @@ -435,7 +445,6 @@ int exynos_init(void) int exynos_power_init(void) { - pmic_init(0); pmic_init_max77686(); return 0; @@ -444,19 +453,21 @@ int exynos_power_init(void) #ifdef CONFIG_USB_GADGET static int s5pc210_phy_control(int on) { - struct pmic *p_pmic; + struct udevice *reg; + int type; - p_pmic = pmic_get("MAX77686_PMIC"); - if (!p_pmic) + if (regulator_get("max77686", ®)) { + error("Regulator get error\n"); return -ENODEV; + } - if (pmic_probe(p_pmic)) - return -1; + type = REGULATOR_TYPE_LDO; if (on) - return max77686_set_ldo_mode(p_pmic, 12, OPMODE_ON); + return regulator_set_mode(reg, type, 12, OPMODE_ON); else - return max77686_set_ldo_mode(p_pmic, 12, OPMODE_LPM); + return regulator_set_mode(reg, type, 12, OPMODE_LPM); + } struct s3c_plat_otg_data s5pc210_otg_data = { @@ -473,7 +484,8 @@ struct s3c_plat_otg_data s5pc210_otg_data = { int board_usb_init(int index, enum usb_init_type init) { #ifdef CONFIG_CMD_USB - struct pmic *p_pmic; + struct udevice *reg; + int type, ret; /* Set Ref freq 0 => 24MHz, 1 => 26MHz*/ /* Odroid Us have it at 24MHz, Odroid Xs at 26MHz */ @@ -491,14 +503,14 @@ int board_usb_init(int index, enum usb_init_type init) /* Power off and on BUCK8 for LAN9730 */ debug("LAN9730 - Turning power buck 8 OFF and ON.\n"); - p_pmic = pmic_get("MAX77686_PMIC"); - if (p_pmic && !pmic_probe(p_pmic)) { - max77686_set_buck_voltage(p_pmic, 8, 750000); - max77686_set_buck_voltage(p_pmic, 8, 3300000); + if (regulator_get("max77686", ®)) { + type = REGULATOR_TYPE_BUCK; + ret = regulator_set_value(reg, type, 8, 750000); + ret |= regulator_set_value(reg, type, 8, 3300000); + if (ret) + error("Can't set regulator\n"); } - #endif - debug("USB_udc_probe\n"); return s3c_udc_probe(&s5pc210_otg_data); }
This commit change the old pmic framework calls with the new ones. Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com> --- Changes v2: - remove board_init_i2c() call - update regulator calls - update headers - samsung/misc.c: include required header --- board/samsung/common/misc.c | 1 + board/samsung/odroid/odroid.c | 52 ++++++++++++++++++++++++++----------------- 2 files changed, 33 insertions(+), 20 deletions(-)