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

Conor Dooley
2024-10-24T17:01:16Z

[-- 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 --]