From: Tarang Raval <tarang.raval@siliconsignals.io>
To: Svyatoslav Ryhel <clamor95@gmail.com>,
Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: "Mauro Carvalho Chehab" <mchehab@kernel.org>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
"Conor Dooley" <conor+dt@kernel.org>,
"Hans Verkuil" <hverkuil@xs4all.nl>,
"Hans de Goede" <hansg@kernel.org>,
"Arnd Bergmann" <arnd@arndb.de>,
"Dongcheng Yan" <dongcheng.yan@intel.com>,
"André Apitzsch" <git@apitzsch.eu>,
"Sylvain Petinot" <sylvain.petinot@foss.st.com>,
"Benjamin Mugnier" <benjamin.mugnier@foss.st.com>,
"Heimir Thor Sverrisson" <heimir.sverrisson@gmail.com>,
"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/2 RESEND] media: i2c: add Sony IMX111 CMOS camera sensor driver
Date: Wed, 29 Oct 2025 15:19:27 +0000 [thread overview]
Message-ID: <PN3P287MB18291BB4CCC68C8BA1260A628BFAA@PN3P287MB1829.INDP287.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <CAPVz0n2SuOcoDn4KZ_zb4NCuaes6nppHRgffWC4yTHmOsbe1vw@mail.gmail.com>
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@gmail.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
next prev parent reply other threads:[~2025-10-29 15:19 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-28 9:21 [PATCH v2 0/2 " Svyatoslav Ryhel
2025-10-28 9:21 ` [PATCH v2 1/2 RESEND] dt-bindings: media: i2c: document Sony IMX111 CMOS sensor Svyatoslav Ryhel
2025-10-28 16:26 ` Sakari Ailus
2025-10-28 16:29 ` Svyatoslav Ryhel
2025-10-28 9:22 ` [PATCH v2 2/2 RESEND] media: i2c: add Sony IMX111 CMOS camera sensor driver Svyatoslav Ryhel
2025-10-28 17:58 ` Sakari Ailus
2025-10-28 18:57 ` Svyatoslav Ryhel
2025-10-29 15:19 ` Tarang Raval [this message]
2025-10-29 15:28 ` Svyatoslav Ryhel
2025-10-30 6:52 ` Tarang Raval
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=PN3P287MB18291BB4CCC68C8BA1260A628BFAA@PN3P287MB1829.INDP287.PROD.OUTLOOK.COM \
--to=tarang.raval@siliconsignals.io \
--cc=arnd@arndb.de \
--cc=benjamin.mugnier@foss.st.com \
--cc=clamor95@gmail.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dongcheng.yan@intel.com \
--cc=git@apitzsch.eu \
--cc=hansg@kernel.org \
--cc=heimir.sverrisson@gmail.com \
--cc=hverkuil@xs4all.nl \
--cc=krzk+dt@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=robh@kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=sylvain.petinot@foss.st.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®