Add ap1302 isp sensor driver by yongxu-wang15 · Pull Request #103541 · zephyrproject-rtos/zephyr

yongxu-wang15 · 2026-02-05T02:48:14Z

Add onnn ap1302 isp driver

Add basic driver for ON Semiconductor AP1302 Image Signal Processor. Advanced features are not implemented in this initial version. Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>

Add lpi2c3 and cmko1 pin for imx95_evk board Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>

Add device tree configuration for AP1302 ISP on i.MX95 EVK: - AP1302 on LPI2C3 (I2C address 0x3C) - ADP5585 and PCAL6408A GPIO expanders - Camera power regulators - MCLK clock configuration Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>

sonarqubecloud · 2026-02-05T03:18:11Z

Quality Gate passed

Issues
2 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

josuah

Good to see ISPs being contributed to Zephyr.

Here are a few feedback points for you to consider, feel free to discuss on each of them or suggest.

Thank you!

josuah · 2026-02-11T23:05:32Z

drivers/video/ap1302.c

+/* Register access functions */
+static int ap1302_write_reg16(const struct device *dev, uint16_t reg, uint16_t val)
+{
+	const struct ap1302_config *cfg = dev->config;
+	uint8_t buf[4];
+	int ret;
+
+	sys_put_be16(reg, &buf[0]);
+	sys_put_be16(val, &buf[2]);
+
+	ret = i2c_write_dt(&cfg->i2c, buf, sizeof(buf));
+	if (ret < 0) {
+		LOG_ERR("Failed to write reg 0x%04x: %d", reg, ret);
+	}
+
+	return ret;
+}
+
+static int ap1302_read_reg16(const struct device *dev, uint16_t reg, uint16_t *val)
+{
+	const struct ap1302_config *cfg = dev->config;
+	uint8_t reg_buf[2];
+	uint8_t val_buf[2];
+	int ret;
+
+	sys_put_be16(reg, reg_buf);
+
+	ret = i2c_write_read_dt(&cfg->i2c, reg_buf, sizeof(reg_buf),
+				val_buf, sizeof(val_buf));
+	if (ret < 0) {
+		LOG_ERR("Failed to read reg 0x%04x: %d", reg, ret);
+		return ret;
+	}
+
+	*val = sys_get_be16(val_buf);
+
+	return 0;
+}


This seems compatible with the MIPI CCI format, and there are utilities to send registers to camera in Zephyr which saves you the need to implement such wrapper function. Would that be interesting for you to use?

You may define the size your I2C target uses like so:

zephyr/drivers/video/imx335.c

Lines 65 to 67 in 2bef40e

#define IMX335_REG8(addr) ((addr) | VIDEO_REG_ADDR16_DATA8)

#define IMX335_REG16(addr) ((addr) | VIDEO_REG_ADDR16_DATA16_LE)

#define IMX335_REG24(addr) ((addr) | VIDEO_REG_ADDR16_DATA24_LE)

Then define register names along with their size (here only 16-bit it seems):

zephyr/drivers/video/imx335.c

Lines 74 to 80 in 2bef40e

#define IMX335_BCWAIT_TIME IMX335_REG8(0x300c)

#define IMX335_CPWAIT_TIME IMX335_REG8(0x300d)

#define IMX335_WINMODE IMX335_REG8(0x3018)

#define IMX335_HTRIMMING_START IMX335_REG16(0x302c)

#define IMX335_HNUM IMX335_REG16(0x302e)

#define IMX335_VMAX IMX335_REG24(0x3030)

#define IMX335_HMAX IMX335_REG16(0x3034)

Then use the functions from the CCI library to sendn I2C commands directly from uint16_t and so forth (passed as 32-bit values):

zephyr/drivers/video/imx335.c

Lines 393 to 396 in 2bef40e

ret = video_write_cci_reg(&cfg->i2c, IMX335_REGHOLD, 1);

if (ret < 0) {

return ret;

}

You do not need to specify the register size (i.e. 16-bit) anywhere, as it is already encoded in the register definition (as seen above).

josuah · 2026-02-11T23:10:17Z

drivers/video/ap1302.c

+		.reset_gpio = GPIO_DT_SPEC_INST_GET_OR(n, reset_gpios, {0}),			\
+		.standby_gpio = GPIO_DT_SPEC_INST_GET_OR(n, standby_gpios, {0}),		\
+		.isp_en_gpio = GPIO_DT_SPEC_INST_GET_OR(n, isp_en_gpios, {0}),			\
+		.firmware_address = DT_INST_PROP(n, firmware_address),				\


Is this firmware something loaded in a memory region?

It is possible that the user cannot know which memory region the firmware is stored in, for instance if their application only knows the name of the flash storage partition it is stored in, but not the actual address.

How about using a storage partition phandle?

josuah · 2026-02-11T23:16:34Z

dts/bindings/video/onnn,ap1302.yaml

+  mipi-data-lanes:
+    type: int
+    required: true
+    description: Number of MIPI CSI-2 data lanes (1-4)


This video driver is what corresponds to a "subdevice" in Linux: a device without a "enqueue()" and "dequeue()" interface, which allows users to control an external chip located upstream to the MIPI receiver of the local SoC.

For instance: an image sensor, an ISP.

This is handled via remote-endpoint-label, port, and endpoint like in Linux.

You might be able to look at MIPID02 which is a MIPI in DVP out device in the same situation as this driver firmware-wise:

https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/video/st%2Cmipid02.yaml

Then mipi-data-lanes and other properties can be taken from the existing .yaml files in Zephyr and just marked required: true.

josuah · 2026-02-11T23:23:03Z

drivers/video/ap1302.c

+static int ap1302_load_firmware(const struct device *dev)
+{


If the access to this binary blob is restricted, this means that this driver is not usable and I do not know if it is possible to merge it (I just need to check and discuss).

If the binary blob can be downloaded publicly, then here is the documentation in integrating binary blobs such as firmware into Zephyr https://docs.zephyrproject.org/latest/contribute/bin_blobs.html

Feel free to ask as many questions as you wish.

josuah · 2026-02-11T23:25:15Z

drivers/video/ap1302.c

+	if (fmt == NULL) {
+		return -EINVAL;
+	}


It is often not required to test the pointers passed at runtime as these are conventionally left for the caller to validate.

josuah · 2026-02-11T23:27:09Z

drivers/video/ap1302.c

+		return -EINVAL;
+	}
+
+	info = ap1302_find_format(fmt->pixelformat);


Are there many formats to support in the long term? If not, maybe a table with a single entry is not needed.

You might also be interested in validating that the format passed is valid according to the table of capabilities, which you can do using this:

zephyr/include/zephyr/drivers/video.h

Lines 894 to 905 in 2bef40e

/**

* @brief Search for a format that matches in a list of capabilities

*

* @param fmts The format capability list to search.

* @param fmt The format to find in the list.

* @param idx The pointer to a number of the first format that matches.

*

* @return 0 when a format is found.

* @return -ENOENT when no matching format is found.

*/

int video_format_caps_index(const struct video_format_cap *fmts, const struct video_format *fmt,

size_t *idx);

josuah · 2026-02-11T23:28:08Z

drivers/video/ap1302.c

+		return -ENOTSUP;
+	}
+
+	ap1302_sanitize_format(fmt);


Once video_format_caps_index() is called, and the return value is 0, the format is expected to be valid according to the list of supported formats of your driver.

josuah · 2026-02-11T23:29:21Z

drivers/video/ap1302.c

+	/* Calculate buffer parameters for YUYV (2 bytes per pixel) */
+	fmt->pitch = fmt->width * 2;
+	fmt->size = fmt->pitch * fmt->height;


There is a helper for doing it available in video_common.h (or maybe video.h).

josuah · 2026-02-11T23:30:47Z

dts/bindings/video/onnn,ap1302.yaml

+# SPDX-License-Identifier: Apache-2.0
+
+description: ONNN AP1302 Camera ISP
+


How about adding an example section to show how to integrate it on devicetree?

examples: - | ...

ngphibang

Thanks for adding this interesting driver.
Could you add a build_all test case for the driver ?
Could you also add support for this driver in the video (capture) sample ?
The ap1302 supports multi-sensors so it would be great to see this first multi-sensor use case in Zephyr, the video framework need to be extended for multi-sensor support, we can work together to see what is missing ...

yongxu-wang15 added 3 commits February 4, 2026 13:42

drivers: video: add ONNN AP1302 ISP driver

797d444

Add basic driver for ON Semiconductor AP1302 Image Signal Processor. Advanced features are not implemented in this initial version. Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>

boards: nxp: imx95_evk: add pinctrl for lpi2c3 and clko1

b79d3ef

Add lpi2c3 and cmko1 pin for imx95_evk board Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>

zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Boards/SoCs platform: NXP MPU platform: NXP NXP area: Video Video subsystem labels Feb 5, 2026

zephyrbot requested review from JarmouniA, JiafeiPan, Zhiqiang-Hou, avolmat-st, dbaluta, iuliana-prodan, josuah, loicpoulain, ngphibang and yangbolu1991 February 5, 2026 02:49

zephyrbot assigned josuah and ngphibang Feb 5, 2026

yongxu-wang15 mentioned this pull request Feb 5, 2026

NXP add imx mipi csi and phy driver #103542

Open

josuah reviewed Feb 11, 2026

View reviewed changes

ngphibang reviewed Feb 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ap1302 isp sensor driver#103541

Add ap1302 isp sensor driver#103541
yongxu-wang15 wants to merge 3 commits intozephyrproject-rtos:mainfrom
nxp-upstream:Add-ap1302-isp-sensor-driver

yongxu-wang15 commented Feb 5, 2026 •

edited

Loading

sonarqubecloud bot commented Feb 5, 2026

josuah left a comment

josuah Feb 11, 2026

josuah Feb 11, 2026

josuah Feb 11, 2026

josuah Feb 11, 2026

josuah Feb 11, 2026

josuah Feb 11, 2026

josuah Feb 11, 2026

josuah Feb 11, 2026

josuah Feb 11, 2026

ngphibang left a comment •

edited

Loading

Labels

4 participants

	#define IMX335_REG8(addr) ((addr) \| VIDEO_REG_ADDR16_DATA8)
	#define IMX335_REG16(addr) ((addr) \| VIDEO_REG_ADDR16_DATA16_LE)
	#define IMX335_REG24(addr) ((addr) \| VIDEO_REG_ADDR16_DATA24_LE)

	#define IMX335_BCWAIT_TIME IMX335_REG8(0x300c)
	#define IMX335_CPWAIT_TIME IMX335_REG8(0x300d)
	#define IMX335_WINMODE IMX335_REG8(0x3018)
	#define IMX335_HTRIMMING_START IMX335_REG16(0x302c)
	#define IMX335_HNUM IMX335_REG16(0x302e)
	#define IMX335_VMAX IMX335_REG24(0x3030)
	#define IMX335_HMAX IMX335_REG16(0x3034)

	ret = video_write_cci_reg(&cfg->i2c, IMX335_REGHOLD, 1);
	if (ret < 0) {
	return ret;
	}

	/**
	* @brief Search for a format that matches in a list of capabilities
	*
	* @param fmts The format capability list to search.
	* @param fmt The format to find in the list.
	* @param idx The pointer to a number of the first format that matches.
	*
	* @return 0 when a format is found.
	* @return -ENOENT when no matching format is found.
	*/
	int video_format_caps_index(const struct video_format_cap fmts, const struct video_format fmt,
	size_t *idx);

		# SPDX-License-Identifier: Apache-2.0

		description: ONNN AP1302 Camera ISP

Conversation

yongxu-wang15 commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

sonarqubecloud bot commented Feb 5, 2026

Quality Gate passed

josuah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngphibang left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Labels

4 participants

yongxu-wang15 commented Feb 5, 2026 •

edited

Loading

ngphibang left a comment •

edited

Loading