| Message ID | 2cc99616ff3dd9bcecb1309cd4d103d70aea862b.1729715599.git.grantpeltier93@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dt-bindings: hwmon: pmbus: add bindings for isl68137 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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 > >
Hi Conor, Thank you for your review! On Thu, Oct 24, 2024 at 06:01:11PM +0100, Conor Dooley wrote: > On Wed, Oct 23, 2024 at 03:53:51PM -0500, Grant Peltier wrote: > > + [...] > > + > > +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. Yes, there are quite a few part numbers (and likely to be more in the future). My intention was to make the driver more user friendly since the variants listed in the driver do not map to something in any of the datasheets. So using those instead would require users to inspect the source of the driver instead of simply referencing their part number(s). > > + > > + 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. In his recent revier, Guenter requested using a standard voltage divider schema as well. I see there is an implementation in maxim,maxim20730.yaml but that differs from the one in voltage-divider.yaml. Should I opt to match maxim,maxim20730.yaml? > > > + 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. > > > + [...] The channel structures are optional to allow users to arbitrarily define voltage dividers for any particular rail. Omitting the channel definitions still allow the device to be instantiated and probed as an I2C device along with all related hwmon PMBus telemetry dictated by the part variant. Thanks again, Grant
On Thu, Oct 24, 2024 at 01:32:09PM -0500, Grant Peltier wrote: > On Thu, Oct 24, 2024 at 06:01:11PM +0100, Conor Dooley wrote: > > On Wed, Oct 23, 2024 at 03:53:51PM -0500, Grant Peltier wrote: > > > + [...] > > > + > > > +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. > > Yes, there are quite a few part numbers (and likely to be more in the > future). My intention was to make the driver more user friendly since the > variants listed in the driver do not map to something in any of the > datasheets. So using those instead would require users to inspect the > source of the driver instead of simply referencing their part number(s). I don't understand. How would a fallback materially change anything in that regard? You still put the compatible corresponding to the device you have in your dts. A fallback means having multiple compatible strings in the property, not a single one corresponding to another device. > > > + 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. > > In his recent revier, Guenter requested using a standard voltage divider > schema as well. I see there is an implementation in maxim,maxim20730.yaml > but that differs from the one in voltage-divider.yaml. Should I opt to > match maxim,maxim20730.yaml? I would rather the standard binding was used, but it would probably involve having to hook up iio-rescale to hwmon? I don't know enough about that, which is why I Cced Peter and Jonathan. > > > +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? > > The channel structures are optional to allow users to arbitrarily define > voltage dividers for any particular rail. Omitting the channel definitions > still allow the device to be instantiated and probed as an I2C device > along with all related hwmon PMBus telemetry dictated by the part variant. I dunno, either the channels are hooked up to something or they are not. If they are, the channels should be populated in the devicetree. Cheers, Conor.
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 + + 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: + 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>; + }; + }; + - | + 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 + }; + }; + };
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