drivers: video: Add Himax HM0360 camera sensor driver #94904

mjs513 · 2025-08-24T12:07:40Z

The camera supports grayscale in qqvga, qvga and vga resolutions. It has been tested on a arduino giga r1.

drivers/video/hm0360.c

sonarqubecloud · 2025-08-25T15:46:44Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

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

See analysis details on SonarQube Cloud

KurtE · 2025-10-11T13:53:55Z

@mjs513 @NinoSc @josuah - Was just curious about what the status of the hm01b0(#94894) and hm0360 (this one).
Was looking to see which ones to try again to work with multiple cameras defined...

josuah · 2025-10-11T20:17:44Z

#94894 is only needing a 2nd review... but before some merge conflicts need to be addressed.

For this current PR, I just need to find a free slice of time as I'm pretty busy and late with work on USB lately.
I do take note that this is being slow, and wish to go back to this as soon as I get a chance!

Thanks for the patience, and apologies for such delay!

mjs513 · 2025-10-15T20:28:59Z

@KurtE - @josuah

Since no one has looked at this yet I had some time from the OV7675 driver so I kind of redid the HM0360 with some lessons learned (probably still have indent issues) and also updated it to use the video cci commands. Figured that would be a better way to go.

Mike

drivers: video Add Himax HM0360 camera sensor driver Adds support for the HM0360 camera. Signed-off-by: Mike S <[email protected]>

sonarqubecloud · 2025-10-16T00:51:38Z

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

josuah

Thank you for this driver!

I have added a few comments, I hope you find them relevant.

You might also want to look at this PR which has already been reviewed, but not yet merged, still could bring some illustrations of how to use the newest video APIs:

https://github.com/zephyrproject-rtos/zephyr/pull/94894/files

josuah · 2025-10-16T09:25:38Z

drivers/video/hm0360.c

This might have been imported from another driver: SCCB is an OmniVision-specific term, and I think there was no issue with Himax versions.

josuah · 2025-10-16T09:26:35Z

drivers/video/hm0360.c

I think Zephyr/Linux typically uses snake_case. In some cases CamelCase is needed though (i.e. use same identifiers as standard documents).

josuah · 2025-10-16T09:27:40Z

drivers/video/hm0360.c

Here, you are reading 2 identifiers following each others and aggregating them in a single uint16_t, good use-case for a HM0360_REG16 and read both at the same time! :)

josuah · 2025-10-16T09:36:44Z

drivers/video/hm0360.c

The Linux CCI library is used so that the user does not have to even think about the size of registers.
Here is how it is possible to use it:

#define HM0360_CCI_TEST_PATTERN_MODE HM0360_REG8(0x0601) ... video_write_cci_reg(&config->bus, HM0360_CCI_TEST_PATTERN_MODE, ctrls->test_pattern.val);

This way, the size of registers is encoded in the registers themselves, and it is possible to forget about low-level details such as address size, data size, or even endianess.

josuah · 2025-10-16T09:38:29Z

drivers/video/hm0360.c

How about naming it HM0360_REG8()? This way, this hides the fact that the address are 16-bit, which becomes an implementation detail.

If having a doubt about what is sent, the VIDEO_LOG_LEVEL_DBG=y will print verbosely everything sent on the I2C bus, this way no ambiguity possible.

josuah · 2025-10-16T10:20:10Z

drivers/video/hm0360.c

This is already checked by the video_format_caps_index function, so this can be removed.

josuah · 2025-10-16T10:23:05Z

drivers/video/hm0360.c

How about something like this?

Suggested change

highres = (fmt->width > 320);

That is, if we would want to validate the input, it is possible to do so before the requests hit the driver, so no need to add the validation effort here IMHO. Open to alternative/discussion!

josuah · 2025-10-16T10:27:35Z

drivers/video/hm0360.c

Minor detail: since highres is a boolean, how about:

Suggested change

if (frmival->numerator <= 10) {

osc_div = highres ? 0x02 : 0x03;

} else if (frmival->numerator <= 30) {

osc_div = highres ? 0x01 : 0x02;

} else {

/* Set to the max possible FPS at this resolution. */

osc_div = highres ? 0x00 : 0x01;

}

josuah · 2025-10-16T10:36:58Z

drivers/video/hm0360.c

+	if (hm0360_set_frmival(dev, &frmival)) {
+		printk("ERROR: Unable to set up video format\n");
+		return false;
+	}


You might want to split the if() away from the function call so that you can return the original error message, false == 0 meaning "success".

Suggested change

if (hm0360_set_frmival(dev, &frmival)) {

printk("ERROR: Unable to set up video format\n");

return false;

}

ret = hm0360_set_frmival(dev, &frmival);

if (ret < 0) {

LOG_ERR("ERROR: Unable to set up video format");

return ret;

}

josuah · 2025-10-16T10:40:44Z

drivers/video/hm0360.c

+		ret |= video_write_cci_reg(&config->bus, HM0360_REG_A16D8(HM0360_COMMAND_UPDATE),
+					   0x01);


It is tempting to do ret |= to reduce the volume of error handling code, but this unfortunately does not work as the error code is not just a boolean but carries information about the type of error (i.e. -IEO from I2C bus, or -EINVAL if the I2C peripheral is misconfigured).

Would it be possible to convert to the more verbose, but more correct idiom?

Suggested change

ret |= video_write_cci_reg(&config->bus, HM0360_REG_A16D8(HM0360_COMMAND_UPDATE),

0x01);

ret = video_write_cci_reg(&config->bus, HM0360_REG_A16D8(HM0360_TEST_PATTERN_MODE),

ctrls->test_pattern.val & 0x01);

if (ret < 0) {

return ret;

}

ret = video_write_cci_reg(&config->bus, HM0360_REG_A16D8(HM0360_COMMAND_UPDATE),

0x01);

if (ret < 0) {

return ret;

}

As an alternative, you can define a local scope for the case and define a struct video_reg { {...}, {...}, }; with everything, and do a single return write_cci_multiregs() with the whole table.

Both are equivalent.

zephyrbot added the area: Video Video subsystem label Aug 24, 2025

zephyrbot requested review from avolmat-st, josuah, loicpoulain and ngphibang August 24, 2025 12:08

zephyrbot assigned josuah and ngphibang Aug 24, 2025

mjs513 changed the title ~~Add Himax HM0360 camera sensor driver~~ drivers: video: Add Himax HM0360 camera sensor driver Aug 24, 2025

dsseng reviewed Aug 24, 2025

View reviewed changes

drivers/video/hm0360.c Show resolved Hide resolved

mjs513 force-pushed the add-HM0360-video-driver branch from f702fc8 to de61b54 Compare August 25, 2025 15:20

mjs513 force-pushed the add-HM0360-video-driver branch from de61b54 to 23e4782 Compare October 15, 2025 20:26

zephyrbot added area: Devicetree Bindings area: Boards/SoCs labels Oct 15, 2025

mjs513 force-pushed the add-HM0360-video-driver branch from 23e4782 to 77c69c7 Compare October 15, 2025 23:17

drivers: video Add Himax HM0360 camera sensor driver

38143b3

drivers: video Add Himax HM0360 camera sensor driver Adds support for the HM0360 camera. Signed-off-by: Mike S <[email protected]>

mjs513 force-pushed the add-HM0360-video-driver branch from 77c69c7 to 38143b3 Compare October 16, 2025 00:17

josuah reviewed Oct 16, 2025

View reviewed changes

+	if (frmival->numerator <= 10) {
+		osc_div = highres ? 0x02 : 0x03;
+	} else if (frmival->numerator <= 30) {
+		osc_div = highres ? 0x01 : 0x02;
+	} else {
+		/* Set to the max possible FPS at this resolution. */
+		osc_div = highres ? 0x00 : 0x01;
+	}

-	if (hm0360_set_frmival(dev, &frmival)) {
-		printk("ERROR: Unable to set up video format\n");
-		return false;
-	}
+	ret = hm0360_set_frmival(dev, &frmival);
+	if (ret < 0) {
+		LOG_ERR("ERROR: Unable to set up video format");
+		return ret;
+	}

		ret \|= video_write_cci_reg(&config->bus, HM0360_REG_A16D8(HM0360_COMMAND_UPDATE),
		0x01);

drivers: video: Add Himax HM0360 camera sensor driver #94904

Are you sure you want to change the base?

drivers: video: Add Himax HM0360 camera sensor driver #94904

Conversation

mjs513 commented Aug 24, 2025

Uh oh!

Uh oh!

sonarqubecloud bot commented Aug 25, 2025

Quality Gate passed

Uh oh!

KurtE commented Oct 11, 2025

Uh oh!

josuah commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjs513 commented Oct 15, 2025

Uh oh!

sonarqubecloud bot commented Oct 16, 2025

Quality Gate passed

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!

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

josuah commented Oct 11, 2025 •

edited

Loading