lkml.org 
[lkml]   [2015]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V1] regulator: pv88060: new regulator driver
On Tue, Nov 17, 2015 at 11:25:21AM +0900, James Ban wrote:

A couple of minor points but overall this looks good:

> + node = of_get_child_by_name(dev->of_node, "regulators");
> + if (!node) {
> + dev_err(dev, "regulators node not found\n");
> + return ERR_PTR(-ENODEV);
> + }
> +
> + num = of_regulator_match(dev, node, pv88060_matches,
> + ARRAY_SIZE(pv88060_matches));

Don't manually parse the DT, use the core support and set of_match and
regulators_node in the regulator_desc.

> + if (i == 0) {
> + pv88060_matches[i].init_data->constraints
> + .valid_modes_mask
> + |= REGULATOR_MODE_FAST
> + | REGULATOR_MODE_NORMAL
> + | REGULATOR_MODE_STANDBY;
> + pv88060_matches[i].init_data->constraints.valid_ops_mask
> + |= REGULATOR_CHANGE_MODE;
> + }

The driver should never modify the constraints that are passed in - the
whole point with constraints is to allow the system to set up what is
safe on that particular system.

> + if (!chip->pdata)
> + chip->pdata = pv88060_parse_regulators_dt(chip->dev);
> +
> + if (IS_ERR(chip->pdata)) {
> + dev_err(chip->dev, "No regulators defined for the platform\n");
> + return PTR_ERR(chip->pdata);
> + }

This should not be an error, the driver should just instantiate all
regulators the device has and at least let people read back the current
state.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2015-11-17 20:21    [W:0.035 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site