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

[Himanshu Jha]

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 <sebastien.bourdelin@xxxxxxxxx>
> > > ---
> > 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