Re: [PATCH 4/8] regulator: da9055: Add DT support

From: Mark Brown
Date: Wed Feb 05 2014 - 13:37:06 EST


On Wed, Feb 05, 2014 at 05:48:35PM +0000, Adam Thomson wrote:

> +#ifdef CONFIG_OF
> +#include <linux/of.h>
> +#include <linux/regulator/of_regulator.h>
> +#endif /* CONFIG_OF */

Don't do ifdefs for includes like this, it's not worth it.

> + for_each_child_of_node(nproot, np) {
> + if (!of_node_cmp(np->name,
> + regulator->info->reg_desc.name)) {
> + config->init_data = of_get_regulator_init_data(
> + &pdev->dev, np);
> + config->of_node = np;
> + break;
> + }
> + }

I think you're looking for of_regulator_match() here.

> if (pdata && pdata->regulators)
> config.init_data = pdata->regulators[pdev->id];
> + else {
> + ret = da9055_regulator_dt_init(pdev, regulator, &config);
> + if (ret < 0)
> + return ret;
> + }

Coding style, both sides of the if should have braces if one does.

> /* Only LDO 5 and 6 has got the over current interrupt */
> if (pdev->id == DA9055_ID_LDO5 || pdev->id == DA9055_ID_LDO6) {
> - irq = platform_get_irq_byname(pdev, "REGULATOR");
> - irq = regmap_irq_get_virq(da9055->irq_data, irq);
> + irq = regmap_irq_get_virq(da9055->irq_data,
> + DA9055_IRQ_REGULATOR);

This seems like a bit of a step backwards - what happened in the MFD
(and why didn't it update the users to avoid breaking bisection)?

Attachment: signature.asc
Description: Digital signature