Re: [PATCH 2/2] media: i2c: hm1092: add Himax HM1092 mono NIR sensor driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The driver incorrectly uses `pm_sleep_ptr()` instead of `pm_ptr()` for the `dev_pm_ops` structure pointer, silently breaking Runtime PM when `CONFIG_PM_SLEEP` is disabled.
- [Medium] V4L2 control handler memory and lock resources are leaked on initialization error paths in `hm1092_init_controls()`.
--

commit 355cde57de68093fd9275a1bf61c01ff0eed341f
Author: Ramshouriesh R <rshouriesh@gmail.com>

media: i2c: hm1092: add Himax HM1092 mono NIR sensor driver

Add a V4L2 subdev driver for the Himax HM1092, a 1 megapixel monochrome
near-infrared image sensor with a single-lane MIPI CSI-2 interface.

> diff --git a/drivers/media/i2c/hm1092.c b/drivers/media/i2c/hm1092.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/media/i2c/hm1092.c
[ ... ]
> +static int hm1092_init_controls(struct hm1092 *hm1092)
> +{
> +	struct v4l2_ctrl_handler *ctrl_hdlr = &hm1092->ctrl_handler;
> +	const struct hm1092_mode *mode = &hm1092_mode_560x360;
> +	struct v4l2_fwnode_device_properties props;
> +	s64 hblank, pixel_rate;
> +	int ret;
> +
> +	v4l2_ctrl_handler_init(ctrl_hdlr, 6);
[ ... ]
> +	v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &hm1092_ctrl_ops,
> +				     V4L2_CID_TEST_PATTERN,
> +				     ARRAY_SIZE(hm1092_test_pattern_menu) - 1,
> +				     0, 0, hm1092_test_pattern_menu);
> +
> +	ret = v4l2_fwnode_device_parse(hm1092->dev, &props);
> +	if (ret)
> +		return ret;

[Severity: Medium]
Could this leak the control handler memory and lock resources if 
v4l2_fwnode_device_parse() fails?

When hm1092_probe() calls hm1092_init_controls() and it fails here,
probe jumps straight to err_power_off, bypassing v4l2_ctrl_handler_free().

> +
> +	v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &hm1092_ctrl_ops, &props);
> +
> +	if (ctrl_hdlr->error)
> +		return ctrl_hdlr->error;

[Severity: Medium]
Similarly, if control creation fails, does returning ctrl_hdlr->error 
directly leave the allocated control handler resources orphaned?

[ ... ]
> +static struct i2c_driver hm1092_i2c_driver = {
> +	.driver = {
> +		.name = "hm1092",
> +		.pm = pm_sleep_ptr(&hm1092_pm_ops),

[Severity: High]
Will this silently drop the Runtime PM callbacks when the kernel is compiled
with CONFIG_PM=y but CONFIG_PM_SLEEP=n?

According to the power management subsystem guidelines, pm_sleep_ptr()
should be used for individual sleep callbacks, while pm_ptr() should be
used for the dev_pm_ops structure pointer. If pm_sleep_ptr() evaluates
to NULL here, the sensor hardware might not power on during
hm1092_enable_streams().

> +		.of_match_table = hm1092_of_match,
> +	},
> +	.probe = hm1092_probe,
> +	.remove = hm1092_remove,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-hm1092-driver-v1-0-d1bd81e233b5@gmail.com?part=2