Skip to content

dts: arm: st: l4: add DCMI node into DTSI file #80199

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
Closed
wants to merge 1 commit into from
Closed

dts: arm: st: l4: add DCMI node into DTSI file #80199

wants to merge 1 commit into from

Conversation

yishai1999
Copy link
Collaborator

Add the DCMI node into stm32l4.dtsi

@zephyrbot zephyrbot added the platform: STM32 ST Micro STM32 label Oct 22, 2024
erwango
erwango requested changes Oct 22, 2024
View reviewed changes
Copy link
Member

@erwango erwango left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for this contribution.

See comment about node location.

Also, dts is used as the source of trust of what is actually supported by drivers. Have you been able to verify this is actually working ?
Then, if possible could you enable in an in-tree board and a related sample so it could be at least comiled and possibily run by CI regularly ?

dts/arm/st/l4/stm32l4.dtsi Outdated
Comment on lines 481 to 488
dcmi: dcmi@50050000 {
compatible = "st,stm32-dcmi";
reg = <0x50050000 0x400>;
interrupts = <85 0>;
interrupt-names = "dcmi";
clocks = <&rcc STM32_CLOCK_BUS_AHB2 0x00004000>;
status = "disabled";
};
Copy link
Member

Choose a reason for hiding this comment

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

This node is not present in all STM32L4 series so, it should be added only in impacted series dtsi files: stm32l496.dtsi and stm32l4p5.dtsi (which will then benefit to all .dtsi including those).

Copy link
Collaborator Author

@yishai1999 yishai1999 Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Will do!
Where did you see this?
I looked at the datasheets (after your comment) and saw it in STM32L496xx, STM32L4A6xG, STM32L4P5xx, and STM32L4Q5xx. Then I saw that stm32l4a6.dtsi includes stm32l496.dtsi and stm32l4q5.dtsi includes stm32l4p5.dtsi. How did you deduce that it's only needed in stm32l496.dtsi and stm32l4p5.dtsi so quickly?

Copy link
Member

Choose a reason for hiding this comment

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

I've looked for definitions in following files: https://github.com/zephyrproject-rtos/hal_stm32/tree/main/stm32cube/stm32l4xx/soc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've pushed the changes.
Just so I understand since I'm pretty new to zephyr - I looked at the files you pointed to but I do see DCMI definitions in other files such as stm32l4s5xx.h.

@yishai1999
dts: arm: st: l4: add DCMI node into DTSI file
1119e56 
Add the DCMI node into stm32l4p5.dtsi and stm32l496.dtsi

Signed-off-by: Yishai Jaffe <[email protected]>
@yishai1999
Copy link
Collaborator Author

Regarding a sample and the CI -
I assumed that since the DCMI periphery is in the data sheet we can assume that it works. Is this not the case?
I haven't tested it out yet, but I'm planning to in the next few days hopefully.
Also, is there a sample/test that exists for stm32h7 which currently has the periphery defined in its DTS?
Sorry if I'm being a noob...

@JarmouniA
Copy link
Collaborator

Also, is there a sample/test that exists for stm32h7 which currently has the periphery defined in its DTS? Sorry if I'm being a noob...

@yishai1999 See #77949

@erwango
Copy link
Member

erwango commented Oct 23, 2024

I assumed that since the DCMI periphery is in the data sheet we can assume that it works.

I wish it could be so simple, but unfortunately not. There is a whole bunch of code that needs to be validated in between.

@erwango erwango added the block: HW Test Testing on hardware required before merging label Oct 23, 2024
FRASTM
FRASTM approved these changes Oct 23, 2024
View reviewed changes
Copy link
Collaborator

@FRASTM FRASTM left a comment

Choose a reason for hiding this comment

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

LGTM in terms of DTS definition

@github-actions GitHub Actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Dec 23, 2024
@github-actions github-actions bot closed this Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erwango erwango erwango requested changes

@FRASTM FRASTM FRASTM approved these changes

@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

@mathieuchopstm mathieuchopstm Awaiting requested review from mathieuchopstm

Assignees

@erwango erwango

Labels
block: HW Test Testing on hardware required before merging platform: STM32 ST Micro STM32 Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yishai1999 @JarmouniA @erwango @FRASTM @zephyrbot