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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[ARM Kernel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]     [Free Online Dating]     [Linux Kernel]     [Linux SCSI]     [XFree86]

Add to Google Powered by Linux