Re: [PATCH v3 2/2] dt-bindings: hwmon: isl68137: add bindings to support voltage dividers

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5399 bytes --]

On Wed, Oct 23, 2024 at 03:53:51PM -0500, Grant Peltier wrote:
> Add devicetree bindings to support declaring optional voltage dividers to
> the rail outputs of supported digital multiphase regulators. Some
> applications require Vout to exceed the voltage range that the Vsense pin
> can detect. This binding definition allows users to define the
> characteristics of a voltage divider placed between Vout and the Vsense
> pin for any rail powered by the device.
> 
> Signed-off-by: Grant Peltier <grantpeltier93@gmail.com>
> ---
>  .../hwmon/pmbus/renesas,isl68137.yaml         | 131 ++++++++++++++++++
>  1 file changed, 131 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml
> new file mode 100644
> index 000000000000..af10c55d547f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml
> @@ -0,0 +1,131 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/hwmon/pmbus/renesas,isl68137.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas Digital Multiphase Voltage Regulators with PMBus
> +
> +maintainers:
> +  - Grant Peltier <grant.peltier.jg@renesas.com>
> +
> +description: |
> +  Renesas digital multiphase voltage regulators with PMBus.
> +  https://www.renesas.com/en/products/power-management/multiphase-power/multiphase-dcdc-switching-controllers
> +
> +properties:
> +  compatible:
> +    enum:
> +      - renesas,isl68220
> +      - renesas,isl68221
> +      - renesas,isl68222
> +      - renesas,isl68223
> +      - renesas,isl68224
> +      - renesas,isl68225
> +      - renesas,isl68226
> +      - renesas,isl68227
> +      - renesas,isl68229
> +      - renesas,isl68233
> +      - renesas,isl68239
> +      - renesas,isl69222
> +      - renesas,isl69223
> +      - renesas,isl69224
> +      - renesas,isl69225
> +      - renesas,isl69227
> +      - renesas,isl69228
> +      - renesas,isl69234
> +      - renesas,isl69236
> +      - renesas,isl69239
> +      - renesas,isl69242
> +      - renesas,isl69243
> +      - renesas,isl69247
> +      - renesas,isl69248
> +      - renesas,isl69254
> +      - renesas,isl69255
> +      - renesas,isl69256
> +      - renesas,isl69259
> +      - renesas,isl69260
> +      - renesas,isl69268
> +      - renesas,isl69269
> +      - renesas,isl69298
> +      - renesas,raa228000
> +      - renesas,raa228004
> +      - renesas,raa228006
> +      - renesas,raa228228
> +      - renesas,raa229001
> +      - renesas,raa229004

Damn, that;s a list and a half, innit! Looking briefly at the driver
change, the match data implies that quite a few of these actually would
be suitable for fallback compatibles.

> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +patternProperties:
> +  "^channel@([0-3])$":
> +    type: object
> +    description:
> +      Container for properties specific to a particular channel (rail).
> +
> +    properties:
> +      reg:
> +        description: The channel (rail) index.
> +        items:
> +          minimum: 0
> +          maximum: 3
> +
> +      renesas,vout-voltage-divider:

There's already a binding for voltage dividers: voltage-divider.yaml
That said, I have no idea how that would work with an extant driver for
the hardware like we have here. I'd imagine it would really have to be
used with iio-hwmon? + Peter and Jonathan, since I don't know how the
driver side of using the voltage divider works.

> +        description:
> +          Resistances of a voltage divider placed between Vout and the voltage
> +          sense pin for the given channel (rail). It has two numbers
> +          representing the resistances of the voltage divider provided as
> +          <R1 R2> which yields an adjusted Vout as
> +          Vout_adj = Vout * (R1 + R2) / R2 given the original Vout as reported
> +          by the Vsense pin.
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 2
> +        maxItems: 2
> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      isl68239@60 {
> +        compatible = "renesas,isl68239";
> +        reg = <0x60>;
> +      };
> +    };

Without any channels, what does this actually do? If you've got no
channels you cannot measure anything making this example invalid?

Thanks,
Conor.

> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      isl68239@60 {
> +        compatible = "renesas,isl68239";
> +        reg = <0x60>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        channel@0 {
> +          reg = <0>;
> +          renesas,vout-voltage-divider = <1000 1000>;  // Reported Vout/Pout would be scaled by 2
> +        };
> +      };
> +    };
> -- 
> 2.39.5
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]