Skip to content

stm32: nucleo u385rg_q: fix wrong spi node assigned to arduino_spi #98361

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 2 commits into from
Nov 6, 2025
Merged

stm32: nucleo u385rg_q: fix wrong spi node assigned to arduino_spi #98361

merged 2 commits into from
Nov 6, 2025
+23 −8

Conversation

@ulrich-a11-y
Copy link
Contributor

@ulrich-a11-y ulrich-a11-y commented Oct 27, 2025 •
edited
Loading

This PR fixes an incorrect peripheral node assignment to arduino_spi and updates the board's device tree to avoid conflicts with other enabled peripheral nodes (i2c3) on PA7 pin.

Fixes: #97543

@zephyrbot zephyrbot added area: I2C platform: STM32 ST Micro STM32 area: DAC Digital-to-Analog Converter area: Boards/SoCs area: Tests Issues related to a particular existing or missing test labels Oct 27, 2025
@ulrich-a11-y ulrich-a11-y mentioned this pull request Oct 27, 2025
Closed
@etienne-lms
Copy link
Contributor

etienne-lms commented Oct 28, 2025

LGTM. @ulrich-a11-y, would you be okay to swap the order of the commits in your series? It would ensure each commit still passes the 2 modified tests. It may be handy if we later bisect into the Git revision history for some unrelated purpose.

Note CI *Compliance Checks" test complains:

File:tests/drivers/dac/dac_api/boards/nucleo_u385rg_q.overlay
 :Add end of line to end of file

(snip)

 :Add end of line to end of file
File:tests/drivers/i2c/i2c_target_api/boards/nucleo_u385rg_q.overlay

@djiatsaf-st, are you okay with the proposed changes, to have the board default supporting Arduino interfaces and have overlay DTS files for DAC and I2C specific tests?

@djiatsaf-st
Copy link
Contributor

djiatsaf-st commented Oct 28, 2025

``

@djiatsaf-st, are you okay with the proposed changes, to have the board default supporting Arduino interfaces and have overlay DTS files for DAC and I2C specific tests?

Yes I am okay with these changes. Arduino uno shield by example can be used with SPI without manually wiring it up.

@martinjaeger martinjaeger removed their assignment Oct 29, 2025
@martinjaeger martinjaeger added area: DAC Digital-to-Analog Converter and removed area: DAC Digital-to-Analog Converter labels Oct 29, 2025
martinjaeger
martinjaeger reviewed Oct 29, 2025
View reviewed changes
mathieuchopstm
mathieuchopstm reviewed Oct 30, 2025
View reviewed changes
etienne-lms
etienne-lms reviewed Oct 30, 2025
View reviewed changes
@martinjaeger martinjaeger dismissed their stale review November 3, 2025 09:57

No DAC related changes.

@martinjaeger martinjaeger assigned erwango and unassigned erwango and martinjaeger Nov 3, 2025
@etienne-lms
Copy link
Contributor

etienne-lms commented Nov 3, 2025

(...) now we would not have any I2C node in the Devicetree on board-level anymore. Did you check if there is any combination of pins to have a suitable default I2C setting? What do STM folks think about this?

i2c1 and i2c2 nodes are still default enabled in the board DTS file.

Regarding i2c3 (used by I2C API test sample), its SCL pin can be muxed on either PA7 or PC0 pins but the former conflicts with spi1 (this P-R) and the later with adc1 node pinctrl, adc1 being used for ADC tests. The proposed change here is fine IMO.

etienne-lms
etienne-lms previously approved these changes Nov 3, 2025
View reviewed changes
Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

LGTM.
I don't understand why CI Compliance checks complains.

@martinjaeger
Copy link
Member

martinjaeger commented Nov 3, 2025

(...) now we would not have any I2C node in the Devicetree on board-level anymore. Did you check if there is any combination of pins to have a suitable default I2C setting? What do STM folks think about this?

i2c1 and i2c2 nodes are still default enabled in the board DTS file.

Regarding i2c3 (used by I2C API test sample), its SCL pin can be muxed on either PA7 or PC0 pins but the former conflicts with spi1 (this P-R) and the later with adc1 node pinctrl, adc1 being used for ADC tests. The proposed change here is fine IMO.

Oops, didn't see i2c1 and i2c2 for some reason. Ok, so that's fine.

LGTM. I don't understand why CI Compliance checks complains.

There is a space character behind the added line, see the marking in the screenshot:

image

etienne-lms
etienne-lms reviewed Nov 3, 2025
View reviewed changes
Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

For commit "tests: drivers: nucleo_u385rg_q: update i2c tests drivers".
Could you explain the rational in the commit message body? E.g.:

Enable and add pinctrl for i2c3 node and disable spi1 node since spi1 is default
enabled on that board (Arduino connector) and i2c3 pinctrl conflicts with.

For commit "boards: st: nucleo_u385rg_q: change arduino_spi to spi1", could you mention in the commit message that spi3 node status property is moved in the node for consistency of node properties ordering.

Implementation LGTM.

@ulrich-a11-y
tests: drivers: nucleo_u385rg_q: update i2c tests drivers
4f16a4d 
Enable and add pinctrl for the I2C3 node, and disable the SPI1 node
since it is enabled by default on this board (via the Arduino connector),
which causes a conflict with the I2C3 pinctrl configuration.

Signed-off-by: Ulrich KAMDEM <kamdemulricharmel@gmail.com>
@ulrich-a11-y
boards: st: nucleo_u385rg_q: change arduino_spi to spi1
6ef85ec 
According to user manual(UM3062, table 16), arduino_spi is connected
to spi1 instead of spi3.

Enable spi1 node and affect the SPI_NSS pin with I/O function
to do the chip select.

Remove i2c3 to avoid conflict with PA7 pin.

Move the status property of dac1 node to maintain consistency
in the ordering of node properties.

NB: The SPI_NSS pin is not listed as an alternate function
for SPI1 in the user manual (UM3062, Table 17).

Signed-off-by: Ulrich KAMDEM <kamdemulricharmel@gmail.com>
@ulrich-a11-y
Copy link
Contributor Author

ulrich-a11-y commented Nov 4, 2025

@etienne-lms , the spi3 node status property is not moved. It's rather the dac1 node.

@zephyrbot zephyrbot requested a review from juickar November 4, 2025 16:21
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 4, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

etienne-lms
etienne-lms approved these changes Nov 4, 2025
View reviewed changes
Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

Right. Thanks for the updates.
LGTM

@ulrich-a11-y
Copy link
Contributor Author

ulrich-a11-y commented Nov 6, 2025

@erwango , PTAL

@cfriedt cfriedt merged commit f2428c6 into zephyrproject-rtos:main Nov 6, 2025
20 checks passed
@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Hi @ulrich-a11-y!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

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

Reviewers

@erwango erwango erwango approved these changes

@etienne-lms etienne-lms etienne-lms approved these changes

@mathieuchopstm mathieuchopstm mathieuchopstm approved these changes

@djiatsaf-st djiatsaf-st djiatsaf-st approved these changes

@FRASTM FRASTM Awaiting requested review from FRASTM

@gautierg-st gautierg-st Awaiting requested review from gautierg-st

@GeorgeCGV GeorgeCGV Awaiting requested review from GeorgeCGV

@marwaiehm-st marwaiehm-st Awaiting requested review from marwaiehm-st

@nashif nashif Awaiting requested review from nashif

@teburd teburd Awaiting requested review from teburd

@juickar juickar Awaiting requested review from juickar

@martinjaeger martinjaeger Awaiting requested review from martinjaeger

Assignees

@erwango erwango

Labels

area: Boards/SoCs area: I2C area: Tests Issues related to a particular existing or missing test platform: STM32 ST Micro STM32

Projects

None yet

Milestone

v4.3.0

Development

Successfully merging this pull request may close these issues.

Nucleo U385RG-Q arduino_spi dts description is wrong

8 participants

@ulrich-a11-y @etienne-lms @djiatsaf-st @martinjaeger @erwango @mathieuchopstm @cfriedt @zephyrbot