* [PATCH] power: regulator: Add support for regulator-force-boot-off
@ 2021-04-08 9:20 ` Stefan Roese
2021-04-08 22:52 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Roese @ 2021-04-08 9:20 UTC (permalink / raw)
To: u-boot
From: Konstantin Porotchkin <kostap@marvell.com>
Add support for regulator-force-boot-off DT property.
This property can be used by the board/device drivers for
turning off regulators on early init stages as pre-requisite
for the other components initialization.
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Simon Glass <sjg@chromium.org>
---
drivers/power/regulator/regulator-uclass.c | 38 ++++++++++++++++++++++
include/power/regulator.h | 23 +++++++++++++
2 files changed, 61 insertions(+)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 4d2e730271f9..423867edced8 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -311,6 +311,17 @@ int regulator_autoset(struct udevice *dev)
return ret;
}
+int regulator_unset(struct udevice *dev)
+{
+ struct dm_regulator_uclass_plat *uc_pdata;
+
+ uc_pdata = dev_get_uclass_plat(dev);
+ if (uc_pdata->force_off)
+ return regulator_set_enable(dev, false);
+
+ return -EMEDIUMTYPE;
+}
+
static void regulator_show(struct udevice *dev, int ret)
{
struct dm_regulator_uclass_plat *uc_pdata;
@@ -443,6 +454,7 @@ static int regulator_pre_probe(struct udevice *dev)
uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay",
0);
+ uc_pdata->force_off = dev_read_bool(dev, "regulator-force-boot-off");
node = dev_read_subnode(dev, "regulator-state-mem");
if (ofnode_valid(node)) {
@@ -495,6 +507,32 @@ int regulators_enable_boot_on(bool verbose)
return ret;
}
+int regulators_enable_boot_off(bool verbose)
+{
+ struct udevice *dev;
+ struct uclass *uc;
+ int ret;
+
+ ret = uclass_get(UCLASS_REGULATOR, &uc);
+ if (ret)
+ return ret;
+ for (uclass_first_device(UCLASS_REGULATOR, &dev);
+ dev;
+ uclass_next_device(&dev)) {
+ ret = regulator_unset(dev);
+ if (ret == -EMEDIUMTYPE) {
+ ret = 0;
+ continue;
+ }
+ if (verbose)
+ regulator_show(dev, ret);
+ if (ret == -ENOSYS)
+ ret = 0;
+ }
+
+ return ret;
+}
+
UCLASS_DRIVER(regulator) = {
.id = UCLASS_REGULATOR,
.name = "regulator",
diff --git a/include/power/regulator.h b/include/power/regulator.h
index da9a065bdde0..fad87c99e5db 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -151,6 +151,7 @@ enum regulator_flag {
* @max_uA* - maximum amperage (micro Amps)
* @always_on* - bool type, true or false
* @boot_on* - bool type, true or false
+ * @force_off* - bool type, true or false
* TODO(sjg at chromium.org): Consider putting the above two into @flags
* @ramp_delay - Time to settle down after voltage change (unit: uV/us)
* @flags: - flags value (see REGULATOR_FLAG_...)
@@ -176,6 +177,7 @@ struct dm_regulator_uclass_plat {
unsigned int ramp_delay;
bool always_on;
bool boot_on;
+ bool force_off;
const char *name;
int flags;
u8 ctrl_reg;
@@ -420,6 +422,15 @@ int regulator_set_mode(struct udevice *dev, int mode_id);
*/
int regulators_enable_boot_on(bool verbose);
+/**
+ * regulators_enable_boot_off() - disable regulators needed for boot
+ *
+ * This disables all regulators which are marked to be off at boot time.
+ *
+ * This effectively calls regulator_unset() for every regulator.
+ */
+int regulators_enable_boot_off(bool verbose);
+
/**
* regulator_autoset: setup the voltage/current on a regulator
*
@@ -439,6 +450,18 @@ int regulators_enable_boot_on(bool verbose);
*/
int regulator_autoset(struct udevice *dev);
+/**
+ * regulator_unset: turn off a regulator
+ *
+ * The setup depends on constraints found in device's uclass's platform data
+ * (struct dm_regulator_uclass_platdata):
+ *
+ * - Disable - will set - if 'force_off' is set to true,
+ *
+ * The function returns on the first-encountered error.
+ */
+int regulator_unset(struct udevice *dev);
+
/**
* regulator_autoset_by_name: setup the regulator given by its uclass's
* platform data name field. The setup depends on constraints found in device's
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] power: regulator: Add support for regulator-force-boot-off
2021-04-08 9:20 ` [PATCH] power: regulator: Add support for regulator-force-boot-off Stefan Roese
@ 2021-04-08 22:52 ` Jaehoon Chung
2021-04-09 0:37 ` Jaehoon Chung
2021-04-09 5:14 ` Stefan Roese
0 siblings, 2 replies; 6+ messages in thread
From: Jaehoon Chung @ 2021-04-08 22:52 UTC (permalink / raw)
To: u-boot
Hi Stefan,
On 4/8/21 6:20 PM, Stefan Roese wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Add support for regulator-force-boot-off DT property.
> This property can be used by the board/device drivers for
> turning off regulators on early init stages as pre-requisite
> for the other components initialization.
>
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
> drivers/power/regulator/regulator-uclass.c | 38 ++++++++++++++++++++++
> include/power/regulator.h | 23 +++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> index 4d2e730271f9..423867edced8 100644
> --- a/drivers/power/regulator/regulator-uclass.c
> +++ b/drivers/power/regulator/regulator-uclass.c
> @@ -311,6 +311,17 @@ int regulator_autoset(struct udevice *dev)
> return ret;
> }
>
> +int regulator_unset(struct udevice *dev)
> +{
> + struct dm_regulator_uclass_plat *uc_pdata;
> +
> + uc_pdata = dev_get_uclass_plat(dev);
> + if (uc_pdata->force_off)
Could you check whether uc_pdata is NULL or not?
It can be returned to NULL.
> + return regulator_set_enable(dev, false);
> +
> + return -EMEDIUMTYPE;
> +}
> +
> static void regulator_show(struct udevice *dev, int ret)
> {
> struct dm_regulator_uclass_plat *uc_pdata;
> @@ -443,6 +454,7 @@ static int regulator_pre_probe(struct udevice *dev)
> uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
> uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay",
> 0);
> + uc_pdata->force_off = dev_read_bool(dev, "regulator-force-boot-off");
>
> node = dev_read_subnode(dev, "regulator-state-mem");
> if (ofnode_valid(node)) {
> @@ -495,6 +507,32 @@ int regulators_enable_boot_on(bool verbose)
> return ret;
> }
>
> +int regulators_enable_boot_off(bool verbose)
> +{
> + struct udevice *dev;
> + struct uclass *uc;
> + int ret;
> +
> + ret = uclass_get(UCLASS_REGULATOR, &uc);
> + if (ret)
> + return ret;
> + for (uclass_first_device(UCLASS_REGULATOR, &dev);
> + dev;
typo?
Best Regards,
Jaehoon Chung
> + uclass_next_device(&dev)) {
> + ret = regulator_unset(dev);
> + if (ret == -EMEDIUMTYPE) {
> + ret = 0;
> + continue;
> + }
> + if (verbose)
> + regulator_show(dev, ret);
> + if (ret == -ENOSYS)
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +
> UCLASS_DRIVER(regulator) = {
> .id = UCLASS_REGULATOR,
> .name = "regulator",
> diff --git a/include/power/regulator.h b/include/power/regulator.h
> index da9a065bdde0..fad87c99e5db 100644
> --- a/include/power/regulator.h
> +++ b/include/power/regulator.h
> @@ -151,6 +151,7 @@ enum regulator_flag {
> * @max_uA* - maximum amperage (micro Amps)
> * @always_on* - bool type, true or false
> * @boot_on* - bool type, true or false
> + * @force_off* - bool type, true or false
> * TODO(sjg at chromium.org): Consider putting the above two into @flags
> * @ramp_delay - Time to settle down after voltage change (unit: uV/us)
> * @flags: - flags value (see REGULATOR_FLAG_...)
> @@ -176,6 +177,7 @@ struct dm_regulator_uclass_plat {
> unsigned int ramp_delay;
> bool always_on;
> bool boot_on;
> + bool force_off;
> const char *name;
> int flags;
> u8 ctrl_reg;
> @@ -420,6 +422,15 @@ int regulator_set_mode(struct udevice *dev, int mode_id);
> */
> int regulators_enable_boot_on(bool verbose);
>
> +/**
> + * regulators_enable_boot_off() - disable regulators needed for boot
> + *
> + * This disables all regulators which are marked to be off at boot time.
> + *
> + * This effectively calls regulator_unset() for every regulator.
> + */
> +int regulators_enable_boot_off(bool verbose);
> +
> /**
> * regulator_autoset: setup the voltage/current on a regulator
> *
> @@ -439,6 +450,18 @@ int regulators_enable_boot_on(bool verbose);
> */
> int regulator_autoset(struct udevice *dev);
>
> +/**
> + * regulator_unset: turn off a regulator
> + *
> + * The setup depends on constraints found in device's uclass's platform data
> + * (struct dm_regulator_uclass_platdata):
> + *
> + * - Disable - will set - if 'force_off' is set to true,
> + *
> + * The function returns on the first-encountered error.
> + */
> +int regulator_unset(struct udevice *dev);
> +
> /**
> * regulator_autoset_by_name: setup the regulator given by its uclass's
> * platform data name field. The setup depends on constraints found in device's
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] power: regulator: Add support for regulator-force-boot-off
2021-04-08 22:52 ` Jaehoon Chung
@ 2021-04-09 0:37 ` Jaehoon Chung
2021-04-09 5:20 ` Stefan Roese
2021-04-09 5:14 ` Stefan Roese
1 sibling, 1 reply; 6+ messages in thread
From: Jaehoon Chung @ 2021-04-09 0:37 UTC (permalink / raw)
To: u-boot
On 4/9/21 7:52 AM, Jaehoon Chung wrote:
> Hi Stefan,
>
> On 4/8/21 6:20 PM, Stefan Roese wrote:
>> From: Konstantin Porotchkin <kostap@marvell.com>
>>
>> Add support for regulator-force-boot-off DT property.
>> This property can be used by the board/device drivers for
>> turning off regulators on early init stages as pre-requisite
>> for the other components initialization.
>>
>> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>> drivers/power/regulator/regulator-uclass.c | 38 ++++++++++++++++++++++
>> include/power/regulator.h | 23 +++++++++++++
>> 2 files changed, 61 insertions(+)
>>
>> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
>> index 4d2e730271f9..423867edced8 100644
>> --- a/drivers/power/regulator/regulator-uclass.c
>> +++ b/drivers/power/regulator/regulator-uclass.c
>> @@ -311,6 +311,17 @@ int regulator_autoset(struct udevice *dev)
>> return ret;
>> }
>>
>> +int regulator_unset(struct udevice *dev)
>> +{
>> + struct dm_regulator_uclass_plat *uc_pdata;
>> +
>> + uc_pdata = dev_get_uclass_plat(dev);
>> + if (uc_pdata->force_off)
>
> Could you check whether uc_pdata is NULL or not?
> It can be returned to NULL.
>
>> + return regulator_set_enable(dev, false);
>> +
>> + return -EMEDIUMTYPE;
>> +}
>> +
>> static void regulator_show(struct udevice *dev, int ret)
>> {
>> struct dm_regulator_uclass_plat *uc_pdata;
>> @@ -443,6 +454,7 @@ static int regulator_pre_probe(struct udevice *dev)
>> uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
>> uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay",
>> 0);
>> + uc_pdata->force_off = dev_read_bool(dev, "regulator-force-boot-off");
>>
>> node = dev_read_subnode(dev, "regulator-state-mem");
>> if (ofnode_valid(node)) {
>> @@ -495,6 +507,32 @@ int regulators_enable_boot_on(bool verbose)
>> return ret;
>> }
>>
>> +int regulators_enable_boot_off(bool verbose)
>> +{
>> + struct udevice *dev;
>> + struct uclass *uc;
>> + int ret;
>> +
>> + ret = uclass_get(UCLASS_REGULATOR, &uc);
>> + if (ret)
>> + return ret;
>> + for (uclass_first_device(UCLASS_REGULATOR, &dev);
>> + dev;
>
> typo?
I have checked that it was not typo. :)
how about making one line?
Best Regards,
Jaehoo Chung
>
> Best Regards,
> Jaehoon Chung
>
>> + uclass_next_device(&dev)) {
>> + ret = regulator_unset(dev);
>> + if (ret == -EMEDIUMTYPE) {
>> + ret = 0;
>> + continue;
>> + }
>> + if (verbose)
>> + regulator_show(dev, ret);
>> + if (ret == -ENOSYS)
>> + ret = 0;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> UCLASS_DRIVER(regulator) = {
>> .id = UCLASS_REGULATOR,
>> .name = "regulator",
>> diff --git a/include/power/regulator.h b/include/power/regulator.h
>> index da9a065bdde0..fad87c99e5db 100644
>> --- a/include/power/regulator.h
>> +++ b/include/power/regulator.h
>> @@ -151,6 +151,7 @@ enum regulator_flag {
>> * @max_uA* - maximum amperage (micro Amps)
>> * @always_on* - bool type, true or false
>> * @boot_on* - bool type, true or false
>> + * @force_off* - bool type, true or false
>> * TODO(sjg at chromium.org): Consider putting the above two into @flags
>> * @ramp_delay - Time to settle down after voltage change (unit: uV/us)
>> * @flags: - flags value (see REGULATOR_FLAG_...)
>> @@ -176,6 +177,7 @@ struct dm_regulator_uclass_plat {
>> unsigned int ramp_delay;
>> bool always_on;
>> bool boot_on;
>> + bool force_off;
>> const char *name;
>> int flags;
>> u8 ctrl_reg;
>> @@ -420,6 +422,15 @@ int regulator_set_mode(struct udevice *dev, int mode_id);
>> */
>> int regulators_enable_boot_on(bool verbose);
>>
>> +/**
>> + * regulators_enable_boot_off() - disable regulators needed for boot
>> + *
>> + * This disables all regulators which are marked to be off at boot time.
>> + *
>> + * This effectively calls regulator_unset() for every regulator.
>> + */
>> +int regulators_enable_boot_off(bool verbose);
>> +
>> /**
>> * regulator_autoset: setup the voltage/current on a regulator
>> *
>> @@ -439,6 +450,18 @@ int regulators_enable_boot_on(bool verbose);
>> */
>> int regulator_autoset(struct udevice *dev);
>>
>> +/**
>> + * regulator_unset: turn off a regulator
>> + *
>> + * The setup depends on constraints found in device's uclass's platform data
>> + * (struct dm_regulator_uclass_platdata):
>> + *
>> + * - Disable - will set - if 'force_off' is set to true,
>> + *
>> + * The function returns on the first-encountered error.
>> + */
>> +int regulator_unset(struct udevice *dev);
>> +
>> /**
>> * regulator_autoset_by_name: setup the regulator given by its uclass's
>> * platform data name field. The setup depends on constraints found in device's
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] power: regulator: Add support for regulator-force-boot-off
2021-04-08 22:52 ` Jaehoon Chung
2021-04-09 0:37 ` Jaehoon Chung
@ 2021-04-09 5:14 ` Stefan Roese
1 sibling, 0 replies; 6+ messages in thread
From: Stefan Roese @ 2021-04-09 5:14 UTC (permalink / raw)
To: u-boot
Hi Jaehoon,
On 09.04.21 00:52, Jaehoon Chung wrote:
> Hi Stefan,
>
> On 4/8/21 6:20 PM, Stefan Roese wrote:
>> From: Konstantin Porotchkin <kostap@marvell.com>
>>
>> Add support for regulator-force-boot-off DT property.
>> This property can be used by the board/device drivers for
>> turning off regulators on early init stages as pre-requisite
>> for the other components initialization.
>>
>> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>> drivers/power/regulator/regulator-uclass.c | 38 ++++++++++++++++++++++
>> include/power/regulator.h | 23 +++++++++++++
>> 2 files changed, 61 insertions(+)
>>
>> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
>> index 4d2e730271f9..423867edced8 100644
>> --- a/drivers/power/regulator/regulator-uclass.c
>> +++ b/drivers/power/regulator/regulator-uclass.c
>> @@ -311,6 +311,17 @@ int regulator_autoset(struct udevice *dev)
>> return ret;
>> }
>>
>> +int regulator_unset(struct udevice *dev)
>> +{
>> + struct dm_regulator_uclass_plat *uc_pdata;
>> +
>> + uc_pdata = dev_get_uclass_plat(dev);
>> + if (uc_pdata->force_off)
>
> Could you check whether uc_pdata is NULL or not?
> It can be returned to NULL.
Sure. I'll add this check in v2.
>> + return regulator_set_enable(dev, false);
>> +
>> + return -EMEDIUMTYPE;
>> +}
>> +
>> static void regulator_show(struct udevice *dev, int ret)
>> {
>> struct dm_regulator_uclass_plat *uc_pdata;
>> @@ -443,6 +454,7 @@ static int regulator_pre_probe(struct udevice *dev)
>> uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
>> uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay",
>> 0);
>> + uc_pdata->force_off = dev_read_bool(dev, "regulator-force-boot-off");
>>
>> node = dev_read_subnode(dev, "regulator-state-mem");
>> if (ofnode_valid(node)) {
>> @@ -495,6 +507,32 @@ int regulators_enable_boot_on(bool verbose)
>> return ret;
>> }
>>
>> +int regulators_enable_boot_off(bool verbose)
>> +{
>> + struct udevice *dev;
>> + struct uclass *uc;
>> + int ret;
>> +
>> + ret = uclass_get(UCLASS_REGULATOR, &uc);
>> + if (ret)
>> + return ret;
>> + for (uclass_first_device(UCLASS_REGULATOR, &dev);
>> + dev;
>
> typo?
See comment in next mail.
Thanks,
Stefan
> Best Regards,
> Jaehoon Chung
>
>> + uclass_next_device(&dev)) {
>> + ret = regulator_unset(dev);
>> + if (ret == -EMEDIUMTYPE) {
>> + ret = 0;
>> + continue;
>> + }
>> + if (verbose)
>> + regulator_show(dev, ret);
>> + if (ret == -ENOSYS)
>> + ret = 0;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> UCLASS_DRIVER(regulator) = {
>> .id = UCLASS_REGULATOR,
>> .name = "regulator",
>> diff --git a/include/power/regulator.h b/include/power/regulator.h
>> index da9a065bdde0..fad87c99e5db 100644
>> --- a/include/power/regulator.h
>> +++ b/include/power/regulator.h
>> @@ -151,6 +151,7 @@ enum regulator_flag {
>> * @max_uA* - maximum amperage (micro Amps)
>> * @always_on* - bool type, true or false
>> * @boot_on* - bool type, true or false
>> + * @force_off* - bool type, true or false
>> * TODO(sjg at chromium.org): Consider putting the above two into @flags
>> * @ramp_delay - Time to settle down after voltage change (unit: uV/us)
>> * @flags: - flags value (see REGULATOR_FLAG_...)
>> @@ -176,6 +177,7 @@ struct dm_regulator_uclass_plat {
>> unsigned int ramp_delay;
>> bool always_on;
>> bool boot_on;
>> + bool force_off;
>> const char *name;
>> int flags;
>> u8 ctrl_reg;
>> @@ -420,6 +422,15 @@ int regulator_set_mode(struct udevice *dev, int mode_id);
>> */
>> int regulators_enable_boot_on(bool verbose);
>>
>> +/**
>> + * regulators_enable_boot_off() - disable regulators needed for boot
>> + *
>> + * This disables all regulators which are marked to be off at boot time.
>> + *
>> + * This effectively calls regulator_unset() for every regulator.
>> + */
>> +int regulators_enable_boot_off(bool verbose);
>> +
>> /**
>> * regulator_autoset: setup the voltage/current on a regulator
>> *
>> @@ -439,6 +450,18 @@ int regulators_enable_boot_on(bool verbose);
>> */
>> int regulator_autoset(struct udevice *dev);
>>
>> +/**
>> + * regulator_unset: turn off a regulator
>> + *
>> + * The setup depends on constraints found in device's uclass's platform data
>> + * (struct dm_regulator_uclass_platdata):
>> + *
>> + * - Disable - will set - if 'force_off' is set to true,
>> + *
>> + * The function returns on the first-encountered error.
>> + */
>> +int regulator_unset(struct udevice *dev);
>> +
>> /**
>> * regulator_autoset_by_name: setup the regulator given by its uclass's
>> * platform data name field. The setup depends on constraints found in device's
>>
>
Viele Gr??e,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] power: regulator: Add support for regulator-force-boot-off
2021-04-09 0:37 ` Jaehoon Chung
@ 2021-04-09 5:20 ` Stefan Roese
2021-04-10 3:49 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Roese @ 2021-04-09 5:20 UTC (permalink / raw)
To: u-boot
Hi Jaehoon,
On 09.04.21 02:37, Jaehoon Chung wrote:
> On 4/9/21 7:52 AM, Jaehoon Chung wrote:
>> Hi Stefan,
>>
>> On 4/8/21 6:20 PM, Stefan Roese wrote:
>>> From: Konstantin Porotchkin <kostap@marvell.com>
>>>
>>> Add support for regulator-force-boot-off DT property.
>>> This property can be used by the board/device drivers for
>>> turning off regulators on early init stages as pre-requisite
>>> for the other components initialization.
>>>
>>> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> ---
>>> drivers/power/regulator/regulator-uclass.c | 38 ++++++++++++++++++++++
>>> include/power/regulator.h | 23 +++++++++++++
>>> 2 files changed, 61 insertions(+)
>>>
>>> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
>>> index 4d2e730271f9..423867edced8 100644
>>> --- a/drivers/power/regulator/regulator-uclass.c
>>> +++ b/drivers/power/regulator/regulator-uclass.c
>>> @@ -311,6 +311,17 @@ int regulator_autoset(struct udevice *dev)
>>> return ret;
>>> }
>>>
>>> +int regulator_unset(struct udevice *dev)
>>> +{
>>> + struct dm_regulator_uclass_plat *uc_pdata;
>>> +
>>> + uc_pdata = dev_get_uclass_plat(dev);
>>> + if (uc_pdata->force_off)
>>
>> Could you check whether uc_pdata is NULL or not?
>> It can be returned to NULL.
>>
>>> + return regulator_set_enable(dev, false);
>>> +
>>> + return -EMEDIUMTYPE;
>>> +}
>>> +
>>> static void regulator_show(struct udevice *dev, int ret)
>>> {
>>> struct dm_regulator_uclass_plat *uc_pdata;
>>> @@ -443,6 +454,7 @@ static int regulator_pre_probe(struct udevice *dev)
>>> uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
>>> uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay",
>>> 0);
>>> + uc_pdata->force_off = dev_read_bool(dev, "regulator-force-boot-off");
>>>
>>> node = dev_read_subnode(dev, "regulator-state-mem");
>>> if (ofnode_valid(node)) {
>>> @@ -495,6 +507,32 @@ int regulators_enable_boot_on(bool verbose)
>>> return ret;
>>> }
>>>
>>> +int regulators_enable_boot_off(bool verbose)
>>> +{
>>> + struct udevice *dev;
>>> + struct uclass *uc;
>>> + int ret;
>>> +
>>> + ret = uclass_get(UCLASS_REGULATOR, &uc);
>>> + if (ret)
>>> + return ret;
>>> + for (uclass_first_device(UCLASS_REGULATOR, &dev);
>>> + dev;
>>
>> typo?
>
> I have checked that it was not typo. :)
> how about making one line?
I could do this. But this would lead to a quite long line:
for (uclass_first_device(UCLASS_REGULATOR, &dev); dev;
uclass_next_device(&dev)) {
Only putting "dev;" into the same line is not appealing, at least not
to me. Please note that this style of uclass looping is pretty
common. It's also used a few lines above in regulators_enable_boot_on():
int regulators_enable_boot_on(bool verbose)
{
struct udevice *dev;
struct uclass *uc;
int ret;
ret = uclass_get(UCLASS_REGULATOR, &uc);
if (ret)
return ret;
for (uclass_first_device(UCLASS_REGULATOR, &dev);
dev;
uclass_next_device(&dev)) {
ret = regulator_autoset(dev);
...
So I would like to keep it as in v1.
Thanks,
Stefan
>
> Best Regards,
> Jaehoo Chung
>
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> + uclass_next_device(&dev)) {
>>> + ret = regulator_unset(dev);
>>> + if (ret == -EMEDIUMTYPE) {
>>> + ret = 0;
>>> + continue;
>>> + }
>>> + if (verbose)
>>> + regulator_show(dev, ret);
>>> + if (ret == -ENOSYS)
>>> + ret = 0;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> UCLASS_DRIVER(regulator) = {
>>> .id = UCLASS_REGULATOR,
>>> .name = "regulator",
>>> diff --git a/include/power/regulator.h b/include/power/regulator.h
>>> index da9a065bdde0..fad87c99e5db 100644
>>> --- a/include/power/regulator.h
>>> +++ b/include/power/regulator.h
>>> @@ -151,6 +151,7 @@ enum regulator_flag {
>>> * @max_uA* - maximum amperage (micro Amps)
>>> * @always_on* - bool type, true or false
>>> * @boot_on* - bool type, true or false
>>> + * @force_off* - bool type, true or false
>>> * TODO(sjg at chromium.org): Consider putting the above two into @flags
>>> * @ramp_delay - Time to settle down after voltage change (unit: uV/us)
>>> * @flags: - flags value (see REGULATOR_FLAG_...)
>>> @@ -176,6 +177,7 @@ struct dm_regulator_uclass_plat {
>>> unsigned int ramp_delay;
>>> bool always_on;
>>> bool boot_on;
>>> + bool force_off;
>>> const char *name;
>>> int flags;
>>> u8 ctrl_reg;
>>> @@ -420,6 +422,15 @@ int regulator_set_mode(struct udevice *dev, int mode_id);
>>> */
>>> int regulators_enable_boot_on(bool verbose);
>>>
>>> +/**
>>> + * regulators_enable_boot_off() - disable regulators needed for boot
>>> + *
>>> + * This disables all regulators which are marked to be off at boot time.
>>> + *
>>> + * This effectively calls regulator_unset() for every regulator.
>>> + */
>>> +int regulators_enable_boot_off(bool verbose);
>>> +
>>> /**
>>> * regulator_autoset: setup the voltage/current on a regulator
>>> *
>>> @@ -439,6 +450,18 @@ int regulators_enable_boot_on(bool verbose);
>>> */
>>> int regulator_autoset(struct udevice *dev);
>>>
>>> +/**
>>> + * regulator_unset: turn off a regulator
>>> + *
>>> + * The setup depends on constraints found in device's uclass's platform data
>>> + * (struct dm_regulator_uclass_platdata):
>>> + *
>>> + * - Disable - will set - if 'force_off' is set to true,
>>> + *
>>> + * The function returns on the first-encountered error.
>>> + */
>>> +int regulator_unset(struct udevice *dev);
>>> +
>>> /**
>>> * regulator_autoset_by_name: setup the regulator given by its uclass's
>>> * platform data name field. The setup depends on constraints found in device's
>>>
>>
>>
>
Viele Gr??e,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] power: regulator: Add support for regulator-force-boot-off
2021-04-09 5:20 ` Stefan Roese
@ 2021-04-10 3:49 ` Jaehoon Chung
0 siblings, 0 replies; 6+ messages in thread
From: Jaehoon Chung @ 2021-04-10 3:49 UTC (permalink / raw)
To: u-boot
Hi Stefan,
On 4/9/21 2:20 PM, Stefan Roese wrote:
> Hi Jaehoon,
>
> On 09.04.21 02:37, Jaehoon Chung wrote:
>> On 4/9/21 7:52 AM, Jaehoon Chung wrote:
>>> Hi Stefan,
>>>
>>> On 4/8/21 6:20 PM, Stefan Roese wrote:
>>>> From: Konstantin Porotchkin <kostap@marvell.com>
>>>>
>>>> Add support for regulator-force-boot-off DT property.
>>>> This property can be used by the board/device drivers for
>>>> turning off regulators on early init stages as pre-requisite
>>>> for the other components initialization.
>>>>
>>>> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>> ---
>>>> ? drivers/power/regulator/regulator-uclass.c | 38 ++++++++++++++++++++++
>>>> ? include/power/regulator.h????????????????? | 23 +++++++++++++
>>>> ? 2 files changed, 61 insertions(+)
>>>>
>>>> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
>>>> index 4d2e730271f9..423867edced8 100644
>>>> --- a/drivers/power/regulator/regulator-uclass.c
>>>> +++ b/drivers/power/regulator/regulator-uclass.c
>>>> @@ -311,6 +311,17 @@ int regulator_autoset(struct udevice *dev)
>>>> ????? return ret;
>>>> ? }
>>>> ? +int regulator_unset(struct udevice *dev)
>>>> +{
>>>> +??? struct dm_regulator_uclass_plat *uc_pdata;
>>>> +
>>>> +??? uc_pdata = dev_get_uclass_plat(dev);
>>>> +??? if (uc_pdata->force_off)
>>>
>>> Could you check whether uc_pdata is NULL or not?
>>> It can be returned to NULL.
>>>
>>>> +??????? return regulator_set_enable(dev, false);
>>>> +
>>>> +??? return -EMEDIUMTYPE;
>>>> +}
>>>> +
>>>> ? static void regulator_show(struct udevice *dev, int ret)
>>>> ? {
>>>> ????? struct dm_regulator_uclass_plat *uc_pdata;
>>>> @@ -443,6 +454,7 @@ static int regulator_pre_probe(struct udevice *dev)
>>>> ????? uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
>>>> ????? uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay",
>>>> ????????????????????????????? 0);
>>>> +??? uc_pdata->force_off = dev_read_bool(dev, "regulator-force-boot-off");
>>>> ? ????? node = dev_read_subnode(dev, "regulator-state-mem");
>>>> ????? if (ofnode_valid(node)) {
>>>> @@ -495,6 +507,32 @@ int regulators_enable_boot_on(bool verbose)
>>>> ????? return ret;
>>>> ? }
>>>> ? +int regulators_enable_boot_off(bool verbose)
>>>> +{
>>>> +??? struct udevice *dev;
>>>> +??? struct uclass *uc;
>>>> +??? int ret;
>>>> +
>>>> +??? ret = uclass_get(UCLASS_REGULATOR, &uc);
>>>> +??? if (ret)
>>>> +??????? return ret;
>>>> +??? for (uclass_first_device(UCLASS_REGULATOR, &dev);
>>>> +???????? dev;
>>>
>>> typo?
>>
>> I have checked that it was not typo. :)
>> how about making one line?
>
> I could do this. But this would lead to a quite long line:
>
> ????for (uclass_first_device(UCLASS_REGULATOR, &dev); dev; uclass_next_device(&dev)) {
>
> Only putting "dev;" into the same line is not appealing, at least not
> to me. Please note that this style of uclass looping is pretty
> common. It's also used a few lines above in regulators_enable_boot_on():
>
> int regulators_enable_boot_on(bool verbose)
> {
> ????struct udevice *dev;
> ????struct uclass *uc;
> ????int ret;
>
> ????ret = uclass_get(UCLASS_REGULATOR, &uc);
> ????if (ret)
> ??????? return ret;
> ????for (uclass_first_device(UCLASS_REGULATOR, &dev);
> ???????? dev;
> ???????? uclass_next_device(&dev)) {
> ??????? ret = regulator_autoset(dev);
> ...
>
> So I would like to keep it as in v1.
Yes, If you prefer to it, keep it as in v1.
I think that can be changed to below.
for (uclass_first_device(UCLASS_REGULATOR, &dev);
dev; uclass_next_device(&dev)) {
ret = reuglator_autoset(dev);
...
}
But it's not critical thing. Thanks!
Best Regards,
Jaehoon Chung
>
> Thanks,
> Stefan
>
>>
>> Best Regards,
>> Jaehoo Chung
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>> +???????? uclass_next_device(&dev)) {
>>>> +??????? ret = regulator_unset(dev);
>>>> +??????? if (ret == -EMEDIUMTYPE) {
>>>> +??????????? ret = 0;
>>>> +??????????? continue;
>>>> +??????? }
>>>> +??????? if (verbose)
>>>> +??????????? regulator_show(dev, ret);
>>>> +??????? if (ret == -ENOSYS)
>>>> +??????????? ret = 0;
>>>> +??? }
>>>> +
>>>> +??? return ret;
>>>> +}
>>>> +
>>>> ? UCLASS_DRIVER(regulator) = {
>>>> ????? .id??????? = UCLASS_REGULATOR,
>>>> ????? .name??????? = "regulator",
>>>> diff --git a/include/power/regulator.h b/include/power/regulator.h
>>>> index da9a065bdde0..fad87c99e5db 100644
>>>> --- a/include/power/regulator.h
>>>> +++ b/include/power/regulator.h
>>>> @@ -151,6 +151,7 @@ enum regulator_flag {
>>>> ?? * @max_uA*??? - maximum amperage (micro Amps)
>>>> ?? * @always_on* - bool type, true or false
>>>> ?? * @boot_on*?? - bool type, true or false
>>>> + * @force_off* - bool type, true or false
>>>> ?? * TODO(sjg at chromium.org): Consider putting the above two into @flags
>>>> ?? * @ramp_delay - Time to settle down after voltage change (unit: uV/us)
>>>> ?? * @flags:???? - flags value (see REGULATOR_FLAG_...)
>>>> @@ -176,6 +177,7 @@ struct dm_regulator_uclass_plat {
>>>> ????? unsigned int ramp_delay;
>>>> ????? bool always_on;
>>>> ????? bool boot_on;
>>>> +??? bool force_off;
>>>> ????? const char *name;
>>>> ????? int flags;
>>>> ????? u8 ctrl_reg;
>>>> @@ -420,6 +422,15 @@ int regulator_set_mode(struct udevice *dev, int mode_id);
>>>> ?? */
>>>> ? int regulators_enable_boot_on(bool verbose);
>>>> ? +/**
>>>> + * regulators_enable_boot_off() - disable regulators needed for boot
>>>> + *
>>>> + * This disables all regulators which are marked to be off at boot time.
>>>> + *
>>>> + * This effectively calls regulator_unset() for every regulator.
>>>> + */
>>>> +int regulators_enable_boot_off(bool verbose);
>>>> +
>>>> ? /**
>>>> ?? * regulator_autoset: setup the voltage/current on a regulator
>>>> ?? *
>>>> @@ -439,6 +450,18 @@ int regulators_enable_boot_on(bool verbose);
>>>> ?? */
>>>> ? int regulator_autoset(struct udevice *dev);
>>>> ? +/**
>>>> + * regulator_unset: turn off a regulator
>>>> + *
>>>> + * The setup depends on constraints found in device's uclass's platform data
>>>> + * (struct dm_regulator_uclass_platdata):
>>>> + *
>>>> + * - Disable - will set - if? 'force_off' is set to true,
>>>> + *
>>>> + * The function returns on the first-encountered error.
>>>> + */
>>>> +int regulator_unset(struct udevice *dev);
>>>> +
>>>> ? /**
>>>> ?? * regulator_autoset_by_name: setup the regulator given by its uclass's
>>>> ?? * platform data name field. The setup depends on constraints found in device's
>>>>
>>>
>>>
>>
>
>
> Viele Gr??e,
> Stefan
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-10 3:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20210408092110epcas1p250a2712e160c9af523223e35e7ea9da2@epcas1p2.samsung.com>
2021-04-08 9:20 ` [PATCH] power: regulator: Add support for regulator-force-boot-off Stefan Roese
2021-04-08 22:52 ` Jaehoon Chung
2021-04-09 0:37 ` Jaehoon Chung
2021-04-09 5:20 ` Stefan Roese
2021-04-10 3:49 ` Jaehoon Chung
2021-04-09 5:14 ` Stefan Roese
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.