power: regulator: Add support for gpio regulators
PATCH 1/1 power: regulator: Add support for gpio regulators
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.
--- 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
drivers/power/regulator/Kconfig +8 -0
@@ -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
drivers/power/regulator/Makefile +1 -0
@@ -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
drivers/power/regulator/gpio-regulator.c new file +136 -0
136 lines not on this page · raw message
include/power/regulator.h +1 -0
@@ -108,6 +108,7 @@ enum regulator_type {REGULATOR_TYPE_BUCK ,REGULATOR_TYPE_DVS ,REGULATOR_TYPE_FIXED , +REGULATOR_TYPE_GPIO , REGULATOR_TYPE_OTHER, };
Signature · 1 line
1.9.1
PATCH 1/1 power: regulator: Add support for gpio regulators
Hello Keerthy,
Quoted 63 lines · Keerthy
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 <[email protected]>--- 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.cdrivers/power/regulator/Kconfig +8
-0@@ -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_RK808drivers/power/regulator/Makefile +1
-0@@ -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.odrivers/power/regulator/gpio-regulator.c new file +13
-0@@ -0,0 +1,136 @@ +/* +* (C) Copyright 2016 Texas Instruments Incorporated, <www.ti.com> +* Keerthy <[email protected]> +* +* 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>
drivers/power/regulator/gpio-regulator.c line 23
Quoted 10 lines
drivers/power/regulator/gpio-regulator.c new file +10
-0+#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.
But as I can see in the Linux, this driver should provide a structure for the gpio states.
Quoted 28 lines
drivers/power/regulator/gpio-regulator.c new file +28
-0+ +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 );
drivers/power/regulator/gpio-regulator.c line 61
Quoted 10 lines
drivers/power/regulator/gpio-regulator.c new file +10
-0+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@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.
Quoted 15 lines
drivers/power/regulator/gpio-regulator.c new file +15
-0+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; +
drivers/power/regulator/gpio-regulator.c line 86
Quoted 10 lines
drivers/power/regulator/gpio-regulator.c new file +10
-0+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.
Quoted 12 lines
drivers/power/regulator/gpio-regulator.c new file +12
-0+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; +
drivers/power/regulator/gpio-regulator.c line 108
Quoted 10 lines
drivers/power/regulator/gpio-regulator.c new file +10
-0+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.
Quoted 33 lines
drivers/power/regulator/gpio-regulator.c new file +28
-0+ 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 ), +};include/power/regulator.h +0
-0
include/power/regulator.h enum regulator_type { · line 114
Quoted 7 lines
include/power/regulator.h +1
-0@@ -108,6 +108,7 @@ enum regulator_type {REGULATOR_TYPE_BUCK ,REGULATOR_TYPE_DVS ,REGULATOR_TYPE_FIXED , +REGULATOR_TYPE_GPIO , REGULATOR_TYPE_OTHER, };
Best regards,
Signature · 4 lines
Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com
PATCH 1/1 power: regulator: Add support for gpio regulators
Quoted 75 lines · Przemyslaw Marczak
Hello Keerthy,
Quoted 72 lines · Keerthy
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 <[email protected]>--- 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.cdrivers/power/regulator/Kconfig +0
-0@@ -58,6 +58,14 @@ config DM_REGULATOR_FIXED features for fixed value regulators. The driver implementsget/set api
for enable and get only for voltage value.
+configDM_REGULATOR_GPIOdrivers/power/regulator/Kconfig +7
-0+ 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_RK808drivers/power/regulator/Makefile +1
-0@@ -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.odrivers/power/regulator/gpio-regulator.c new file +18
-0@@ -0,0 +1,136 @@ +/* +* (C) Copyright 2016 Texas Instruments Incorporated, <www.ti.com> +* Keerthy <[email protected]> +* +* 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 ; +
Quoted 8 lines
drivers/power/regulator/gpio-regulator.c line 23
Quoted 5 lines
drivers/power/regulator/gpio-regulator.c new file +5
-0+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.
Quoted 3 lines
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.
Quoted 41 lines
Quoted 29 lines
drivers/power/regulator/gpio-regulator.c new file +28
-0+ +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 );drivers/power/regulator/gpio-regulator.c line 61
Quoted 10 lines
drivers/power/regulator/gpio-regulator.c new file +10
-0+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.
Quoted 6 lines
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.
Quoted 20 lines
Quoted 20 lines
drivers/power/regulator/gpio-regulator.c new file +19
-0+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);
Quoted 8 lines
drivers/power/regulator/gpio-regulator.c line 86
Quoted 6 lines
drivers/power/regulator/gpio-regulator.c new file +6
-0+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.
Quoted 18 lines
Quoted 18 lines
drivers/power/regulator/gpio-regulator.c new file +17
-0+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 ;
Quoted 8 lines
drivers/power/regulator/gpio-regulator.c line 108
Quoted 5 lines
drivers/power/regulator/gpio-regulator.c new file +5
-0+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.
Quoted 35 lines
Quoted 35 lines
drivers/power/regulator/gpio-regulator.c new file +28
-0+ 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 ), +};include/power/regulator.h +0
-0@@ -108,6 +108,7 @@ enum regulator_type { REGULATOR_TYPE_BUCK,
Quoted 8 lines
include/power/regulator.h enum regulator_type { · line 114
Quoted 6 lines
include/power/regulator.h +1
-0REGULATOR_TYPE_DVS ,REGULATOR_TYPE_FIXED , +REGULATOR_TYPE_GPIO , REGULATOR_TYPE_OTHER, };Best regards,
Thanks for the review,
- Keerthy
Quoted 1 line
PATCH 1/1 power: regulator: Add support for gpio regulators
Quoted 89 lines · Keerthy
Quoted 77 lines · Przemyslaw Marczak
Hello Keerthy,
Quoted 74 lines · Keerthy
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 <[email protected]>--- 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.cdrivers/power/regulator/Kconfig +0
-0@@ -58,6 +58,14 @@ config DM_REGULATOR_FIXED features for fixed value regulators. The driver implementsget/set api
for enable and get only for voltage value.
+configDM_REGULATOR_GPIOdrivers/power/regulator/Kconfig +4
-0+ bool "Enable Driver Model for GPIO REGULATOR" + depends on DM_REGULATOR + ---help--- + This config enables implementation of driver-model regulatoruclass
drivers/power/regulator/Kconfig +3
-0+ 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_RK808drivers/power/regulator/Makefile +1
-0@@ -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.odrivers/power/regulator/gpio-regulator.c new file +18
-0@@ -0,0 +1,136 @@ +/* +* (C) Copyright 2016 Texas Instruments Incorporated, <www.ti.com> +* Keerthy <[email protected]> +* +* 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 ; +Quoted 8 lines
drivers/power/regulator/gpio-regulator.c line 23
Quoted 5 lines
drivers/power/regulator/gpio-regulator.c new file +5
-0+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.
Quoted 3 lines
min_uV,max_uVrepresent the minimum and maximum voltages indm_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.
Quoted 53 lines
Quoted 3 lines
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.
Quoted 46 lines
Quoted 29 lines
drivers/power/regulator/gpio-regulator.c new file +28
-0+ +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 );drivers/power/regulator/gpio-regulator.c line 61
Quoted 10 lines
drivers/power/regulator/gpio-regulator.c new file +10
-0+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.
Quoted 6 lines
Quoted 1 line
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_platdatastructure 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.
Quoted 105 lines
Quoted 20 lines
Quoted 20 lines
drivers/power/regulator/gpio-regulator.c new file +19
-0+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);Quoted 8 lines
drivers/power/regulator/gpio-regulator.c line 86
Quoted 6 lines
drivers/power/regulator/gpio-regulator.c new file +6
-0+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.
Quoted 18 lines
Quoted 18 lines
drivers/power/regulator/gpio-regulator.c new file +17
-0+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 ;Quoted 8 lines
drivers/power/regulator/gpio-regulator.c line 108
Quoted 5 lines
drivers/power/regulator/gpio-regulator.c new file +5
-0+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.
Quoted 36 lines
Quoted 36 lines
drivers/power/regulator/gpio-regulator.c new file +27
-0+ 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),drivers/power/regulator/gpio-regulator.c new file +1
-0+};include/power/regulator.h +0
-0@@ -108,6 +108,7 @@ enum regulator_type { REGULATOR_TYPE_BUCK,Quoted 8 lines
include/power/regulator.h enum regulator_type { · line 114
Quoted 6 lines
include/power/regulator.h +1
-0REGULATOR_TYPE_DVS ,REGULATOR_TYPE_FIXED , +REGULATOR_TYPE_GPIO , REGULATOR_TYPE_OTHER, };Best regards,
Quoted 3 lines
Thanks for the review,
- Keerthy
You are welcome
Signature · 4 lines
Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com
PATCH 1/1 power: regulator: Add support for gpio regulators
Quoted 159 lines · Przemyslaw Marczak
Quoted 89 lines · Keerthy
Quoted 77 lines · Przemyslaw Marczak
Hello Keerthy,
Quoted 74 lines · Keerthy
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 <[email protected]>--- 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.cdrivers/power/regulator/Kconfig +0
-0@@ -58,6 +58,14 @@ config DM_REGULATOR_FIXED features for fixed value regulators. The driver implementsget/set api
for enable and get only for voltage value.
+configDM_REGULATOR_GPIOdrivers/power/regulator/Kconfig +4
-0+ bool "Enable Driver Model for GPIO REGULATOR" + depends on DM_REGULATOR + ---help--- + This config enables implementation of driver-model regulatoruclass
drivers/power/regulator/Kconfig +3
-0+ 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_RK808drivers/power/regulator/Makefile +1
-0@@ -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.odrivers/power/regulator/gpio-regulator.c new file +18
-0@@ -0,0 +1,136 @@ +/* +* (C) Copyright 2016 Texas Instruments Incorporated, <www.ti.com> +* Keerthy <[email protected]> +* +* 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 ; +Quoted 8 lines
drivers/power/regulator/gpio-regulator.c line 23
Quoted 5 lines
drivers/power/regulator/gpio-regulator.c new file +5
-0+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.Quoted 3 lines
min_uV,max_uVrepresent the minimum and maximum voltages indm_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.
Quoted 53 lines
Quoted 3 lines
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.
Quoted 46 lines
Quoted 29 lines
drivers/power/regulator/gpio-regulator.c new file +28
-0+ +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 );drivers/power/regulator/gpio-regulator.c line 61
Quoted 10 lines
drivers/power/regulator/gpio-regulator.c new file +10
-0+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.Quoted 6 lines
Quoted 1 line
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_platdatastructure and set states corresponding to requested voltages.
Quoted 8 lines
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.
Yes. I sent a V2 :-) with your comments fixed.
Quoted 112 lines
Quoted 106 lines
Quoted 21 lines
Quoted 21 lines
drivers/power/regulator/gpio-regulator.c new file +19
-0+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);Quoted 8 lines
drivers/power/regulator/gpio-regulator.c line 86
Quoted 6 lines
drivers/power/regulator/gpio-regulator.c new file +6
-0+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.
Quoted 18 lines
Quoted 18 lines
drivers/power/regulator/gpio-regulator.c new file +17
-0+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 ;Quoted 8 lines
drivers/power/regulator/gpio-regulator.c line 108
Quoted 5 lines
drivers/power/regulator/gpio-regulator.c new file +5
-0+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.
Quoted 36 lines
Quoted 36 lines
drivers/power/regulator/gpio-regulator.c new file +27
-0+ 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),drivers/power/regulator/gpio-regulator.c new file +1
-0+};include/power/regulator.h +0
-0@@ -108,6 +108,7 @@ enum regulator_type { REGULATOR_TYPE_BUCK,Quoted 8 lines
include/power/regulator.h enum regulator_type { · line 114
Quoted 6 lines
include/power/regulator.h +1
-0REGULATOR_TYPE_DVS ,REGULATOR_TYPE_FIXED , +REGULATOR_TYPE_GPIO , REGULATOR_TYPE_OTHER, };Best regards,
Quoted 3 lines
Thanks for the review,
- KeerthyYou are welcome