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.

Sign up for GitHub

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

Jump to bottom

tests: adc_api: Use new DT_LABEL macro for getting device name #24623

Merged
merged 1 commit into from
Apr 23, 2020
Merged

tests: adc_api: Use new DT_LABEL macro for getting device name #24623

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions tests/drivers/adc/adc_api/src/test_adc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
#elif defined(CONFIG_BOARD_NRF51DK_NRF51422)

#include <hal/nrf_adc.h>
#define ADC_DEVICE_NAME DT_ADC_0_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, nordic_nrf_adc))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we be sure that DT_INST(0, nordic_nrf_adc) is ADC_0? Shouldn't we use DT_NODELABEL() here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing in this case its know that ADC is a singleton, since the driver uses DT_INST.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was more of a general comment to the conversions performed in this PR. Some of the coversions convert from ADC_1 to DT_INST(0, ...).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the driver is multi-instance at all, the INST ordering should be assumed to be basically random, so this is indeed an issue in general (though for this particular line of source it's a singleton).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbolivar-nordic I might have been a bit too trigger-happy with this one. @henrikbrixandersen please feel free to continue the conversation here and then we can create a separate PR if necessary to tweak whatever is needed.

Copy link
Contributor

@mbolivar-nordic mbolivar-nordic Apr 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think the INST conversions should be removed and replaced with either NODELABEL or ALIAS.

Edit: in cases where ambiguity is possible, not this particular line

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer NODELABEL

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or in this case, for a lot of these PATH would make sense since they are board specific ifdefs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either NODELABEL or PATH seems fine as way to resolve this issue

#define ADC_RESOLUTION 10
#define ADC_GAIN ADC_GAIN_1_3
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand All @@ -51,7 +51,7 @@
defined(CONFIG_BOARD_ADAFRUIT_FEATHER_NRF52840)

#include <hal/nrf_saadc.h>
#define ADC_DEVICE_NAME DT_ADC_0_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, nordic_nrf_saadc))
#define ADC_RESOLUTION 10
#define ADC_GAIN ADC_GAIN_1_6
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand All @@ -63,7 +63,7 @@

#elif defined(CONFIG_BOARD_FRDM_K22F)

#define ADC_DEVICE_NAME DT_ADC_0_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, nxp_kinetis_adc16))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand All @@ -73,7 +73,7 @@

#elif defined(CONFIG_BOARD_FRDM_K64F)

#define ADC_DEVICE_NAME DT_ADC_1_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, nxp_kinetis_adc16))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand All @@ -82,7 +82,7 @@

#elif defined(CONFIG_BOARD_FRDM_KL25Z)

#define ADC_DEVICE_NAME DT_ADC_0_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, nxp_kinetis_adc16))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand All @@ -91,7 +91,7 @@

#elif defined(CONFIG_BOARD_FRDM_KW41Z)

#define ADC_DEVICE_NAME DT_ADC_0_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, nxp_kinetis_adc16))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand All @@ -100,7 +100,7 @@

#elif defined(CONFIG_BOARD_HEXIWEAR_K64)

#define ADC_DEVICE_NAME DT_ADC_0_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, nxp_kinetis_adc16))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand All @@ -109,7 +109,7 @@

#elif defined(CONFIG_BOARD_HEXIWEAR_KW40Z)

#define ADC_DEVICE_NAME DT_ADC_0_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, nxp_kinetis_adc16))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand All @@ -119,7 +119,7 @@
#elif defined(CONFIG_BOARD_SAM_E70_XPLAINED) || \
defined(CONFIG_BOARD_SAM_V71_XULT)

#define ADC_DEVICE_NAME DT_ADC_0_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, atmel_sam_afec))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_EXTERNAL0
Expand All @@ -144,15 +144,15 @@
defined(CONFIG_BOARD_NUCLEO_L073RZ) || \
defined(CONFIG_BOARD_NUCLEO_WB55RG) || \
defined(CONFIG_BOARD_NUCLEO_L152RE)
#define ADC_DEVICE_NAME DT_ADC_1_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, st_stm32_adc))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
#define ADC_ACQUISITION_TIME ADC_ACQ_TIME_DEFAULT
#define ADC_1ST_CHANNEL_ID 0

#elif defined(CONFIG_BOARD_NUCLEO_F302R8)
#define ADC_DEVICE_NAME DT_ADC_1_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, st_stm32_adc))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand All @@ -161,15 +161,15 @@
#define ADC_1ST_CHANNEL_ID 1

#elif defined(CONFIG_BOARD_NUCLEO_L476RG)
#define ADC_DEVICE_NAME DT_ADC_1_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, st_stm32_adc))
#define ADC_RESOLUTION 10
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
#define ADC_ACQUISITION_TIME ADC_ACQ_TIME_DEFAULT
#define ADC_1ST_CHANNEL_ID 1

#elif defined(CONFIG_BOARD_TWR_KE18F)
#define ADC_DEVICE_NAME DT_ADC_0_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, nxp_kinetis_adc12))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand All @@ -179,7 +179,7 @@

#elif defined(CONFIG_BOARD_MEC15XXEVB_ASSY6853) || \
defined(CONFIG_BOARD_MEC1501MODULAR_ASSY6885)
#define ADC_DEVICE_NAME DT_ADC_0_NAME
#define ADC_DEVICE_NAME DT_LABEL(DT_INST(0, microchip_xec_adc))
#define ADC_RESOLUTION 12
#define ADC_GAIN ADC_GAIN_1
#define ADC_REFERENCE ADC_REF_INTERNAL
Expand Down