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.

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

drivers: video: csi: Change sensor dev to source dev #72623

Merged
merged 1 commit into from May 15, 2024

Conversation

ngphibang
Copy link
Contributor

The CSI can connect to either a camera sensor (as on i.MX RT10xx) or a MIPI CSI-2 receiver (as on i.MX RT11xx). To be generic, change the naming from sensor dev to source dev.

This PR is splitted from #69810 to ease the review process and from #72420 to avoid mutual dependency with #71859

@ngphibang
Copy link
Contributor Author

Hi,
Could we merge this one in priority as the PR is rather trivial (just a renaming) and it is a dependency for several PRs ?

@josuah
Copy link
Contributor

josuah commented May 13, 2024

sensor is also causing confusion with the sensor driver API. Good to rename it.

sdev could be thought as stream-dev, or source-dev, or sink-dev or subdev (which conveniently mimicks Linux subdev API, which is kind of the purpose here...)

A sink could also be an associated device, such as the HDMI to MIPI bridges chips 1, 2, or HDMI CEC controllers 1.

An alternative is to use remote-endpoint for this, so that for instance, some source with many ports/endpoint, and different resolution per ports can get controls specific to every port declare. Probably not going to happen before one of such complex video device becomes supported by Zephyr though...

P.S.: There is a chat channel #pr-help dedicated to catching attention onto a PR.
There is currently no maintainer for the video subsystem, which means lower guarantee of response time.

@DerekSnell DerekSnell removed their request for review May 13, 2024 14:02
@@ -14,7 +14,8 @@ properties:
interrupts:
required: true

sensor:
sdev:
Copy link
Member

Choose a reason for hiding this comment

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

this name is not descriptive, also is this worth breaking backward compatibility

Copy link
Contributor Author

@ngphibang ngphibang May 13, 2024

Choose a reason for hiding this comment

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

@decsny : I can change to "source_dev" (?) to be more descriptive. But the old name "sensor" is definitely not ok for the new support in RT11xx. As explained in the commit message, "sensor" is chosen because at that time, the CSI driver supports only RT10xx where it connects to a sensor. But in RT11xx, it connects to MIPI CSI2 instead.

Copy link
Member

Choose a reason for hiding this comment

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

ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, done

decsny
decsny previously approved these changes May 13, 2024
danieldegrasse
danieldegrasse previously approved these changes May 14, 2024
@loicpoulain
Copy link
Collaborator

Can we simply name it 'source'

The CSI can connect to either a camera sensor (as on i.MX RT10xx) or
a MIPI CSI-2 receiver (as on i.MX RT11xx). To be generic, change the
naming from sensor to source.

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

Yes, done. But now, I need @danieldegrasse and @decsny approve the PR again ...

@nashif nashif merged commit ba1565b into zephyrproject-rtos:main May 15, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Video Video subsystem platform: NXP Drivers NXP Semiconductors, drivers platform: NXP NXP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants