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-keer...@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-keer...@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.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
