| 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
| ||
|
Message-ID: <aVPEkhP2587xcF78@kekkonen.localdomain>
Date: Tue, 30 Dec 2025 14:24:50 +0200
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Himanshu Bhavani <himanshu.bhavani@...iconsignals.io>
Cc: robh@...nel.org, krzk+dt@...nel.org,
Elgin Perumbilly <elgin.perumbilly@...iconsignals.io>,
Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Hans Verkuil <hverkuil@...nel.org>,
Hans de Goede <hansg@...nel.org>,
Mehdi Djait <mehdi.djait@...ux.intel.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
André Apitzsch <git@...tzsch.eu>,
Benjamin Mugnier <benjamin.mugnier@...s.st.com>,
Sylvain Petinot <sylvain.petinot@...s.st.com>,
Dongcheng Yan <dongcheng.yan@...el.com>,
Svyatoslav Ryhel <clamor95@...il.com>,
Jingjing Xiong <jingjing.xiong@...el.com>,
Heimir Thor Sverrisson <heimir.sverrisson@...il.com>,
linux-media@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 2/2] media: i2c: add os05b10 image sensor driver
Hi Himanshu,
Thanks for the set. A few comments below...
On Fri, Dec 19, 2025 at 02:15:19PM +0530, Himanshu Bhavani wrote:
> +static int os05b10_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> +{
> + struct os05b10 *os05b10 = to_os05b10(sd);
> + int ret;
> +
> + ret = cci_write(os05b10->cci, OS05B10_REG_CTRL_MODE,
> + OS05B10_MODE_STANDBY, NULL);
> + if (ret)
> + dev_err(os05b10->dev, "%s failed to set stream off\n", __func__);
I'd drop the function name here, it's obvious where this comes from. Same
elsewhere.
> +
> + pm_runtime_put(os05b10->dev);
> +
> + return ret;
> +}
> +
> +static int os05b10_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state)
> +{
> + struct v4l2_mbus_framefmt *format;
> + const struct os05b10_mode *mode;
> +
> + /* Initialize try_fmt */
> + format = v4l2_subdev_state_get_format(state, 0);
> +
> + mode = &supported_modes_10bit[0];
> + format->code = MEDIA_BUS_FMT_SBGGR10_1X10;
> +
> + /* Update image pad formate */
> + format->width = mode->width;
> + format->height = mode->height;
> + format->field = V4L2_FIELD_NONE;
> + format->colorspace = V4L2_COLORSPACE_RAW;
> + format->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> + format->xfer_func = V4L2_XFER_FUNC_NONE;
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops os05b10_video_ops = {
> + .s_stream = v4l2_subdev_s_stream_helper,
> +};
> +
> +static const struct v4l2_subdev_pad_ops os05b10_pad_ops = {
> + .enum_mbus_code = os05b10_enum_mbus_code,
> + .get_fmt = v4l2_subdev_get_fmt,
> + .set_fmt = os05b10_set_pad_format,
> + .get_selection = os05b10_get_selection,
> + .enum_frame_size = os05b10_enum_frame_size,
> + .enable_streams = os05b10_enable_streams,
> + .disable_streams = os05b10_disable_streams,
> +};
> +
> +static const struct v4l2_subdev_internal_ops os05b10_internal_ops = {
> + .init_state = os05b10_init_state,
> +};
> +
> +static const struct v4l2_subdev_ops os05b10_subdev_ops = {
> + .video = &os05b10_video_ops,
> + .pad = &os05b10_pad_ops,
> +};
> +
> +static const struct v4l2_ctrl_ops os05b10_ctrl_ops = {
> + .s_ctrl = os05b10_set_ctrl,
> +};
> +
> +static int os05b10_identify_module(struct os05b10 *os05b10)
> +{
> + int ret;
> + u64 val;
> +
> + ret = cci_read(os05b10->cci, OS05B10_REG_CHIP_ID, &val, NULL);
> + if (ret)
> + return dev_err_probe(os05b10->dev, ret,
> + "failed to read chip id %x\n",
> + OS05B10_CHIP_ID);
> +
> + if (val != OS05B10_CHIP_ID)
> + return dev_err_probe(os05b10->dev, -ENODEV,
> + "chip id mismatch: %x!=%llx\n",
> + OS05B10_CHIP_ID, val);
> +
> + return 0;
> +}
> +
> +static int os05b10_power_on(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct os05b10 *os05b10 = to_os05b10(sd);
> + unsigned long delay_us;
> + int ret;
> +
> + /* Enable power rails */
> + ret = regulator_bulk_enable(ARRAY_SIZE(os05b10_supply_name),
> + os05b10->supplies);
> + if (ret) {
> + dev_err(os05b10->dev, "failed to enable regulators\n");
> + return ret;
> + }
> +
> + /* Enable xclk */
> + ret = clk_prepare_enable(os05b10->xclk);
> + if (ret) {
> + dev_err(os05b10->dev, "failed to enable clock\n");
> + goto err_regulator_off;
> + }
> +
> + gpiod_set_value_cansleep(os05b10->reset_gpio, 0);
> +
> + /* Delay T1 */
> + fsleep(5 * USEC_PER_MSEC);
> +
> + /* Delay T2 (8192 cycles before SCCB/I2C access) */
> + delay_us = DIV_ROUND_UP(8192, OS05B10_XCLK_FREQ / 1000 / 1000);
> + usleep_range(delay_us, delay_us * 2);
> +
> + return 0;
> +
> +err_regulator_off:
> + regulator_bulk_disable(ARRAY_SIZE(os05b10_supply_name),
> + os05b10->supplies);
> +
> + return ret;
> +}
> +
> +static int os05b10_power_off(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct os05b10 *os05b10 = to_os05b10(sd);
> +
> + gpiod_set_value_cansleep(os05b10->reset_gpio, 1);
> +
> + regulator_bulk_disable(ARRAY_SIZE(os05b10_supply_name), os05b10->supplies);
Can you run
$ ./scripts/checkpatch.pl --strict --max-line-length=80
on the set, please?
> + clk_disable_unprepare(os05b10->xclk);
> +
> + return 0;
> +}
> +
> +static int os05b10_parse_endpoint(struct os05b10 *os05b10)
> +{
> + struct v4l2_fwnode_endpoint bus_cfg = {
> + .bus_type = V4L2_MBUS_CSI2_DPHY
> + };
> + unsigned long link_freq_bitmap;
> + struct fwnode_handle *ep;
> + int ret;
> +
> + ep = fwnode_graph_get_next_endpoint(dev_fwnode(os05b10->dev), NULL);
Please use fwnode-graph_get_endpoint_by_id() instead.
> + if (!ep) {
> + dev_err(os05b10->dev, "Failed to get next endpoint\n");
> + return -ENXIO;
> + }
> +
> + ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
> + fwnode_handle_put(ep);
> + if (ret)
> + return ret;
> +
> + if (bus_cfg.bus.mipi_csi2.num_data_lanes != 4) {
> + ret = dev_err_probe(os05b10->dev, -EINVAL,
> + "only 4 data lanes are supported\n");
> + goto error_out;
> + }
> +
> + ret = v4l2_link_freq_to_bitmap(os05b10->dev, bus_cfg.link_frequencies,
> + bus_cfg.nr_of_link_frequencies,
> + link_frequencies,
> + ARRAY_SIZE(link_frequencies),
> + &link_freq_bitmap);
> +
> + if (ret)
> + dev_err(os05b10->dev, "only 600MHz frequency is available\n");
> +
> +error_out:
> + v4l2_fwnode_endpoint_free(&bus_cfg);
> +
> + return ret;
> +}
--
Kind regards,
Sakari Ailus
Powered by blists - more mailing lists