Re: [PATCH 6/6] mfd: TPS65910: Improve regulator init data

From: Kyle Manna
Date: Mon Oct 24 2011 - 12:13:55 EST


On 10/19/2011 09:00 AM, Mark Brown wrote:
On Tue, Oct 18, 2011 at 01:26:28PM -0500, Kyle Manna wrote:
Improve the interface between platform code/board files to the TPS65910
Again, *always* CC maintainers on patches.

This was an oversight on my part.


regulators. The TWL4030/6030 code was used as an example interface.
This isn't a good sign...

I've reviewed other PMICs (ie Wolfson Micro ;) ) and will post an updated series with an interface similar to what is used there. The new approach makes more sense and keeps the code/patch small.


This improved interface will allow use of the regulators without
specifying all the constraints. Also gets rid of an assumption that
the platform pass in an array of correct size and was unchecked.
You've not described the changes between the two interfaces. Note that
empty constraints should be absolutely fine with the API.

+ if (init_data->constraints.name)
+ pmic->desc[i].name = init_data->constraints.name;
+ else
+ pmic->desc[i].name = info[i].name;
No, this is broken. The name of the regulator is a fixed property of
the device and isn't something that ought to be overridden per system.

Understood.


+ /* TPS65910 and TPS65911 Regulators */
+ rdev = add_regulator(pmic, info, TPS65910_REG_VRTC,
+ pmic_plat_data->vrtc);
+ if (IS_ERR(rdev))
+ return PTR_ERR(rdev);
+ rdev = add_regulator(pmic, info, TPS65910_REG_VIO,
+ pmic_plat_data->vio);
+
+ if (IS_ERR(rdev))
+ return PTR_ERR(rdev);
+
+ rdev = add_regulator(pmic, info, TPS65910_REG_VDD1,
+ pmic_plat_data->vdd1);
+ if (IS_ERR(rdev))
+ return PTR_ERR(rdev);
This looks like a regression - we've gone from looping over an array
which is nice and simple to explicit code for each individual regulator
giving us lots of repetitive code...

Will be revised.
-err_unregister_regulator:
- while (--i>= 0)
- regulator_unregister(pmic->rdev[i]);
- kfree(pmic->rdev);
...and loosing all our cleanup if things go wrong which isn't great
either.

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