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

punitvara · 2018-08-24T05:02:19Z

Remove old adc_qmsi_ss driver and use DesignWare driver.
Make DW driver to use dts.
Update grove sensors ( light and temperature sensor) driver to use new ADC APIs.
Update adc_api test for ARC board and it working fine.
Addressed all comments from Remove QMSI sensor subsystem ADC driver and use Designware driver for ARC. #8857

codecov-io · 2018-08-24T05:55:39Z

Codecov Report

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

@@           Coverage Diff           @@
##           master    #9605   +/-   ##
=======================================
  Coverage   52.17%   52.17%           
=======================================
  Files         212      212           
  Lines       25914    25914           
  Branches     5583     5583           
=======================================
  Hits        13520    13520           
  Misses      10145    10145           
  Partials     2249     2249

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 fb10377...5432989. Read the comment docs.

punitvara · 2018-08-24T06:33:05Z

@nashif @MaureenHelm Your review comments are addressed given on #8857. Please let me know if you feel I should provide any updates.

MaureenHelm

Thank you for fixing all the direct references to adc_info_dev, that part looks good now. There are additional comments in #9602 that still need to be addressed. Please also start running sanitycheck locally and fix the failures. You can do it like this:

$ sanitycheck -T tests/drivers/adc -T samples/sensor/grove_light -l

Please run gitlint on all of your commits, there is an issue with this one:

Commit b538745b9a:
1: UC3 Title does not follow [subsystem]: [subject]: "boards:quark_se_c1000_ss_devboard: Enable ADC node"

punitvara · 2018-08-25T12:56:46Z

@MaureenHelm I have addressed all your comments from #9602 and #8857
Can you please review once ?

MaureenHelm

Please fix the CI failures. Instructions to run them locally are here: #9605 (review)

MaureenHelm · 2018-08-25T20:21:31Z

drivers/adc/adc_dw.c

 #endif

+/* TODO: Enable ADC power down capability */
+#ifdef CONFIG_ADC_DW_POWER_DOWN


Where is this defined? I don't see a Kconfig for it. If there's no way to use the power down functionality, you should remove it so we don't have dead code lying around.

ok. Removed.

MaureenHelm · 2018-08-25T20:23:58Z

drivers/adc/adc_dw.c

-	sys_out32(ADC_INT_DSB|ADC_SEQ_PTR_RST, adc_base + ADC_CTRL);
+	/* There is no provision in the IP to set
+	 * gain and reference voltage settings. So
+	 * ignoring these settings.


You need to return an error for unsupported settings

MaureenHelm · 2018-08-25T20:25:26Z

drivers/adc/adc_dw.c

+	info->active_channels |= 1 << channel_id;

-	info->state = ADC_STATE_DISABLED;
+	adc_dw_enable(dev);


This should be in adc_dw_init instead

MaureenHelm · 2018-08-25T20:33:16Z

drivers/adc/adc_dw.c

+				(BIT(i) & info->active_channels)) {
+			info->channels |= BIT(i);
+		}
+	}


You can replace the for loop with info->channels = seq_tbl->channels & info->active_channels

MaureenHelm · 2018-08-25T20:45:13Z

drivers/adc/adc_dw.c

-	};
+};

 static struct adc_config adc_config_dev = {


This should be const

MaureenHelm · 2018-08-25T20:48:27Z

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

 #define ADC_1ST_CHANNEL_ID	0
 #define ADC_1ST_CHANNEL_INPUT	0

+#elif defined(CONFIG_ARC)


This should be a board name. Same throughout this file

This still needs to be fixed

MaureenHelm · 2018-08-25T20:51:16Z

drivers/adc/Kconfig.dw


 endchoice

-config ADC_DW_SAMPLE_WIDTH


Please also remove the related testcases in tests/drivers/adc/adc_api/testcase.yaml

MaureenHelm · 2018-08-25T20:59:19Z

samples/sensor/grove_temperature/prj.conf

 CONFIG_GROVE_TEMPERATURE_SENSOR_V1_X=y
-CONFIG_GROVE_LCD_RGB=y
+
+#CONFIG_GROVE_LCD_RGB=y


Just remove it

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]>

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]>

Updated to work with new ADC API. Signed-off-by: Justin Watson <[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]>

Enable ADC node in dts. Signed-off-by: Punit Vara <[email protected]>

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

Remove Kconfig which are replaced by dts and unnecessary macros which is not useful anymore. 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]>

punitvara · 2018-08-26T07:10:57Z

@MaureenHelm Thanks for review. I addressed all your comments and also tested changes with grove sensors. Everything is working fine. Please have a look once.

MaureenHelm

This is much better, thank you. Just a couple of minor issues left.

MaureenHelm · 2018-08-26T17:21:44Z

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

 #define ADC_1ST_CHANNEL_ID	0
 #define ADC_1ST_CHANNEL_INPUT	0

+#elif defined(CONFIG_ARC)


This still needs to be fixed

MaureenHelm · 2018-08-26T17:23:47Z

samples/sensor/grove_temperature/src/main.c

please provide a meaningful message here, SAMPLE seems like a leftover debug message.

MaureenHelm · 2018-08-26T17:26:45Z

tests/drivers/adc/adc_api/testcase.yaml

Instead of a filter, remove the adc line from boards/x86/quark_d2000_crb/quark_d2000_crb.yaml

Add ARC related parameters and modified test case to work this test case on arc sensor subsystem. Signed-off-by: Punit Vara <[email protected]>

check return value for sample fetch. Conditional printk added to print adc value. Signed-off-by: Punit Vara <[email protected]>

punitvara · 2018-08-26T19:04:21Z

@nashif Done. Please let me know if anything need to be addressed.

nashif · 2018-08-27T01:16:03Z

samples/sensor/grove_temperature/prj.conf

why is this being removed? isn't that part of the sample to display the sensor data on the LCD?

@nashif, Arduino 101 board was hanging when we enabled RGB initially so we disabled it and continued our debugging and verified the ADC completely.

Today we spent more time on this LCD issue and looks like there is platform/HW related issue with arduino_101 and Groove RGB LCD. Arduino 101 pin voltage measurements:
VCC = 5.0v
SCL = 0v
SDA = 0v
So clearly something is not hooked up properly at HW level on arduino 101 with Grove LCD.

To further root cause the issue, we then connected the LCD to D2000 board and ran the test and able to see the LCD up and showing the values we programmed. D2000 pin measurements:
VCC = 5.0v
SCL = 3.3v
SDA = 3.3v

Out of the three whitelisted boards, Arduino Due build fails and only D2000 works fine with Grove LCD.

So, we can enable Grove LCD only for D2000 when we push the changes for D2000 QMSI ADC in separate PR.

On some boards you will need to use 2 pull-up resistors (10k Ohm) between the
SCL/SDA lines and 3.3V

@nashif After connecting two resistors of 10k connecting to SCA and SDA lines , I can see output temperature value on LCD. I am going to push it now.

Use Kconfig to get device name. Make LIBC conditional to get output. Signed-off-by: Punit Vara <[email protected]>

Remove ADC support from d2000 as it is not migrated to new ADC apis. Signed-off-by: Punit Vara <[email protected]>

Remove unnecessary sample width scenario. Signed-off-by: Punit Vara <[email protected]>

MaureenHelm · 2018-08-27T15:48:44Z

Looks good. I pulled your commits into #7691

punitvara requested review from MaureenHelm, anangl, carlescufi, dbkinder, galak, nashif and ruuddw as code owners August 24, 2018 05:02

punitvara force-pushed the adc_api_rework branch from 983312e to e9f7b17 Compare August 24, 2018 05:09

punitvara mentioned this pull request Aug 24, 2018

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

Closed

ramakrishnapallala requested a review from dcpleung August 24, 2018 05:32

punitvara force-pushed the adc_api_rework branch from e9f7b17 to 641e225 Compare August 24, 2018 06:57

MaureenHelm requested changes Aug 24, 2018

View reviewed changes

punitvara force-pushed the adc_api_rework branch 8 times, most recently from c5605c0 to 3ebe054 Compare August 25, 2018 12:55

MaureenHelm requested changes Aug 25, 2018

View reviewed changes

punitvara force-pushed the adc_api_rework branch 3 times, most recently from c633e4c to 73f4c4a Compare August 26, 2018 06:12

anangl added 3 commits August 26, 2018 11:43

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

e0bbea4

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

5725e1a

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]>

anangl and others added 11 commits August 26, 2018 11:43

drivers: adc: Select HAS_DTS_ADC for all ADC drivers

ffec718

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

01b7490

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]>

sam: adc: Updated SAM ADC driver.

58155dd

Updated to work with new ADC API. Signed-off-by: Justin Watson <[email protected]>

drivers: Use Designware driver for sensor subsystem

7233ca6

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

7416938

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

boards: quark_se_c1000_ss_devboard: Enable ADC node

91a7965

Enable ADC node in dts. Signed-off-by: Punit Vara <[email protected]>

drivers: adc: Update new ADC APIs

42890c8

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

drivers: Kconfig.dw: Remove unnecessary Kconfig

0d80554

Remove Kconfig which are replaced by dts and unnecessary macros which is not useful anymore. Signed-off-by: Punit Vara <[email protected]>

drivers: grove: Modify light sensor

44b61ac

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

drivers: grove: Update temperature driver

33e5b6c

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

punitvara force-pushed the adc_api_rework branch 2 times, most recently from 04cff66 to 6f75b49 Compare August 26, 2018 07:08

MaureenHelm requested changes Aug 26, 2018

View reviewed changes

punitvara added 2 commits August 27, 2018 00:32

tests: adc_api: Add ARC related parameters for ADC

c640996

Add ARC related parameters and modified test case to work this test case on arc sensor subsystem. Signed-off-by: Punit Vara <[email protected]>

samples: grove_light: Provide device name from Kconfig

abed718

check return value for sample fetch. Conditional printk added to print adc value. Signed-off-by: Punit Vara <[email protected]>

punitvara force-pushed the adc_api_rework branch from 6f75b49 to 6db4ead Compare August 26, 2018 19:02

nashif requested changes Aug 27, 2018

View reviewed changes

punitvara added 3 commits August 27, 2018 19:33

samples: grove_temperature: Use device name from Kconfig

94d8d09

Use Kconfig to get device name. Make LIBC conditional to get output. Signed-off-by: Punit Vara <[email protected]>

boards: quark_d2000_crb: Remove adc support

352dde4

Remove ADC support from d2000 as it is not migrated to new ADC apis. Signed-off-by: Punit Vara <[email protected]>

tests: adc_api: Remove sample width test scenario

5432989

Remove unnecessary sample width scenario. Signed-off-by: Punit Vara <[email protected]>

punitvara force-pushed the adc_api_rework branch from 6db4ead to 5432989 Compare August 27, 2018 14:06

nashif approved these changes Aug 27, 2018

View reviewed changes

punitvara force-pushed the adc_api_rework branch 2 times, most recently from 6f75b49 to 5432989 Compare August 27, 2018 15:20

MaureenHelm closed this Aug 27, 2018

punitvara mentioned this pull request Aug 30, 2018

ADC: ADC fails to work when fetch multiple sequence entries #2952

Closed

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

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

Uh oh!

Conversation

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

Uh oh!

Uh oh!

codecov-io commented Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

punitvara commented Aug 24, 2018

Uh oh!

MaureenHelm left a comment

Choose a reason for hiding this comment

Uh oh!

punitvara commented Aug 25, 2018

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!

punitvara commented Aug 26, 2018

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!

punitvara commented Aug 26, 2018

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!

punitvara commented Aug 24, 2018 •

edited

Loading

codecov-io commented Aug 24, 2018 •

edited

Loading