On Thu, 15 Sep 2016 09:27:22 +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 <marcel.ziswi...@toradex.com>
>
> ---
>
> Changes in v3:
> - Introduce new patch to honour optionality of fixed regulator enable
> GPIO.
>
> Changes in v2: None
>
> drivers/power/regulator/fixed.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/power/regulator/fixed.c b/drivers/power/regulator/fixed.c
> index 37b8400..c68da70 100644
> --- a/drivers/power/regulator/fixed.c
> +++ b/drivers/power/regulator/fixed.c
> @@ -37,11 +37,12 @@ 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 = &dev_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);
> + debug("Fixed reg optional enable GPIO - not found! Error: %d",
> + ret);
If we're changing this, should we tighten up the error handling? It's
only ENOENT that we want to ignore here, other errors should probably
make fixed_regulator_ofdata_to_platdata() fail.
> /* Get optional ramp up delay */
> dev_pdata->startup_delay_us = fdtdec_get_uint(gd->fdt_blob,
> @@ -87,8 +88,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;
This looks much better to me (I thought I had a similar change locally
but I can't find it now).
> return dm_gpio_get_value(&dev_pdata->gpio);
> }
> @@ -98,8 +100,9 @@ static int fixed_regulator_set_enable(struct udevice *dev,
> bool enable)
> struct fixed_regulator_platdata *dev_pdata = dev_get_platdata(dev);
> int ret;
>
> + /* Enable GPIO is optional */
> if (!dev_pdata->gpio.dev)
> - return -ENOSYS;
> + return 0;
I'm not sure about this change, the current behaviour seems correct to
me. After this we're pretending that fixed_set_enable(dev, false) has
succeeded, when it has not.
> ret = dm_gpio_set_value(&dev_pdata->gpio, enable);
> if (ret) {
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
