lkml.org 
[lkml]   [2015]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3 4/5] regulators: tps65912: Add regulator driver for the TPS65912 PMIC
    From
    Date
    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.


    \
     
     \ /
      Last update: 2015-09-29 20:41    [W:7.715 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site