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

[Mark Brown]

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