search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.77k stars 6.57k forks source link

ADC_DT_SPEC_GET not working for channels >= 10 #47119

Closed aurel32 closed 2 years ago

aurel32 commented 2 years ago

Describe the bug

I am trying to define a channel of an ADC that way in the device tree:

    channel@f {
            reg = <15>;
            zephyr,gain = "ADC_GAIN_1";
            zephyr,reference = "ADC_REF_INTERNAL";
            zephyr,acquisition-time = <ADC_ACQ_TIME_MAX>;
            zephyr,resolution = <12>;
    }

Then I use the ADC_DT_SPEC_GET macro to populate and a struct adc_dt_spec. Passing this structure to adc_channel_setup_dt(), I get error ENOTSUP.

I have tracked down that to the fact that ADC_DT_SPEC_STRUCT uses UTIL_CAT(channel_, input) while input is defined in the device tree as an hexadecimal value.

A workaround is to use a decimal value channel@15 in the device tree, however this cause the following warning at build time: unit address and first address in 'reg' (0xf) don't match for /soc/adc@40022000/channel@15

To Reproduce Steps to reproduce the behavior:

  1. mkdir build; cd build
  2. cmake -DBOARD=nucleo_h7a3zi_q zephyr/samples/drivers/adc
  3. make
  4. See error
*** Booting Zephyr OS build zephyr-v3.1.0-1038-g7fe91f0e87c2  ***
Could not setup channel #0 (-134)

Expected behavior

*** Booting Zephyr OS build zephyr-v3.1.0-1038-g7fe91f0e87c2  ***
ADC reading:
- ADC_1, channel 15: 592 = 476 mV

Environment (please complete the following information):

rjmaris commented 5 months ago

I still encounter this sort of warnings, with v3.4.0 as well as with v3.6.0. So I was a bit wondering that the problem has been solved (a few days ago, I searched for whether this warning was already covered). Today I recognized that the warnings are output to the stderr stream (Linux workstation). When I say west build [...] 2> err.out, then those warnings do not appear, except in the file err.out.

@fabiobaltieri: a reason to reopen this issue?

aurel32 commented 5 months ago

I still encounter this sort of warnings, with v3.4.0 as well as with v3.6.0. So I was a bit wondering that the problem has been solved (a few days ago, I searched for whether this warning was already covered). Today I recognized that the warnings are output to the stderr stream (Linux workstation). When I say west build [...] 2> err.out, then those warnings do not appear, except in the file err.out.

@rjmaris The initial issue is definitely fixed. What warning do you have exactly?

rjmaris commented 5 months ago

In order to allow quick verification by anybody, I have done two builds on boards that define channels >=10 in their overlay files in samples/drivers/adc/adc_dt boards that includes >=10. Note that the warning also occurs before the split of adc samples into the current standard samples and the sequenced samples.

$ west build -p -b saml21_xpro samples/drivers/adc/adc_dt/        
-- west build: making build dir /home/rob/zephyrproject/zephyr/build pristine
-- west build: generating a build system                                                           
Loading Zephyr default modules (Zephyr base).
-- Application: /home/rob/zephyrproject/zephyr/samples/drivers/adc/adc_dt
-- CMake version: 3.25.1
-- Found Python3: /usr/bin/python3 (found suitable version "3.9.2", minimum required is "3.8") found components: Interpreter 
-- Cache files will be written to: /home/rob/.cache/zephyr
-- Zephyr version: 3.6.99 (/home/rob/zephyrproject/zephyr)
-- Found west (found suitable version "1.1.0", minimum required is "0.14.0")
-- Board: saml21_xpro, qualifiers: saml21j18b
-- ZEPHYR_TOOLCHAIN_VARIANT not set, trying to locate Zephyr SDK
-- Found host-tools: zephyr 0.16.3 (/opt/zephyr-sdk-0.16.3)
-- Found toolchain: zephyr 0.16.3 (/opt/zephyr-sdk-0.16.3)
-- Found Dtc: /opt/zephyr-sdk-0.16.3/sysroots/x86_64-pokysdk-linux/usr/bin/dtc (found suitable version "1.6.0", minimum required is "1.4.6") 
-- Found BOARD.dts: /home/rob/zephyrproject/zephyr/boards/atmel/sam0/saml21_xpro/saml21_xpro.dts
-- Found devicetree overlay: /home/rob/zephyrproject/zephyr/samples/drivers/adc/adc_dt/boards/saml21_xpro.overlay
#### here: ####
unit address and first address in 'reg' (0xd) don't match for /soc/adc@43000c00/channel@13
-- Generated zephyr.dts: /home/rob/zephyrproject/zephyr/build/zephyr/zephyr.dts
-- Generated devicetree_generated.h: /home/rob/zephyrproject/zephyr/build/zephyr/include/generated/devicetree_generated.h
-- Including generated dts.cmake file: /home/rob/zephyrproject/zephyr/build/zephyr/dts.cmake
/home/rob/zephyrproject/zephyr/build/zephyr/zephyr.dts:54.19-59.5: Warning (unique_unit_address_if_enabled): /soc/pm@40000400: duplicate unit-address (also used in node /soc/mclk@40000400)
Parsing /home/rob/zephyrproject/zephyr/Kconfig
Loaded configuration '/home/rob/zephyrproject/zephyr/boards/atmel/sam0/saml21_xpro/saml21_xpro_defconfig'
Merged configuration '/home/rob/zephyrproject/zephyr/samples/drivers/adc/adc_dt/prj.conf'
Configuration saved to '/home/rob/zephyrproject/zephyr/build/zephyr/.config'
Kconfig header saved to '/home/rob/zephyrproject/zephyr/build/zephyr/include/generated/autoconf.h'
-- Found GnuLd: /opt/zephyr-sdk-0.16.3/arm-zephyr-eabi/arm-zephyr-eabi/bin/ld.bfd (found version "2.38") 
-- The C compiler identification is GNU 12.2.0
-- The CXX compiler identification is GNU 12.2.0
-- The ASM compiler identification is GNU
-- Found assembler: /opt/zephyr-sdk-0.16.3/arm-zephyr-eabi/bin/arm-zephyr-eabi-gcc
-- Using ccache: /usr/bin/ccache
-- Configuring done
-- Generating done
-- Build files have been written to: /home/rob/zephyrproject/zephyr/build

In this case, the build goes without a warning, but when I replace the channel@f by channel@15 at line 29 of sam4s_xplained.overlay, the warning is output:

$ west build -b sam4s_xplained samples/drivers/adc/adc_dt/
[0/1] Re-running CMake...
Loading Zephyr default modules (Zephyr base (cached)).
-- Application: /home/rob/zephyrproject/zephyr/samples/drivers/adc/adc_dt
-- CMake version: 3.25.1
-- Cache files will be written to: /home/rob/.cache/zephyr
-- Zephyr version: 3.6.99 (/home/rob/zephyrproject/zephyr)
-- Found west (found suitable version "1.1.0", minimum required is "0.14.0")
-- Board: sam4s_xplained, qualifiers: sam4s16c
-- Found host-tools: zephyr 0.16.3 (/opt/zephyr-sdk-0.16.3)
-- Found toolchain: zephyr 0.16.3 (/opt/zephyr-sdk-0.16.3)
-- Found BOARD.dts: /home/rob/zephyrproject/zephyr/boards/atmel/sam/sam4s_xplained/sam4s_xplained.dts
-- Found devicetree overlay: /home/rob/zephyrproject/zephyr/samples/drivers/adc/adc_dt/boards/sam4s_xplained.overlay
#### here: ####
unit address and first address in 'reg' (0xf) don't match for /soc/adc@40038000/channel@15
-- Generated zephyr.dts: /home/rob/zephyrproject/zephyr/build/zephyr/zephyr.dts
-- Generated devicetree_generated.h: /home/rob/zephyrproject/zephyr/build/zephyr/include/generated/devicetree_generated.h
-- Including generated dts.cmake file: /home/rob/zephyrproject/zephyr/build/zephyr/dts.cmake
Parsing /home/rob/zephyrproject/zephyr/Kconfig
Loaded configuration '/home/rob/zephyrproject/zephyr/build/zephyr/.config'
No change to configuration in '/home/rob/zephyrproject/zephyr/build/zephyr/.config'
No change to Kconfig header in '/home/rob/zephyrproject/zephyr/build/zephyr/include/generated/autoconf.h'
-- Using ccache: /usr/bin/ccache
-- Configuring done
-- Generating done
-- Build files have been written to: /home/rob/zephyrproject/zephyr/build

(edit, added) The latter sample shows that channel@f is just hex for 15, and must in this case (still) be so to prevent the warning - according to the first post in this issue.

rjmaris commented 5 months ago

I see that the warning was not the principal motivation of this issue. That what's discussed by me is related to scripts/dts/python-devicetree/src/devicetree/edtlib.py, where this comparison makes the trouble:

node.regs[0].addr != node.unit_addr

Changing this would require the start of a new issue.

node.unit_addr is the xx in channel@xx, and is always interpreted as hex and stored as decimal in node.unit_addr. Therefore, the comparison always fails because reg 15 is compared to 21 when I write channel@15. Fact is: many overlay files in demos say channel@decimal value. Apparently it is not affecting the ADC function at all when channel@ has no proper value. What is determining is the reg value only.

A solution that overcomes the problem (without the need to modify a bunch of overlay files) would be as follows: only @value with hex digits would be interpreted as hex; otherwise decimal. In the case that channel is always in the range 0...15, this would be a fine workaround that allows @f or @15 (and e.g. @c for @12).

aurel32 commented 5 months ago

I see that the warning was not the principal motivation of this issue.

Indeed. The issue was actually that the node address was wrongly parsed as a decimal value. So the warning was just the consequence of using a decimal value as a workaround.

That what's discussed by me is related to scripts/dts/python-devicetree/src/devicetree/edtlib.py, where this comparison makes the trouble:

node.regs[0].addr != node.unit_addr

Changing this would require the start of a new issue.

node.unit_addr is the xx in channel@xx, and is always interpreted as hex and stored as decimal in node.unit_addr.

This is correct and matches the device tree specification: https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#naming-and-valid-characters

Therefore, the comparison always fails because reg 15 is compared to 21 when I write channel@15. Fact is: many overlay files in demos say channel@decimal value. Apparently it is not affecting the ADC function at all when channel@ has no proper value. What is determining is the reg value only.

It must be a hex, and is now correctly interpreted like that, that's why you get the warning about the device tree mismatch.

A solution that overcomes the problem (without the need to modify a bunch of overlay files) would be as follows: only @value with hex digits would be interpreted as hex; otherwise decimal. In the case that channel is always in the range 0...15, this would be a fine workaround that allows @f or @15 (and e.g. @c for @12).

This is the real issue. The overlay files in demos should be fixed.

rjmaris commented 5 months ago

Meanwhile I recognize that only a single board out there in adc demo has a false declaration. And regarding me: I once started adc coding with Zephyr v3.1.x, using a sample demo as blueprint. At that time, most of the overlays only had an io-channels node. Upon migration to v3.2.0, the user was advised to view the nrf52840dk_nrf52840.overlay as an example of the new overlay requirements from this version on: in this case channel 0, 1 and 7 with individual parameters. And <reg> also 0, 1 and 7. So I considered both values as decimal...
But I also wondered about the Why of this apparent redundancy. Of course this also applies when channel is expressed as hex value (BTW, hex given as a one digit number is very rare) - later I saw some places with e.g. channel@f, and wondered what it could mean. When it would have been notified as @0f I would immediately see that as hex. But as the DTS coding style document states, leading zeroes are not permitted.

Because the channel address is actually meaningless, I could simply list the individual channel entries with consecutive numbering, while in <reg> the channel number is specified. When the warning would not occur in the build output...