Skip to content

drivers: adc: Add support for RaspberryPi Pico ADC #45363

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
Merged
merged 8 commits into from
Sep 6, 2022
Merged

drivers: adc: Add support for RaspberryPi Pico ADC #45363

merged 8 commits into from
Sep 6, 2022

Conversation

@soburi
Copy link
Member

@soburi soburi commented May 5, 2022

Adding RaspberryPi Pico ADC driver.

The ADC has internally connected temperature sensor,
Add property to enable this.

The ADC has a single VREF. VCC usually connects to it,
but it may not be in a case.
Add property to make configurable it.

This driver was created with reference to the adc_emul implementation.

This PR testing with tests/drivers/adc/adc_api.
At this time, twister couldn't with rpi_pico board.

samles/driver/adc is also worked.

@github-actions github-actions bot added area: ADC Analog-to-Digital Converter (ADC) area: API Changes to public APIs area: Boards area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Documentation area: Samples Samples area: Tests Issues related to a particular existing or missing test labels May 5, 2022
@soburi soburi force-pushed the support_pico_adc branch 5 times, most recently from 7a2bc7a to a61bbd1 Compare May 5, 2022 04:47
@yonsch yonsch added the platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico) label May 5, 2022
@zephyrbot zephyrbot requested a review from mbolivar-nordic May 6, 2022 11:36
@yonsch
Copy link
Contributor

yonsch commented May 8, 2022

@soburi The STM32 series has a similar temperature sensor, implemented in drivers/sensor/stm32_temp. I think it would be best to follow that example for consistency.

@soburi
Copy link
Member Author

soburi commented May 8, 2022 •
edited
Loading

Hi, @yonsch,

@soburi The STM32 series has a similar temperature sensor, implemented in drivers/sensor/stm32_temp. I think it would be best to follow that example for consistency.

I understood.
Removing The temperature_sensor option in this PR , And issue another PR to support it as a sensor.

@yonsch
Copy link
Contributor

yonsch commented May 8, 2022

You can keep it in this PR if you want, just do it in a separate commit if you do. The point is to have a compatible and a "sensor" implementation.

@soburi soburi force-pushed the support_pico_adc branch from a61bbd1 to 316cd89 Compare May 12, 2022 16:56
@soburi
Copy link
Member Author

soburi commented May 12, 2022

Hi, @yonsch,

You can keep it in this PR if you want, just do it in a separate commit if you do. The point is to have a compatible and a "sensor" implementation.

I removed the temperature-related code from this PR.
Because it makes the interface clean. It is just only writing a flag to registers,
I think it is also able to implement in sensor code.

mbolivar-nordic
mbolivar-nordic reviewed May 13, 2022
View reviewed changes
dts/bindings/adc/raspberrypi,pico-adc.yaml Outdated
Comment on lines 11 to 12
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic May 13, 2022

Choose a reason for hiding this comment

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

Why is this here? It looks unnecessary to me.

Copy link
Member Author

@soburi soburi May 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Removed it. It's my carelessness.
I implemented it without interrupts firstly,
and I forgot to remove it after the implementation changed.

Copy link
Contributor

@mbolivar-nordic mbolivar-nordic May 13, 2022

Choose a reason for hiding this comment

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

Thanks!

@soburi soburi force-pushed the support_pico_adc branch from 316cd89 to 089d63d Compare May 13, 2022 19:01
@mbolivar-nordic
Copy link
Contributor

mbolivar-nordic commented May 13, 2022

@soburi thank you for your update. Do you think you can also rebase this PR? There is a conflict in rpi_pico.dts.

yonsch
yonsch previously approved these changes Aug 21, 2022
View reviewed changes
@soburi
Copy link
Member Author

soburi commented Sep 1, 2022

Hi, @anangl @mbolivar-nordic @galak @stephanosio @carlescufi @nashif

I would like to proceed with this PR (and, also below), so could you give a review this PR if possible?

zephyrproject-rtos/hal_rpi_pico#2 (comment)

As you can see in the conversation log, the points pointed out in the review have solved.
So I think there are no problems.

I would like to put it into the next release if possible.

nordicjm
nordicjm reviewed Sep 5, 2022
View reviewed changes
Copy link
Contributor

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

Looks good, will approve once pull link is replaced with commit ID.

Future update to support the 5th channel (temperature) would be ideal

carlescufi
carlescufi previously approved these changes Sep 5, 2022
View reviewed changes
@carlescufi
Copy link
Member

carlescufi commented Sep 6, 2022

@soburi zephyrproject-rtos/hal_rpi_pico#2 is merged now.

@soburi
manifests: Rename Pico-SDK's adc_read() to pico_adc_read()
d833a08 
adc_read() that defined in adc.h of PICO-SDK
conflicts with zephyr's ADC API.
Rename it to avoid compile errors.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
modules: hal_rpi_pico: Enable ADC driver
4ba664c 
Enable ADC driver.
Add the path of the ADC driver header into include paths.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
dts: bindings: adc: Add RaspberryPi Pico ADC
b80b3ff 
Define RaspberryPi Pico ADC.

The ADC has internally connected temperature sensor,
Add property to enable this.

The ADC has a single VREF. VCC usually connects to it,
but it may not be in a case.
Add property to make configurable it.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
drivers: adc: Add support for RaspberryPi Pico ADC
09c8b2f 
Introducing RaspberryPi Pico ADC driver.
This driver was created with reference to the adc_emul implementation.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
boards: arm: rpi_pico: Enabling ADC
7e7ec7d 
Make ADC available. Also enable internal temperature sensor.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
tests: drivers: adc: Add support for rpi_pico
7967a09 
Add support for rpi_pico board to adc_api test.

At this time, twister couldn't with rpi_pico board,
Test it built with west and checked the console output.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
samples: drivers: adc: Add rpi_pico support
1db7fad 
Add support for rpi_pico board to adc driver sample.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
drivers: adc: shell: Support RaspberryPi Pico ADC
38c2ba4 
Add ADC shell support for RaspberryPi Pico.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi soburi dismissed stale reviews from carlescufi and yonsch via 38c2ba4 September 6, 2022 13:21
@zephyrbot zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Sep 6, 2022
@carlescufi carlescufi merged commit f3fd686 into zephyrproject-rtos:main Sep 6, 2022
@soburi soburi deleted the support_pico_adc branch October 3, 2022 02:50
@yonsch
Copy link
Contributor

yonsch commented Dec 14, 2022

@soburi Are you still planning to work on the temperature sensor?

@soburi
Copy link
Member Author

soburi commented Dec 16, 2022

@yonsch
Oh! I forgot it completely.
Thank you for notification.
I'm a little busy right now, so I think I'll be able to handle it by the end of the year.
If you are already starting to handle it, I will leave it to you.

@yonsch
Copy link
Contributor

yonsch commented Dec 16, 2022

Thanks @soburi , I didn't start working on it. If you want to finish it yourself go ahead, it's not urgent. If it's too much trouble I can do it no problem.

@soburi soburi mentioned this pull request Jan 16, 2023
Open
57 tasks
soburi added a commit to soburi/zephyr that referenced this pull request Aug 2, 2023
@soburi
CODEOWNERS: add codeowner of adc_rpi_pico
30d82ed 
Add myself as codeowner of previously committed driver.

- zephyrproject-rtos#45363

Signed-off-by: TOKITA Hiroshi <[email protected]>
fabiobaltieri pushed a commit that referenced this pull request Aug 3, 2023
@soburi @fabiobaltieri
CODEOWNERS: add codeowner of adc_rpi_pico
7729de4 
Add myself as codeowner of previously committed driver.

- #45363

Signed-off-by: TOKITA Hiroshi <[email protected]>
kunoh pushed a commit to kunoh/zephyr that referenced this pull request Aug 7, 2023
@soburi @kunoh
CODEOWNERS: add codeowner of adc_rpi_pico
833a669 
Add myself as codeowner of previously committed driver.

- zephyrproject-rtos#45363

Signed-off-by: TOKITA Hiroshi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yonsch yonsch yonsch approved these changes

@nordicjm nordicjm nordicjm approved these changes

@carlescufi carlescufi carlescufi left review comments

@nashif nashif Awaiting requested review from nashif

@galak galak Awaiting requested review from galak

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@anangl anangl Awaiting requested review from anangl

@mbolivar-nordic mbolivar-nordic Awaiting requested review from mbolivar-nordic

Assignees

@anangl anangl

Labels

area: ADC Analog-to-Digital Converter (ADC) area: API Changes to public APIs area: Boards area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: Documentation area: Modules area: Samples Samples area: Tests Issues related to a particular existing or missing test manifest manifest-hal_rpi_pico platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@soburi @yonsch @mbolivar-nordic @zephyrbot @carlescufi @pdgendt @nordicjm @anangl