On Fri, 2023-11-10 at 18:42 +0000, Conor Dooley wrote: > Yo, > > On Fri, Nov 10, 2023 at 04:18:45PM +0100, Nuno Sa wrote: > > Add bindings for the LTC4282 High Current Hot Swap Controller with I2C > > Compatible Monitoring. > > > > Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx> > > --- > > .../bindings/hwmon/adi,ltc4282.yaml | 228 ++++++++++++++++++ > > MAINTAINERS | 6 + > > 2 files changed, 234 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > > > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > > b/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > > new file mode 100644 > > index 000000000000..0a5d540f014e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml > > @@ -0,0 +1,228 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/hwmon/adi,ltc4282.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Analog Devices LTC4282 I2C High Current Hot Swap Controller over I2C > > + > > +maintainers: > > + - Nuno Sa <nuno.sa@xxxxxxxxxx> > > + > > +description: | > > + Analog Devices LTC4282 I2C High Current Hot Swap Controller over I2C. > > + > > + > > https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4282.pdf > > + > > + > > Extra blank line here FYI. > Right... > > +properties: > > + compatible: > > + enum: > > + - adi,ltc4282 > > + > > + reg: > > + maxItems: 1 > > + > > + vdd-supply: true > > + > > + clocks: > > + maxItems: 1 > > + > > + adi,clkout-mode: > > + description: | > > + Controls in which mode the CLKOUT PIN should work: > > + 0 - Configures the CLKOUT pin to output the internal system clock > > + 1 - Configures the CLKOUT pin to output the internal conversion > > time > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1] > > I really am not a fan of these types of properties. Part of me says that > if you're outputting clocks from this device, then you should be a clock > controller. How do consumers of this @clkout@ pin get the rate of the > clock? I explained it to Guenter as he also argued about this. I'll wait for more feedback but it's likely this will just turn into a clock provider, yes. > I'd kinda be expecting to see a clocks property with a maxItems of 1 and > clock-names with two, mutually exclusive, options. > > The other part says, and it applies in multiple places here, that having > integer properties with non-integer meanings is a poor ABI. I'd be vastly > happier if the various instances in this file became enums of strings, > or $re┤evant-unit so that a dts containing these properties is > immediately understandable. Well, I think you're mentioning the 'gpio-mode' 'and under/over-voltage- dividers'. I think for both it's clear that having the relevant units is not feasible (at least I'm not seeing a way of properly do it). As for the strings, well, I don't have any much to argue other than: 1) It's pattern seen in a lot of bindings - yes, that's not an excuse to copy bad/wrong things over new bindings - but, honestly, it's the first time I have someone complaining about it so I never thought it was wrong. 2) It makes much more easier to handle the properties in the driver (yeah, I know that, as far as you're concerned, this does not matter to you :)) So yeah, if you insist on it, no strong reasons on my side to not do it. As long as I see some consistency down the road :)). - Nuno Sá
