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.

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

Add ov5640 camera driver #71854

Merged
merged 3 commits into from
May 17, 2024
Merged

Add ov5640 camera driver #71854

merged 3 commits into from
May 17, 2024

Conversation

ngphibang
Copy link
Contributor

@ngphibang ngphibang commented Apr 24, 2024 •
edited
Loading

This PR adds ov5640 camera driver. It is splitted from this to ease the review procerss.

It already got reviewed by Daniel Degrasse and others

@zephyrbot zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Video Video subsystem labels Apr 24, 2024
danieldegrasse
danieldegrasse reviewed Apr 25, 2024
View reviewed changes
Copy link
Collaborator

@danieldegrasse danieldegrasse 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.

We should add an entry for this camera driver to the application devicetree overlay in tests/drivers/build_all/video, so that this driver gets built within CI. Also- please take a look at CI failures when you get a chance:)

soc/nxp/imxrt/imxrt11xx/soc.c Outdated Show resolved Hide resolved
@josuah
Copy link
Collaborator

josuah commented May 4, 2024

Thank you for adding this driver!

Here are a few complementary sources for anyone interested in this sensor, more or less obvious:

Some might use extra information from the vendors not available in the datasheet, which is great but not really self-sufficient.

loicpoulain
loicpoulain previously approved these changes May 6, 2024
View reviewed changes
Copy link
Collaborator

@loicpoulain loicpoulain left a comment

Choose a reason for hiding this comment

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

LGTM for ov5640.

@ngphibang
dts: bindings: Add bindings for ov5640 camera
f483aad 
Add bindings for ov5640 camera sensor

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
@ngphibang
drivers: video: Add ov5640 camera driver
ccb7b01 
Add driver to support ov5640 camera sensor

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
@ngphibang ngphibang mentioned this pull request May 6, 2024
Merged
@ngphibang
tests: drivers: build_all: video: Add entry for ov5640
4c0b845 
Add entry for ov5640 and add mimxrt1170_evk to the test platforms

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
@ngphibang
Copy link
Contributor Author

ngphibang commented May 6, 2024 •
edited
Loading

@danieldegrasse : Thanks. Is it ok for you now ?

@ngphibang
Copy link
Contributor Author

ngphibang commented May 6, 2024 •
edited
Loading

@loicpoulain : I force pushed to address @danieldegrasse 's comments as well as to resolve the merge conflict (there's a DCMI camera driver merged meanwhile). Could you please re-approve the PR ?
Thanks

@danieldegrasse danieldegrasse mentioned this pull request May 7, 2024
Merged
@ngphibang
Copy link
Contributor Author

ngphibang commented May 9, 2024 •
edited
Loading

@galak Could you help approve and merge this PR, please ? It got 2 approvals but one is cleared due to forced push (to resolve merge conflict)
Thanks !

@ngphibang ngphibang mentioned this pull request May 12, 2024
Merged
@DerekSnell DerekSnell removed their request for review May 13, 2024 14:08
@ngphibang
Copy link
Contributor Author

@loicpoulain Could you please approve the PR again so that it get merged as it blocks other PR as well ?
Thanks !

@ngphibang ngphibang mentioned this pull request May 13, 2024
Closed
@loicpoulain loicpoulain self-requested a review May 13, 2024 17:07
@ngphibang
Copy link
Contributor Author

@galak: Could you help ?

@dleach02
Copy link
Member

@decsny can you review as a co-assignee. The bot assigned @galak this because of the dts change.

@decsny
Copy link
Member

decsny commented May 17, 2024

dts change is trivial, reassigning to loic because I don't know anything about video

@decsny decsny assigned loicpoulain and unassigned galak and decsny May 17, 2024
@MaureenHelm MaureenHelm merged commit 04293dc into zephyrproject-rtos:main May 17, 2024
21 checks passed
@josuah josuah mentioned this pull request Jul 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@loicpoulain loicpoulain loicpoulain approved these changes

@dleach02 dleach02 dleach02 approved these changes

@danieldegrasse danieldegrasse danieldegrasse approved these changes

@decsny decsny Awaiting requested review from decsny

@galak galak Awaiting requested review from galak

@EmilioCBen EmilioCBen Awaiting requested review from EmilioCBen

@mmahadevan108 mmahadevan108 Awaiting requested review from mmahadevan108

@yvanderv yvanderv Awaiting requested review from yvanderv

Assignees

@loicpoulain loicpoulain

Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Video Video subsystem platform: NXP NXP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@ngphibang @josuah @dleach02 @decsny @loicpoulain @danieldegrasse @MaureenHelm @galak @zephyrbot