Re: [PATCH 3/4] regulator: qcom: Refactor of-parsing code

[Bjorn Andersson]

On Tue 03 Mar 06:13 PST 2015, Mark Brown wrote:

> On Mon, Mar 02, 2015 at 08:25:39PM -0800, Bjorn Andersson wrote:
> 
> > +	if ((vreg->parts->uV.mask || vreg->parts->mV.mask) &&
> > +	    (!initdata->constraints.min_uV || !initdata->constraints.max_uV)) {
> > +		dev_err(&pdev->dev, "no voltage specified for regulator\n");
> > +		return -EINVAL;
> > +	}
> 
> OK, so it looks like we're using it because we can't read the initial
> hardware state - can we introduce a specific interface for that perhaps?
> A call to query the current constraints might do it.  Or possibly just
> change your earlier patch to make sure the constraints are marked const
> would do it, reading them isn't the problem that modifying them is.  I
> might've missed some other usage somewhere though.

No, the reason for init_data is this snippet:

/* Regulators with ia property suppports drms */
if (vreg->parts->ia.mask)
	init_data->constraints.valid_ops_mask |= REGULATOR_CHANGE_DRMS;

This patch just moves the code around to make it possible to implement
patch 4 as cleanly as possible.


The quoted code checks to see that for regulators with voltage-nobs
(could have checked for set_voltage ops) the of-parser did find voltage
limits. So it's a sanity check that discards regulators with missing
dt parameters.

I did drop this in patch 4, as  it's not strictly needed. But the result
is that we will successfully register the regulator and the consumers
will be faced with errors upon trying to set voltage or enable it.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/