Re: [U-Boot] [PATCH v3 15/17] odroid: board: add support to dm pmic api

2015-04-03 Thread Przemyslaw Marczak

Hello Simon,

On 03/29/2015 03:08 PM, Simon Glass wrote:

  Hi Przemyslaw,

On 24 March 2015 at 14:30, Przemyslaw Marczak  wrote:

This commit change the old pmic framework calls with the new ones.

Signed-off-by: Przemyslaw Marczak 
---
Changes v2:
- remove board_init_i2c() call
- update regulator calls
- update headers
- samsung/misc.c: include required header

Changes v3:
- adjust regulator calls to new api
---
  board/samsung/common/misc.c   |   1 +
  board/samsung/odroid/odroid.c | 113 +-
  2 files changed, 91 insertions(+), 23 deletions(-)

diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c
index 1a77c82..f0d69d4 100644
--- a/board/samsung/common/misc.c
+++ b/board/samsung/common/misc.c
@@ -16,6 +16,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 

diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c
index ae41c29..aa3b0ff 100644
--- a/board/samsung/odroid/odroid.c
+++ b/board/samsung/odroid/odroid.c
@@ -12,7 +12,9 @@
  #include 
  #include 
  #include 
+#include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -405,15 +407,62 @@ static void board_gpio_init(void)

  static int pmic_init_max77686(void)
  {
-   struct pmic *p = pmic_get("MAX77686_PMIC");
+   struct udevice *dev;
+   int ret;

-   if (pmic_probe(p))
-   return -ENODEV;
+   ret = regulator_get("VDDQ_EMMC_1.8V", &dev);
+   if (ret) {
+   error("Regulator get error: %d", ret);
+   return ret;
+   }
+
+   ret = regulator_set_value(dev, 180);
+   if (ret) {
+   error("Regulator %s value setting error: %d", dev->name, ret);
+   return ret;
+   }
+
+   ret = regulator_set_enable(dev, true);
+   if (ret) {
+   error("Regulator %s enable error: %d", dev->name, ret);
+   return ret;
+   }


How about adding a function that finds a regulator, sets its voltage
and enables it? Then you can avoid duplicating the same code 3 times.



Yes, will add something like this.


+
+   ret = regulator_get("TFLASH_2.8V", &dev);
+   if (ret) {
+   error("Regulator get error: %d", ret);
+   return ret;
+   }
+
+   ret = regulator_set_value(dev, 280);
+   if (ret) {
+   error("Regulator %s value setting error: %d", dev->name, ret);
+   return ret;
+   }
+
+   ret = regulator_set_enable(dev, true);
+   if (ret) {
+   error("Regulator %s enable error: %d", dev->name, ret);
+   return ret;
+   }
+
+   ret = regulator_get("VDDQ_EMMC_2.8V", &dev);
+   if (ret) {
+   error("Regulator get error: %d", ret);
+   return ret;
+   }

-   /* Set LDO Voltage */
-   max77686_set_ldo_voltage(p, 20, 180);   /* LDO20 eMMC */
-   max77686_set_ldo_voltage(p, 21, 280);   /* LDO21 SD */
-   max77686_set_ldo_voltage(p, 22, 280);   /* LDO22 eMMC */
+   ret = regulator_set_value(dev, 280);
+   if (ret) {
+   error("Regulator %s value setting error: %d", dev->name, ret);
+   return ret;
+   }
+
+   ret = regulator_set_enable(dev, true);
+   if (ret) {
+   error("Regulator %s enable error: %d", dev->name, ret);
+   return ret;
+   }

 return 0;
  }
@@ -434,7 +483,6 @@ int exynos_init(void)

  int exynos_power_init(void)
  {
-   pmic_init(0);
 pmic_init_max77686();

 return 0;
@@ -443,19 +491,20 @@ int exynos_power_init(void)
  #ifdef CONFIG_USB_GADGET
  static int s5pc210_phy_control(int on)
  {
-   struct pmic *p_pmic;
-
-   p_pmic = pmic_get("MAX77686_PMIC");
-   if (!p_pmic)
-   return -ENODEV;
+   struct udevice *dev;
+   int ret;

-   if (pmic_probe(p_pmic))
-   return -1;
+   ret = regulator_get("VDD_UOTG_3.0V", &dev);
+   if (ret) {
+   error("Regulator get error: %d", ret);
+   return ret;
+   }

 if (on)
-   return max77686_set_ldo_mode(p_pmic, 12, OPMODE_ON);
+   return regulator_set_mode(dev, OPMODE_ON);
 else
-   return max77686_set_ldo_mode(p_pmic, 12, OPMODE_LPM);
+   return regulator_set_mode(dev, OPMODE_LPM);
+
  }

  struct s3c_plat_otg_data s5pc210_otg_data = {
@@ -472,7 +521,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 *dev;
+   int ret;

 /* Set Ref freq 0 => 24MHz, 1 => 26MHz*/
 /* Odroid Us have it at 24MHz, Odroid Xs at 26MHz */
@@ -490,14 +540,31 @@ 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");


Re: [U-Boot] [PATCH v3 15/17] odroid: board: add support to dm pmic api

2015-03-29 Thread Simon Glass
 Hi Przemyslaw,

On 24 March 2015 at 14:30, Przemyslaw Marczak  wrote:
> This commit change the old pmic framework calls with the new ones.
>
> Signed-off-by: Przemyslaw Marczak 
> ---
> Changes v2:
> - remove board_init_i2c() call
> - update regulator calls
> - update headers
> - samsung/misc.c: include required header
>
> Changes v3:
> - adjust regulator calls to new api
> ---
>  board/samsung/common/misc.c   |   1 +
>  board/samsung/odroid/odroid.c | 113 
> +-
>  2 files changed, 91 insertions(+), 23 deletions(-)
>
> diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c
> index 1a77c82..f0d69d4 100644
> --- a/board/samsung/common/misc.c
> +++ b/board/samsung/common/misc.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c
> index ae41c29..aa3b0ff 100644
> --- a/board/samsung/odroid/odroid.c
> +++ b/board/samsung/odroid/odroid.c
> @@ -12,7 +12,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -405,15 +407,62 @@ static void board_gpio_init(void)
>
>  static int pmic_init_max77686(void)
>  {
> -   struct pmic *p = pmic_get("MAX77686_PMIC");
> +   struct udevice *dev;
> +   int ret;
>
> -   if (pmic_probe(p))
> -   return -ENODEV;
> +   ret = regulator_get("VDDQ_EMMC_1.8V", &dev);
> +   if (ret) {
> +   error("Regulator get error: %d", ret);
> +   return ret;
> +   }
> +
> +   ret = regulator_set_value(dev, 180);
> +   if (ret) {
> +   error("Regulator %s value setting error: %d", dev->name, ret);
> +   return ret;
> +   }
> +
> +   ret = regulator_set_enable(dev, true);
> +   if (ret) {
> +   error("Regulator %s enable error: %d", dev->name, ret);
> +   return ret;
> +   }

How about adding a function that finds a regulator, sets its voltage
and enables it? Then you can avoid duplicating the same code 3 times.

> +
> +   ret = regulator_get("TFLASH_2.8V", &dev);
> +   if (ret) {
> +   error("Regulator get error: %d", ret);
> +   return ret;
> +   }
> +
> +   ret = regulator_set_value(dev, 280);
> +   if (ret) {
> +   error("Regulator %s value setting error: %d", dev->name, ret);
> +   return ret;
> +   }
> +
> +   ret = regulator_set_enable(dev, true);
> +   if (ret) {
> +   error("Regulator %s enable error: %d", dev->name, ret);
> +   return ret;
> +   }
> +
> +   ret = regulator_get("VDDQ_EMMC_2.8V", &dev);
> +   if (ret) {
> +   error("Regulator get error: %d", ret);
> +   return ret;
> +   }
>
> -   /* Set LDO Voltage */
> -   max77686_set_ldo_voltage(p, 20, 180);   /* LDO20 eMMC */
> -   max77686_set_ldo_voltage(p, 21, 280);   /* LDO21 SD */
> -   max77686_set_ldo_voltage(p, 22, 280);   /* LDO22 eMMC */
> +   ret = regulator_set_value(dev, 280);
> +   if (ret) {
> +   error("Regulator %s value setting error: %d", dev->name, ret);
> +   return ret;
> +   }
> +
> +   ret = regulator_set_enable(dev, true);
> +   if (ret) {
> +   error("Regulator %s enable error: %d", dev->name, ret);
> +   return ret;
> +   }
>
> return 0;
>  }
> @@ -434,7 +483,6 @@ int exynos_init(void)
>
>  int exynos_power_init(void)
>  {
> -   pmic_init(0);
> pmic_init_max77686();
>
> return 0;
> @@ -443,19 +491,20 @@ int exynos_power_init(void)
>  #ifdef CONFIG_USB_GADGET
>  static int s5pc210_phy_control(int on)
>  {
> -   struct pmic *p_pmic;
> -
> -   p_pmic = pmic_get("MAX77686_PMIC");
> -   if (!p_pmic)
> -   return -ENODEV;
> +   struct udevice *dev;
> +   int ret;
>
> -   if (pmic_probe(p_pmic))
> -   return -1;
> +   ret = regulator_get("VDD_UOTG_3.0V", &dev);
> +   if (ret) {
> +   error("Regulator get error: %d", ret);
> +   return ret;
> +   }
>
> if (on)
> -   return max77686_set_ldo_mode(p_pmic, 12, OPMODE_ON);
> +   return regulator_set_mode(dev, OPMODE_ON);
> else
> -   return max77686_set_ldo_mode(p_pmic, 12, OPMODE_LPM);
> +   return regulator_set_mode(dev, OPMODE_LPM);
> +
>  }
>
>  struct s3c_plat_otg_data s5pc210_otg_data = {
> @@ -472,7 +521,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 *dev;
> +   int ret;
>
> /* Set Ref freq 0 => 24MHz, 1 => 26MHz*/
> /* Odroid Us have it at 24MHz, Odroid Xs at 26MHz */
> @@ -490,14 +540