Re: [PATCH v2 5/6] regulator: tps65086: Add regulator driver for the TPS65086 PMIC

From: Andrew F. Davis
Date: Tue Nov 03 2015 - 12:57:26 EST


On 11/03/2015 10:31 AM, Mark Brown wrote:
On Tue, Nov 03, 2015 at 09:57:46AM -0600, Andrew F. Davis wrote:
On 10/31/2015 09:37 PM, Mark Brown wrote:
On Fri, Oct 30, 2015 at 05:41:43PM -0500, Andrew F. Davis wrote:

+ struct regulator_desc desc;
+ unsigned int decay_reg;
+ unsigned int decay_mask;

The decay_reg and decay_mask fields appear to be entirely write only,
why are they present?

Not sure what you mean, they are written to differently for each regulator,
and read when setting the decay mode in _probe.

Ugh, that's a bit hidden (mostly due to not using the callback).

+ ret = of_regulator_match(&pdev->dev, pdev->dev.of_node,
+ tps65086_matches,
+ ARRAY_SIZE(tps65086_matches));
+ if (ret < 0) {
+ dev_err(tps->dev, "Error parsing regulator init data\n");
+ return ret;
+ }

Please match regulators using the fields in the regulator_desc rather
than open coding.

I need to match to get the driver specific information for each regulator,
if I let regulator_register do it I never get a chance to process the match,
unless there is a callback or something I'm missing.

There's a callback you are missing, of_parse_cb().


Ah, that will make things easier, I'll re-spin with the callback.
--
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/