Hi Mark,

Looking at this now.

quoted

+static __devinit int
+ab8500_regulator_of_probe(struct platform_device *pdev, struct device_node *np)
+{
+	struct regulator_init_data *ab8500_regulator;
+	struct device_node *child;
+	int err, value, i, id = 0;
+
+	/* Initialise regulator registers to platform specific values. */
+	for (i = 0; i<  ARRAY_SIZE(ab8500_reg_init); i++) {
+		err = of_property_read_u32(np, ab8500_reg_init[i].of_name,&value);
+		if (err<  0)
+			return err;
+
+		err = ab8500_regulator_init_registers(pdev, i, value);
+		if (err<  0)
+			return err;

You should be using of_regulator_match() for this (I think it's supposed
to do an equivalent job...) rather than open coding.

I've ripped this out completely and the code appears to continue be 
fully functional. Happy days! :)

quoted

+	/* Register each ab8500 regulator found in the Device Tree. */
+	for_each_child_of_node(np, child) {
+		ab8500_regulator = of_get_regulator_init_data(&pdev->dev, child);

Definitely don't do this - you should unconditionally register all the
regulators that physically exist, this allows users to inspect their
state even if they aren't used.

It's possible the original driver has this bug (I didn't check but

The original driver places each of the registers inside a structure 
within the driver itself and recursively registers them from there. The 
constraints are united with the correct element using #defines.

Can't we just assume that all of the regulators will be put into the 
Device Tree? As this is what I'll be doing.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog