lkml.org 
[lkml]   [2022]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v2 02/15] dt-bindings: power: supply: Add Mediatek MT6370 Charger
    From
    On 17/06/2022 03:19, ChiaEn Wu wrote:
    > Hi Krzysztof,
    >
    > Thanks for your helpful comments! I have so some questions want to ask
    > you below.
    >
    > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年6月17日 週五 清晨5:05寫道:
    >>
    >> On 13/06/2022 04:11, ChiaEn Wu wrote:
    >>> From: ChiaEn Wu <chiaen_wu@richtek.com>
    >>>
    >>> Add Mediatek MT6370 Charger binding documentation.
    >>>
    >>> Signed-off-by: ChiaEn Wu <chiaen_wu@richtek.com>
    >>> ---
    >>> .../power/supply/mediatek,mt6370-charger.yaml | 60 +++++++++++++++++++
    >>> 1 file changed, 60 insertions(+)
    >>> create mode 100644 Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml
    >>>
    >>> diff --git a/Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml b/Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml
    >>> new file mode 100644
    >>> index 000000000000..b63553ebb15b
    >>> --- /dev/null
    >>> +++ b/Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml
    >>> @@ -0,0 +1,60 @@
    >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
    >>> +%YAML 1.2
    >>> +---
    >>> +$id: http://devicetree.org/schemas/power/supply/mediatek,mt6370-charger.yaml#
    >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
    >>> +
    >>> +title: Mediatek MT6370 Battery Charger
    >>> +
    >>> +maintainers:
    >>> + - ChiaEn Wu <chiaen_wu@richtek.com>
    >>> +
    >>> +description: |
    >>> + This module is part of the MT6370 MFD device.
    >>> + Provides Battery Charger, Boost for OTG devices and BC1.2 detection.
    >>> +
    >>> +properties:
    >>> + compatible:
    >>> + const: mediatek,mt6370-charger
    >>> +
    >>> + interrupts:
    >>> + description: |
    >>> + Specify what irqs are needed to be handled by MT6370 Charger driver. IRQ
    >>> + "MT6370_IRQ_CHG_MIVR", "MT6370_IRQ_ATTACH" and "MT6370_IRQ_OVPCTRL_UVP_D"
    >>> + are required.
    >>> + items:
    >>> + - description: BC1.2 done irq
    >>> + - description: usb plug in irq
    >>> + - description: mivr irq
    >>> +
    >>> + interrupt-names:
    >>> + items:
    >>> + - const: attach_i
    >>> + - const: uvp_d_evt
    >>> + - const: mivr
    >>> +
    >>> + io-channels:
    >>> + description: |
    >>> + Use ADC channel to read vbus, ibus, ibat, etc., info. Ibus ADC channel
    >>> + is required.
    >>
    >> Add io-channel-names and describe each item - what type of ADC it is
    >> expected to be.
    >>
    >
    > I'm afraid I might not be understanding what you mean.
    > I will try to add some text in "description" and "io-channel-names", like below
    > ----------------------------------
    > io-channels:
    > description: |
    > Use ADC channel to read VBUS, IBUS, IBAT, etc., info. Ibus ADC channel
    > is required. It can be seen in
    > include/dt-bindings/iio/adc/mediatek,mt6370_adc.h
    > minItems: 1
    > maxItems: 9
    >
    > io-channel-names:
    > items:
    > - const: vbusdiv5
    > - const: vbusdiv2

    Almost. The best would be something like this:
    Documentation/devicetree/bindings/power/supply/cpcap-charger.yaml
    so also "items" with description under "io-channels". You need to skip
    maxItems in such case (keep minItems).

    > - ...
    > ----------------------------------
    > Did these modifications meet your expectations?
    >
    >>> + minItems: 1
    >>> + maxItems: 9
    >>> +
    >>> + usb-otg-vbus-regulator:
    >>> + type: object
    >>> + description: OTG boost regulator.
    >>> + $ref: /schemas/regulator/regulator.yaml#
    >>
    >> unevaluatedProperties: false
    >
    > I will add this in the next patch.
    >
    >>
    >>> +
    >>> + properties:
    >>> + enable-gpio:
    >>
    >> "gpios", so:
    >> enable-gpios
    >
    > If this otg regulator only uses one GPIO Pin, do I still need to
    > change to "gpios"?

    Yes, because "gpios" is the preferred suffix. This is requirement for
    all such properties. enable-gpios are also documented here:
    Documentation/devicetree/bindings/gpio/gpio-consumer-common.yaml


    > If so, I will refine it along with the regulator "enable-gpio" in MFD
    > dt-binding.

    Yes, there it should be "enable-gpios" as well.




    Best regards,
    Krzysztof

    \
     
     \ /
      Last update: 2022-06-18 01:12    [W:3.276 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site