Re: [PATCH v3 4/5] regulators: tps65912: Add regulator driver for the TPS65912 PMIC

From: Andrew F. Davis
Date: Tue Sep 29 2015 - 14:09:23 EST

On 09/29/2015 10:13 AM, Mark Brown wrote:
On Fri, Sep 25, 2015 at 03:10:04PM -0500, Andrew F. Davis wrote:
On 09/25/2015 01:05 PM, Mark Brown wrote:
On Thu, Sep 24, 2015 at 09:52:53AM -0500, Andrew F. Davis wrote:

+ match = of_match_device(tps65912_regulator_of_match_table, &pdev->dev);
+ if (!match)
+ return -ENODEV;

+ template = match->data;
+ id = template->id;
+ init_data = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
+ &regulators[id]);

Don't open code this stuff, use the core DT matching in the
regulator_desc instead.

I assume you are referring to your additions in a0c7b164ad11? If so I'm not

Please don't refer to commits by ID only, include a human readable
description of the commit for the benefit of any humans who might read
your mail.


Forgot about those pesky humans :), OK, will do from now on.

sure that will save me anything as my probe function is called with a DT
match already, so no searching is needed.

You've not understood what that change is replacing, the code I'm
quoting above is exactly that code. Check out some of the existing
drivers using this API.


Looking at other drivers that use this API they all call regulator_register
in a loop in their probe, once for each possible regulator, in this case
letting the API do the DT node search makes sense. My probe on the other-hand
is only called when we already have a DT match, therefor searching is not
necessary and all I have to do is call of_get_regulator_init_data myself on
the already found DT node. No need to add node names to my regulator_desc
and make the API re-search for the node.

I have further cleaned up this code to show this.

If this is acceptable, I'll push the re-spin so you can review further.

Please also use subject lines mathcing the style for the subsystem.

I'm not sure I know what you mean? What is wrong with my subject line, it looks
like the others I've looked at?

You are using "regulators:", the regulator API uses "regulator:" - this
should be really clear if you do something like a git shortlog.


Ah, slipped right past me, thanks for clarifying, fixed.
--
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/