Hi Axel Thanks for the patch
On Wed, 17 Apr 2013, Axel Lin wrote: > The same table can be used for multiple instance of pdev, so we don't need to > allocate memory for of_regulator_match table per pdev. > > Signed-off-by: Axel Lin <axel....@ingics.com> > --- > drivers/regulator/as3711-regulator.c | 46 > ++++++++++++++-------------------- > 1 file changed, 19 insertions(+), 27 deletions(-) > > diff --git a/drivers/regulator/as3711-regulator.c > b/drivers/regulator/as3711-regulator.c > index 0539b3e..dd1a089 100644 > --- a/drivers/regulator/as3711-regulator.c > +++ b/drivers/regulator/as3711-regulator.c > @@ -278,52 +278,44 @@ static struct as3711_regulator_info as3711_reg_info[] = > { > > #define AS3711_REGULATOR_NUM ARRAY_SIZE(as3711_reg_info) > > -static const char *as3711_regulator_of_names[AS3711_REGULATOR_NUM] = { > - [AS3711_REGULATOR_SD_1] = "sd1", > - [AS3711_REGULATOR_SD_2] = "sd2", > - [AS3711_REGULATOR_SD_3] = "sd3", > - [AS3711_REGULATOR_SD_4] = "sd4", > - [AS3711_REGULATOR_LDO_1] = "ldo1", > - [AS3711_REGULATOR_LDO_2] = "ldo2", > - [AS3711_REGULATOR_LDO_3] = "ldo3", > - [AS3711_REGULATOR_LDO_4] = "ldo4", > - [AS3711_REGULATOR_LDO_5] = "ldo5", > - [AS3711_REGULATOR_LDO_6] = "ldo6", > - [AS3711_REGULATOR_LDO_7] = "ldo7", > - [AS3711_REGULATOR_LDO_8] = "ldo8", > +static struct of_regulator_match as3711_regulator_matches[] = { > + { .name = "sd1" }, > + { .name = "sd2" }, > + { .name = "sd3" }, > + { .name = "sd4" }, > + { .name = "ldo1" }, > + { .name = "ldo2" }, > + { .name = "ldo3" }, > + { .name = "ldo4" }, > + { .name = "ldo5" }, > + { .name = "ldo6" }, > + { .name = "ldo7" }, > + { .name = "ldo8" }, Please keep explicit indices to match this array's members to the as3711_reg_info[] array. > }; > > static int as3711_regulator_parse_dt(struct device *dev, > struct device_node **of_node, const int count) > { > struct as3711_regulator_pdata *pdata = dev_get_platdata(dev); > - struct device_node *regulators = > - of_find_node_by_name(dev->parent->of_node, "regulators"); > - struct of_regulator_match *matches, *match; > + struct device_node *regulators; > + struct of_regulator_match *match; > int ret, i; > > + regulators = of_find_node_by_name(dev->parent->of_node, "regulators"); What was wrong with the original code? I don't see a difference, this seems to be an unrelated stylistic change, please, don't do this. > if (!regulators) { > dev_err(dev, "regulator node not found\n"); > return -ENODEV; > } > > - matches = devm_kzalloc(dev, sizeof(*matches) * count, GFP_KERNEL); > - if (!matches) > - return -ENOMEM; > - > - for (i = 0, match = matches; i < count; i++, match++) { > - match->name = as3711_regulator_of_names[i]; > - match->driver_data = as3711_reg_info + i; This is a separate change. I was probably copy-pasting this parsing from some other driver and didn't realise, that .driver_data isn't actually used. And that was the reason why I copied the array at run-time. I won't play a patch-police here, asking you to split this into a separate patch, but please, could you at least make a remark in the commit message, confirming my understanding. Or maybe I'm wrong and .driver_data is needed? Then the whole your patch might not be right. > - } > - > - ret = of_regulator_match(dev->parent, regulators, matches, count); > + ret = of_regulator_match(dev->parent, regulators, > + as3711_regulator_matches, count); > of_node_put(regulators); > if (ret < 0) { > dev_err(dev, "Error parsing regulator init data: %d\n", ret); > return ret; > } > > - for (i = 0, match = matches; i < count; i++, match++) > + for (i = 0, match = as3711_regulator_matches; i < count; i++, match++) > if (match->of_node) { > pdata->init_data[i] = match->init_data; > of_node[i] = match->of_node; > -- > 1.7.10.4 Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/