[PATCH] regulator: tps6586x: register regulator even if no init data

[PATCH] regulator: tps6586x: register regulator even if no init data

[Laxman Dewangan]

Register all TPS6586x regulators even if there is no regulator
init data for platform i.e. without any user-supplied constraints.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
This patch make sure that all chip regulators get registered
even there is no user-supplied.
This is based on discussion on following patch about registering
rails without init data.
	max8907: fix use of possibly NULL idata

 drivers/mfd/tps6586x.c                 |   16 ++++++++--------
 drivers/regulator/tps6586x-regulator.c |    3 +++
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index 380a3c8..f5dac8a 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -396,19 +396,19 @@ static struct tps6586x_platform_data *tps6586x_parse_dt(struct i2c_client *clien
 	for (i = 0, j = 0; i < num && j < count; i++) {
 		struct regulator_init_data *reg_idata;
 
-		if (!tps6586x_matches[i].init_data)
-			continue;
-
 		reg_idata  = tps6586x_matches[i].init_data;
 		devs[j].name = "tps6586x-regulator";
 		devs[j].platform_data = tps6586x_matches[i].init_data;
 		devs[j].id = (int)tps6586x_matches[i].driver_data;
-		if (devs[j].id == TPS6586X_ID_SYS)
-			sys_rail_name = reg_idata->constraints.name;
 
-		if ((devs[j].id == TPS6586X_ID_LDO_5) ||
-			(devs[j].id == TPS6586X_ID_LDO_RTC))
-			reg_idata->supply_regulator = sys_rail_name;
+		if (tps6586x_matches[i].init_data) {
+			if (devs[j].id == TPS6586X_ID_SYS)
+				sys_rail_name = reg_idata->constraints.name;
+
+			if ((devs[j].id == TPS6586X_ID_LDO_5) ||
+				(devs[j].id == TPS6586X_ID_LDO_RTC))
+				reg_idata->supply_regulator = sys_rail_name;
+		}
 
 		devs[j].of_node = tps6586x_matches[i].of_node;
 		j++;
diff --git a/drivers/regulator/tps6586x-regulator.c b/drivers/regulator/tps6586x-regulator.c
index ce1e7cb..4508222 100644
--- a/drivers/regulator/tps6586x-regulator.c
+++ b/drivers/regulator/tps6586x-regulator.c
@@ -332,6 +332,9 @@ static int __devinit tps6586x_regulator_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, rdev);
 
+	if (!pdev->dev.platform_data)
+		return 0;
+
 	return tps6586x_regulator_set_slew_rate(pdev);
 }
 
-- 
1.7.1.1

[PATCH] regulator: tps65910: register regulator even if no init data

[Laxman Dewangan]

Register all TPS65910 regulators even if there is no regulator
init data for platform i.e. without any user-supplied constraints.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
This patch make sure that all chip regulators get registered
even there is no user-supplied.
This is based on discussion on following patch about registering
rails without init data.
        max8907: fix use of possibly NULL idata

 drivers/regulator/tps65910-regulator.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index 793adda..1f758c9 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -1105,11 +1105,6 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
 
 		reg_data = pmic_plat_data->tps65910_pmic_init_data[i];
 
-		/* Regulator API handles empty constraints but not NULL
-		 * constraints */
-		if (!reg_data)
-			continue;
-
 		/* Register the regulators */
 		pmic->info[i] = info;
 
-- 
1.7.1.1

Re: [PATCH] regulator: tps6586x: register regulator even if no init data

[Stephen Warren]

On 08/28/2012 03:29 AM, Laxman Dewangan wrote:
> Register all TPS6586x regulators even if there is no regulator
> init data for platform i.e. without any user-supplied constraints.

> diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c

> @@ -396,19 +396,19 @@ static struct tps6586x_platform_data *tps6586x_parse_dt(struct i2c_client *clien
>  	for (i = 0, j = 0; i < num && j < count; i++) {
>  		struct regulator_init_data *reg_idata;
>  
> -		if (!tps6586x_matches[i].init_data)
> -			continue;
> -
>  		reg_idata  = tps6586x_matches[i].init_data;
>  		devs[j].name = "tps6586x-regulator";
>  		devs[j].platform_data = tps6586x_matches[i].init_data;
>  		devs[j].id = (int)tps6586x_matches[i].driver_data;
> -		if (devs[j].id == TPS6586X_ID_SYS)
> -			sys_rail_name = reg_idata->constraints.name;
>  
> -		if ((devs[j].id == TPS6586X_ID_LDO_5) ||
> -			(devs[j].id == TPS6586X_ID_LDO_RTC))
> -			reg_idata->supply_regulator = sys_rail_name;
> +		if (tps6586x_matches[i].init_data) {

The variable that's being used inside this block is reg_idata.
Admittedly the value of that variable is tps6586x_matches[i].init_data,
but I think it'd be much cleaner if the if statement checked reg_idata
directly; that way, someone reading the code wouldn't have to look above
to find out that reg_idata and tps6586x_matches[i].init_data are related.

> +			if (devs[j].id == TPS6586X_ID_SYS)
> +				sys_rail_name = reg_idata->constraints.name;

In the MAX8907 patch this attempts to duplicate; if there is no init
data where the user gives an explicit name, the name from the descriptor
is used:

if (idata && idata->constraints.name)
    mbatt_rail_name = idata->constraints.name;
else
    mbatt_rail_name = pmic->desc[i].name;

Shouldn't that same algorithm be used here?

> diff --git a/drivers/regulator/tps6586x-regulator.c b/drivers/regulator/tps6586x-regulator.c

> @@ -332,6 +332,9 @@ static int __devinit tps6586x_regulator_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, rdev);
>  
> +	if (!pdev->dev.platform_data)
> +		return 0;
> +
>  	return tps6586x_regulator_set_slew_rate(pdev);
>  }

Sorry, I don't immediately see why that's related?

Re: [PATCH] regulator: tps6586x: register regulator even if no init data

[Laxman Dewangan]

On Tuesday 28 August 2012 07:48 PM, Stephen Warren wrote:
>
>>
>> -		if ((devs[j].id == TPS6586X_ID_LDO_5) ||
>> -			(devs[j].id == TPS6586X_ID_LDO_RTC))
>> -			reg_idata->supply_regulator = sys_rail_name;
>> +		if (tps6586x_matches[i].init_data) {
> The variable that's being used inside this block is reg_idata.
> Admittedly the value of that variable is tps6586x_matches[i].init_data,
> but I think it'd be much cleaner if the if statement checked reg_idata
> directly; that way, someone reading the code wouldn't have to look above
> to find out that reg_idata and tps6586x_matches[i].init_data are related.
>

OK, will take care in my next patch.


>> +			if (devs[j].id == TPS6586X_ID_SYS)
>> +				sys_rail_name = reg_idata->constraints.name;
> In the MAX8907 patch this attempts to duplicate; if there is no init
> data where the user gives an explicit name, the name from the descriptor
> is used:
>
> if (idata&&  idata->constraints.name)
>      mbatt_rail_name = idata->constraints.name;
> else
>      mbatt_rail_name = pmic->desc[i].name;
>
> Shouldn't that same algorithm be used here?

the desc is not available here in mfd file and so not possible. Each 
regulator register as different platform driver and hence does not have 
the other instance platform data.
However, this will be get fixed once my next patch for moving the dt 
parsing from mfd core to regulator is there. At that time I will take 
care of this.

>>
>> +	if (!pdev->dev.platform_data)
>> +		return 0;
>> +
>>   	return tps6586x_regulator_set_slew_rate(pdev);
>>   }
> Sorry, I don't immediately see why that's related?
The function tps6586x_regulator_set_slew_rate() use the platform data 
which is not available if init_data is not provided by platform.