| 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:
<PN3P287MB18291BB4CCC68C8BA1260A628BFAA@PN3P287MB1829.INDP287.PROD.OUTLOOK.COM>
Date: Wed, 29 Oct 2025 15:19:27 +0000
From: Tarang Raval <tarang.raval@...iconsignals.io>
To: Svyatoslav Ryhel <clamor95@...il.com>, Sakari Ailus
<sakari.ailus@...ux.intel.com>
CC: Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Hans Verkuil <hverkuil@...all.nl>, Hans de Goede <hansg@...nel.org>, Arnd
Bergmann <arnd@...db.de>, Dongcheng Yan <dongcheng.yan@...el.com>,
André Apitzsch <git@...tzsch.eu>, Sylvain Petinot
<sylvain.petinot@...s.st.com>, Benjamin Mugnier
<benjamin.mugnier@...s.st.com>, Heimir Thor Sverrisson
<heimir.sverrisson@...il.com>, "linux-media@...r.kernel.org"
<linux-media@...r.kernel.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2 RESEND] media: i2c: add Sony IMX111 CMOS camera
sensor driver
Hi Svyatoslav,
> > On Tue, Oct 28, 2025 at 11:22:00AM +0200, Svyatoslav Ryhel wrote:
> > > Add a v4l2 sub-device driver for the Sony IMX111 image sensor. This is a
> > > camera sensor using the i2c bus for control and the csi-2 bus for data.
> > >
> > > The following features are supported:
> > > - manual exposure, digital and analog gain control support
> > > - pixel rate/link freq control support
> > > - supported resolution up to 3280x2464 for single shot capture
> > > - supported resolution up to 1920x1080 @ 30fps for video
> > > - supported bayer order output SGBRG10 and SGBRG8
> > >
> > > Camera module seems to be partially compatible with Nokia SMIA but it
> > > lacks a few registers required for clock calculations and has different
> > > vendor-specific per-mode configurations which makes it incompatible with
> > > existing CCS driver.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@...il.com>
...
> > > +#include <linux/clk.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/media.h>
> > > +#include <linux/module.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/ratelimit.h>
> > > +#include <linux/regmap.h>
> > > +#include <linux/regulator/consumer.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/string.h>
> > > +#include <linux/types.h>
> > > +#include <linux/videodev2.h>
> > > +#include <linux/units.h>
> > > +
> > > +#include <media/media-entity.h>
> > > +#include <media/v4l2-async.h>
> > > +#include <media/v4l2-cci.h>
> > > +#include <media/v4l2-ctrls.h>
> > > +#include <media/v4l2-device.h>
> > > +#include <media/v4l2-fwnode.h>
> > > +#include <media/v4l2-event.h>
> > > +#include <media/v4l2-image-sizes.h>
> > > +#include <media/v4l2-subdev.h>
> > > +#include <media/v4l2-mediabus.h>
A few of those headers seem to be unused and can be removed
Like:
#include <linux/ratelimit.h>
#include <linux/slab.h>
#include <linux/string.h>
#include <media/v4l2-event.h>
#include <media/v4l2-image-sizes.h>
...
> > > +/* product information registers */
> > > +#define IMX111_PRODUCT_ID CCI_REG16(0x0000)
> > > +#define IMX111_CHIP_ID 0x111
> > > +#define IMX111_REVISION CCI_REG8(0x0002)
> > > +#define IMX111_MANUFACTURER_ID CCI_REG8(0x0003)
> > > +#define IMX111_SMIA_VER CCI_REG8(0x0004)
> > > +#define IMX111_FRAME_COUNTER CCI_REG8(0x0005)
> > > +#define IMX111_PIXEL_ORDER CCI_REG8(0x0006)
> > > +
> > > +/* general configuration registers */
> > > +#define IMX111_STREAMING_MODE CCI_REG8(0x0100)
> > > +#define IMX111_MODE_STANDBY 0
> > > +#define IMX111_MODE_STREAMING 1
> > > +#define IMX111_IMAGE_ORIENTATION CCI_REG8(0x0101)
> > > +#define IMX111_IMAGE_HFLIP BIT(0)
> > > +#define IMX111_IMAGE_VFLIP BIT(1)
> > > +#define IMX111_SOFTWARE_RESET CCI_REG8(0x0103)
> > > +#define IMX111_RESET_ON 1
> > > +#define IMX111_GROUP_WRITE CCI_REG8(0x0104)
> > > +#define IMX111_GROUP_WRITE_ON 1
> > > +#define IMX111_FRAME_DROP CCI_REG8(0x0105)
> > > +#define IMX111_FRAME_DROP_ON 1
> > > +#define IMX111_CHANNEL_ID CCI_REG8(0x0110)
> > > +#define IMX111_SIGNALLING_MODE CCI_REG8(0x0111)
> > > +#define IMX111_DATA_DEPTH CCI_REG16(0x0112)
> > > +#define IMX111_DATA_DEPTH_RAW8 0x08
> > > +#define IMX111_DATA_DEPTH_RAW10 0x0a
> > > +
> > > +/* integration time registers */
> > > +#define IMX111_INTEGRATION_TIME CCI_REG16(0x0202)
> > > +#define IMX111_INTEGRATION_TIME_MIN 0x1
> > > +#define IMX111_INTEGRATION_TIME_MAX 0xffff
> > > +#define IMX111_INTEGRATION_TIME_STEP 1
> > > +
> > > +/* analog gain control */
> > > +#define IMX111_REG_ANALOG_GAIN CCI_REG8(0x0205)
> > > +#define IMX111_ANA_GAIN_MIN 0
> > > +#define IMX111_ANA_GAIN_MAX 240
> > > +#define IMX111_ANA_GAIN_STEP 1
> > > +#define IMX111_ANA_GAIN_DEFAULT 0
> > > +
> > > +/* digital gain control */
> > > +#define IMX111_REG_DIG_GAIN_GREENR CCI_REG16(0x020e)
> > > +#define IMX111_REG_DIG_GAIN_RED CCI_REG16(0x0210)
> > > +#define IMX111_REG_DIG_GAIN_BLUE CCI_REG16(0x0212)
> > > +#define IMX111_REG_DIG_GAIN_GREENB CCI_REG16(0x0214)
> > > +#define IMX111_DGTL_GAIN_MIN 0x0100
> > > +#define IMX111_DGTL_GAIN_MAX 0x0fff
> > > +#define IMX111_DGTL_GAIN_DEFAULT 0x0100
> > > +#define IMX111_DGTL_GAIN_STEP 1
> > > +
> > > +/* clock configuration registers */
> > > +#define IMX111_PIXEL_CLK_DIVIDER_PLL1 CCI_REG8(0x0301) /* fixed to 10 */
> > > +#define IMX111_SYSTEM_CLK_DIVIDER_PLL1 CCI_REG8(0x0303) /* fixed to 1 */
> > > +#define IMX111_PRE_PLL_CLK_DIVIDER_PLL1 CCI_REG8(0x0305)
> > > +#define IMX111_PLL_MULTIPLIER_PLL1 CCI_REG8(0x0307)
> > > +#define IMX111_PLL_SETTLING_TIME CCI_REG8(0x303c)
> > > +#define IMX111_PLL_SETTLING_TIME_DEFAULT 200
> > > +#define IMX111_POST_DIVIDER CCI_REG8(0x30a4)
> > > +#define IMX111_POST_DIVIDER_DIV1 2
> > > +#define IMX111_POST_DIVIDER_DIV2 0
> > > +#define IMX111_POST_DIVIDER_DIV4 1
> > > +
> > > +/* frame timing registers */
> > > +#define IMX111_VERTICAL_TOTAL_LENGTH CCI_REG16(0x0340)
> > > +#define IMX111_HORIZONTAL_TOTAL_LENGTH CCI_REG16(0x0342)
> > > +
> > > +/* image size registers */
> > > +#define IMX111_HORIZONTAL_START CCI_REG16(0x0344)
> > > +#define IMX111_VERTICAL_START CCI_REG16(0x0346)
> > > +#define IMX111_HORIZONTAL_END CCI_REG16(0x0348)
> > > +#define IMX111_VERTICAL_END CCI_REG16(0x034a)
> > > +#define IMX111_IMAGE_WIDTH CCI_REG16(0x034c)
> > > +#define IMX111_IMAGE_HEIGHT CCI_REG16(0x034e)
In the mode register settings, you can use the above macros.
> > > +static const struct cci_reg_sequence mode_820x614[] = {
> > > + { CCI_REG8(0x0340), 0x04 }, { CCI_REG8(0x0341), 0xec },
> > > + { CCI_REG8(0x0342), 0x0d }, { CCI_REG8(0x0343), 0xd0 },
> > > + { CCI_REG8(0x0344), 0x00 }, { CCI_REG8(0x0345), 0x08 },
> > > + { CCI_REG8(0x0346), 0x00 }, { CCI_REG8(0x0347), 0x34 },
> > > + { CCI_REG8(0x0348), 0x0c }, { CCI_REG8(0x0349), 0xd7 },
> > > + { CCI_REG8(0x034a), 0x09 }, { CCI_REG8(0x034b), 0xcb },
> > > + { CCI_REG8(0x034c), 0x03 }, { CCI_REG8(0x034d), 0x34 },
> > > + { CCI_REG8(0x034e), 0x02 }, { CCI_REG8(0x034f), 0x66 },
Here, you can use those macros.
Likewise, in every mode.
> > > + { CCI_REG8(0x0381), 0x05 }, { CCI_REG8(0x0383), 0x03 },
> > > + { CCI_REG8(0x0385), 0x05 }, { CCI_REG8(0x0387), 0x03 },
> > > + { CCI_REG8(0x3033), 0x00 }, { CCI_REG8(0x303d), 0x10 },
> > > + { CCI_REG8(0x303e), 0x40 }, { CCI_REG8(0x3040), 0x08 },
> > > + { CCI_REG8(0x3041), 0x97 }, { CCI_REG8(0x3048), 0x01 },
> > > + { CCI_REG8(0x304c), 0x6f }, { CCI_REG8(0x304d), 0x03 },
> > > + { CCI_REG8(0x3064), 0x12 }, { CCI_REG8(0x3073), 0x00 },
> > > + { CCI_REG8(0x3074), 0x11 }, { CCI_REG8(0x3075), 0x11 },
> > > + { CCI_REG8(0x3076), 0x11 }, { CCI_REG8(0x3077), 0x11 },
> > > + { CCI_REG8(0x3079), 0x00 }, { CCI_REG8(0x307a), 0x00 },
> > > + { CCI_REG8(0x309b), 0x28 }, { CCI_REG8(0x309c), 0x13 },
> > > + { CCI_REG8(0x309e), 0x00 }, { CCI_REG8(0x30a0), 0x14 },
> > > + { CCI_REG8(0x30a1), 0x09 }, { CCI_REG8(0x30aa), 0x03 },
> > > + { CCI_REG8(0x30b2), 0x03 }, { CCI_REG8(0x30d5), 0x09 },
> > > + { CCI_REG8(0x30d6), 0x00 }, { CCI_REG8(0x30d7), 0x00 },
> > > + { CCI_REG8(0x30d8), 0x00 }, { CCI_REG8(0x30d9), 0x00 },
> > > + { CCI_REG8(0x30de), 0x04 }, { CCI_REG8(0x30df), 0x20 },
> > > + { CCI_REG8(0x3102), 0x08 }, { CCI_REG8(0x3103), 0x22 },
> > > + { CCI_REG8(0x3104), 0x20 }, { CCI_REG8(0x3105), 0x00 },
> > > + { CCI_REG8(0x3106), 0x87 }, { CCI_REG8(0x3107), 0x00 },
> > > + { CCI_REG8(0x3108), 0x03 }, { CCI_REG8(0x3109), 0x02 },
> > > + { CCI_REG8(0x310a), 0x03 }, { CCI_REG8(0x315c), 0x9c },
> > > + { CCI_REG8(0x315d), 0x9b }, { CCI_REG8(0x316e), 0x9d },
> > > + { CCI_REG8(0x316f), 0x9c }, { CCI_REG8(0x3318), 0x7a },
> > > + { CCI_REG8(0x3348), 0xe0 },
> > > +};
...
> > > +static int imx111_init_controls(struct imx111 *sensor)
> > > +{
> > > + const struct v4l2_ctrl_ops *ops = &imx111_ctrl_ops;
> > > + struct device *dev = regmap_get_device(sensor->regmap);
> > > + struct v4l2_fwnode_device_properties props;
> > > + struct v4l2_subdev *sd = &sensor->sd;
> > > + struct v4l2_ctrl_handler *hdl = &sensor->hdl;
> > > + s64 pixel_rate_min, pixel_rate_max;
> > > + int i, ret;
> > > +
> > > + ret = v4l2_fwnode_device_parse(dev, &props);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + ret = v4l2_ctrl_handler_init(hdl, 13);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + pixel_rate_min = div_u64(sensor->pixel_clk_raw, 2 * IMX111_DATA_DEPTH_RAW10);
> > > + pixel_rate_max = div_u64(sensor->pixel_clk_raw, 2 * IMX111_DATA_DEPTH_RAW8);
> > > + sensor->pixel_rate = v4l2_ctrl_new_std(hdl, NULL, V4L2_CID_PIXEL_RATE,
> > > + pixel_rate_min, pixel_rate_max,
> > > + 1, div_u64(sensor->pixel_clk_raw,
> > > + 2 * sensor->data_depth));
> > > +
> > > + sensor->link_freq = v4l2_ctrl_new_int_menu(hdl, NULL, V4L2_CID_LINK_FREQ,
> > > + 0, 0, &sensor->default_link_freq);
> > > + if (sensor->link_freq)
> > > + sensor->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > > +
> > > + v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN,
> > > + IMX111_ANA_GAIN_MIN, IMX111_ANA_GAIN_MAX,
> > > + IMX111_ANA_GAIN_STEP, IMX111_ANA_GAIN_DEFAULT);
> > > +
> > > + v4l2_ctrl_new_std(hdl, ops, V4L2_CID_DIGITAL_GAIN,
> > > + IMX111_DGTL_GAIN_MIN, IMX111_DGTL_GAIN_MAX,
> > > + IMX111_DGTL_GAIN_STEP, IMX111_DGTL_GAIN_DEFAULT);
> > > +
> > > + sensor->hflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, 0);
> > > + if (sensor->hflip)
> > > + sensor->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> > > +
> > > + sensor->vflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, 0);
> > > + if (sensor->vflip)
> > > + sensor->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> >
> > Could you also add VBLANK and HBLANK controls, please?
> >
>
> I may try to, but since no datasheet is available, my suggestion may
> be quite arbitrary. In this iteration v/hblank are considered to be 0.
The registers IMX111_VERTICAL_TOTAL_LENGTH and IMX111_HORIZONTAL_TOTAL_LENGTH
are used to set the vertical and horizontal total lengths in every mode.
I believe you can use these registers to implement VBLANK and HBLANK.
You can find the appropriate values for these from the mode register settings.
Best Regards,
Tarang
Powered by blists - more mailing lists