Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NPCX Tachometer driver compiled despite status = "disabled" #48198

Closed
aaronemassey opened this issue Jul 22, 2022 · 1 comment · Fixed by #48205
Closed

NPCX Tachometer driver compiled despite status = "disabled" #48198

aaronemassey opened this issue Jul 22, 2022 · 1 comment · Fixed by #48205
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug

Comments

@aaronemassey
Copy link
Member

Describe the bug

With #48048 changes, we noticed
our builds were failing on board overlays using npcx without a tachometer driver
due to the following warning:

/zephyr/drivers/sensor/nuvoton_tach_npcx/tach_nuvoton_npcx.c:359:39: warning: 'tach_npcx_driver_api' defined but not used [-Wunused-const-variable=]
  359 | static const struct sensor_driver_api tach_npcx_driver_api = {
      |                                       ^~~~~~~~~~~~~~~~~~~~
/zephyr/drivers/sensor/nuvoton_tach_npcx/tach_nuvoton_npcx.c:310:12: warning: 'tach_npcx_init' defined but not used [-Wunused-function]
  310 | static int tach_npcx_init(const struct device *dev)
      |            ^~~~~~~~~~~~~~

This can be reproduced in upstream Zephyr using the npcx9m6f_evb board.

To Reproduce
Steps to reproduce the behavior:

  1. In boards/arm/npcx9m6f_evb/npcx9m6f_evb.dts disable the &tach1 node.
  2. Run west build -b npcx9m6f_evb samples/sensor/adc_cmp_npcx -d build-npcx --pristine.
  3. See aforementioned warnings.

Expected behavior

No warnings. My understanding is that
#48048 should be preventing the
tachometer driver from being compiled due to the status being "disabled".

Impact

This is likely only a simple example of a greater problem may significantly
impact other boards.

Logs and console output

Log-tail of West build.

/zephyr/drivers/sensor/nuvoton_tach_npcx/tach_nuvoton_npcx.c:310:12: warning: 'tach_npcx_init' defined but not used [-Wunused-function]
  310 | static int tach_npcx_init(const struct device *dev)
      |            ^~~~~~~~~~~~~~
[139/161] Linking C static library zephyr/libzephyr.a
[140/161] Linking C static library zephyr/lib/libc/minimal/liblib__libc__minimal.a
[141/161] Linking C static library zephyr/arch/arch/arm/core/aarch32/mpu/libarch__arm__core__aarch32__mpu.a
[142/161] Building C object zephyr/drivers/gpio/CMakeFiles/drivers__gpio.dir/gpio_npcx.c.obj
[143/161] Linking C static library zephyr/drivers/timer/libdrivers__timer.a
[144/161] Linking C static library zephyr/drivers/sensor/nuvoton_tach_npcx/libdrivers__sensor__nuvoton_tach_npcx.a
[145/161] Building C object zephyr/arch/arch/arm/core/aarch32/cortex_m/CMakeFiles/arch__arm__core__aarch32__cortex_m.dir/fault.c.obj
[146/161] Linking C static library zephyr/kernel/libkernel.a
[147/161] Building C object zephyr/drivers/adc/CMakeFiles/drivers__adc.dir/adc_npcx.c.obj
[148/161] Linking C static library zephyr/drivers/gpio/libdrivers__gpio.a
[149/161] Linking C static library zephyr/arch/arch/arm/core/aarch32/cortex_m/libarch__arm__core__aarch32__cortex_m.a
[150/161] Linking C static library zephyr/drivers/adc/libdrivers__adc.a
[151/161] Linking C executable zephyr/zephyr_pre0.elf

[152/161] Generating dev_handles.c
[153/161] Building C object zephyr/CMakeFiles/zephyr_pre1.dir/misc/empty_file.c.obj
[154/161] Building C object zephyr/CMakeFiles/zephyr_pre1.dir/dev_handles.c.obj
[155/161] Linking C executable zephyr/zephyr_pre1.elf

[156/161] Generating linker.cmd
[157/161] Generating isr_tables.c, isrList.bin
[158/161] Building C object zephyr/CMakeFiles/zephyr_final.dir/misc/empty_file.c.obj
[159/161] Building C object zephyr/CMakeFiles/zephyr_final.dir/isr_tables.c.obj
[160/161] Building C object zephyr/CMakeFiles/zephyr_final.dir/dev_handles.c.obj
[161/161] Linking C executable zephyr/zephyr.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:       19944 B       192 KB     10.14%
            SRAM:        5432 B        64 KB      8.29%
        IDT_LIST:          0 GB         2 KB      0.00%

******************************
***        SUCCESS         ***
******************************

Additional context

Assigning to @galak for comment as he likely has significant context to suggest
next steps.

Note:

The warning goes away if we remove the config TACH_NPCX entry
zephyr/soc/arm/nuvoton_npcx/npcx9/Kconfig.defconfig.series:

config I2C_NPCX
	default y
	depends on I2C

- config TACH_NPCX
- 	default y
- 	depends on SENSOR
-
config SPI_NPCX_FIU
	default y
	depends on SPI

This may suggest an fault in the order or method fo Kconfig processing or maybe
these Kconfig.defconfig.series files should just be removed?

@aaronemassey aaronemassey added the bug The issue is a bug, or the PR is fixing a bug label Jul 22, 2022
@aaronemassey
Copy link
Member Author

@keith-zephyr ^

galak added a commit to galak/zephyr that referenced this issue Jul 23, 2022
Now that sensor drivers are enabled based on devicetree
we need to remove any cases of them getting enabled by
Kconfig.defconfig* files as this can lead to errors.

Typically the Kconfig.defconfig* will blindly enable a
sensor and not respect the devicetree state of the sensor.
Additionally we can get problems with prj.conf/defconfig
as well getting incorrectly overridden.

Fixes zephyrproject-rtos#48198

Signed-off-by: Kumar Gala <galak@kernel.org>
carlescufi pushed a commit that referenced this issue Jul 25, 2022
Now that sensor drivers are enabled based on devicetree
we need to remove any cases of them getting enabled by
Kconfig.defconfig* files as this can lead to errors.

Typically the Kconfig.defconfig* will blindly enable a
sensor and not respect the devicetree state of the sensor.
Additionally we can get problems with prj.conf/defconfig
as well getting incorrectly overridden.

Fixes #48198

Signed-off-by: Kumar Gala <galak@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants