Re: [U-Boot] [PATCH v4 3/5] regulator: fixed: honour optionality of enable gpio

[Stephen Warren]

On 09/16/2016 01:33 AM, Marcel Ziswiler wrote:

According to the binding documentation the fixed regulator enable GPIO
is optional. However so far registration thereof failed if no enable
GPIO was specified. Fix this by making it entirely optional whether an
enable GPIO is used.



diff --git a/drivers/power/regulator/fixed.c b/drivers/power/regulator/fixed.c



@@ -37,11 +39,14 @@ static int fixed_regulator_ofdata_to_platdata(struct 
udevice *dev)
/* Set type to fixed */
uc_pdata->type = REGULATOR_TYPE_FIXED;

-   /* Get fixed regulator gpio desc */
+   /* Get fixed regulator optional enable GPIO desc */
gpio = _pdata->gpio;
ret = gpio_request_by_name(dev, "gpio", 0, gpio, GPIOD_IS_OUT);
-   if (ret)
-   debug("Fixed regulator gpio - not found! Error: %d", ret);
+   if (ret) {
+   debug("Fixed regulator optional enable GPIO: %d\n", ret);
+   if (ret != -ENOENT)
+   return ret;


That message doesn't exactly make it obvious that it's describing a "not 
found" condition. Still, it is a debug() statement, so presumably anyone 
seeing the issue can just look at the code to see what's going on and 
work it out easily enough. So, aside from the DEBUG setting you already 
meantioned,


Acked-by: Stephen Warren 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v4 3/5] regulator: fixed: honour optionality of enable gpio

[Marcel Ziswiler]

On Fri, 2016-09-16 at 09:33 +0200, Marcel Ziswiler wrote:
> According to the binding documentation the fixed regulator enable
> GPIO
> is optional. However so far registration thereof failed if no enable
> GPIO was specified. Fix this by making it entirely optional whether
> an
> enable GPIO is used.
> 
> Signed-off-by: Marcel Ziswiler 
> 
> ---
> 
> Changes in v4:
> - Use full regulator rather than reg wording again in debug message
> as
>   suggested by Przemyslaw.
> - Tighten up error handling as suggested by John.
> - Fix set_enable() as suggested by Stephen.
> 
> Changes in v3:
> - Introduce new patch to honour optionality of fixed regulator enable
>   GPIO.
> 
> Changes in v2: None
> 
>  drivers/power/regulator/fixed.c | 22 --
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/power/regulator/fixed.c
> b/drivers/power/regulator/fixed.c
> index 37b8400..6ffb25d 100644
> --- a/drivers/power/regulator/fixed.c
> +++ b/drivers/power/regulator/fixed.c
> @@ -6,6 +6,8 @@
>   * SPDX-License-Identifier:  GPL-2.0+
>   */
>  
> +#define DEBUG
> +

Sorry, I just noticed I left DEBUG enabled here, Bummer. I will remove
that in a final version.

>  #include 
>  #include 
>  #include 
> @@ -37,11 +39,14 @@ static int
> fixed_regulator_ofdata_to_platdata(struct udevice *dev)
>   /* Set type to fixed */
>   uc_pdata->type = REGULATOR_TYPE_FIXED;
>  
> - /* Get fixed regulator gpio desc */
> + /* Get fixed regulator optional enable GPIO desc */
>   gpio = _pdata->gpio;
>   ret = gpio_request_by_name(dev, "gpio", 0, gpio,
> GPIOD_IS_OUT);
> - if (ret)
> - debug("Fixed regulator gpio - not found! Error: %d",
> ret);
> + if (ret) {
> + debug("Fixed regulator optional enable GPIO: %d\n",
> ret);
> + if (ret != -ENOENT)
> + return ret;
> + }
>  
>   /* Get optional ramp up delay */
>   dev_pdata->startup_delay_us = fdtdec_get_uint(gd->fdt_blob,
> @@ -87,8 +92,9 @@ static bool fixed_regulator_get_enable(struct
> udevice *dev)
>  {
>   struct fixed_regulator_platdata *dev_pdata =
> dev_get_platdata(dev);
>  
> + /* Enable GPIO is optional */
>   if (!dev_pdata->gpio.dev)
> - return false;
> + return true;
>  
>   return dm_gpio_get_value(_pdata->gpio);
>  }
> @@ -98,8 +104,12 @@ static int fixed_regulator_set_enable(struct
> udevice *dev, bool enable)
>   struct fixed_regulator_platdata *dev_pdata =
> dev_get_platdata(dev);
>   int ret;
>  
> - if (!dev_pdata->gpio.dev)
> - return -ENOSYS;
> + /* Enable GPIO is optional */
> + if (!dev_pdata->gpio.dev) {
> + if (!enable)
> + return -ENOSYS;
> + return 0;
> + }
>  
>   ret = dm_gpio_set_value(_pdata->gpio, enable);
>   if (ret) {
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v4 3/5] regulator: fixed: honour optionality of enable gpio

[Marcel Ziswiler]

According to the binding documentation the fixed regulator enable GPIO
is optional. However so far registration thereof failed if no enable
GPIO was specified. Fix this by making it entirely optional whether an
enable GPIO is used.

Signed-off-by: Marcel Ziswiler 

---

Changes in v4:
- Use full regulator rather than reg wording again in debug message as
  suggested by Przemyslaw.
- Tighten up error handling as suggested by John.
- Fix set_enable() as suggested by Stephen.

Changes in v3:
- Introduce new patch to honour optionality of fixed regulator enable
  GPIO.

Changes in v2: None

 drivers/power/regulator/fixed.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/power/regulator/fixed.c b/drivers/power/regulator/fixed.c
index 37b8400..6ffb25d 100644
--- a/drivers/power/regulator/fixed.c
+++ b/drivers/power/regulator/fixed.c
@@ -6,6 +6,8 @@
  * SPDX-License-Identifier:GPL-2.0+
  */
 
+#define DEBUG
+
 #include 
 #include 
 #include 
@@ -37,11 +39,14 @@ static int fixed_regulator_ofdata_to_platdata(struct 
udevice *dev)
/* Set type to fixed */
uc_pdata->type = REGULATOR_TYPE_FIXED;
 
-   /* Get fixed regulator gpio desc */
+   /* Get fixed regulator optional enable GPIO desc */
gpio = _pdata->gpio;
ret = gpio_request_by_name(dev, "gpio", 0, gpio, GPIOD_IS_OUT);
-   if (ret)
-   debug("Fixed regulator gpio - not found! Error: %d", ret);
+   if (ret) {
+   debug("Fixed regulator optional enable GPIO: %d\n", ret);
+   if (ret != -ENOENT)
+   return ret;
+   }
 
/* Get optional ramp up delay */
dev_pdata->startup_delay_us = fdtdec_get_uint(gd->fdt_blob,
@@ -87,8 +92,9 @@ static bool fixed_regulator_get_enable(struct udevice *dev)
 {
struct fixed_regulator_platdata *dev_pdata = dev_get_platdata(dev);
 
+   /* Enable GPIO is optional */
if (!dev_pdata->gpio.dev)
-   return false;
+   return true;
 
return dm_gpio_get_value(_pdata->gpio);
 }
@@ -98,8 +104,12 @@ static int fixed_regulator_set_enable(struct udevice *dev, 
bool enable)
struct fixed_regulator_platdata *dev_pdata = dev_get_platdata(dev);
int ret;
 
-   if (!dev_pdata->gpio.dev)
-   return -ENOSYS;
+   /* Enable GPIO is optional */
+   if (!dev_pdata->gpio.dev) {
+   if (!enable)
+   return -ENOSYS;
+   return 0;
+   }
 
ret = dm_gpio_set_value(_pdata->gpio, enable);
if (ret) {
-- 
2.5.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot