Re: [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 30, 2024 at 11:30:42AM -0500, Rob Herring wrote:
> On Thu, Aug 29, 2024 at 04:30:58PM -0500, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@xxxxxxxxxxx>
> > 
> > Document the Texas instruments BQ25703 series of charger managers/
> > buck/boost regulators.
> > 
> > Signed-off-by: Chris Morgan <macromorgan@xxxxxxxxxxx>
> > ---
> >  .../devicetree/bindings/mfd/ti,bq25703a.yaml  | 143 ++++++++++++++++++
> >  1 file changed, 143 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml b/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> > new file mode 100644
> > index 000000000000..e555aa60f9ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> > @@ -0,0 +1,143 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/ti,bq25703a.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: BQ25703 Charger Manager/Buck/Boost Converter
> 
> BQ25703A?

I'm not entirely sure on the nomenclature. I know for sure I have a
BQ25703A in my device, but I don't know if the A is important or not.
I'll just drop it until I know for sure to alleviate confusion.

> 
> > +
> > +maintainers:
> > +  - Chris Morgan <macromorgan@xxxxxxxxxxx>
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,bq25703a
> > +
> > +  reg:
> > +    const: 0x6b
> > +    description: I2C slave address
> 
> Drop description.

Acknowledged.

> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  power-supplies:
> > +    description:
> > +      phandle of the power supply that provides input power
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> 
> Already has a type. You need a reference to power-supply.yaml at the 
> top level and 'maxItems: 1' here.

Gotcha, thank you.

> 
> > +
> > +  ti,charge-current:
> > +    description:
> > +      maximum current to apply to charging the battery
> > +    minimum: 0
> > +    maximum: 8128000
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> I guess this is copied from other TI parts, but really this should move 
> to a property with a unit suffix. Or these shared properties moved to a 
> shared schema so we aren't redefining the type multiple times.
> 
> Same for the others here.

Correct, I tried to reuse the same TI specific properties. I suppose I
could also eliminate ti,charge-current and ti,max-charge-voltage, and
then just require a phandle to a simple-battery node which contains
those two values in a vendor independent form. What do you think?

ti,current-limit and ti,minimum-system-voltage though are other
possible questions I have. Basically I could also create a vsys
regulator that represents the vsys coming off this device too (I
currently omit this entirely). This regulator wouldn't be programable
but would report the existing input current and input voltage for
the system, which in my case I'm pretty sure is stepped down to 5v
or less and then fed into an RK806 PMIC (I lack the schematics so
I don't know for sure). Of course if I did that then I'd have a
valid reason to add a "regulators" node since I'd have more than one
regulator.

> 
> > +
> > +  ti,current-limit:
> > +    description:
> > +      maximum total input current allowed
> > +    minimum: 50000
> > +    maximum: 6400000
> > +    default: 3250000
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  ti,max-charge-voltage:
> > +    description:
> > +      maximum voltage to apply to charging the battery
> > +    minimum: 1024000
> > +    maximum: 19200000
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  ti,minimum-sys-voltage:
> > +    description:
> > +      minimum system voltage while on battery power, with default value
> > +      depending based on cell configuration
> > +    minimum: 1024000
> > +    maximum: 16128000
> > +    default:
> > +      enum: [3584000, 6144000, 9216000, 16128000]
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  regulators:
> > +    type: object
> > +    additionalProperties: false
> > +    description:
> > +      Boost converter regulator output of bq257xx
> 
> Doesn't this apply to "usb-otg-vbus"?
> 
> Really, only one regulator, so you don't need a container node.
> 

It does, that's a mistake on my part. And see above; I could add a
regulator for the vsys (even though you wouldn't be able to set the
voltage or current).

> > +
> > +    properties:
> > +      "usb-otg-vbus":
> 
> Don't need quotes.

Acknowledged.

> 
> > +        type: object
> > +        $ref: /schemas/regulator/regulator.yaml
> > +
> > +        properties:
> > +          regulator-name: true
> > +          regulator-min-microamp:
> > +            minimum: 0
> > +            maximum: 6350000
> > +          regulator-max-microamp:
> > +            minimum: 0
> > +            maximum: 6350000
> > +          regulator-min-microvolt:
> > +            minimum: 4480000
> > +            maximum: 20800000
> > +          regulator-max-microvolt:
> > +            minimum: 4480000
> > +            maximum: 20800000
> > +          enable-gpios:
> > +            description:
> > +              The BQ25703 may require both a register write and a GPIO
> > +              toggle to enable the boost regulator.
> > +
> > +        additionalProperties: true
> 
> Nope.
> 

Acknowledged.

> > +
> > +        required:
> > +          - regulator-name
> > +          - regulator-min-microamp
> > +          - regulator-max-microamp
> > +          - regulator-min-microvolt
> > +          - regulator-max-microvolt
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - power-supplies
> > +  - ti,charge-current
> > +  - ti,current-limit
> > +  - ti,max-charge-voltage
> > +  - ti,minimum-sys-voltage
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/pinctrl/rockchip.h>
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        bq25703: bq25703@6b {
> 
> charger@6b
> 

Acknowledged.

> > +            compatible = "ti,bq25703a";
> > +            reg = <0x6b>;
> > +            interrupt-parent = <&gpio0>;
> > +            interrupts = <RK_PD5 IRQ_TYPE_LEVEL_LOW>;
> > +            power-supplies = <&fusb302>;
> > +            ti,charge-current = <2500000>;
> > +            ti,current-limit = <5000000>;
> > +            ti,max-charge-voltage = <8750000>;
> > +            ti,minimum-sys-voltage = <7400000>;
> > +
> > +            regulators {
> > +                usb_otg_vbus: usb-otg-vbus {
> > +                    enable-gpios = <&gpio4 RK_PA6 GPIO_ACTIVE_HIGH>;
> > +                    regulator-max-microamp = <960000>;
> > +                    regulator-max-microvolt = <5088000>;
> > +                    regulator-min-microamp = <512000>;
> > +                    regulator-min-microvolt = <4992000>;
> > +                    regulator-name = "usb_otg_vbus";
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> > -- 
> > 2.34.1
> > 

Thank you for your feedback,
Chris




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux