Video: addition of the imx335 driver #88825

avolmat-st · 2025-04-19T21:23:29Z

This PR introduces the CSI IMX335 Sensor driver (5MPix). The sensor supports both 10 and 12 bits bayer format in 2 or 4 csi lanes.
It is based on the CCI helper in order to have a cleaner code.
I kept few TODO within the source code:

For the time being, only 10 bit in 2 data lanes is supported. data-lane DT property isn't checked.
The sensor can be clock using various clock-rate. Register control for the various possible clock rate is in place however check of the DT in order to read the clock rate isn't in place (we could add a fixed-clock within the DT for that purpose). For the time being this is fixed to 24MHz.

This PR depends on #87935

josuah

This is awesome and we can even order the module from here.

I did my best to proofread without the datasheet (as often with image sensors, secret).

A few comments in case these were omissions. If these are intentional, please keep them like so!

josuah · 2025-04-19T22:27:53Z

drivers/video/imx335.c

+	ret = imx335_set_input_clk(dev, MHZ(24));
+	if (ret) {
+		LOG_ERR("Unable to configure INCK");
+		return -EIO;


Suggested change

return -EIO;

return ret;

josuah · 2025-04-19T22:28:05Z

drivers/video/imx335.c

+	ret = video_write_cci_multi(&cfg->i2c, imx335_mode_2l_10b);
+	if (ret) {
+		LOG_ERR("Unable to initialize the sensor");
+		return -EIO;


Suggested change

return -EIO;

return ret;

josuah · 2025-04-19T22:28:13Z

drivers/video/imx335.c

+	ret = video_write_cci_multi(&cfg->i2c, imx335_init_params);
+	if (ret) {
+		LOG_ERR("Unable to initialize the sensor");
+		return -EIO;


Suggested change

return -EIO;

return ret;

josuah · 2025-04-19T22:30:46Z

drivers/video/imx335.c

+		break;
+	default:
+		LOG_ERR("Unsupported inck freq (%d)\n", rate);
+		return -EIO;


Suggested change

return -EIO;

return -EINVAL;

josuah · 2025-04-19T22:43:19Z

If interested in making test of this sensor more easy, it is possible to add the devicetree snippets needed to integrate this sensor with a compatible board via a shield. For instance like these:

Permitting to run west build --board stm32n6... --shield b_cams_imx samples/drivers/video/capture for instance.

avolmat-st · 2025-04-20T12:24:36Z

Thanks for all your comments. I think all of your comments are valid so I will correct the code.
Regarding the shield, I already have it but haven't yet pushed it yet. Reason for that is that, for the N6 this is based on the work done on the DCMIPP so since I haven't pushed it yet I haven't push yet the shield as well. The shield connect to a connector which expose the 22 pins "RPi" type connector.

ngphibang

LGTM in general. Eager for more features coming in the future (chipID, controls, data lanes, resolution, etc.) :-).

Could you add an entry to https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/drivers/build_all/video/app.overlay so that we can have a minimal built-only CI test. Without it, we cannot detect even only compilation errors. As you can see, the PRs has "passed" CI although it cannot be compiled because of the dependency on #87935.

Thanks

drivers/video/imx335.c

josuah · 2025-04-20T18:33:34Z

Converting to draft until the dependency is merged as suggested earlier.

josuah · 2025-04-21T12:12:49Z

I completely forgot to check that the sensor needed to be added to tests/drivers/build_all/video/app.overlay!
Here is an example to save time.

avolmat-st · 2025-04-23T20:39:23Z

I push a new version of the driver, all rebased on top of the HEAD, meaning top of new ctrl and video dev frameworks.

avolmat-st · 2025-05-19T21:36:59Z

PR updates include the followings:

addition of CSI 15 and 22 pins connectors
addition of frmival ops within the imx335
addition of a clock source in order to indicate the frequency of the input quartz of the sensor
addition of the shield for the B_CAMS_IMX board for STM32 boards

The shield is relying on several labels in order to connect the board DT: there are more than we could expect, which is due to the fact that the DCMIPP (with what it is being used on the STM32N6 DK board) has several output (capture) endpoints, and also has a port for the sensor part. So it is necessary to:

identify the i2c bus: &csi_22pins_i2c
identify the capture interface (overall) to enable: &csi_22pins_interface
identity the input port (into which the sensor is connected): &csi_22pins_ep_in
identify the capture point (aka the one given to zephyr,camera): &csi_22pins_capture_port
The shield if also relying on a &csi_22pins_connector which is provided by the board.

I first posted this PR so that it is easier to see the difference compare to previous version of the PR. Next I'll post the rebased version of the PR to be up to date with the current zephyr HEAD (the driver here still have the endpoint arg for example)

avolmat-st · 2025-05-20T19:46:00Z

Removal of the 15pin CSI connector from this PR, I will push it within the GC2145 / CSI PR in order to avoid dependencies between PR.
Added the CCI PR commit first in the PR to allow proper build of the PR

avolmat-st · 2025-05-20T20:45:11Z

Rebased on top of HEAD in order to be up to date in terms of video api.
This rebased included the removal of the endpoint argument of all API functions and enum video_buf_type within the set_stream function.

avolmat-st · 2025-05-20T21:40:17Z

Fixed the webp image filename to correct the documentation build issue.

avolmat-st · 2025-05-21T06:13:45Z

Simply rebased on top of head of main branch since the CCI PR has now been merged.

josuah

Only one blocking point for the video_enum_frmival() API.
Feel free to skip the rest from me!

Thank you for this first Sony sensor contributed in this quality driver!

dts/bindings/video/sony,imx335.yaml

drivers/video/imx335.c

dts/bindings/gpio/csi,cam-22pins-connector.yaml

include/zephyr/dt-bindings/gpio/csi-22pins-connector.h

boards/shields/st_b_cams_imx_mb1854/doc/index.rst

josuah · 2025-05-21T11:30:35Z

boards/shields/st_b_cams_imx_mb1854/st_b_cams_imx_mb1854.overlay

+	status = "okay";
+};
+
+&csi_22pins_ep_in {


[non-blocking: discussion]

I went with such approach at first, but got some recommendation from @ngphibang:

I think the right way is keeping the video sdma node and label it as a dvp_20pin_interface as in https://github.com/zephyrproject-rtos/zephyr/blob/main/boards/nxp/mimxrt1064_evk/mimxrt1064_evk.dts#L352-#L354

boards: frdm_mcxn947: turn OV7670 into a shield #89131 (comment)

The endpoint should be inside the interface node.

boards: frdm_mcxn947: turn OV7670 into a shield #89131 (comment)

First commenting on this one, seems to be the one that could lead to discussions. In our case, this is being used on the STM32N6 for example and the DCMIPP has 3 output endpoints (one for each pipe). So the DT node looks like something like that:

csi_22pins_interface: dcmipp { ports { port@0 { csi_22pins_ep_in: endpoint { here the endpoint which is connected to the sensor endpoint } } port@1 { endpoint@0 { first output endpoint / pipe of the dcmipp } csi_22pins_capture_port: endpoint@1 { 2nd output endpoint / pipe of the dcmipp } endpoint@2 { 3rd output endpoint / pipe of the dcmipp } } }

So here, there are actually plenty of information I need in order to "connect" the shield to the dcmipp.
1st, I need to set to enable the dcmipp node (it can't be always enabled, it should only be when there is a sensor connected to it). Hence the "interface" label: csi_22pins_interface.
Then I need to configure the input endpoint of the dcmipp and link it to the sensor endpoint, hence csi_22pins_ep_in.
I need to add the sensor to the right i2c interface, hence csi_22pins_i2c
Then I need to select an output pipe, here the 2nd one is selected via csi_22pins_capture_port and zephyr,camera = csi_22pins_capture_port.

I have the feeling that it is actually quite complicated, due to the fact that there are multiple output endpoints which are part of a 2nd port.

ouch, formatting is gone ... sorry about that.

(I added the formatting back)
Yes somehow the csi_22pins_ep_in allowed the shield to know which endpoint to select.

In this case, I think you could put all the port@1 (with 3 endpoints) into the SoC devicetree, not in the shield because they are "fixed" and just the port@0 (which is flexible, which connects to the sensor) into the shield, by reserving the port@0 firts in the SoC somehow like :

port@0 { reg = <1>; };

https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/arm/nxp/nxp_rt11xx.dtsi#L920-#L943

no ?

@ngphibang In this case, I think you could put all the port@1 (with 3 endpoints) into the SoC devicetree, not in the shield because they are "fixed" and just the port@0 (which is flexible, which connects to the sensor) into the shield, by reserving the port@0 firts in the SoC somehow like :

This would be a convention that drivers take to make it easy for shields...
We need some place where to document that to help new drivers being written.
Some kind of "video driver author handbook" in the long run too.
I will ask where to insert it in the docs.

An "Implementing..." section in the (hopefully already existing) driver class page? https://docs.zephyrproject.org/latest/hardware/peripherals/sensor/index.html#implementing-sensor-drivers -- @kartben

Honestly, I still don't really understand why we need to put &csi_22pins_ep_in outside the interface node :-) ... but it's just for my knowdledge

If we can guarantee that on all board having a RPi 22 pins CSI connector the interface IP "input" endpoint (to which to connect the sensor endpoint) is going to be at:

csi_22pins_interface { ports { port@0 { endpoint { // HERE }; } } }

then there is no need for such label indeed, but can we guarantee that ?
If yes then then only csi_22pins_interface is enough.

I see now. Thank you !

boards/shields/st_b_cams_imx_mb1854/st_b_cams_imx_mb1854.overlay

drivers/video/imx335.c

Introduction of bindings for the imx335 sensor. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

Add support for the Sony IMX335 CSI sensor. This sensor supports resolution of 2592x1944 in RGGB bayer format either 10 or 12 bits and using 2 or 4 CSI lanes. For the time being only 10 bits on 2 CSI lanes is supported via this commit. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

Add RaspberryPi CSI 22 pins camera connector description. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

Introduce the B_CAMS_IMX camera shield for STM32 which embeds a IMX335 camera sensor. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

sonarqubecloud · 2025-05-22T07:58:49Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

drivers/video/imx335.c

josuah

Only very minor discussions left, nothing blocking anymore for my part. 👍

boards/shields/st_b_cams_imx_mb1854/st_b_cams_imx_mb1854.overlay

josuah · 2025-05-22T11:33:55Z

Changing the assignees based on their feedback. Do feel free to revert that if that was a mistake.

github-actions bot added area: Video Video subsystem platform: NXP Drivers NXP Semiconductors, drivers labels Apr 19, 2025

github-actions bot requested review from dbaluta, decsny, dleach02, josuah, loicpoulain, manuargue, mmahadevan108, ngphibang and Raymond0225 April 19, 2025 21:24

github-actions bot assigned dleach02 and mmahadevan108 Apr 19, 2025

josuah previously approved these changes Apr 19, 2025

View reviewed changes

avolmat-st dismissed josuah’s stale review via 442eb3e April 20, 2025 13:29

avolmat-st force-pushed the video_imx335_driver branch from 95979fe to 442eb3e Compare April 20, 2025 13:29

josuah previously approved these changes Apr 20, 2025

View reviewed changes

josuah mentioned this pull request Apr 20, 2025

drivers: video: common: introduce CCI utilities #87935

Merged

josuah added platform: STM32 ST Micro STM32 area: camera labels Apr 20, 2025

ngphibang reviewed Apr 20, 2025

View reviewed changes

drivers/video/imx335.c Outdated Show resolved Hide resolved

drivers/video/imx335.c Outdated Show resolved Hide resolved

josuah marked this pull request as draft April 20, 2025 18:33

ngphibang mentioned this pull request Apr 21, 2025

Implement video control framework #82158

Merged

josuah self-requested a review April 21, 2025 15:54

avolmat-st dismissed josuah’s stale review via 50bbd8c April 23, 2025 20:39

github-actions bot requested review from jfischer-no and kartben May 19, 2025 21:29

avolmat-st force-pushed the video_imx335_driver branch 2 times, most recently from 96d7e02 to 629703f Compare May 20, 2025 19:44

avolmat-st force-pushed the video_imx335_driver branch from 629703f to e59d591 Compare May 20, 2025 20:44

avolmat-st force-pushed the video_imx335_driver branch from e59d591 to e8b587c Compare May 20, 2025 21:40

avolmat-st force-pushed the video_imx335_driver branch from e8b587c to c493ab8 Compare May 21, 2025 06:13

josuah requested changes May 21, 2025

View reviewed changes

ngphibang reviewed May 21, 2025

View reviewed changes

drivers/video/imx335.c Outdated Show resolved Hide resolved

Alain Volmat added 4 commits May 21, 2025 23:14

dts: bindings: video: imx335: addition of imx335 description

7a57a31

Introduction of bindings for the imx335 sensor. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

dts: bindings: addition of RaspberryPi CSI 22 pins camera connector

d64f4dd

Add RaspberryPi CSI 22 pins camera connector description. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

shields: st_b_cams_imx_mb1854: add imx335 based MB1854 board shield

83e9d4b

Introduce the B_CAMS_IMX camera shield for STM32 which embeds a IMX335 camera sensor. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

avolmat-st force-pushed the video_imx335_driver branch from c493ab8 to 83e9d4b Compare May 22, 2025 07:29

josuah reviewed May 22, 2025

View reviewed changes

drivers/video/imx335.c Show resolved Hide resolved

josuah approved these changes May 22, 2025

View reviewed changes

boards/shields/st_b_cams_imx_mb1854/st_b_cams_imx_mb1854.overlay Show resolved Hide resolved

josuah assigned ngphibang and josuah and unassigned mmahadevan108 and dleach02 May 22, 2025

ngphibang approved these changes May 22, 2025

View reviewed changes

kartben merged commit d0240ab into zephyrproject-rtos:main May 22, 2025
26 checks passed

josuah mentioned this pull request Jul 4, 2025

drivers: video: image sensor list (from Linux, others) #76378

Open

Video: addition of the imx335 driver #88825

Video: addition of the imx335 driver #88825

Uh oh!

Conversation

avolmat-st commented Apr 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josuah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josuah commented Apr 19, 2025

Uh oh!

avolmat-st commented Apr 20, 2025

Uh oh!

ngphibang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

josuah commented Apr 20, 2025

Uh oh!

josuah commented Apr 21, 2025

Uh oh!

avolmat-st commented Apr 23, 2025

Uh oh!

avolmat-st commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avolmat-st commented May 20, 2025

Uh oh!

avolmat-st commented May 20, 2025

Uh oh!

avolmat-st commented May 20, 2025

Uh oh!

avolmat-st commented May 21, 2025

Uh oh!

josuah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avolmat-st May 21, 2025 • edited by josuah Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngphibang May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avolmat-st May 22, 2025 • edited by josuah Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avolmat-st commented Apr 19, 2025 •

edited

Loading

avolmat-st commented May 19, 2025 •

edited

Loading

avolmat-st May 21, 2025 •

edited by josuah

Loading

ngphibang May 21, 2025 •

edited

Loading

avolmat-st May 22, 2025 •

edited by josuah

Loading

josuah left a comment •

edited

Loading