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.

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

stm32 comparator #85022

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

KozhinovAlexander
Copy link
Collaborator

@KozhinovAlexander KozhinovAlexander commented Feb 1, 2025

Add stm32 comparator driver and simple comparator sample.

I've used to test it on nucleo_h745zi_q (m7 core only).

Build command with newly added sample:

west build -p auto -d samples/drivers/comparator/build -b nucleo_h745zi_q/stm32h745xx/m7 samples/drivers/comparator  -DDTC_OVERLAY_FILE=samples/drivers/comparator/boards/st/nucleo_h745zi_q.overlay  

My build result output:

-- Zephyr version: 4.0.99 (...../zephyr), build: v4.0.0-4417-g2c7031229fe0
[153/153] Linking C executable zephyr/zephyr.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:       31316 B         1 MB      2.99%
             RAM:        6848 B       512 KB      1.31%
          EXTMEM:          0 GB       256 MB      0.00%
           SRAM1:          0 GB       128 KB      0.00%
           SRAM2:          0 GB       128 KB      0.00%
           SRAM3:         16 KB        32 KB     50.00%
           SRAM4:          0 GB        64 KB      0.00%
            DTCM:          0 GB       128 KB      0.00%
        IDT_LIST:          0 GB        32 KB      0.00%

The resulting output in the console could look like:

*** Booting Zephyr OS build v4.0.0-4417-g2c7031229fe0 ***
- comp_trigger_cb: comparator@5800380c output: 0
- comparator@5800380c output: 0
- comparator@5800380c output: 0
- comparator@5800380c output: 0
- comp_trigger_cb: comparator@5800380c output: 1
- comparator@5800380c output: 1
- comparator@5800380c output: 1
- comparator@5800380c output: 1
- comp_trigger_cb: comparator@5800380c output: 0
- comparator@5800380c output: 0
- comparator@5800380c output: 0
- comparator@5800380c output: 0
- comp_trigger_cb: comparator@5800380c output: 1
- comparator@5800380c output: 1
- comparator@5800380c output: 1
- comparator@5800380c output: 1

NOTE: To see the output value changing and comp_trigger_cb called the pins PB0 and PB2 shall be connected physically on the nucleo_h745zi_q board. Please consider the overlay file in the sample to find more about pinouts used.

I've not tested the driver with any other stm32, but I am planning to add support for stm32g4 soon.
Any testers with appropriate board (any stm32h7 would be fine as a good first choice) are very welcome.

@@ -9,5 +9,6 @@ zephyr_library_sources_ifdef(CONFIG_USERSPACE comparator_handlers.c)
zephyr_library_sources_ifdef(CONFIG_COMPARATOR_FAKE_COMP comparator_fake_comp.c)
zephyr_library_sources_ifdef(CONFIG_COMPARATOR_MCUX_ACMP comparator_mcux_acmp.c)
zephyr_library_sources_ifdef(CONFIG_COMPARATOR_NRF_COMP comparator_nrf_comp.c)
zephyr_library_sources_ifdef(CONFIG_COMPARATOR_STM32_COMP comparator_stm32_comp.c)
Copy link
Contributor

Choose a reason for hiding this comment

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

sort alphabetically

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@@ -21,6 +21,7 @@ config COMPARATOR_INIT_PRIORITY
rsource "Kconfig.fake_comp"
rsource "Kconfig.mcux_acmp"
rsource "Kconfig.nrf_comp"
rsource "Kconfig.stm32_comp"
Copy link
Contributor

Choose a reason for hiding this comment

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

here as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@KozhinovAlexander KozhinovAlexander force-pushed the stm32_comparator branch 5 times, most recently from 7cc8784 to 455e0ec Compare February 2, 2025 22:32
Add comparator sample with simple triggering behaviour

Signed-off-by: Alexander Kozhinov <[email protected]>
Add comparator driver for ST's stm32 series.

Signed-off-by: Alexander Kozhinov <[email protected]>
Copy link
Collaborator

@bjarki-andreasen bjarki-andreasen left a comment

Choose a reason for hiding this comment

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

The driver looks good :) The sample is not needed I believe. The test suite in tests/drivers/comparator/gpio_loopback validates the API behavior with a GPIO connected to the comp input. The build all test suite allows you to build varoius dt configurations tests/drivers/build_all/comparator, and lastly, given the nature of a comparator, just enabling the comparator shell in a blank application is sample enough :)

Please add the comp to the two test suites to validate and build the driver in CI

@KozhinovAlexander
Copy link
Collaborator Author

Thank you for your review.

The driver looks good :) The sample is not needed I believe. The test suite in tests/drivers/comparator/gpio_loopback validates the API behavior with a GPIO connected to the comp input. The build all test suite allows you to build varoius dt configurations tests/drivers/build_all/comparator, and lastly, given the nature of a comparator, just enabling the comparator shell in a blank application is sample enough :)

Makes sense. I have few questions just for my understanding:

  • Is there also any HW or only twister, where tests are running?

  • Will test-suite test the stm32 driver too?

Please add the comp to the two test suites to validate and build the driver in CI

Will do of course. Thanks.

Regarding comparator sample: I think - the major advantage of the sample is to give a very basic example coupled with physical functionality. Especially in my experience samples allow very fast peripheral testing on new hardware, device-tree problem findings with new BSP.
Additionally all the zephyr samples give west build command for an arbitrary sample. People learn it faster, than running twister, I would assume. Thus it is always faster for them to go with sample than with test.
Of course we may discuss it in a separate PR for sample. For the sake of simplicity I will drop the sample in this PR.

@bjarki-andreasen
Copy link
Collaborator

bjarki-andreasen commented Feb 3, 2025

Thank you for your review.

The driver looks good :) The sample is not needed I believe. The test suite in tests/drivers/comparator/gpio_loopback validates the API behavior with a GPIO connected to the comp input. The build all test suite allows you to build varoius dt configurations tests/drivers/build_all/comparator, and lastly, given the nature of a comparator, just enabling the comparator shell in a blank application is sample enough :)

Makes sense. I have few questions just for my understanding:

* Is there also any HW or only twister, where tests are running?

* Will test-suite test the stm32 driver too?

Many vendors, including STM I believe, run twister tests on hardware in their own CI, zephyr only runs emulated platforms in CI. That's why we specify fixtures in the zephyr test suites, like gpio_loopback, to specify what hardware they can run on :) That's why the gpio_loopback comparator test was created, its widely supported by upstream boards (and simple to set up locally of course)

I don't know exactly what their gpio_loopback looks like, or if they test the board you use in this PR, maybe @FRASTM knows more?

Please add the comp to the two test suites to validate and build the driver in CI

Will do of course. Thanks.

Regarding comparator sample: I think - the major advantage of the sample is to give a very basic example coupled with physical functionality. Especially in my experience samples allow very fast peripheral testing on new hardware, device-tree problem findings with new BSP. Additionally all the zephyr samples give west build command for an arbitrary sample. People learn it faster, than running twister, I would assume. Thus it is always faster for them to go with sample than with test. Of course we may discuss it in a separate PR for sample. For the sake of simplicity I will drop the sample in this PR.

Tests don't require twister to run, they are just normal applications with a nice framework to set up cases without needing a bunch of printk() everywhere.

Simply west build -p -b nucleo_h745zi_q zephyr/tests/drivers/comparator/gpio_loopback and flash it (after you add the board overlay for it in the test suite of course). So, its just as fast as the sample, but about 90% less duplication :)

The reason I favor the shell above a "fixed" scenario like the sample in this PR, is that actually playing around with a comparator usually requires some external signal generator to generate a voltage or an edge, the GPIO case only shows that the API "works", not that the comparator is actually measuring and behaving correctly. That is quick to see using the shell to test "real" varying signals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants