Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: skakit=codeaurora.org@mg.codeaurora.org
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Tue, 28 Sep 2021 18:14:16 +0530
From:   skakit@codeaurora.org
To:     Mark Brown <broonie@kernel.org>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Lee Jones <lee.jones@linaro.org>,
        Liam Girdwood <lgirdwood@gmail.com>, mka@chromium.org,
        swboyd@chromium.org, Das Srinagesh <gurus@codeaurora.org>,
        David Collins <collinsd@codeaurora.org>, kgunda@codeaurora.org,
        Subbaraman Narayanamurthy <subbaram@codeaurora.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] regulator: dt-bindings: Add pm8008 regulator bindings
In-Reply-To: <20210917154818.GC4700@sirena.org.uk>
References: <1631875538-22473-1-git-send-email-skakit@codeaurora.org>
 <1631875538-22473-3-git-send-email-skakit@codeaurora.org>
 <20210917154818.GC4700@sirena.org.uk>
Message-ID: <b192c1e056e0ea2fc959651065e4ce7b@codeaurora.org>
User-Agent: Roundcube Webmail/1.3.9
Precedence: bulk

On 2021-09-17 21:18, Mark Brown wrote:
> On Fri, Sep 17, 2021 at 04:15:36PM +0530, Satya Priya wrote:
> 
>> +    properties:
>> +      reg:
>> +        maxItems: 1
>> +      regulator-name: true
>> +      regulator-min-microvolt: true
>> +      regulator-max-microvolt: true
> 
> You shouldn't be forcing these properties, it should be perfectly OK 
> for
> boards to have fixed voltages especially for example during bringup or
> for debugging.
> 

Okay. I will remove these.

>> +      qcom,min-dropout-voltage:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description:
>> +          Specifies the minimum voltage in microvolts that the parent
>> +          supply regulator must output, above the output of this
>> +          regulator.
> 
> If this is needed in DT it should be a generic property since most
> regulators have some requirement here however usually it's a fixed
> property of the silicon and should therefore just gets set in the
> regulator_desc as min_dropout_uV - I'd strongly recommend having a
> default there even if there's some requirement for it to be set per
> board.

Yeah, we are setting the default values for this(headroom_uv) from 
driver. Please see below

struct regulator_data {
      char        *name;
      char        *supply_name;
      int     min_uv;
      int     max_uv;
      int     min_dropout_uv;
};

static const struct regulator_data reg_data[PM8008_MAX_LDO] = {
      /* name  parent      min_uv  max_uv  headroom_uv */
     {"l1", "vdd_l1_l2",  528000, 1504000, 225000},
     {"l2", "vdd_l1_l2",  528000, 1504000, 225000},
     {"l3", "vdd_l3_l4", 1504000, 3400000, 200000},
     {"l4", "vdd_l3_l4", 1504000, 3400000, 200000},
     {"l5", "vdd_l5",    1504000, 3400000, 300000},
     {"l6", "vdd_l6",    1504000, 3400000, 300000},
     {"l7", "vdd_l7",    1504000, 3400000, 300000},
};

Inside Register LDO API:

pm8008_reg->rdesc.min_dropout_uV = reg_data[i].min_dropout_uv;
of_property_read_u32(reg_node, "qcom,min-dropout-voltage",
                   &pm8008_reg->rdesc.min_dropout_uV);