* [PATCH] shtc1: add support for device tree bindings
@ 2020-07-03 3:48 Chris Ruehl
2020-07-03 3:48 ` [PATCH 1/2] hwmon: " Chris Ruehl
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Chris Ruehl @ 2020-07-03 3:48 UTC (permalink / raw)
To: Chris Ruehl
Cc: Jack Lo, devicetree, Jean Delvare, Guenter Roeck, linux-hwmon,
linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] hwmon: shtc1: add support for device tree bindings 2020-07-03 3:48 [PATCH] shtc1: add support for device tree bindings Chris Ruehl @ 2020-07-03 3:48 ` Chris Ruehl 2020-07-03 5:48 ` Guenter Roeck 2020-07-03 11:12 ` kernel test robot 2020-07-03 3:48 ` [PATCH 2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml Chris Ruehl 2020-07-03 5:35 ` [PATCH] shtc1: add support for device tree bindings Guenter Roeck 2 siblings, 2 replies; 10+ messages in thread From: Chris Ruehl @ 2020-07-03 3:48 UTC (permalink / raw) To: Chris Ruehl Cc: Jack Lo, devicetree, Jean Delvare, Guenter Roeck, linux-hwmon, linux-kernel 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] hwmon: shtc1: add support for device tree bindings 2020-07-03 3:48 ` [PATCH 1/2] hwmon: " Chris Ruehl @ 2020-07-03 5:48 ` Guenter Roeck 2020-07-05 0:34 ` Chris Ruehl 2020-07-03 11:12 ` kernel test robot 1 sibling, 1 reply; 10+ messages in thread From: Guenter Roeck @ 2020-07-03 5:48 UTC (permalink / raw) To: Chris Ruehl; +Cc: Jack Lo, devicetree, Jean Delvare, linux-hwmon, linux-kernel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] hwmon: shtc1: add support for device tree bindings 2020-07-03 5:48 ` Guenter Roeck @ 2020-07-05 0:34 ` Chris Ruehl 0 siblings, 0 replies; 10+ messages in thread From: Chris Ruehl @ 2020-07-05 0:34 UTC (permalink / raw) To: Guenter Roeck Cc: Jack Lo, devicetree, Jean Delvare, linux-hwmon, linux-kernel 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] hwmon: shtc1: add support for device tree bindings 2020-07-03 3:48 ` [PATCH 1/2] hwmon: " Chris Ruehl 2020-07-03 5:48 ` Guenter Roeck @ 2020-07-03 11:12 ` kernel test robot 1 sibling, 0 replies; 10+ messages in thread From: kernel test robot @ 2020-07-03 11:12 UTC (permalink / raw) To: Chris Ruehl Cc: kbuild-all, clang-built-linux, Jack Lo, devicetree, Jean Delvare, Guenter Roeck, linux-hwmon, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml 2020-07-03 3:48 [PATCH] shtc1: add support for device tree bindings Chris Ruehl 2020-07-03 3:48 ` [PATCH 1/2] hwmon: " Chris Ruehl @ 2020-07-03 3:48 ` Chris Ruehl 2020-07-03 5:49 ` Guenter Roeck 2020-07-03 5:35 ` [PATCH] shtc1: add support for device tree bindings Guenter Roeck 2 siblings, 1 reply; 10+ messages in thread From: Chris Ruehl @ 2020-07-03 3:48 UTC (permalink / raw) To: Chris Ruehl Cc: Jack Lo, devicetree, Jean Delvare, Guenter Roeck, Rob Herring, linux-hwmon, linux-kernel 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml 2020-07-03 3:48 ` [PATCH 2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml Chris Ruehl @ 2020-07-03 5:49 ` Guenter Roeck 2020-07-05 0:30 ` Chris Ruehl 0 siblings, 1 reply; 10+ messages in thread From: Guenter Roeck @ 2020-07-03 5:49 UTC (permalink / raw) To: Chris Ruehl Cc: Jack Lo, devicetree, Jean Delvare, Rob Herring, linux-hwmon, linux-kernel 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"; > + }; > + }; > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml 2020-07-03 5:49 ` Guenter Roeck @ 2020-07-05 0:30 ` Chris Ruehl 2020-07-05 2:35 ` Guenter Roeck 0 siblings, 1 reply; 10+ messages in thread From: Chris Ruehl @ 2020-07-05 0:30 UTC (permalink / raw) To: Guenter Roeck Cc: Jack Lo, devicetree, Jean Delvare, Rob Herring, linux-hwmon, linux-kernel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml 2020-07-05 0:30 ` Chris Ruehl @ 2020-07-05 2:35 ` Guenter Roeck 0 siblings, 0 replies; 10+ messages in thread From: Guenter Roeck @ 2020-07-05 2:35 UTC (permalink / raw) To: Chris Ruehl Cc: Jack Lo, devicetree, Jean Delvare, Rob Herring, linux-hwmon, linux-kernel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] shtc1: add support for device tree bindings 2020-07-03 3:48 [PATCH] shtc1: add support for device tree bindings Chris Ruehl 2020-07-03 3:48 ` [PATCH 1/2] hwmon: " Chris Ruehl 2020-07-03 3:48 ` [PATCH 2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml Chris Ruehl @ 2020-07-03 5:35 ` Guenter Roeck 2 siblings, 0 replies; 10+ messages in thread From: Guenter Roeck @ 2020-07-03 5:35 UTC (permalink / raw) To: Chris Ruehl; +Cc: Jack Lo, devicetree, Jean Delvare, linux-hwmon, linux-kernel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-07-05 2:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-03 3:48 [PATCH] shtc1: add support for device tree bindings Chris Ruehl 2020-07-03 3:48 ` [PATCH 1/2] hwmon: " Chris Ruehl 2020-07-03 5:48 ` Guenter Roeck 2020-07-05 0:34 ` Chris Ruehl 2020-07-03 11:12 ` kernel test robot 2020-07-03 3:48 ` [PATCH 2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml Chris Ruehl 2020-07-03 5:49 ` Guenter Roeck 2020-07-05 0:30 ` Chris Ruehl 2020-07-05 2:35 ` Guenter Roeck 2020-07-03 5:35 ` [PATCH] shtc1: add support for device tree bindings Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).