Skip to content

Conversation

@uLipe
Copy link
Member

@uLipe uLipe commented Sep 14, 2024

This change is required to fix dts error of unknown vendor when parsing the compatible string when instancing gc2145.

Although it builds applications correctly, this error makes the ci to fail if some board instance this sensor on its dts, this is caused because it was not using the already existing prefix galaxycore, it was merged using gc prefix instead.

@uLipe uLipe self-assigned this Sep 14, 2024
@zephyrbot zephyrbot added the size: XS A PR changing only a single line of code label Sep 14, 2024
@uLipe uLipe added area: Video Video subsystem and removed size: XS A PR changing only a single line of code labels Sep 14, 2024
@uLipe uLipe added the size: XS A PR changing only a single line of code label Sep 14, 2024
Copy link
Contributor

@kartben kartben left a comment

Choose a reason for hiding this comment

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

This looks wrong? Why wasn't the existing galaxycore vendor picked in the first place? The proper fix would be to update the binding/driver to use that instead of gc

@uLipe
Copy link
Member Author

uLipe commented Sep 14, 2024

Oh, that is true, I will change this PR, I did not realize there is a prefix yet, thanks @kartben

of the compatible driver, use galaxycore
instead of gc.

Signed-off-by: Felipe Neves <[email protected]>
@uLipe uLipe force-pushed the bugfix/gc2145_prefix branch from bcf40be to cd17ad2 Compare September 14, 2024 18:28
@uLipe uLipe changed the title dts: vendor-prefixes: add Galaxy Core gc prefix in the vendor-prefix drivers: video: gc2145: fixes compatible previx of gc2145 Sep 14, 2024
@uLipe uLipe requested a review from kartben September 14, 2024 18:30
@uLipe uLipe removed the size: XS A PR changing only a single line of code label Sep 17, 2024
@uLipe uLipe changed the title drivers: video: gc2145: fixes compatible previx of gc2145 drivers: video: gc2145: fixes compatible prefix of gc2145 Sep 17, 2024
@uLipe
Copy link
Member Author

uLipe commented Sep 17, 2024

Hi Folks, can we merge this? I would like to have this one merged beefore the camera support from Nicla Vision (or other use case for GC2145) comes in.

Thank you :)

@nashif
Copy link
Member

nashif commented Sep 18, 2024

@uLipe Please do not self-assign PRs, the zephyr bot usually does that, you should only be assigned if you are the maintainer of the area being changed.

@kartben
Copy link
Contributor

kartben commented Sep 18, 2024

Hi Folks, can we merge this? I would like to have this one merged beefore the camera support from Nicla Vision (or other use case for GC2145) comes in.

Thank you :)

It will get merged in the next merge batch. It hadn't been in review for 48 business hours until very recently, so that's why it hasn't been merged yet :)

@nashif nashif merged commit c24e8a3 into zephyrproject-rtos:main Sep 18, 2024
@uLipe
Copy link
Member Author

uLipe commented Sep 18, 2024

@uLipe Please do not self-assign PRs, the zephyr bot usually does that, you should only be assigned if you are the maintainer of the area being changed.

Thank you for pointing @nashif I was getting the wrong understanding of what assign PR means, it is clear now.

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

Labels

area: Video Video subsystem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants