[U-Boot] [PATCH] power: regulator: Add support for gpio regulators
Przemyslaw Marczak
p.marczak at samsung.com
Thu Sep 15 13:42:02 CEST 2016
On 09/15/2016 12:34 PM, Keerthy wrote:
>
>
> On Thursday 15 September 2016 03:04 PM, Przemyslaw Marczak wrote:
>> Hello Keerthy,
>>
>> On 09/15/2016 10:25 AM, Keerthy wrote:
>>> Add support for gpio regulators. As of now this driver caters
>>> to gpio regulators with one gpio. Supports setting voltage values to
>>> gpio
>>> regulators and retrieving the values.
>>>
>>> Signed-off-by: Keerthy <j-keerthy at ti.com>
>>> ---
>>> drivers/power/regulator/Kconfig | 8 ++
>>> drivers/power/regulator/Makefile | 1 +
>>> drivers/power/regulator/gpio-regulator.c | 136
>>> +++++++++++++++++++++++++++++++
>>> include/power/regulator.h | 1 +
>>> 4 files changed, 146 insertions(+)
>>> create mode 100644 drivers/power/regulator/gpio-regulator.c
>>>
>>> diff --git a/drivers/power/regulator/Kconfig
>>> b/drivers/power/regulator/Kconfig
>>> index 2510474..4776011 100644
>>> --- a/drivers/power/regulator/Kconfig
>>> +++ b/drivers/power/regulator/Kconfig
>>> @@ -58,6 +58,14 @@ config DM_REGULATOR_FIXED
>>> features for fixed value regulators. The driver implements
>>> get/set api
>>> for enable and get only for voltage value.
>>> +config DM_REGULATOR_GPIO
>>> + bool "Enable Driver Model for GPIO REGULATOR"
>>> + depends on DM_REGULATOR
>>> + ---help---
>>> + This config enables implementation of driver-model regulator
>>> uclass
>>> + features for gpio regulators. The driver implements get/set for
>>> + voltage value.
>>> +
>>> config REGULATOR_RK808
>>> bool "Enable driver for RK808 regulators"
>>> depends on DM_REGULATOR && PMIC_RK808
>>> diff --git a/drivers/power/regulator/Makefile
>>> b/drivers/power/regulator/Makefile
>>> index 2093048..2d350cb 100644
>>> --- a/drivers/power/regulator/Makefile
>>> +++ b/drivers/power/regulator/Makefile
>>> @@ -10,6 +10,7 @@ obj-$(CONFIG_REGULATOR_ACT8846) += act8846.o
>>> obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
>>> obj-$(CONFIG_DM_REGULATOR_PFUZE100) += pfuze100.o
>>> obj-$(CONFIG_$(SPL_)DM_REGULATOR_FIXED) += fixed.o
>>> +obj-$(CONFIG_$(SPL_)DM_REGULATOR_GPIO) += gpio-regulator.o
>>> obj-$(CONFIG_REGULATOR_RK808) += rk808.o
>>> obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
>>> obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o
>>> diff --git a/drivers/power/regulator/gpio-regulator.c
>>> b/drivers/power/regulator/gpio-regulator.c
>>> new file mode 100644
>>> index 0000000..9c8832e
>>> --- /dev/null
>>> +++ b/drivers/power/regulator/gpio-regulator.c
>>> @@ -0,0 +1,136 @@
>>> +/*
>>> + * (C) Copyright 2016 Texas Instruments Incorporated, <www.ti.com>
>>> + * Keerthy <j-keerthy at ti.com>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <fdtdec.h>
>>> +#include <errno.h>
>>> +#include <dm.h>
>>> +#include <i2c.h>
>>> +#include <asm/gpio.h>
>>> +#include <power/pmic.h>
>>> +#include <power/regulator.h>
>>> +
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +struct gpio_regulator_platdata {
>>> + struct gpio_desc gpio; /* GPIO for regulator voltage control */
>>> + int gpio_low_uV;
>>> + int gpio_high_uV;
>>> +};
>>
>> The low/high values can be provided by regulator's uclass driver in
>> struct of type "dm_regulator_uclass_platdata", as for other regulators.
>
> min_uV, max_uV represent the minimum and maximum voltages in
> dm_regulator_uclass_platdata. I would not use them here.
>
Ah right, sorry I just get wrong the name of those variables above - as
min/max uV, but your point was about the GPIO lo/hi state.
>>
>> But as I can see in the Linux, this driver should provide a structure
>> for the gpio states.
>
> Yes so i am keeping the voltage values for 0 and 1 states in a driver
> specific struct.
>
>>
>>> +
>>> +static int gpio_regulator_ofdata_to_platdata(struct udevice *dev)
>>> +{
>>> + struct dm_regulator_uclass_platdata *uc_pdata;
>>> + struct gpio_regulator_platdata *dev_pdata;
>>> + struct gpio_desc *gpio;
>>> + const void *blob = gd->fdt_blob;
>>> + int node = dev->of_offset;
>>> + int ret, count, i;
>>> + u32 states_array[8];
>>> +
>>> + dev_pdata = dev_get_platdata(dev);
>>> + uc_pdata = dev_get_uclass_platdata(dev);
>>> + if (!uc_pdata)
>>> + return -ENXIO;
>>> +
>>> + /* Set type to gpio */
>>> + uc_pdata->type = REGULATOR_TYPE_GPIO;
>>> +
>>> + /*
>>> + * Get gpio regulator gpio desc
>>> + * Assuming one GPIO per regulator.
>>> + * Can be extended later to multiple GPIOs
>>> + * per gpio-regulator. As of now no instance with multiple
>>> + * gpios is presnt
>>> + */
>>> + gpio = &dev_pdata->gpio;
>>> + ret = gpio_request_by_name(dev, "gpios", 0, gpio, GPIOD_IS_OUT);
>>> + if (ret)
>>> + debug("regulator gpio - not found! Error: %d", ret);
>>> +
>>> + count = fdtdec_get_int_array_count(blob, node, "states",
>>> + states_array, 8);
>>> +
>>> + if (!count)
>>> + return -EINVAL;
>>> +
>>> + for (i = 0; i < count; i += 2) {
>> The below condition is valid for most devices.
>>
>> In Linux we can find dts for some ARMs in which I can find at least two
>> boards,
>> with inverted meaning of state/voltage, so "0", can be also valid for
>> the high uV.
>>
>> I think, this driver should keep possible states in platdata.
>
> Okay. I get the point. So i will have both states and corresponding
> states voltages stored in gpio_regulator_platdata structure and set
> states corresponding to requested voltages.
>
Yes, so actually the only issue is low/high interpretation.
Maybe the easiest way is a simple array with the voltage,
since there are only two states. But maybe the name of gpio_low/hi..
is quite misleading. What about an array gpio_state_uV[2] or something
like that?
Then you can use GPIO state's true/false as array index for get method.
>>
>>> + if (states_array[i + 1] == 0)
>>> + dev_pdata->gpio_low_uV = states_array[i];
>>> + else
>>> + dev_pdata->gpio_high_uV = states_array[i];
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int gpio_regulator_get_value(struct udevice *dev)
>>> +{
>>> + struct dm_regulator_uclass_platdata *uc_pdata;
>>> + struct gpio_regulator_platdata *dev_pdata = dev_get_platdata(dev);
>>> + int enable;
>>> +
>>> + if (!dev_pdata->gpio.dev)
>>> + return -ENOSYS;
>>> +
>>> + uc_pdata = dev_get_uclass_platdata(dev);
>>> + if (uc_pdata->min_uV > uc_pdata->max_uV) {
>>> + debug("Invalid constraints for: %s\n", uc_pdata->name);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + enable = dm_gpio_get_value(&dev_pdata->gpio);
>> The returned value (enable) should be compared to the states kept in
>> platdata.
>
> Sure.
>
>>
>>> + if (enable)
>>> + return dev_pdata->gpio_high_uV;
>>> + else
>>> + return dev_pdata->gpio_low_uV;
>>> +}
>>> +
>>> +static int gpio_regulator_set_value(struct udevice *dev, int uV)
>>> +{
>>> + struct gpio_regulator_platdata *dev_pdata = dev_get_platdata(dev);
>>> + int ret;
>>> + bool enable;
>>> +
>>> + if (!dev_pdata->gpio.dev)
>>> + return -ENOSYS;
>>> +
>>> + if (uV == dev_pdata->gpio_high_uV)
>>> + enable = 1;
>>> + else if (uV == dev_pdata->gpio_low_uV)
>>> + enable = 0;
>>> + else
>>> + return -EINVAL;
>>> +
>>
>> And also here you should get the "enable" value from states kept in
>> platdata.
>
> Okay.
>
>>
>>> + ret = dm_gpio_set_value(&dev_pdata->gpio, enable);
>>> + if (ret) {
>>> + error("Can't set regulator : %s gpio to: %d\n", dev->name,
>>> + enable);
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct dm_regulator_ops gpio_regulator_ops = {
>>> + .get_value = gpio_regulator_get_value,
>>> + .set_value = gpio_regulator_set_value,
>>> +};
>>> +
>>> +static const struct udevice_id gpio_regulator_ids[] = {
>>> + { .compatible = "regulator-gpio" },
>>> + { },
>>> +};
>>> +
>>> +U_BOOT_DRIVER(gpio_regulator) = {
>>> + .name = "gpio regulator",
>>> + .id = UCLASS_REGULATOR,
>>> + .ops = &gpio_regulator_ops,
>>> + .of_match = gpio_regulator_ids,
>>> + .ofdata_to_platdata = gpio_regulator_ofdata_to_platdata,
>>> + .platdata_auto_alloc_size = sizeof(struct
>>> gpio_regulator_platdata),
>>> +};
>>> diff --git a/include/power/regulator.h b/include/power/regulator.h
>>> index 8ae6b14..5d327e6 100644
>>> --- a/include/power/regulator.h
>>> +++ b/include/power/regulator.h
>>> @@ -108,6 +108,7 @@ enum regulator_type {
>>> REGULATOR_TYPE_BUCK,
>>> REGULATOR_TYPE_DVS,
>>> REGULATOR_TYPE_FIXED,
>>> + REGULATOR_TYPE_GPIO,
>>> REGULATOR_TYPE_OTHER,
>>> };
>>>
>>
>> Best regards,
>
> Thanks for the review,
> - Keerthy
>
You are welcome
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
More information about the U-Boot
mailing list