linux-kernel - Re: [PATCH v2 2/2] media: i2c: add os05b10 image sensor driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <umcwwm4kqnh3cdgg65zxjhzlgbwh34yd55h2rwqe7xugmsz67d@3uvgtsp6lhgh>
Date: Thu, 11 Dec 2025 12:15:15 +0100
From: Mehdi Djait <mehdi.djait@...ux.intel.com>
To: Himanshu Bhavani <himanshu.bhavani@...iconsignals.io>
Cc: robh@...nel.org, krzk+dt@...nel.org, sakari.ailus@...ux.intel.com, 
	Elgin Perumbilly <elgin.perumbilly@...iconsignals.io>, Mauro Carvalho Chehab <mchehab@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Hans Verkuil <hverkuil@...nel.org>, 
	Hans de Goede <hansg@...nel.org>, Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>, André Apitzsch <git@...tzsch.eu>, 
	Dongcheng Yan <dongcheng.yan@...el.com>, Sylvain Petinot <sylvain.petinot@...s.st.com>, 
	Benjamin Mugnier <benjamin.mugnier@...s.st.com>, Bryan O'Donoghue <bryan.odonoghue@...aro.org>, 
	Svyatoslav Ryhel <clamor95@...il.com>, Heimir Thor Sverrisson <heimir.sverrisson@...il.com>, 
	Hardevsinh Palaniya <hardevsinh.palaniya@...iconsignals.io>, linux-media@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] media: i2c: add os05b10 image sensor driver

Hello Himanshu,

Thank you for the patch!

On Thu, Dec 11, 2025 at 04:24:07PM +0530, Himanshu Bhavani wrote:
> Add a v4l2 subdevice driver for the Omnivision OS05B10 sensor.
> 
> The Omnivision OS05B10 image sensor with an active
> array size of 2592 x 1944.
> 
> The following features are supported:
> - Manual exposure an gain control support
> - vblank/hblank control support
> - Supported resolution: 2592 x 1944 @ 60fps (SBGGR10)
> 
> Co-developed-by: Elgin Perumbilly <elgin.perumbilly@...iconsignals.io>
> Signed-off-by: Elgin Perumbilly <elgin.perumbilly@...iconsignals.io>
> Signed-off-by: Himanshu Bhavani <himanshu.bhavani@...iconsignals.io>

[..]

> 
> +config VIDEO_OS05B10
> +        tristate "OmniVision OS05B10 sensor support"
> +        select V4L2_CCI_I2C
> +        help
> +          This is a Video4Linux2 sensor driver for Omnivision
> +          OS05B10 camera sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +          module's name is ov9734.

ov9734 ? copy-paste error ?

> +

[..]

> +static int os05b10_probe(struct i2c_client *client)
> +{
> +	struct os05b10 *os05b10;
> +	unsigned int xclk_freq;
> +	int ret;
> +
> +	os05b10 = devm_kzalloc(&client->dev, sizeof(*os05b10), GFP_KERNEL);
> +	if (!os05b10)
> +		return -ENOMEM;
> +
> +	os05b10->client = client;
> +	os05b10->dev = &client->dev;
> +
> +	v4l2_i2c_subdev_init(&os05b10->sd, client, &os05b10_subdev_ops);
> +
> +	os05b10->cci = devm_cci_regmap_init_i2c(client, 16);
> +	if (IS_ERR(os05b10->cci))
> +		return dev_err_probe(os05b10->dev, PTR_ERR(os05b10->cci),
> +				     "failed to initialize CCI\n");
> +
> +	/* Get system clock (xclk) */

I don't think this comment adds value

> +	os05b10->xclk = devm_clk_get(os05b10->dev, NULL);

Use devm_v4l2_sensor_clk_get() to get the clock

> +	if (IS_ERR(os05b10->xclk))
> +		return dev_err_probe(os05b10->dev, PTR_ERR(os05b10->xclk),
> +				     "failed to get xclk\n");
> +
> +	xclk_freq = clk_get_rate(os05b10->xclk);
> +	if (xclk_freq != OS05B10_XCLK_FREQ)
> +		return dev_err_probe(os05b10->dev, -EINVAL,
> +				     "xclk frequency not supported: %d Hz\n",
> +				     xclk_freq);
> +
> +	ret = os05b10_get_regulators(os05b10);
> +	if (ret)
> +		return dev_err_probe(os05b10->dev, ret, "failed to get regulators\n");
> +
> +	ret = os05b10_parse_endpoint(os05b10);
> +	if (ret) {
> +		dev_err_probe(os05b10->dev, ret,
> +			      "failed to parse endpoint configuration\n");
> +		return ret;

return dev_err_probe() like above.

> +	}
> +
> +	os05b10->reset_gpio = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(os05b10->reset_gpio))
> +		return dev_err_probe(os05b10->dev, PTR_ERR(os05b10->reset_gpio),
> +				     "failed to get reset GPIO\n");
> +

--
Kind Regards
Mehdi Djait