[PATCH] shtc1: add support for device tree bindings

[PATCH] shtc1: add support for device tree bindings

[Chris Ruehl]

Add support for DTS bindings to the shtc driver
The patches add the compatible table and of_property_read* to the
shtc1.c. Newly created Yaml document has been released to the
Documentation/devicetree/hwmon/sensirion,shtc1.yaml

Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
---
 Version 1

[PATCH 1/2] hwmon: shtc1: add support for device tree bindings

[Chris Ruehl]

Add support for DTS bindings to the shtc driver, use CONFIG_OF
to compile in the code if needed.

Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
---
 drivers/hwmon/shtc1.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/hwmon/shtc1.c b/drivers/hwmon/shtc1.c
index a0078ccede03..3bcabc1cbce8 100644
--- a/drivers/hwmon/shtc1.c
+++ b/drivers/hwmon/shtc1.c
@@ -14,6 +14,9 @@
 #include <linux/err.h>
 #include <linux/delay.h>
 #include <linux/platform_data/shtc1.h>
+#ifdef CONFIG_OF
+#include <linux/of.h>
+#endif
 
 /* commands (high precision mode) */
 static const unsigned char shtc1_cmd_measure_blocking_hpm[]    = { 0x7C, 0xA2 };
@@ -196,6 +199,10 @@ static int shtc1_probe(struct i2c_client *client,
 	enum shtcx_chips chip = id->driver_data;
 	struct i2c_adapter *adap = client->adapter;
 	struct device *dev = &client->dev;
+#ifdef CONFIG_OF
+	struct device_node *np = dev->of_node;
+	u8 value;
+#endif
 
 	if (!i2c_check_functionality(adap, I2C_FUNC_I2C)) {
 		dev_err(dev, "plain i2c transactions not supported\n");
@@ -235,6 +242,20 @@ static int shtc1_probe(struct i2c_client *client,
 
 	if (client->dev.platform_data)
 		data->setup = *(struct shtc1_platform_data *)dev->platform_data;
+
+#ifdef CONFIG_OF
+	if (np) {
+		if (of_property_read_bool(np, "sensirion,blocking_io")) {
+			of_property_read_u8(np, "sensirion,blocking_io", &value);
+			data->setup.blocking_io = (value > 0) ? true : false;
+		}
+		if (of_property_read_bool(np, "sensicon,high_precision")) {
+			of_property_read_u8(np, "sensirion,high_precision", &value);
+			data->setup.high_precision = (value > 0) ? true : false;
+		}
+	}
+#endif
+
 	shtc1_select_command(data);
 	mutex_init(&data->update_lock);
 
@@ -257,6 +278,15 @@ static const struct i2c_device_id shtc1_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, shtc1_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id shtc1_of_match[] = {
+	{ .compatible = "sensirion,shtc1" },
+	{ .compatible = "sensirion,shtw1" },
+	{ .compatible = "sensirion,shtc3" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, shtc1_of_match);
+#endif
 static struct i2c_driver shtc1_i2c_driver = {
 	.driver.name  = "shtc1",
 	.probe        = shtc1_probe,
-- 
2.20.1

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

[Chris Ruehl]

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)
+
+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";
+    };
+  };
-- 
2.20.1

Re: [PATCH] shtc1: add support for device tree bindings

[Guenter Roeck]

On 7/2/20 8:48 PM, Chris Ruehl wrote:
> Add support for DTS bindings to the shtc driver
> The patches add the compatible table and of_property_read* to the
> shtc1.c. Newly created Yaml document has been released to the
> Documentation/devicetree/hwmon/sensirion,shtc1.yaml
> 
> Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
> ---
>  Version 1
> 

There is no patch.

Guenter

Re: [PATCH 1/2] hwmon: shtc1: add support for device tree bindings

[Guenter Roeck]

On 7/2/20 8:48 PM, Chris Ruehl wrote:
> Add support for DTS bindings to the shtc driver, use CONFIG_OF
> to compile in the code if needed.
> 

Ah, here it is. The introducing patch should say something like "[PATCH 0/2]".

> Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
> ---
>  drivers/hwmon/shtc1.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/hwmon/shtc1.c b/drivers/hwmon/shtc1.c
> index a0078ccede03..3bcabc1cbce8 100644
> --- a/drivers/hwmon/shtc1.c
> +++ b/drivers/hwmon/shtc1.c
> @@ -14,6 +14,9 @@
>  #include <linux/err.h>
>  #include <linux/delay.h>
>  #include <linux/platform_data/shtc1.h>
> +#ifdef CONFIG_OF

No. Please no conditional includes.

> +#include <linux/of.h>
> +#endif
>  
>  /* commands (high precision mode) */
>  static const unsigned char shtc1_cmd_measure_blocking_hpm[]    = { 0x7C, 0xA2 };
> @@ -196,6 +199,10 @@ static int shtc1_probe(struct i2c_client *client,
>  	enum shtcx_chips chip = id->driver_data;
>  	struct i2c_adapter *adap = client->adapter;
>  	struct device *dev = &client->dev;
> +#ifdef CONFIG_OF
> +	struct device_node *np = dev->of_node;
> +	u8 value;
> +#endif
>  
>  	if (!i2c_check_functionality(adap, I2C_FUNC_I2C)) {
>  		dev_err(dev, "plain i2c transactions not supported\n");
> @@ -235,6 +242,20 @@ static int shtc1_probe(struct i2c_client *client,
>  
>  	if (client->dev.platform_data)
>  		data->setup = *(struct shtc1_platform_data *)dev->platform_data;
> +
> +#ifdef CONFIG_OF

Unnecessary ifdef. Selection of devicetree data or not can be made
based on np != NULL. Also, if devictree data is available, platform
data should be ignored to avoid confusion.

> +	if (np) {
> +		if (of_property_read_bool(np, "sensirion,blocking_io")) {
> +			of_property_read_u8(np, "sensirion,blocking_io", &value);
> +			data->setup.blocking_io = (value > 0) ? true : false;
> +		}
Why this complexity, and why not just make the property a boolean ?

> +		if (of_property_read_bool(np, "sensicon,high_precision")) {
> +			of_property_read_u8(np, "sensirion,high_precision", &value);
> +			data->setup.high_precision = (value > 0) ? true : false;

"sensicon,high_precision" should also be a boolean.

> +		}
> +	}
> +#endif
> +
>  	shtc1_select_command(data);
>  	mutex_init(&data->update_lock);
>  
> @@ -257,6 +278,15 @@ static const struct i2c_device_id shtc1_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, shtc1_id);
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id shtc1_of_match[] = {
> +	{ .compatible = "sensirion,shtc1" },
> +	{ .compatible = "sensirion,shtw1" },
> +	{ .compatible = "sensirion,shtc3" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, shtc1_of_match);
> +#endif
>  static struct i2c_driver shtc1_i2c_driver = {
>  	.driver.name  = "shtc1",
>  	.probe        = shtc1_probe,
> 
Not sure how this works without setting of_match_table. I guess it just works
accidentally because .id_table also provides a devicetree match. Still,
of_match_table should be set.

Guenter

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

[Guenter Roeck]

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";
> +    };
> +  };
> 

Re: [PATCH 1/2] hwmon: shtc1: add support for device tree bindings

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

Hi Chris,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v5.8-rc3 next-20200703]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Chris-Ruehl/hwmon-shtc1-add-support-for-device-tree-bindings/20200703-124921
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: arm-randconfig-r012-20200701 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project ca464639a1c9dd3944eb055ffd2796e8c2e7639f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/hwmon/shtc1.c:282:34: warning: unused variable 'shtc1_of_match' [-Wunused-const-variable]
   static const struct of_device_id shtc1_of_match[] = {
                                    ^
   1 warning generated.

vim +/shtc1_of_match +282 drivers/hwmon/shtc1.c

   280	
   281	#ifdef CONFIG_OF
 > 282	static const struct of_device_id shtc1_of_match[] = {
   283		{ .compatible = "sensirion,shtc1" },
   284		{ .compatible = "sensirion,shtw1" },
   285		{ .compatible = "sensirion,shtc3" },
   286		{ }
   287	};
   288	MODULE_DEVICE_TABLE(of, shtc1_of_match);
   289	#endif
   290	static struct i2c_driver shtc1_i2c_driver = {
   291		.driver.name  = "shtc1",
   292		.probe        = shtc1_probe,
   293		.id_table     = shtc1_id,
   294	};
   295	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28301 bytes --]

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

[Chris Ruehl]

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";
>> +    };
>> +  };
>>

-- 
GTSYS Limited RFID Technology
9/F, Unit E, R07, Kwai Shing Industrial Building Phase 2,
42-46 Tai Lin Pai Road, Kwai Chung, N.T., Hong Kong
Tel (852) 9079 9521

Disclaimer: https://www.gtsys.com.hk/email/classified.html

Re: [PATCH 1/2] hwmon: shtc1: add support for device tree bindings

[Chris Ruehl]

Hi Guenter,

On 3/7/2020 1:48 pm, Guenter Roeck wrote:
> On 7/2/20 8:48 PM, Chris Ruehl wrote:
>> Add support for DTS bindings to the shtc driver, use CONFIG_OF
>> to compile in the code if needed.
>>
> 
> Ah, here it is. The introducing patch should say something like "[PATCH 0/2]".
> 
>> Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
>> ---
>>   drivers/hwmon/shtc1.c | 30 ++++++++++++++++++++++++++++++
>>   1 file changed, 30 insertions(+)
>>
>> diff --git a/drivers/hwmon/shtc1.c b/drivers/hwmon/shtc1.c
>> index a0078ccede03..3bcabc1cbce8 100644
>> --- a/drivers/hwmon/shtc1.c
>> +++ b/drivers/hwmon/shtc1.c
>> @@ -14,6 +14,9 @@
>>   #include <linux/err.h>
>>   #include <linux/delay.h>
>>   #include <linux/platform_data/shtc1.h>
>> +#ifdef CONFIG_OF
> 
> No. Please no conditional includes.

OK, will be fixed and same for below.

> 
>> +#include <linux/of.h>
>> +#endif
>>   
>>   /* commands (high precision mode) */
>>   static const unsigned char shtc1_cmd_measure_blocking_hpm[]    = { 0x7C, 0xA2 };
>> @@ -196,6 +199,10 @@ static int shtc1_probe(struct i2c_client *client,
>>   	enum shtcx_chips chip = id->driver_data;
>>   	struct i2c_adapter *adap = client->adapter;
>>   	struct device *dev = &client->dev;
>> +#ifdef CONFIG_OF
>> +	struct device_node *np = dev->of_node;
>> +	u8 value;
>> +#endif
>>   
>>   	if (!i2c_check_functionality(adap, I2C_FUNC_I2C)) {
>>   		dev_err(dev, "plain i2c transactions not supported\n");
>> @@ -235,6 +242,20 @@ static int shtc1_probe(struct i2c_client *client,
>>   
>>   	if (client->dev.platform_data)
>>   		data->setup = *(struct shtc1_platform_data *)dev->platform_data;
>> +
>> +#ifdef CONFIG_OF
> 
> Unnecessary ifdef. Selection of devicetree data or not can be made
> based on np != NULL. Also, if devictree data is available, platform
> data should be ignored to avoid confusion.

Ok, I wasn't aware this rule. Will change it.

> 
>> +	if (np) {
>> +		if (of_property_read_bool(np, "sensirion,blocking_io")) {
>> +			of_property_read_u8(np, "sensirion,blocking_io", &value);
>> +			data->setup.blocking_io = (value > 0) ? true : false;
>> +		}
> Why this complexity, and why not just make the property a boolean ?
> 
>> +		if (of_property_read_bool(np, "sensicon,high_precision")) {
>> +			of_property_read_u8(np, "sensirion,high_precision", &value);
>> +			data->setup.high_precision = (value > 0) ? true : false;
> 
> "sensicon,high_precision" should also be a boolean.
> 
>> +		}
>> +	}
>> +#endif
>> +
>>   	shtc1_select_command(data);
>>   	mutex_init(&data->update_lock);
>>   
>> @@ -257,6 +278,15 @@ static const struct i2c_device_id shtc1_id[] = {
>>   };
>>   MODULE_DEVICE_TABLE(i2c, shtc1_id);
>>   
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id shtc1_of_match[] = {
>> +	{ .compatible = "sensirion,shtc1" },
>> +	{ .compatible = "sensirion,shtw1" },
>> +	{ .compatible = "sensirion,shtc3" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, shtc1_of_match);
>> +#endif
>>   static struct i2c_driver shtc1_i2c_driver = {
>>   	.driver.name  = "shtc1",
>>   	.probe        = shtc1_probe,
>>
> Not sure how this works without setting of_match_table. I guess it just works
> accidentally because .id_table also provides a devicetree match. Still,
> of_match_table should be set.

Thanks, I will take care of this in the v2 version.

> 
> Guenter
> 

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

[Guenter Roeck]

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