Message-ID: <9c1f4d0e-83c9-014e-bdb1-38b4b0fb2bbf@linaro.org>
Date:   Thu, 16 Feb 2023 10:12:15 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH 1/2] dt-bindings: regulator: Add bindings for Richtek
 RT5739
Content-Language: en-US
To:     cy_huang@richtek.com, broonie@kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org
Cc:     lgirdwood@gmail.com, u0084500@gmail.com,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
References: <1676426457-1389-1-git-send-email-cy_huang@richtek.com>
 <1676426457-1389-2-git-send-email-cy_huang@richtek.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <1676426457-1389-2-git-send-email-cy_huang@richtek.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 15/02/2023 03:00, cy_huang@richtek.com wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Add the binding document for Richtek RT5739.

Subject: drop second/last, redundant "bindings for". The "dt-bindings"
prefix is already stating that these are bindings.

> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> ---
>  .../bindings/regulator/richtek,rt5739.yaml         | 80 ++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> new file mode 100644
> index 00000000..7dc4f78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/richtek,rt5739.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RT5739 2.4MHz 3.5A Step-Down Converter
> +
> +maintainers:
> +  - ChiYuan Huang <cy_huang@richtek.com>
> +
> +description: |
> +  The RT5739 is a step-down switching voltage regulator that delivers a
> +  digitally programmable output from an input voltage supply of 2.5V to 5.5V.
> +  The output voltage is programmed through an I2C interface capable of
> +  operating up to 3.4MHz.
> +
> +  Using a proprietary architecture with synchronous rectification, the RT5739
> +  is capable of delivering 3.5A continuously at over 80% efficiency,
> +  maintaining that efficiency at load current as low as 10mA. The regulator
> +  operates at a normal fixed frequency of 2.4MHz, which reduces the value of
> +  the external components. 

Can we drop the marketing from kernel? Last part of sentence is not
related to this submission at all. The internal frequency also looks
unrelated to the topic...

> Additional output capacitance can be added to
> +  improve regulation during load transients without affecting stability.
> +
> +allOf:
> +  - $ref: regulator.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - richtek,rt5739
> +
> +  reg:
> +    maxItems: 1
> +
> +  enable-gpios:
> +    maxItems: 1
> +
> +  richtek,vsel-active-high:
> +    description: |
> +      If property is present, use the 'VSEL1' register group for buck control.
> +      Else, use the 'VSEL0' register group. This depends on external hardware
> +      'VSEL' pin connecton.
> +    type: boolean
> +
> +  regulator-allowed-modes:
> +    description: |
> +      buck allowed operating mode
> +        0: Auto PFM/PWM mode
> +        1: Forced PWM mode
> +    maxItems: 2
> +    items:
> +      enum: [0, 1]

So you always require two items? Thus I wonder what's the point of
having it in DT? To skip the property entirely if none of the modes are
allowed?


Best regards,
Krzysztof