diff mbox series

[2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml

Message ID 20200703034856.12846-3-chris.ruehl@gtsys.com.hk
State Superseded, archived
Headers show
Series None | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema fail build log

Commit Message

Chris Ruehl July 3, 2020, 3:48 a.m. UTC
Add documentation for the newly added DTS support in the shtc1 driver.

Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
---
 .../bindings/hwmon/sensirion,shtc1.yaml       | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml

Comments

Guenter Roeck July 3, 2020, 5:49 a.m. UTC | #1
On 7/2/20 8:48 PM, Chris Ruehl wrote:
> Add documentation for the newly added DTS support in the shtc1 driver.
> 
> Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
> ---
>  .../bindings/hwmon/sensirion,shtc1.yaml       | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml b/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
> new file mode 100644
> index 000000000000..e3e292bc6d7d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/sensirion,shtc1.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sensirion SHTC1 Humidity and Temperature Sensor IC
> +
> +maintainers:
> +  - jdelvare@suse.com
> +
> +description: |
> +  The SHTC1, SHTW1 and SHTC3 are digital humidity and temperature sensor
> +  designed especially for battery-driven high-volume consumer electronics
> +  applications.
> +  For further information refere to Documentation/hwmon/shtc1.rst
> +
> +  This binding document describes the binding for the hardware monitor
> +  portion of the driver.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - sensirion,shtc1
> +      - sensirion,shtw1
> +      - sensirion,shtc3
> +
> +  reg: I2C address 0x70
> +
> +Optional properties:
> +  sensirion,blocking_io: |
> +    u8, if > 0 the i2c bus hold until measure finished (default 0)
> +  sensirion,high_precision: |
> +    u8, if > 0 aquire data with high precision (default 1)
> +

Why u8 and not boolean ?

Guenter

> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +Example:
> +  &i2c1 {
> +    status = "okay";
> +    clock-frequency = <400000>;
> +
> +    shtc3@70 {
> +      compatible = "sensirion,shtc3";
> +      reg = <0x70>
> +      sensirion,blocking_io = <1>;
> +      status = "okay";
> +    };
> +  };
>
Chris Ruehl July 5, 2020, 12:30 a.m. UTC | #2
Hi Guenter,

On 3/7/2020 1:49 pm, Guenter Roeck wrote:
> On 7/2/20 8:48 PM, Chris Ruehl wrote:
>> Add documentation for the newly added DTS support in the shtc1 driver.
>>
>> Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
>> ---
>>   .../bindings/hwmon/sensirion,shtc1.yaml       | 53 +++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml b/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
>> new file mode 100644
>> index 000000000000..e3e292bc6d7d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
>> @@ -0,0 +1,53 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/sensirion,shtc1.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Sensirion SHTC1 Humidity and Temperature Sensor IC
>> +
>> +maintainers:
>> +  - jdelvare@suse.com
>> +
>> +description: |
>> +  The SHTC1, SHTW1 and SHTC3 are digital humidity and temperature sensor
>> +  designed especially for battery-driven high-volume consumer electronics
>> +  applications.
>> +  For further information refere to Documentation/hwmon/shtc1.rst
>> +
>> +  This binding document describes the binding for the hardware monitor
>> +  portion of the driver.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - sensirion,shtc1
>> +      - sensirion,shtw1
>> +      - sensirion,shtc3
>> +
>> +  reg: I2C address 0x70
>> +
>> +Optional properties:
>> +  sensirion,blocking_io: |
>> +    u8, if > 0 the i2c bus hold until measure finished (default 0)
>> +  sensirion,high_precision: |
>> +    u8, if > 0 aquire data with high precision (default 1)
>> +
> Why u8 and not boolean ?
>
> Guenter
The author of the driver make high_precision default (recommend) in the code,
if I use boolean, then the device tree _must_ have have the 
sensirion,high_precision set
or I need to do the opposite and define sensirion,low_precision.
(blocking_io = false default, high_precision = true default)

that's the reason I was thinking use a u8 and test with of_property_read_bool to 
check
the presence of it and set it if value > 0.


Chris.

>
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +Example:
>> +  &i2c1 {
>> +    status = "okay";
>> +    clock-frequency = <400000>;
>> +
>> +    shtc3@70 {
>> +      compatible = "sensirion,shtc3";
>> +      reg = <0x70>
>> +      sensirion,blocking_io = <1>;
>> +      status = "okay";
>> +    };
>> +  };
>>
Guenter Roeck July 5, 2020, 2:35 a.m. UTC | #3
On 7/4/20 5:30 PM, Chris Ruehl wrote:
> Hi Guenter,
> 
> On 3/7/2020 1:49 pm, Guenter Roeck wrote:
>> On 7/2/20 8:48 PM, Chris Ruehl wrote:
>>> Add documentation for the newly added DTS support in the shtc1 driver.
>>>
>>> Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
>>> ---
>>>   .../bindings/hwmon/sensirion,shtc1.yaml       | 53 +++++++++++++++++++
>>>   1 file changed, 53 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml b/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
>>> new file mode 100644
>>> index 000000000000..e3e292bc6d7d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
>>> @@ -0,0 +1,53 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/hwmon/sensirion,shtc1.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Sensirion SHTC1 Humidity and Temperature Sensor IC
>>> +
>>> +maintainers:
>>> +  - jdelvare@suse.com
>>> +
>>> +description: |
>>> +  The SHTC1, SHTW1 and SHTC3 are digital humidity and temperature sensor
>>> +  designed especially for battery-driven high-volume consumer electronics
>>> +  applications.
>>> +  For further information refere to Documentation/hwmon/shtc1.rst
>>> +
>>> +  This binding document describes the binding for the hardware monitor
>>> +  portion of the driver.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - sensirion,shtc1
>>> +      - sensirion,shtw1
>>> +      - sensirion,shtc3
>>> +
>>> +  reg: I2C address 0x70
>>> +
>>> +Optional properties:
>>> +  sensirion,blocking_io: |
>>> +    u8, if > 0 the i2c bus hold until measure finished (default 0)
>>> +  sensirion,high_precision: |
>>> +    u8, if > 0 aquire data with high precision (default 1)
>>> +
>> Why u8 and not boolean ?
>>
>> Guenter
> The author of the driver make high_precision default (recommend) in the code,
> if I use boolean, then the device tree _must_ have have the sensirion,high_precision set
> or I need to do the opposite and define sensirion,low_precision.
> (blocking_io = false default, high_precision = true default)
> 
> that's the reason I was thinking use a u8 and test with of_property_read_bool to check
> the presence of it and set it if value > 0.
> 

Devicetree properties are supposed to be independent from actual implementations.
Declaring "we must do so because of an existing implementation" would set a really
bad precedence - everyone could use that later on to push through arbitrary sets
of devicetree properties. On top of that, this is supposed to be a new set of
bindings, with no one using it today. Any argument along the line of "must have"
seems irrelevant, since there is no real concern about devicetree backwards
compatibility. And on top of all that, I find the currently suggested code
really awkward and convoluted.

With that in mind, I personally would neither accept your argument nor your code.
If you object to defining sensirion,high_precision as boolean, and at the same
time object to defining sensirion,low_precision as well, I'd say, fine, then
let's stick with what we have today.

Anyway, I'll follow Rob's guidance.

Guenter
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml b/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
new file mode 100644
index 000000000000..e3e292bc6d7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml
@@ -0,0 +1,53 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/sensirion,shtc1.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sensirion SHTC1 Humidity and Temperature Sensor IC
+
+maintainers:
+  - jdelvare@suse.com
+
+description: |
+  The SHTC1, SHTW1 and SHTC3 are digital humidity and temperature sensor
+  designed especially for battery-driven high-volume consumer electronics
+  applications.
+  For further information refere to Documentation/hwmon/shtc1.rst
+
+  This binding document describes the binding for the hardware monitor
+  portion of the driver.
+
+properties:
+  compatible:
+    enum:
+      - sensirion,shtc1
+      - sensirion,shtw1
+      - sensirion,shtc3
+
+  reg: I2C address 0x70
+
+Optional properties:
+  sensirion,blocking_io: |
+    u8, if > 0 the i2c bus hold until measure finished (default 0)
+  sensirion,high_precision: |
+    u8, if > 0 aquire data with high precision (default 1)
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+Example:
+  &i2c1 {
+    status = "okay";
+    clock-frequency = <400000>;
+
+    shtc3@70 {
+      compatible = "sensirion,shtc3";
+      reg = <0x70>
+      sensirion,blocking_io = <1>;
+      status = "okay";
+    };
+  };