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

boards: nxp: mimxrt1170_evk: Enable video feature #72632

Closed
wants to merge 1 commit into from
Closed

boards: nxp: mimxrt1170_evk: Enable video feature #72632

wants to merge 1 commit into from

Conversation

ngphibang
Copy link
Contributor

Enable video feature on cm7 evk and evkb which are tested with ov5640 camera.

This PR is split from #69810 to ease the review process.
This PR is dependent on #72434 which needs to be merged first.

@ngphibang
boards: nxp: mimxrt1170_evk: Enable video feature
a6059fe 
Enable video feature on cm7 evk and evkb which are tested with ov5640
camera.

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
decsny
decsny requested changes May 13, 2024
View reviewed changes
Copy link
Member

@decsny decsny left a comment

Choose a reason for hiding this comment

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

you cannot list support for a feature that isn't supported, this cannot be a separate PR

@ngphibang
Copy link
Contributor Author

Yes, to support the feature, ofcourse other PRs needs to be merged first. We can merge this PR as the last one to "announce" about the feature.

@danieldegrasse danieldegrasse mentioned this pull request May 13, 2024
Merged
@decsny
Copy link
Member

decsny commented May 13, 2024 •
edited

Yes, to support the feature, ofcourse other PRs needs to be merged first. We can merge this PR as the last one to "announce" about the feature.

no, it has to be merged in the same PR, you can't start testing something after it's merged

@ngphibang
Copy link
Contributor Author

ngphibang commented May 13, 2024 •
edited

no, it has to be merged in the same PR, ...

But which PR I need to push this in ? because the feature is not a single PR but a set of the following PRs (somes are already merged).

#71854: no dependencies
#72623: no dependencies
#72631: no dependencies
#71859: dependent on #72623 and #71854
#72420: dependent on #71859 (and its dependencies)
#72434: dependent on #71859 (and its dependencies)

@dleach02
Copy link
Member

Which ever PR actually enables the video features, pull these changes to it and close this PR.

@ngphibang
Copy link
Contributor Author

ngphibang commented May 14, 2024 •
edited

@dleach02 : Ok, I added this into #72434 as this is the most dependent PR and can be considered as the last one which completes the feature. Thanks

@ngphibang ngphibang closed this May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@decsny decsny decsny requested changes

@danieldegrasse danieldegrasse Awaiting requested review from danieldegrasse

@DerekSnell DerekSnell Awaiting requested review from DerekSnell

@dleach02 dleach02 Awaiting requested review from dleach02

@EmilioCBen EmilioCBen Awaiting requested review from EmilioCBen

@mmahadevan108 mmahadevan108 Awaiting requested review from mmahadevan108

@yvanderv yvanderv Awaiting requested review from yvanderv

Assignees

@dleach02 dleach02

Labels
platform: NXP NXP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ngphibang @decsny @dleach02 @zephyrbot