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:

quoted

On Wed, Oct 23, 2024 at 03:53:51PM -0500, Grant Peltier wrote:

quoted

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

quoted

quoted

+  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.

quoted

quoted

+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.