[PATCH] power: regulator: Add support for regulator-force-boot-off

[Stefan Roese]

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 at 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 at marvell.com>
>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>> Cc: Jaehoon Chung <jh80.chung at samsung.com>
>>> Cc: Simon Glass <sjg at 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