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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 28 Aug 2012 20:26:38 +0530
From:	Laxman Dewangan <ldewangan@...dia.com>
To:	Stephen Warren <swarren@...dotorg.org>
CC:	"broonie@...nsource.wolfsonmicro.com" 
	<broonie@...nsource.wolfsonmicro.com>,
	"sameo@...ux.intel.com" <sameo@...ux.intel.com>,
	"lrg@...com" <lrg@...com>,
	"swarren@...dotorg.com" <swarren@...dotorg.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] regulator: tps6586x: register regulator even if no init
 data

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/