Re: [PATCH v2 3/5] regulator: helper routine to extract regulator_init_data

On Mon, Oct 24, 2011 at 03:59:50PM +0200, Grant Likely wrote:

On Mon, Oct 24, 2011 at 09:40:26PM +0800, Shawn Guo wrote:

quoted

On Mon, Oct 24, 2011 at 03:06:37PM +0200, Mark Brown wrote:

quoted

On Mon, Oct 24, 2011 at 09:04:31PM +0800, Shawn Guo wrote:

quoted

If we can attach the device_node of 'regulators' node to dev->of_node
when calling regulator_register(regulator_desc, dev, ...) from
regulator driver, the regulator core will be able to find all nodes under
'regulators' using for_each_child_of_node(dev->of_node, child).

Please provide concrete examples of the bindings you're talking about,
the really important thing here is how sane the bindings look and I've
really got no idea what any of what you're talking about will look like
or if they make sense.

The only thing different from what I attached last time is the
compatible string added to 'regulators' node.

        ecspi@70010000 { /* ECSPI1 */
                fsl,spi-num-chipselects = <2>;
                cs-gpios = <&gpio3 24 0>, /* GPIO4_24 */
                           <&gpio3 25 0>; /* GPIO4_25 */
                status = "okay";

                pmic: mc13892@0 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        compatible = "fsl,mc13892";
                        spi-max-frequency = <6000000>;
                        reg = <0>;
                        mc13xxx-irq-gpios = <&gpio0 8 0>; /* GPIO1_8 */

                        regulators {
                        	compatible = "fsl,mc13892-regulator";

                                sw1reg: mc13892_sw1 {
                                        regulator-min-uV = <600000>;
                                        regulator-max-uV = <1375000>;
                                        regulator-change-voltage;
                                        regulator-boot-on;
                                        regulator-always-on;
                                };

                                sw2reg: mc13892_sw2 {
                                        regulator-min-uV = <900000>;
                                        regulator-max-uV = <1850000>;
                                        regulator-change-voltage;
                                        regulator-boot-on;
                                        regulator-always-on;
                                };

                                ......
                        };

                        leds {
                                ......
                        };

                        buttons {
                                ......
                        };
                };

                flash: at45db321d@1 {
                        ......
                };
        };

quoted

quoted

hesitate to hack this into mfd_add_devices(), so I would like to add
compatible string "fsl,mc13892-regulators" to node 'regulators' and
find the node using of_find_compatible_node(dev->parent, NULL,
"fsl,mc13892-regulators").

It's not immediately obvious to me that having a binding for the
regulators separately makes sense, it's not a usefully distinct device.

Fair point.  Actually, I also hate to have the finding of node
'regulators' plugged into regulator driver.  What about following
change to address Grant's concern on global device tree search?
 

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 8fe132d..29dcf90 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c

@@ -2673,7 +2673,8 @@ struct regulator_dev *regulator_register(struct regulator_desc *regulator_desc,
        BLOCKING_INIT_NOTIFIER_HEAD(&rdev->notifier);

        /* find device_node and attach it */
-       rdev->dev.of_node = of_find_node_by_name(NULL, regulator_desc->name);
+       rdev->dev.of_node = of_find_node_by_name(dev->parent->of_node,
+                                                regulator_desc->name);

of_find_node_by_name() doesn't work that way.  The first argument is a
starting point, but it doesn't restrict the search to children of a
node.

for_each_child_of_node() is what you want to use when iterating over
the children which unfortunately changes the structure of this
function.

The dev->parent->of_node is meant to point to node 'pmic: mc13892@0'.
And the intention here is not to iterate over the children, but to
start a search from a reasonable point rather than the top root node.

-- 
Regards,
Shawn