Messages in this thread Patch in this message | | | Subject | Re: [PATCH 1/2] dt-bindings: hwmon: add binding documentation for adt7475 | From | Guenter Roeck <> | Date | Tue, 6 Nov 2018 21:07:57 -0800 |
| |
On 11/6/18 8:27 PM, Chris Packham wrote: > On 7/11/18 5:24 PM, Guenter Roeck wrote: >> On 11/6/18 8:00 PM, Chris Packham wrote: >>> With the addition of the invert-pwm property the adt7475 needs its own >>> binding documentation rather being captured under trivial-devices.txt. >>> >>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >>> --- >>> .../devicetree/bindings/hwmon/adt7475.txt | 22 +++++++++++++++++++ >>> .../devicetree/bindings/trivial-devices.txt | 4 ---- >>> 2 files changed, 22 insertions(+), 4 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/hwmon/adt7475.txt >>> >>> diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.txt b/Documentation/devicetree/bindings/hwmon/adt7475.txt >>> new file mode 100644 >>> index 000000000000..79255439e157 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/hwmon/adt7475.txt >>> @@ -0,0 +1,22 @@ >>> +*ADT7475 hwmon sensor. >>> + >>> +Required properties: >>> +- compatible: One of >>> + "adi,adt7473" >>> + "adi,adt7475" >>> + "adi,adt7476" >>> + "adi,adt7490" >>> + >>> +- reg: I2C address >>> + >>> +optional properties: >>> + >>> +- invert-pwm: This configures the PWM to use logic low for 100% duty cycle. >>> + >>> +Example: >>> + >>> +hwmon@2e { >>> + compatible = ; >>> + reg = <0x2e>; >>> + invert-pwm; >> >> If I understand correctly, the flag is set per pwm channel. A single global >> property seems inappropriate. > > Yes it is per-channel. But I was having a hard time imagining a hardware > design that would use both inverted and non-inverted at the same time. >
People can be inventive. Sometimes too much so.
> Do you have a preference for how the properties should look? > > invert-pwm[123]? > > invert-pwm = <0x7>;? > Ultimately we'll have to find a generic way of defining devicetree properties for hardware monitoring devices, not just for pwm but for all sensor types. A sub-node per channel seems to be the most likely answer, but I think that is a long way off (and will require lengthy discussions about acceptable properties).
I was looking into pwm DT properties, but they define a set of flags (PWM_POLARITY_INVERTED, PWM_POLARITY_NORMAL).
The g762 driver uses "pwm_polarity". Underscore - hmm. Other drivers use module parameters, sysfs attributes (pwmX_invert in asc7621 driver), or platform data (g762, max6639).
Maybe pwm-polarity = <PWM_POLARITY_NORMAL | PWM_POLARITY_INVERTED> ? No idea if that makes sense. Or just a boolean pwm-polarity-inverted.
Thinking about it per channel vs. per chip ... other drivers also seem to use a single property / attribute for the entire chip, so that is fine here as well. We can always extend it if needed.
Anyway, I am fine with whatever Rob accepts.
Guenter
>> >> Guenter >> >>> +}; >>> diff --git a/Documentation/devicetree/bindings/trivial-devices.txt b/Documentation/devicetree/bindings/trivial-devices.txt >>> index 69c934aec13b..4f29100d6bbf 100644 >>> --- a/Documentation/devicetree/bindings/trivial-devices.txt >>> +++ b/Documentation/devicetree/bindings/trivial-devices.txt >>> @@ -14,10 +14,6 @@ ad,ad7414 SMBus/I2C Digital Temperature Sensor in 6-Pin SOT with SMBus Alert an >>> ad,adm9240 ADM9240: Complete System Hardware Monitor for uProcessor-Based Systems >>> adi,adt7461 +/-1C TDM Extended Temp Range I.C >>> adt7461 +/-1C TDM Extended Temp Range I.C >>> -adi,adt7473 +/-1C TDM Extended Temp Range I.C >>> -adi,adt7475 +/-1C TDM Extended Temp Range I.C >>> -adi,adt7476 +/-1C TDM Extended Temp Range I.C >>> -adi,adt7490 +/-1C TDM Extended Temp Range I.C >>> adi,adxl345 Three-Axis Digital Accelerometer >>> adi,adxl346 Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too) >>> ams,iaq-core AMS iAQ-Core VOC Sensor >>> >> >> > >
| |