2019-01-11 21:20:07

by Sebastien Bourdelin

[permalink] [raw]
Subject: [PATCH v2 1/2] iio: chemical: bme680: Add device-tree support

This commit allow the driver to work with device-tree.

Signed-off-by: Sebastien Bourdelin <[email protected]>
---
v1 -> v2:
- add missing of.h header in bme680_spi.c
---
drivers/iio/chemical/bme680_i2c.c | 7 +++++++
drivers/iio/chemical/bme680_spi.c | 9 +++++++++
2 files changed, 16 insertions(+)

diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c
index 06d4be539d2e..94a36ebdf0b2 100644
--- a/drivers/iio/chemical/bme680_i2c.c
+++ b/drivers/iio/chemical/bme680_i2c.c
@@ -70,10 +70,17 @@ static const struct acpi_device_id bme680_acpi_match[] = {
};
MODULE_DEVICE_TABLE(acpi, bme680_acpi_match);

+static const struct of_device_id bme680_of_i2c_match[] = {
+ { .compatible = "bosch,bme680", },
+ {},
+}
+MODULE_DEVICE_TABLE(of, bme680_of_i2c_match);
+
static struct i2c_driver bme680_i2c_driver = {
.driver = {
.name = "bme680_i2c",
.acpi_match_table = ACPI_PTR(bme680_acpi_match),
+ .of_match_table = of_match_ptr(bme680_of_i2c_match),
},
.probe = bme680_i2c_probe,
.id_table = bme680_i2c_id,
diff --git a/drivers/iio/chemical/bme680_spi.c b/drivers/iio/chemical/bme680_spi.c
index c9fb05e8d0b9..caa57287a911 100644
--- a/drivers/iio/chemical/bme680_spi.c
+++ b/drivers/iio/chemical/bme680_spi.c
@@ -6,9 +6,11 @@
*/
#include <linux/acpi.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/regmap.h>
#include <linux/spi/spi.h>

+
#include "bme680.h"

static int bme680_regmap_spi_write(void *context, const void *data,
@@ -110,10 +112,17 @@ static const struct acpi_device_id bme680_acpi_match[] = {
};
MODULE_DEVICE_TABLE(acpi, bme680_acpi_match);

+static const struct of_device_id bme680_of_spi_match[] = {
+ { .compatible = "bosch,bme680", },
+ {},
+}
+MODULE_DEVICE_TABLE(of, bme680_of_spi_match);
+
static struct spi_driver bme680_spi_driver = {
.driver = {
.name = "bme680_spi",
.acpi_match_table = ACPI_PTR(bme680_acpi_match),
+ .of_match_table = of_match_ptr(bme680_of_spi_match),
},
.probe = bme680_spi_probe,
.id_table = bme680_spi_id,
--
2.20.1



2019-01-11 21:20:13

by Sebastien Bourdelin

[permalink] [raw]
Subject: [PATCH v2 2/2] dt-bindings: iio: chemical: Add bindings for bme680

BME680 is a pressure/temperature/humidity/voc sensor.

Signed-off-by: Sebastien Bourdelin <[email protected]>
---
.../devicetree/bindings/iio/chemical/bme680.txt | 11 +++++++++++
1 file changed, 11 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/chemical/bme680.txt

diff --git a/Documentation/devicetree/bindings/iio/chemical/bme680.txt b/Documentation/devicetree/bindings/iio/chemical/bme680.txt
new file mode 100644
index 000000000000..885a1b918340
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/bme680.txt
@@ -0,0 +1,11 @@
+Bosch Sensortec BME680 pressure/temperature/humidity/voc sensors
+
+Required properties:
+- compatible: must be "bosch,bme680"
+
+Example:
+
+bme680@77 {
+ compatible = "bosch,bme680";
+ reg = <0x77>;
+};
--
2.20.1


2019-01-12 09:44:58

by Himanshu Jha

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: chemical: bme680: Add device-tree support

On Fri, Jan 11, 2019 at 03:53:58PM -0500, Sebastien Bourdelin wrote:
> This commit allow the driver to work with device-tree.
>
> Signed-off-by: Sebastien Bourdelin <[email protected]>
> ---

I get the following compilation failure:

Below I have `allyesconfig` except 'BME680' configure as [M]
in case you wish to reproduce.

himanshu@himanshu-Vostro-3559:~/linux-next$ grep -i -w 'CONFIG_BME680\|CONFIG_ACPI\|CONFIG_OF' .config
CONFIG_ACPI=y
CONFIG_OF=y
CONFIG_BME680=m
himanshu@himanshu-Vostro-3559:~/linux-next$ sudo make drivers/iio/chemical/bme680_spi.o
make[1]: Nothing to be done for 'all'.
CALL scripts/checksyscalls.sh
DESCEND objtool
CC [M] drivers/iio/chemical/bme680_spi.o
In file included from ./include/linux/acpi.h:41:0,
from drivers/iio/chemical/bme680_spi.c:7:
./include/linux/module.h:213:1: error: expected ‘,’ or ‘;’ before ‘extern’
extern typeof(name) __mod_##type##__##name##_device_table \
^
drivers/iio/chemical/bme680_spi.c:119:1: note: in expansion of macro ‘MODULE_DEVICE_TABLE’
MODULE_DEVICE_TABLE(of, bme680_of_spi_match);
^~~~~~~~~~~~~~~~~~~
scripts/Makefile.build:291: recipe for target 'drivers/iio/chemical/bme680_spi.o' failed
make[1]: *** [drivers/iio/chemical/bme680_spi.o] Error 1
Makefile:1741: recipe for target 'drivers/iio/chemical/bme680_spi.o' failed
make: *** [drivers/iio/chemical/bme680_spi.o] Error 2

BUT if:

himanshu@himanshu-Vostro-3559:~/linux-next$ make allyesconfig
scripts/kconfig/conf --allyesconfig Kconfig
#
# configuration written to .config
#

himanshu@himanshu-Vostro-3559:~/linux-next$ sudo make drivers/iio/chemical/bme680_spi.o
scripts/kconfig/conf --syncconfig Kconfig
make[1]: Nothing to be done for 'all'.
HOSTCC scripts/dtc/dtc.o
HOSTCC scripts/dtc/flattree.o
HOSTCC scripts/dtc/fstree.o
HOSTCC scripts/dtc/data.o
HOSTCC scripts/dtc/livetree.o
HOSTCC scripts/dtc/treesource.o
HOSTCC scripts/dtc/srcpos.o
HOSTCC scripts/dtc/checks.o
HOSTCC scripts/dtc/util.o
LEX scripts/dtc/dtc-lexer.lex.c
YACC scripts/dtc/dtc-parser.tab.h
HOSTCC scripts/dtc/dtc-lexer.lex.o
YACC scripts/dtc/dtc-parser.tab.c
HOSTCC scripts/dtc/dtc-parser.tab.o
HOSTLD scripts/dtc/dtc
CC scripts/mod/empty.o
MKELF scripts/mod/elfconfig.h
HOSTCC scripts/mod/modpost.o
CC scripts/mod/devicetable-offsets.s
HOSTCC scripts/mod/file2alias.o
HOSTCC scripts/mod/sumversion.o
HOSTLD scripts/mod/modpost
CC kernel/bounds.s
CC arch/x86/kernel/asm-offsets.s
CALL scripts/checksyscalls.sh
DESCEND objtool
CC drivers/iio/chemical/bme680_spi.o

Compiles without any issues.

Also, wondering when is 0x77 i2c address used, since I tested
this on 3 different boards with 0x76(when SDO is connected to GND)

And why do I connect SDO to ground everytime ?
It is because if SDO pin is left floating then I2C address will be
undefined as said in datasheet + I have observed this while testing.

Actallly, I don't understand what "VDIDO" is, as explained in the
datasheet.


Anyway, if the above compilation issue is not a problem, then

Acked-by: Himanshu Jha <[email protected]>


Thanks
--
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

2019-01-12 19:11:10

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: chemical: bme680: Add device-tree support

On Fri, 11 Jan 2019 15:53:58 -0500
Sebastien Bourdelin <[email protected]> wrote:

> This commit allow the driver to work with device-tree.
>
> Signed-off-by: Sebastien Bourdelin <[email protected]>
Minor stuff inline.

J
> ---
> v1 -> v2:
> - add missing of.h header in bme680_spi.c
> ---
> drivers/iio/chemical/bme680_i2c.c | 7 +++++++
> drivers/iio/chemical/bme680_spi.c | 9 +++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c
> index 06d4be539d2e..94a36ebdf0b2 100644
> --- a/drivers/iio/chemical/bme680_i2c.c
> +++ b/drivers/iio/chemical/bme680_i2c.c
> @@ -70,10 +70,17 @@ static const struct acpi_device_id bme680_acpi_match[] = {
> };
> MODULE_DEVICE_TABLE(acpi, bme680_acpi_match);
>
> +static const struct of_device_id bme680_of_i2c_match[] = {
> + { .compatible = "bosch,bme680", },
> + {},
> +}
> +MODULE_DEVICE_TABLE(of, bme680_of_i2c_match);
> +
> static struct i2c_driver bme680_i2c_driver = {
> .driver = {
> .name = "bme680_i2c",
> .acpi_match_table = ACPI_PTR(bme680_acpi_match),
> + .of_match_table = of_match_ptr(bme680_of_i2c_match),

As below. just = bme680...

> },
> .probe = bme680_i2c_probe,
> .id_table = bme680_i2c_id,
> diff --git a/drivers/iio/chemical/bme680_spi.c b/drivers/iio/chemical/bme680_spi.c
> index c9fb05e8d0b9..caa57287a911 100644
> --- a/drivers/iio/chemical/bme680_spi.c
> +++ b/drivers/iio/chemical/bme680_spi.c
> @@ -6,9 +6,11 @@
> */
> #include <linux/acpi.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/regmap.h>
> #include <linux/spi/spi.h>
>
> +

This white space change should not be here.

> #include "bme680.h"
>
> static int bme680_regmap_spi_write(void *context, const void *data,
> @@ -110,10 +112,17 @@ static const struct acpi_device_id bme680_acpi_match[] = {
> };
> MODULE_DEVICE_TABLE(acpi, bme680_acpi_match);
>
> +static const struct of_device_id bme680_of_spi_match[] = {
> + { .compatible = "bosch,bme680", },
> + {},
> +}
> +MODULE_DEVICE_TABLE(of, bme680_of_spi_match);
> +
> static struct spi_driver bme680_spi_driver = {
> .driver = {
> .name = "bme680_spi",
> .acpi_match_table = ACPI_PTR(bme680_acpi_match),
> + .of_match_table = of_match_ptr(bme680_of_spi_match),
Please don't use of_match_ptr. We actually want this entry to be there even
if devicetree is not in use. This is because there is a magic ACPI hid
that can use this table even from ACPI.

> },
> .probe = bme680_spi_probe,
> .id_table = bme680_spi_id,


2019-01-12 19:11:39

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: chemical: bme680: Add device-tree support

On Sat, 12 Jan 2019 15:12:26 +0530
Himanshu Jha <[email protected]> wrote:

> On Fri, Jan 11, 2019 at 03:53:58PM -0500, Sebastien Bourdelin wrote:
> > This commit allow the driver to work with device-tree.
> >
> > Signed-off-by: Sebastien Bourdelin <[email protected]>
> > ---
>
> I get the following compilation failure:
>
> Below I have `allyesconfig` except 'BME680' configure as [M]
> in case you wish to reproduce.
>
> himanshu@himanshu-Vostro-3559:~/linux-next$ grep -i -w 'CONFIG_BME680\|CONFIG_ACPI\|CONFIG_OF' .config
> CONFIG_ACPI=y
> CONFIG_OF=y
> CONFIG_BME680=m
> himanshu@himanshu-Vostro-3559:~/linux-next$ sudo make drivers/iio/chemical/bme680_spi.o
> make[1]: Nothing to be done for 'all'.
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CC [M] drivers/iio/chemical/bme680_spi.o
> In file included from ./include/linux/acpi.h:41:0,
> from drivers/iio/chemical/bme680_spi.c:7:
> ./include/linux/module.h:213:1: error: expected ‘,’ or ‘;’ before ‘extern’
> extern typeof(name) __mod_##type##__##name##_device_table \
> ^
> drivers/iio/chemical/bme680_spi.c:119:1: note: in expansion of macro ‘MODULE_DEVICE_TABLE’
> MODULE_DEVICE_TABLE(of, bme680_of_spi_match);
> ^~~~~~~~~~~~~~~~~~~

huh? That one had me confused. Google however got me there quickly.
Missing semi colon on the line above MODULE_DEVICE_TABLE.

J

> scripts/Makefile.build:291: recipe for target 'drivers/iio/chemical/bme680_spi.o' failed
> make[1]: *** [drivers/iio/chemical/bme680_spi.o] Error 1
> Makefile:1741: recipe for target 'drivers/iio/chemical/bme680_spi.o' failed
> make: *** [drivers/iio/chemical/bme680_spi.o] Error 2
>
> BUT if:
>
> himanshu@himanshu-Vostro-3559:~/linux-next$ make allyesconfig
> scripts/kconfig/conf --allyesconfig Kconfig
> #
> # configuration written to .config
> #
>
> himanshu@himanshu-Vostro-3559:~/linux-next$ sudo make drivers/iio/chemical/bme680_spi.o
> scripts/kconfig/conf --syncconfig Kconfig
> make[1]: Nothing to be done for 'all'.
> HOSTCC scripts/dtc/dtc.o
> HOSTCC scripts/dtc/flattree.o
> HOSTCC scripts/dtc/fstree.o
> HOSTCC scripts/dtc/data.o
> HOSTCC scripts/dtc/livetree.o
> HOSTCC scripts/dtc/treesource.o
> HOSTCC scripts/dtc/srcpos.o
> HOSTCC scripts/dtc/checks.o
> HOSTCC scripts/dtc/util.o
> LEX scripts/dtc/dtc-lexer.lex.c
> YACC scripts/dtc/dtc-parser.tab.h
> HOSTCC scripts/dtc/dtc-lexer.lex.o
> YACC scripts/dtc/dtc-parser.tab.c
> HOSTCC scripts/dtc/dtc-parser.tab.o
> HOSTLD scripts/dtc/dtc
> CC scripts/mod/empty.o
> MKELF scripts/mod/elfconfig.h
> HOSTCC scripts/mod/modpost.o
> CC scripts/mod/devicetable-offsets.s
> HOSTCC scripts/mod/file2alias.o
> HOSTCC scripts/mod/sumversion.o
> HOSTLD scripts/mod/modpost
> CC kernel/bounds.s
> CC arch/x86/kernel/asm-offsets.s
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CC drivers/iio/chemical/bme680_spi.o
>
> Compiles without any issues.
>
> Also, wondering when is 0x77 i2c address used, since I tested
> this on 3 different boards with 0x76(when SDO is connected to GND)
>
> And why do I connect SDO to ground everytime ?
> It is because if SDO pin is left floating then I2C address will be
> undefined as said in datasheet + I have observed this while testing.
>
> Actallly, I don't understand what "VDIDO" is, as explained in the
> datasheet.
>
>
> Anyway, if the above compilation issue is not a problem, then
>
> Acked-by: Himanshu Jha <[email protected]>
>
>
> Thanks


2019-01-12 19:16:26

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: iio: chemical: Add bindings for bme680

On Fri, 11 Jan 2019 15:53:59 -0500
Sebastien Bourdelin <[email protected]> wrote:

> BME680 is a pressure/temperature/humidity/voc sensor.
>
> Signed-off-by: Sebastien Bourdelin <[email protected]>

Hmm. We could add the VDD and VDIO regulators perhaps.
Driver assumes they are on currently but we'll get a board where control
is needed sooner or later. I'm not that fussed about this though.

Jonathan

> ---
> .../devicetree/bindings/iio/chemical/bme680.txt | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/chemical/bme680.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/chemical/bme680.txt b/Documentation/devicetree/bindings/iio/chemical/bme680.txt
> new file mode 100644
> index 000000000000..885a1b918340
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/bme680.txt
> @@ -0,0 +1,11 @@
> +Bosch Sensortec BME680 pressure/temperature/humidity/voc sensors
> +
> +Required properties:
> +- compatible: must be "bosch,bme680"
> +
> +Example:
> +
> +bme680@77 {
> + compatible = "bosch,bme680";
> + reg = <0x77>;
> +};


2019-01-14 20:04:03

by Sebastien Bourdelin

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: chemical: bme680: Add device-tree support

Hi,

On 1/12/19 1:22 PM, Jonathan Cameron wrote:
> On Fri, 11 Jan 2019 15:53:58 -0500
> Sebastien Bourdelin <[email protected]> wrote:
>
>> This commit allow the driver to work with device-tree.
>>
>> Signed-off-by: Sebastien Bourdelin <[email protected]>
> Minor stuff inline.
>
> J
>> ---
>> v1 -> v2:
>> - add missing of.h header in bme680_spi.c
>> ---
>> drivers/iio/chemical/bme680_i2c.c | 7 +++++++
>> drivers/iio/chemical/bme680_spi.c | 9 +++++++++
>> 2 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c
>> index 06d4be539d2e..94a36ebdf0b2 100644
>> --- a/drivers/iio/chemical/bme680_i2c.c
>> +++ b/drivers/iio/chemical/bme680_i2c.c
>> @@ -70,10 +70,17 @@ static const struct acpi_device_id bme680_acpi_match[] = {
>> };
>> MODULE_DEVICE_TABLE(acpi, bme680_acpi_match);
>>
>> +static const struct of_device_id bme680_of_i2c_match[] = {
>> + { .compatible = "bosch,bme680", },
>> + {},
>> +}
>> +MODULE_DEVICE_TABLE(of, bme680_of_i2c_match);
>> +
>> static struct i2c_driver bme680_i2c_driver = {
>> .driver = {
>> .name = "bme680_i2c",
>> .acpi_match_table = ACPI_PTR(bme680_acpi_match),
>> + .of_match_table = of_match_ptr(bme680_of_i2c_match),
> As below. just = bme680...
Got it, thanks!
>
>> },
>> .probe = bme680_i2c_probe,
>> .id_table = bme680_i2c_id,
>> diff --git a/drivers/iio/chemical/bme680_spi.c b/drivers/iio/chemical/bme680_spi.c
>> index c9fb05e8d0b9..caa57287a911 100644
>> --- a/drivers/iio/chemical/bme680_spi.c
>> +++ b/drivers/iio/chemical/bme680_spi.c
>> @@ -6,9 +6,11 @@
>> */
>> #include <linux/acpi.h>
>> #include <linux/module.h>
>> +#include <linux/of.h>
>> #include <linux/regmap.h>
>> #include <linux/spi/spi.h>
>>
>> +
> This white space change should not be here.
My bad.
>> #include "bme680.h"
>>
>> static int bme680_regmap_spi_write(void *context, const void *data,
>> @@ -110,10 +112,17 @@ static const struct acpi_device_id bme680_acpi_match[] = {
>> };
>> MODULE_DEVICE_TABLE(acpi, bme680_acpi_match);
>>
>> +static const struct of_device_id bme680_of_spi_match[] = {
>> + { .compatible = "bosch,bme680", },
>> + {},
>> +}
>> +MODULE_DEVICE_TABLE(of, bme680_of_spi_match);
>> +
>> static struct spi_driver bme680_spi_driver = {
>> .driver = {
>> .name = "bme680_spi",
>> .acpi_match_table = ACPI_PTR(bme680_acpi_match),
>> + .of_match_table = of_match_ptr(bme680_of_spi_match),
> Please don't use of_match_ptr. We actually want this entry to be there even
> if devicetree is not in use. This is because there is a magic ACPI hid
> that can use this table even from ACPI.
>
Sure.
>> },
>> .probe = bme680_spi_probe,
>> .id_table = bme680_spi_id,

2019-01-14 20:04:34

by Sebastien Bourdelin

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: chemical: bme680: Add device-tree support

Hi,

On 1/12/19 4:42 AM, Himanshu Jha wrote:
> On Fri, Jan 11, 2019 at 03:53:58PM -0500, Sebastien Bourdelin wrote:
>> This commit allow the driver to work with device-tree.
>>
>> Signed-off-by: Sebastien Bourdelin <[email protected]>
>> ---
> I get the following compilation failure:
>
> Below I have `allyesconfig` except 'BME680' configure as [M]
> in case you wish to reproduce.
>
> himanshu@himanshu-Vostro-3559:~/linux-next$ grep -i -w 'CONFIG_BME680\|CONFIG_ACPI\|CONFIG_OF' .config
> CONFIG_ACPI=y
> CONFIG_OF=y
> CONFIG_BME680=m
> himanshu@himanshu-Vostro-3559:~/linux-next$ sudo make drivers/iio/chemical/bme680_spi.o
> make[1]: Nothing to be done for 'all'.
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CC [M] drivers/iio/chemical/bme680_spi.o
> In file included from ./include/linux/acpi.h:41:0,
> from drivers/iio/chemical/bme680_spi.c:7:
> ./include/linux/module.h:213:1: error: expected ‘,’ or ‘;’ before ‘extern’
> extern typeof(name) __mod_##type##__##name##_device_table \
> ^
> drivers/iio/chemical/bme680_spi.c:119:1: note: in expansion of macro ‘MODULE_DEVICE_TABLE’
> MODULE_DEVICE_TABLE(of, bme680_of_spi_match);
> ^~~~~~~~~~~~~~~~~~~
> scripts/Makefile.build:291: recipe for target 'drivers/iio/chemical/bme680_spi.o' failed
> make[1]: *** [drivers/iio/chemical/bme680_spi.o] Error 1
> Makefile:1741: recipe for target 'drivers/iio/chemical/bme680_spi.o' failed
> make: *** [drivers/iio/chemical/bme680_spi.o] Error 2
Thanks for the test, this is bad, i will fix that!
> BUT if:
>
> himanshu@himanshu-Vostro-3559:~/linux-next$ make allyesconfig
> scripts/kconfig/conf --allyesconfig Kconfig
> #
> # configuration written to .config
> #
>
> himanshu@himanshu-Vostro-3559:~/linux-next$ sudo make drivers/iio/chemical/bme680_spi.o
> scripts/kconfig/conf --syncconfig Kconfig
> make[1]: Nothing to be done for 'all'.
> HOSTCC scripts/dtc/dtc.o
> HOSTCC scripts/dtc/flattree.o
> HOSTCC scripts/dtc/fstree.o
> HOSTCC scripts/dtc/data.o
> HOSTCC scripts/dtc/livetree.o
> HOSTCC scripts/dtc/treesource.o
> HOSTCC scripts/dtc/srcpos.o
> HOSTCC scripts/dtc/checks.o
> HOSTCC scripts/dtc/util.o
> LEX scripts/dtc/dtc-lexer.lex.c
> YACC scripts/dtc/dtc-parser.tab.h
> HOSTCC scripts/dtc/dtc-lexer.lex.o
> YACC scripts/dtc/dtc-parser.tab.c
> HOSTCC scripts/dtc/dtc-parser.tab.o
> HOSTLD scripts/dtc/dtc
> CC scripts/mod/empty.o
> MKELF scripts/mod/elfconfig.h
> HOSTCC scripts/mod/modpost.o
> CC scripts/mod/devicetable-offsets.s
> HOSTCC scripts/mod/file2alias.o
> HOSTCC scripts/mod/sumversion.o
> HOSTLD scripts/mod/modpost
> CC kernel/bounds.s
> CC arch/x86/kernel/asm-offsets.s
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CC drivers/iio/chemical/bme680_spi.o
>
> Compiles without any issues.
Hum, weird it compiles actually :s
> Also, wondering when is 0x77 i2c address used, since I tested
> this on 3 different boards with 0x76(when SDO is connected to GND)
>
> And why do I connect SDO to ground everytime ?
> It is because if SDO pin is left floating then I2C address will be
> undefined as said in datasheet + I have observed this while testing.

Ah, thanks for the reminder, actually i have only tested with a floating

SDO, but i will probably use the GND instead even if i didn't had any

problem right now.

> Actallly, I don't understand what "VDIDO" is, as explained in the
> datasheet.
>
>
> Anyway, if the above compilation issue is not a problem, then
>
> Acked-by: Himanshu Jha <[email protected]>
>
>
> Thanks

2019-01-14 20:19:25

by Sebastien Bourdelin

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: iio: chemical: Add bindings for bme680


On 1/12/19 1:28 PM, Jonathan Cameron wrote:
> On Fri, 11 Jan 2019 15:53:59 -0500
> Sebastien Bourdelin <[email protected]> wrote:
>
>> BME680 is a pressure/temperature/humidity/voc sensor.
>>
>> Signed-off-by: Sebastien Bourdelin <[email protected]>
> Hmm. We could add the VDD and VDIO regulators perhaps.
> Driver assumes they are on currently but we'll get a board where control
> is needed sooner or later. I'm not that fussed about this though.

If that's fine with you, i prefer to leave it as it is right now as i
don't feel

confident enough to correctly explain it in the Documentation.

But if you have more input, you are more then welcome!

> Jonathan
>
>> ---
>> .../devicetree/bindings/iio/chemical/bme680.txt | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/chemical/bme680.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/chemical/bme680.txt b/Documentation/devicetree/bindings/iio/chemical/bme680.txt
>> new file mode 100644
>> index 000000000000..885a1b918340
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/chemical/bme680.txt
>> @@ -0,0 +1,11 @@
>> +Bosch Sensortec BME680 pressure/temperature/humidity/voc sensors
>> +
>> +Required properties:
>> +- compatible: must be "bosch,bme680"
>> +
>> +Example:
>> +
>> +bme680@77 {
>> + compatible = "bosch,bme680";
>> + reg = <0x77>;
>> +};

2019-01-16 09:33:20

by Himanshu Jha

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: chemical: bme680: Add device-tree support

Hi Sebastien,

On Mon, Jan 14, 2019 at 03:00:41PM -0500, sebastien bourdelin wrote:
> Hi,
>
> On 1/12/19 4:42 AM, Himanshu Jha wrote:
> > On Fri, Jan 11, 2019 at 03:53:58PM -0500, Sebastien Bourdelin wrote:
> > > This commit allow the driver to work with device-tree.
> > >
> > > Signed-off-by: Sebastien Bourdelin <[email protected]>
> > > ---
> > I get the following compilation failure:
> >
> > Below I have `allyesconfig` except 'BME680' configure as [M]
> > in case you wish to reproduce.
> >
> > himanshu@himanshu-Vostro-3559:~/linux-next$ grep -i -w 'CONFIG_BME680\|CONFIG_ACPI\|CONFIG_OF' .config
> > CONFIG_ACPI=y
> > CONFIG_OF=y
> > CONFIG_BME680=m
> > himanshu@himanshu-Vostro-3559:~/linux-next$ sudo make drivers/iio/chemical/bme680_spi.o
> > make[1]: Nothing to be done for 'all'.
> > CALL scripts/checksyscalls.sh
> > DESCEND objtool
> > CC [M] drivers/iio/chemical/bme680_spi.o
> > In file included from ./include/linux/acpi.h:41:0,
> > from drivers/iio/chemical/bme680_spi.c:7:
> > ./include/linux/module.h:213:1: error: expected ‘,’ or ‘;’ before ‘extern’
> > extern typeof(name) __mod_##type##__##name##_device_table \
> > ^
> > drivers/iio/chemical/bme680_spi.c:119:1: note: in expansion of macro ‘MODULE_DEVICE_TABLE’
> > MODULE_DEVICE_TABLE(of, bme680_of_spi_match);
> > ^~~~~~~~~~~~~~~~~~~
> > scripts/Makefile.build:291: recipe for target 'drivers/iio/chemical/bme680_spi.o' failed
> > make[1]: *** [drivers/iio/chemical/bme680_spi.o] Error 1
> > Makefile:1741: recipe for target 'drivers/iio/chemical/bme680_spi.o' failed
> > make: *** [drivers/iio/chemical/bme680_spi.o] Error 2
> Thanks for the test, this is bad, i will fix that!
> > BUT if:
> >
> > himanshu@himanshu-Vostro-3559:~/linux-next$ make allyesconfig
> > scripts/kconfig/conf --allyesconfig Kconfig
> > #
> > # configuration written to .config
> > #
> >
> > himanshu@himanshu-Vostro-3559:~/linux-next$ sudo make drivers/iio/chemical/bme680_spi.o
> > scripts/kconfig/conf --syncconfig Kconfig
> > make[1]: Nothing to be done for 'all'.
> > HOSTCC scripts/dtc/dtc.o
> > HOSTCC scripts/dtc/flattree.o
> > HOSTCC scripts/dtc/fstree.o
> > HOSTCC scripts/dtc/data.o
> > HOSTCC scripts/dtc/livetree.o
> > HOSTCC scripts/dtc/treesource.o
> > HOSTCC scripts/dtc/srcpos.o
> > HOSTCC scripts/dtc/checks.o
> > HOSTCC scripts/dtc/util.o
> > LEX scripts/dtc/dtc-lexer.lex.c
> > YACC scripts/dtc/dtc-parser.tab.h
> > HOSTCC scripts/dtc/dtc-lexer.lex.o
> > YACC scripts/dtc/dtc-parser.tab.c
> > HOSTCC scripts/dtc/dtc-parser.tab.o
> > HOSTLD scripts/dtc/dtc
> > CC scripts/mod/empty.o
> > MKELF scripts/mod/elfconfig.h
> > HOSTCC scripts/mod/modpost.o
> > CC scripts/mod/devicetable-offsets.s
> > HOSTCC scripts/mod/file2alias.o
> > HOSTCC scripts/mod/sumversion.o
> > HOSTLD scripts/mod/modpost
> > CC kernel/bounds.s
> > CC arch/x86/kernel/asm-offsets.s
> > CALL scripts/checksyscalls.sh
> > DESCEND objtool
> > CC drivers/iio/chemical/bme680_spi.o
> >
> > Compiles without any issues.
> Hum, weird it compiles actually :s

I think this behavior is observed due to:

include/linux/module.h +212

#ifdef MODULE
/* Creates an alias so file2alias.c can find device table. */
#define MODULE_DEVICE_TABLE(type, name) \
extern typeof(name) __mod_##type##__##name##_device_table \
__attribute__ ((unused, alias(__stringify(name))))
#else /* !MODULE */
#define MODULE_DEVICE_TABLE(type, name)
#endif

So, when we build the driver as a module[M] then macro expansion
takes place giving us the compiler warning.

OTOH, if the driver is built as builtin[*] then marco expands
to nothing or simply goes away. And `;' completes the struct
declaration while silencing the warning.

static const struct of_device_id bme680_of_spi_match[] = {
{ .compatible = "bosch,bme680", },
{},
}
MODULE_DEVICE_TABLE(of, bme680_of_spi_match);

converts to:

static const struct of_device_id bme680_of_spi_match[] = {
{ .compatible = "bosch,bme680", },
{},
}
;
^^^

Amazing!
Correct me if I'm wrong somewhere, took me 2 hours to figure
that out :D

Also, I some additional interesting observations:

When buitin[*] -> no symbol tables in the RO segment of object file

himanshu@himanshu-Vostro-3559:~/linux-next$ nm drivers/iio/chemical/bme680_spi.o
0000000000000000 d __addressable_bme680_spi_driver_init130
00000000000001a0 r bme680_acpi_match
U bme680_core_probe
0000000000000000 r bme680_of_spi_match
00000000000000a0 d bme680_regmap_bus
U bme680_regmap_config
0000000000000000 t bme680_regmap_spi_read
0000000000000010 t bme680_regmap_spi_write
0000000000000000 d bme680_spi_driver
0000000000000000 t bme680_spi_driver_exit
0000000000000000 t bme680_spi_driver_init
00000000000001e0 r bme680_spi_id
0000000000000070 t bme680_spi_probe
U _dev_err
U __devm_regmap_init
U driver_unregister
0000000000000000 d __exitcall_bme680_spi_driver_exit
0000000000000000 t __initcall_bme680_spi_driver_init6
U regmap_read
U regmap_update_bits_base
U regmap_write
U spi_get_device_id
U __spi_register_driver
U spi_setup
U spi_write_then_read
U __stack_chk_fail


While when [M] -> we can see the symbol tables in the RO segment

himanshu@himanshu-Vostro-3559:~/linux-next$ nm drivers/iio/chemical/bme680_spi.o
00000000000001a0 r bme680_acpi_match
U bme680_core_probe
0000000000000000 r bme680_of_spi_match
00000000000000a0 d bme680_regmap_bus
U bme680_regmap_config
0000000000000000 t bme680_regmap_spi_read
0000000000000010 t bme680_regmap_spi_write
0000000000000000 d bme680_spi_driver
0000000000000000 t bme680_spi_driver_exit
0000000000000000 t bme680_spi_driver_init
00000000000001e0 r bme680_spi_id
0000000000000070 t bme680_spi_probe
0000000000000000 T cleanup_module
U _dev_err
U __devm_regmap_init
U driver_unregister
0000000000000000 T init_module
00000000000001a0 R __mod_acpi__bme680_acpi_match_device_table <---
0000000000000000 R __mod_of__bme680_of_spi_match_device_table <---
00000000000001e0 R __mod_spi__bme680_spi_id_device_table <---
U regmap_read
U regmap_update_bits_base
U regmap_write
U spi_get_device_id
U __spi_register_driver
U spi_setup
U spi_write_then_read
U __stack_chk_fail
U __this_module
0000000000000033 r __UNIQUE_ID_author38
000000000000000f r __UNIQUE_ID_description39
0000000000000000 r __UNIQUE_ID_license40


Thanks!
--
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

2019-01-17 00:45:17

by Sebastien Bourdelin

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: chemical: bme680: Add device-tree support

Hi Himanshu,

On 1/15/19 1:41 PM, Himanshu Jha wrote:
> ...
> himanshu@himanshu-Vostro-3559:~/linux-next$ sudo make drivers/iio/chemical/bme680_spi.o
> scripts/kconfig/conf --syncconfig Kconfig
> make[1]: Nothing to be done for 'all'.
> HOSTCC scripts/dtc/dtc.o
> HOSTCC scripts/dtc/flattree.o
> HOSTCC scripts/dtc/fstree.o
> HOSTCC scripts/dtc/data.o
> HOSTCC scripts/dtc/livetree.o
> HOSTCC scripts/dtc/treesource.o
> HOSTCC scripts/dtc/srcpos.o
> HOSTCC scripts/dtc/checks.o
> HOSTCC scripts/dtc/util.o
> LEX scripts/dtc/dtc-lexer.lex.c
> YACC scripts/dtc/dtc-parser.tab.h
> HOSTCC scripts/dtc/dtc-lexer.lex.o
> YACC scripts/dtc/dtc-parser.tab.c
> HOSTCC scripts/dtc/dtc-parser.tab.o
> HOSTLD scripts/dtc/dtc
> CC scripts/mod/empty.o
> MKELF scripts/mod/elfconfig.h
> HOSTCC scripts/mod/modpost.o
> CC scripts/mod/devicetable-offsets.s
> HOSTCC scripts/mod/file2alias.o
> HOSTCC scripts/mod/sumversion.o
> HOSTLD scripts/mod/modpost
> CC kernel/bounds.s
> CC arch/x86/kernel/asm-offsets.s
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CC drivers/iio/chemical/bme680_spi.o
>
> Compiles without any issues.
>> Hum, weird it compiles actually :s
> I think this behavior is observed due to:
>
> include/linux/module.h +212
>
> #ifdef MODULE
> /* Creates an alias so file2alias.c can find device table. */
> #define MODULE_DEVICE_TABLE(type, name) \
> extern typeof(name) __mod_##type##__##name##_device_table \
> __attribute__ ((unused, alias(__stringify(name))))
> #else /* !MODULE */
> #define MODULE_DEVICE_TABLE(type, name)
> #endif
>
> So, when we build the driver as a module[M] then macro expansion
> takes place giving us the compiler warning.
>
> OTOH, if the driver is built as builtin[*] then marco expands
> to nothing or simply goes away. And `;' completes the struct
> declaration while silencing the warning.
>
> static const struct of_device_id bme680_of_spi_match[] = {
> { .compatible = "bosch,bme680", },
> {},
> }
> MODULE_DEVICE_TABLE(of, bme680_of_spi_match);
>
> converts to:
>
> static const struct of_device_id bme680_of_spi_match[] = {
> { .compatible = "bosch,bme680", },
> {},
> }
> ;
> ^^^
>
> Amazing!
> Correct me if I'm wrong somewhere, took me 2 hours to figure
> that out :D

Ahah, nice!

Thanks a lot for the explanation!

> Also, I some additional interesting observations:
>
> When buitin[*] -> no symbol tables in the RO segment of object file
>
> himanshu@himanshu-Vostro-3559:~/linux-next$ nm drivers/iio/chemical/bme680_spi.o
> 0000000000000000 d __addressable_bme680_spi_driver_init130
> 00000000000001a0 r bme680_acpi_match
> U bme680_core_probe
> 0000000000000000 r bme680_of_spi_match
> 00000000000000a0 d bme680_regmap_bus
> U bme680_regmap_config
> 0000000000000000 t bme680_regmap_spi_read
> 0000000000000010 t bme680_regmap_spi_write
> 0000000000000000 d bme680_spi_driver
> 0000000000000000 t bme680_spi_driver_exit
> 0000000000000000 t bme680_spi_driver_init
> 00000000000001e0 r bme680_spi_id
> 0000000000000070 t bme680_spi_probe
> U _dev_err
> U __devm_regmap_init
> U driver_unregister
> 0000000000000000 d __exitcall_bme680_spi_driver_exit
> 0000000000000000 t __initcall_bme680_spi_driver_init6
> U regmap_read
> U regmap_update_bits_base
> U regmap_write
> U spi_get_device_id
> U __spi_register_driver
> U spi_setup
> U spi_write_then_read
> U __stack_chk_fail
>
>
> While when [M] -> we can see the symbol tables in the RO segment
>
> himanshu@himanshu-Vostro-3559:~/linux-next$ nm drivers/iio/chemical/bme680_spi.o
> 00000000000001a0 r bme680_acpi_match
> U bme680_core_probe
> 0000000000000000 r bme680_of_spi_match
> 00000000000000a0 d bme680_regmap_bus
> U bme680_regmap_config
> 0000000000000000 t bme680_regmap_spi_read
> 0000000000000010 t bme680_regmap_spi_write
> 0000000000000000 d bme680_spi_driver
> 0000000000000000 t bme680_spi_driver_exit
> 0000000000000000 t bme680_spi_driver_init
> 00000000000001e0 r bme680_spi_id
> 0000000000000070 t bme680_spi_probe
> 0000000000000000 T cleanup_module
> U _dev_err
> U __devm_regmap_init
> U driver_unregister
> 0000000000000000 T init_module
> 00000000000001a0 R __mod_acpi__bme680_acpi_match_device_table <---
> 0000000000000000 R __mod_of__bme680_of_spi_match_device_table <---
> 00000000000001e0 R __mod_spi__bme680_spi_id_device_table <---
> U regmap_read
> U regmap_update_bits_base
> U regmap_write
> U spi_get_device_id
> U __spi_register_driver
> U spi_setup
> U spi_write_then_read
> U __stack_chk_fail
> U __this_module
> 0000000000000033 r __UNIQUE_ID_author38
> 000000000000000f r __UNIQUE_ID_description39
> 0000000000000000 r __UNIQUE_ID_license40
>
>
> Thanks!
Thanks to you :)

2019-01-19 16:59:40

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: iio: chemical: Add bindings for bme680

On Mon, 14 Jan 2019 15:17:10 -0500
sebastien bourdelin <[email protected]> wrote:

> On 1/12/19 1:28 PM, Jonathan Cameron wrote:
> > On Fri, 11 Jan 2019 15:53:59 -0500
> > Sebastien Bourdelin <[email protected]> wrote:
> >
> >> BME680 is a pressure/temperature/humidity/voc sensor.
> >>
> >> Signed-off-by: Sebastien Bourdelin <[email protected]>
> > Hmm. We could add the VDD and VDIO regulators perhaps.
> > Driver assumes they are on currently but we'll get a board where control
> > is needed sooner or later. I'm not that fussed about this though.
>
> If that's fine with you, i prefer to leave it as it is right now as i
> don't feel
>
> confident enough to correctly explain it in the Documentation.
>
> But if you have more input, you are more then welcome!
Given we aren't planning to do more than turn them on and that'll work
fine with stub regulators (so they can effectively be optional for ever),
there is no particular need to put them in now if you prefer not to.

Jonathan

>
> > Jonathan
> >
> >> ---
> >> .../devicetree/bindings/iio/chemical/bme680.txt | 11 +++++++++++
> >> 1 file changed, 11 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/iio/chemical/bme680.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/chemical/bme680.txt b/Documentation/devicetree/bindings/iio/chemical/bme680.txt
> >> new file mode 100644
> >> index 000000000000..885a1b918340
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/iio/chemical/bme680.txt
> >> @@ -0,0 +1,11 @@
> >> +Bosch Sensortec BME680 pressure/temperature/humidity/voc sensors
> >> +
> >> +Required properties:
> >> +- compatible: must be "bosch,bme680"
> >> +
> >> +Example:
> >> +
> >> +bme680@77 {
> >> + compatible = "bosch,bme680";
> >> + reg = <0x77>;
> >> +};


2019-01-21 21:47:06

by Sebastien Bourdelin

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: iio: chemical: Add bindings for bme680


On 1/19/19 11:57 AM, Jonathan Cameron wrote:
> On Mon, 14 Jan 2019 15:17:10 -0500
> sebastien bourdelin <[email protected]> wrote:
>
>> On 1/12/19 1:28 PM, Jonathan Cameron wrote:
>>> On Fri, 11 Jan 2019 15:53:59 -0500
>>> Sebastien Bourdelin <[email protected]> wrote:
>>>
>>>> BME680 is a pressure/temperature/humidity/voc sensor.
>>>>
>>>> Signed-off-by: Sebastien Bourdelin <[email protected]>
>>> Hmm. We could add the VDD and VDIO regulators perhaps.
>>> Driver assumes they are on currently but we'll get a board where control
>>> is needed sooner or later. I'm not that fussed about this though.
>> If that's fine with you, i prefer to leave it as it is right now as i
>> don't feel
>>
>> confident enough to correctly explain it in the Documentation.
>>
>> But if you have more input, you are more then welcome!
> Given we aren't planning to do more than turn them on and that'll work
> fine with stub regulators (so they can effectively be optional for ever),
> there is no particular need to put them in now if you prefer not to.
>
> Jonathan
Ok Thanks Jonathan!
>>> Jonathan
>>>
>>>> ---
>>>> .../devicetree/bindings/iio/chemical/bme680.txt | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/iio/chemical/bme680.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/chemical/bme680.txt b/Documentation/devicetree/bindings/iio/chemical/bme680.txt
>>>> new file mode 100644
>>>> index 000000000000..885a1b918340
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/chemical/bme680.txt
>>>> @@ -0,0 +1,11 @@
>>>> +Bosch Sensortec BME680 pressure/temperature/humidity/voc sensors
>>>> +
>>>> +Required properties:
>>>> +- compatible: must be "bosch,bme680"
>>>> +
>>>> +Example:
>>>> +
>>>> +bme680@77 {
>>>> + compatible = "bosch,bme680";
>>>> + reg = <0x77>;
>>>> +};