DW ADC driver #9602

nashif · 2018-08-23T22:03:22Z

restored from deleted branch.

This commit adds ADC nodes to DTS files for nRF SoCs and introduces corresponding bindings for these nodes. Signed-off-by: Andrzej Głąbek <[email protected]>

ADC nodes are enabled in DTS for the following nRF development kits: - nrf51_pca10028 - nrf52_pca10040 - nrf52_pca20020 - nrf52810_pca10040 - nrf52840_pca10056 Signed-off-by: Andrzej Głąbek <[email protected]>

This commit replaces the API for ADC drivers with a reworked one. As requested in the issue zephyrproject-rtos#3980, the adc_enable/adc_disable functions are removed. Additionaly, some new features are introduced, like: - asynchronous calls - configuration of channels - multi-channel sampling Common parts of code that are supposed to appear in each implementation of the driver (like locking, synchronization, triggering of consecutive samplings) are provided in the "adc_context.h" file to keep consistency with the SPI driver. Syscalls are no longer present in the API because the functions starting read requests cannot use them, since they can be provided with a callback that is executed in the ISR context, and there is no point in supporting syscalls only for the channels configuration. "adc_api" test is updated and extended with additional test cases, with intention to show how the API is supposed to be used. "adc_simple" test is removed as it does not seem to add much value. Signed-off-by: Andrzej Głąbek <[email protected]>

This commit adds translation layers to make nrfx drivers for the nRF ADC (nRF51 series) and SAADC (nRF52 series) peripherals accessible via the Zephyr's API. The SAADC peripheral is accessed using nrfx HAL only as it turns out that usage of the nrfx driver in this case would be inconvenient and would unnecessarily complicate the shim. Signed-off-by: Andrzej Głąbek <[email protected]>

This adds to the adc_api test board-specific configurations for the following boards: - nrf51_pca10028 - nrf52_pca10040 - nrf52840_pca10056 "adc" is indicated as supported in the yaml files for these boards so that they are included in sanitycheck. Signed-off-by: Andrzej Głąbek <[email protected]>

To require relevant DTS entries on all platforms providing the ADC driver. Signed-off-by: Andrzej Głąbek <[email protected]>

Major rework of the mcux adc16 driver to convert it to the new adc api. Currently supports a subset of the api features including synchronous and asynchronous reads, and consecutive reads triggered by the kernel timer. Does not yet support some of the channel configuration options such as gain and reference voltage because the hardware only allows these options to be configured by peripheral instance rather than by channel. The values are currently hardcoded in the driver, but in the future we could introduce some flexibility per instance via device tree attributes. Does not yet support consecutive reads triggered by a hardware timer. Signed-off-by: Maureen Helm <[email protected]>

Adds board-specific macros to enable the adc_api test to run on kinetis boards. This restores the test to support all the nxp boards that were supported before the new adc api was introduced. Signed-off-by: Maureen Helm <[email protected]>

Remove adc_qmsi_ss to use designware driver for sensor subsystem Signed-off-by: Punit Vara <[email protected]>

Get required data from dts tree. Signed-off-by: Punit Vara <[email protected]>

Replace new ADC apis for designware driver Signed-off-by: Punit Vara <[email protected]>

Include parameters for ARC and update test case. Signed-off-by: Punit Vara <[email protected]>

Replace old ADC apis with new ones. Signed-off-by: Punit Vara <[email protected]>

Replace old ADC driver APIs with new ones. Signed-off-by: Punit Vara <[email protected]>

codecov-io · 2018-08-23T23:14:00Z

Codecov Report

Merging #9602 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #9602   +/-   ##
=======================================
  Coverage   52.15%   52.15%           
=======================================
  Files         212      212           
  Lines       25916    25916           
  Branches     5582     5582           
=======================================
  Hits        13517    13517           
  Misses      10149    10149           
  Partials     2250     2250

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d0b221...3da3ffc. Read the comment docs.

MaureenHelm

This is better than the previous version, but there are still a lot of direct references to the static adc_info_dev that should instead come from looking up a pointer in the device struct.

MaureenHelm · 2018-08-24T00:31:33Z

dts/bindings/iio/adc/intel,qmsi-adc.yaml

+      type: string
+      category: required
+      description: compatible strings
+      constraint: "intel,qmsi-adc"


This should be designware not qmsi, right @nashif ?

MaureenHelm · 2018-08-24T00:33:15Z

drivers/adc/adc_dw.c

+		ADC_CONTEXT_INIT_TIMER(adc_info_dev, ctx),
+		ADC_CONTEXT_INIT_LOCK(adc_info_dev, ctx),
+		ADC_CONTEXT_INIT_SYNC(adc_info_dev, ctx),
+		.state = ADC_STATE_IDLE,


Remove the extra tab

MaureenHelm · 2018-08-24T00:41:36Z

drivers/adc/adc_dw.c

-	if (info->state != ADC_STATE_IDLE) {
-		return 1;
+	if (channel_id >= DW_CHANNEL_COUNT) {
+		return -EINVAL;


You seem to be ignoring everything in channel_cfg except for channel_id. Please check the other configuration settings and return errors if they're not supported.

In the DW IP, we are not setting any other parameters except channel id. I will put comment that hardware don't support any of those.

MaureenHelm · 2018-08-24T00:49:41Z

drivers/adc/adc_dw.c

-		tmp_val |= (entry[0].channel_id & FIVE_BITS_SET);
-		sys_out32(tmp_val, adc_base + ADC_SEQ);
+	/* multiple channels not supported to read, if bitmask "channels"
+	 * has more than 1 bit set than return error


You can handle this at the driver level, similar to mcux. See mcux_adc16_start_channel and mcux_adc16_isr

Ok will look into it.

MaureenHelm · 2018-08-24T00:53:38Z

drivers/adc/adc_dw.c

+	adc_context_lock(&adc_info_dev.ctx, false, NULL);
+
+	ret = adc_dw_read_request(dev, seq_tbl);
+	adc_dw_disable(dev);


It's inconsistent to disable the adc after synchronous reads but not asynchronous reads. You should remove this.

yes this we will remove.

MaureenHelm · 2018-08-24T00:58:00Z

drivers/adc/adc_dw.c

+	set_resolution(dev, seq_tbl);
+	adc_context_start_read(&adc_info_dev.ctx, seq_tbl);
+	error = adc_context_wait_for_completion(&adc_info_dev.ctx);
+	adc_context_release(&adc_info_dev.ctx, error);


Fix up these direct references to adc_info_dev. to be info-> instead

MaureenHelm · 2018-08-24T01:01:25Z

drivers/adc/adc_dw.c

+	u32_t tmp_val;
+	u32_t ctrl;
+
+	adc_dw_enable(adc_info_dev.dev);


Remove this as a consequence of removing adc_dw_disable()

MaureenHelm · 2018-08-24T01:06:12Z

tests/drivers/adc/adc_api/src/test_adc.c

 #define ADC_1ST_CHANNEL_ID	1
 #define ADC_1ST_CHANNEL_INPUT	0

+#elif defined(CONFIG_ARC)


This should be a board name

MaureenHelm · 2018-08-24T01:07:44Z

tests/drivers/adc/adc_api/src/test_adc.c

+	};
 	const struct adc_sequence sequence = {
+#if defined(CONFIG_ARC)
+		.options     = &options,


This is causing CI failures on non ARC boards because options isn't being used.

MaureenHelm · 2018-08-24T01:08:56Z

drivers/grove/light_sensor.c


+static struct adc_sequence_options options = {
+			.interval_us = 12,
+			.extra_samplings = 0,


Remove the extra tab

nashif · 2018-08-24T14:11:39Z

@punitvara @dcpleung please look at the feedback here, I will close this PR to avoid confusion.

anangl and others added 14 commits August 23, 2018 14:53

dts: nrf: Add ADC nodes and bindings for nRF SoCs

f501258

This commit adds ADC nodes to DTS files for nRF SoCs and introduces corresponding bindings for these nodes. Signed-off-by: Andrzej Głąbek <[email protected]>

boards: nrf: Enable ADC nodes in DTS for nRF boards

35fb7d9

ADC nodes are enabled in DTS for the following nRF development kits: - nrf51_pca10028 - nrf52_pca10040 - nrf52_pca20020 - nrf52810_pca10040 - nrf52840_pca10056 Signed-off-by: Andrzej Głąbek <[email protected]>

drivers: adc: Select HAS_DTS_ADC for all ADC drivers

070fc3d

To require relevant DTS entries on all platforms providing the ADC driver. Signed-off-by: Andrzej Głąbek <[email protected]>

tests: adc_api: Add support for kinetis boards

46796df

Adds board-specific macros to enable the adc_api test to run on kinetis boards. This restores the test to support all the nxp boards that were supported before the new adc api was introduced. Signed-off-by: Maureen Helm <[email protected]>

drivers: Use Designware driver for sensor subsystem

a7e5b4a

Remove adc_qmsi_ss to use designware driver for sensor subsystem Signed-off-by: Punit Vara <[email protected]>

dts: arc: Use dts tree for designware driver

4c12059

Get required data from dts tree. Signed-off-by: Punit Vara <[email protected]>

drivers: adc: Update new ADC APIs

40e0bfd

Replace new ADC apis for designware driver Signed-off-by: Punit Vara <[email protected]>

tests: adc_api: Update test case for ARC

f3ea62e

Include parameters for ARC and update test case. Signed-off-by: Punit Vara <[email protected]>

drivers: grove: Modify light sensor

d5e4267

Replace old ADC apis with new ones. Signed-off-by: Punit Vara <[email protected]>

drivers: grove: Update temperature driver

3da3ffc

Replace old ADC driver APIs with new ones. Signed-off-by: Punit Vara <[email protected]>

nashif requested review from MaureenHelm, anangl, carlescufi, galak and ruuddw as code owners August 23, 2018 22:03

MaureenHelm requested changes Aug 24, 2018

View reviewed changes

MaureenHelm requested a review from punitvara August 24, 2018 13:26

nashif closed this Aug 24, 2018

punitvara mentioned this pull request Aug 25, 2018

Remove QMSI sensor subsystem ADC driver and use Designware driver for ARC. #9605

Closed

DW ADC driver #9602

DW ADC driver #9602

Uh oh!

Conversation

nashif commented Aug 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Aug 23, 2018

Codecov Report

Uh oh!

MaureenHelm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nashif commented Aug 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nashif commented Aug 23, 2018 •

edited

Loading