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.
Attachment:
signature.asc
Description: PGP signature
