linux-kernel - Re: [PATCH 2/2] ARM: dt: tegra: cardhu: register core regulator tps65911

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 22 May 2012 22:39:04 +0530
From:	Laxman Dewangan <ldewangan@...dia.com>
To:	Stephen Warren <swarren@...dotorg.org>
CC:	Stephen Warren <swarren@...dia.com>,
	"olof@...om.net" <olof@...om.net>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH 2/2] ARM: dt: tegra: cardhu: register core regulator tps65911

On Tuesday 22 May 2012 10:10 PM, Stephen Warren wrote:
> On 05/22/2012 07:05 AM, Laxman Dewangan wrote:
>> Add device info for the PMIC device tps65911 in tegra-cardhu
>> dts file. This device supports the multiple regulator rails,
>> gpio, interrupts.
> FYI, patch 1 in this series looks fine. Some comments below though:
>
>> diff --git a/arch/arm/boot/dts/tegra-cardhu.dts b/arch/arm/boot/dts/tegra-cardhu.dts
>> +		tps65911: tps65911@2d {
>> +			compatible = "ti,tps65911";
>> +			reg =<0x2d>;
>> +
>> +			#gpio-cells =<2>;
>> +			gpio-controller;
>> +
>> +			regulators {
> Please add the following properties here:
>
> 	#address-cells =<1>;
> 	#size-cells =<0>;
>
>> +				vdd1_reg: vdd1 {
> This node name should be "regulator", since nodes are generally named
> after the class of object they represent. Since all the nodes will then
> have the same name, you'll need to add a unit address ("@nnnn") to the
> node name.
>

Nop, we can not do it. The node name should match with the name 
mentioned in driver otherwise the regulator node search will fail
Following is the excerpt of the code:
int of_regulator_match(struct device *dev, struct device_node *node,
                        struct of_regulator_match *matches,
                        unsigned int num_matches)
{

         for (i = 0; i < num_matches; i++) {
                 struct of_regulator_match *match = &matches[i];
                 struct device_node *child;

                 child = of_find_node_by_name(node, match->name);
                 if (!child)
                         continue;

:::::::::::
}

static struct of_regulator_match tps65911_matches[] = {
         { .name = "vrtc",       .driver_data = (void *) 
&tps65911_regs[0] },
         { .name = "vio",        .driver_data = (void *) 
&tps65911_regs[1] },
         { .name = "vdd1",       .driver_data = (void *) 
&tps65911_regs[2] },
         { .name = "vdd2",       .driver_data = (void *) 
&tps65911_regs[3] },
         { .name = "vddctrl",    .driver_data = (void *) 
&tps65911_regs[4] },
         { .name = "ldo1",       .driver_data = (void *) 
&tps65911_regs[5] },
         { .name = "ldo2",       .driver_data = (void *) 
&tps65911_regs[6] },
  :::::::::::::::::::::::::::::::::::::
         { .name = "ldo8",       .driver_data = (void *) 
&tps65911_regs[12] },
};



So only we can do it as
     reg_vdd1: vdd1 {
                 reg = <0>;
             :::::::::
     };

     reg_vdd2: vdd2 {
             reg = < 1>;
             :::::::::::
     };

> Nitpicky, but the labels might be more logical as reg_vdd1 rather than
> vdd1_reg, but not a big deal.
>
> So, please replace the line above with:
>
> 	reg_vdd1: regulator@0 {
> 		reg =<0>;
>

Why do we really require the reg at all?
I dont think any usage of doing this.


--
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/