From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
To: Tarang Raval <tarang.raval@siliconsignals.io>,
sakari.ailus@linux.intel.com, mehdi.djait@linux.intel.com
Cc: Himanshu Bhavani <himanshu.bhavani@siliconsignals.io>,
Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Hans Verkuil <hverkuil+cisco@kernel.org>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 03/17] media: i2c: os05b10: add register definitions and use them in init table
Date: Fri, 24 Jul 2026 16:09:39 +0300 [thread overview]
Message-ID: <3673f7ac-4a86-4576-9a27-9e736a69c45f@linaro.org> (raw)
In-Reply-To: <20260718200912.16001-4-tarang.raval@siliconsignals.io>
Hi Tarang.
On 7/18/26 23:08, Tarang Raval wrote:
> Define named register macros for OS05B10 and replace raw register
> addresses in the common initialization array with the new definitions.
> This improves readability and maintainability without changing
> functionality.
>
> Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
> ---
> drivers/media/i2c/os05b10.c | 110 +++++++++++++++++++++++-------------
> 1 file changed, 70 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/media/i2c/os05b10.c b/drivers/media/i2c/os05b10.c
> index 84621370d1da..96395cd7a2ca 100644
> --- a/drivers/media/i2c/os05b10.c
> +++ b/drivers/media/i2c/os05b10.c
> @@ -38,6 +38,20 @@
> #define OS05B10_MODE_STANDBY 0x00
> #define OS05B10_MODE_STREAMING 0x01
>
> +#define OS05B10_REG_PLL_CTRL_01 CCI_REG8(0x0301)
> +#define OS05B10_REG_PLL_CTRL_03 CCI_REG8(0x0303)
> +#define OS05B10_REG_PLL_CTRL_05 CCI_REG8(0x0305)
> +#define OS05B10_REG_PLL_CTRL_06 CCI_REG8(0x0306)
> +#define OS05B10_REG_PLL_CTRL_25 CCI_REG8(0x0325)
Here I see a potential to remove a few lines by announcing
a parameterised macro like
#define OS05B10_REG_PLL_CTRL(n) CCI_REG8(0x0300 + (n))
> +
> +#define OS05B10_REG_MIPI_SC_CTRL CCI_REG8(0x3016)
> +#define OS05B10_4_LANE_MODE 0x72
> +#define OS05B10_2_LANE_MODE 0x32
> +
> +#define OS05B10_REG_MIPI_SC_CTRL_1 CCI_REG8(0x3022)
> +#define OS05B10_10BIT_MODE 0x01
> +#define OS05B10_12BIT_MODE 0x61
> +
> #define OS05B10_REG_EXPOSURE CCI_REG24(0x3500)
> #define OS05B10_EXPOSURE_MIN 2
> #define OS05B10_EXPOSURE_STEP 1
> @@ -49,11 +63,41 @@
> #define OS05B10_ANALOG_GAIN_STEP 1
> #define OS05B10_ANALOG_GAIN_DEFAULT 0x80
>
> +#define OS05B10_REG_DIGITAL_GAIN CCI_REG16(0x350a)
> +#define OS05B10_DIGITAL_GAIN_MIN 0x400
> +#define OS05B10_DIGITAL_GAIN_MAX 0x3fff
> +#define OS05B10_DIGITAL_GAIN_STEP 1
> +#define OS05B10_DIGITAL_GAIN_DEFAULT 0x400
#define OS05B10_DIGITAL_GAIN_DEFAULT OS05B10_DIGITAL_GAIN_MIN
Or just use OS05B10_DIGITAL_GAIN_MIN whenever default one is needed.
> +
> +#define OS05B10_REG_ANALOG_GAIN_SHORT CCI_REG16(0x350c)
> +#define OS05B10_REG_DIGITAL_GAIN_SHORT CCI_REG16(0x350e)
> +#define OS05B10_REG_EXPOSURE_SHORT CCI_REG24(0x3510)
> +
> +#define OS05B10_REG_X_ADDR_START CCI_REG16(0x3800)
> +#define OS05B10_REG_Y_ADDR_START CCI_REG16(0x3802)
> +#define OS05B10_REG_X_ADDR_END CCI_REG16(0x3804)
> +#define OS05B10_REG_Y_ADDR_END CCI_REG16(0x3806)
> +#define OS05B10_REG_X_OUTPUT_SIZE CCI_REG16(0x3808)
> +#define OS05B10_REG_Y_OUTPUT_SIZE CCI_REG16(0x380a)
> +
> #define OS05B10_REG_HTS CCI_REG16(0x380c)
>
> #define OS05B10_REG_VTS CCI_REG16(0x380e)
> #define OS05B10_VTS_MAX 0x7fff
>
> +#define OS05B10_REG_ISP_X_WIN CCI_REG16(0x3810)
> +#define OS05B10_REG_ISP_Y_WIN CCI_REG16(0x3812)
> +#define OS05B10_REG_X_INC_ODD CCI_REG8(0x3814)
> +#define OS05B10_REG_X_INC_EVEN CCI_REG8(0x3815)
> +#define OS05B10_REG_Y_INC_ODD CCI_REG8(0x3816)
> +#define OS05B10_REG_Y_INC_EVEN CCI_REG8(0x3817)
> +
> +#define OS05B10_REG_FORMAT1 CCI_REG8(0x3820)
> +#define OS05B10_MIRROR BIT(3)
> +#define OS05B10_FLIP GENMASK(5, 4)
> +
> +#define OS05B10_REG_FORMAT2 CCI_REG8(0x3821)
> +
> #define OS05B10_LINK_FREQ_600MHZ (600 * HZ_PER_MHZ)
>
> static const struct v4l2_rect os05b10_native_area = {
> @@ -77,30 +121,25 @@ static const char * const os05b10_supply_name[] = {
> };
>
> static const struct cci_reg_sequence os05b10_common_regs[] = {
> - { CCI_REG8(0x0301), 0x44 },
> - { CCI_REG8(0x0303), 0x02 },
> - { CCI_REG8(0x0305), 0x32 },
> - { CCI_REG8(0x0306), 0x00 },
> - { CCI_REG8(0x0325), 0x3b },
> + { OS05B10_REG_PLL_CTRL_01, 0x44 },
> + { OS05B10_REG_PLL_CTRL_03, 0x02 },
> + { OS05B10_REG_PLL_CTRL_05, 0x32 },
> + { OS05B10_REG_PLL_CTRL_06, 0x00 },
> + { OS05B10_REG_PLL_CTRL_25, 0x3b },
> { CCI_REG8(0x3002), 0x21 },
> - { CCI_REG8(0x3016), 0x72 },
> + { OS05B10_REG_MIPI_SC_CTRL, 0x72 },
{ OS05B10_REG_MIPI_SC_CTRL, OS05B10_4_LANE_MODE },
> { CCI_REG8(0x301e), 0xb4 },
> { CCI_REG8(0x301f), 0xd0 },
> { CCI_REG8(0x3021), 0x03 },
> - { CCI_REG8(0x3022), 0x01 },
> + { OS05B10_REG_MIPI_SC_CTRL_1, 0x01 },
{ OS05B10_REG_MIPI_SC_CTRL_1, OS05B10_10BIT_MODE },
> { CCI_REG8(0x3107), 0xa1 },
> { CCI_REG8(0x3108), 0x7d },
> { CCI_REG8(0x3109), 0xfc },
> { CCI_REG8(0x3503), 0x88 },
> - { CCI_REG8(0x350a), 0x04 },
> - { CCI_REG8(0x350b), 0x00 },
> - { CCI_REG8(0x350c), 0x00 },
> - { CCI_REG8(0x350d), 0x80 },
> - { CCI_REG8(0x350e), 0x04 },
> - { CCI_REG8(0x350f), 0x00 },
> - { CCI_REG8(0x3510), 0x00 },
> - { CCI_REG8(0x3511), 0x00 },
> - { CCI_REG8(0x3512), 0x20 },
> + { OS05B10_REG_DIGITAL_GAIN, 0x0400 },
{ OS05B10_REG_DIGITAL_GAIN, OS05B10_DIGITAL_GAIN_DEFAULT }, // or OS05B10_DIGITAL_GAIN_MIN
> + { OS05B10_REG_ANALOG_GAIN_SHORT, 0x0080 },
> + { OS05B10_REG_DIGITAL_GAIN_SHORT, OS05B10_DIGITAL_GAIN_DEFAULT },
> + { OS05B10_REG_EXPOSURE_SHORT, 0x000020 },
> { CCI_REG8(0x3600), 0x4d },
> { CCI_REG8(0x3601), 0x08 },
> { CCI_REG8(0x3610), 0x87 },
> @@ -274,34 +313,25 @@ static const struct cci_reg_sequence os05b10_common_regs[] = {
> { CCI_REG8(0x37f5), 0x00 },
> { CCI_REG8(0x37f6), 0x00 },
> { CCI_REG8(0x37f7), 0x00 },
> - { CCI_REG8(0x3800), 0x01 },
> - { CCI_REG8(0x3801), 0x30 },
> - { CCI_REG8(0x3802), 0x00 },
> - { CCI_REG8(0x3803), 0x00 },
> - { CCI_REG8(0x3804), 0x0b },
> - { CCI_REG8(0x3805), 0x5f },
> - { CCI_REG8(0x3806), 0x07 },
> - { CCI_REG8(0x3807), 0xa7 },
> - { CCI_REG8(0x3808), 0x0a },
> - { CCI_REG8(0x3809), 0x20 },
> - { CCI_REG8(0x380a), 0x07 },
> - { CCI_REG8(0x380b), 0x98 },
> - { CCI_REG8(0x380c), 0x06 },
> - { CCI_REG8(0x380d), 0xd0 },
> - { CCI_REG8(0x3810), 0x00 },
> - { CCI_REG8(0x3811), 0x08 },
> - { CCI_REG8(0x3812), 0x00 },
> - { CCI_REG8(0x3813), 0x08 },
> - { CCI_REG8(0x3814), 0x01 },
> - { CCI_REG8(0x3815), 0x01 },
> - { CCI_REG8(0x3816), 0x01 },
> - { CCI_REG8(0x3817), 0x01 },
> + { OS05B10_REG_X_ADDR_START, 0x0130 },
> + { OS05B10_REG_Y_ADDR_START, 0x0000 },
> + { OS05B10_REG_X_ADDR_END, 0x0b5f },
> + { OS05B10_REG_Y_ADDR_END, 0x07a7 },
> + { OS05B10_REG_X_OUTPUT_SIZE, 0x0a20 },
> + { OS05B10_REG_Y_OUTPUT_SIZE, 0x0798 },
> + { OS05B10_REG_HTS, 0x06d0 },
> + { OS05B10_REG_ISP_X_WIN, 0x0008 },
> + { OS05B10_REG_ISP_Y_WIN, 0x0008 },
> + { OS05B10_REG_X_INC_ODD, 0x01 },
> + { OS05B10_REG_X_INC_EVEN, 0x01 },
> + { OS05B10_REG_Y_INC_ODD, 0x01 },
> + { OS05B10_REG_Y_INC_EVEN, 0x01 },
> { CCI_REG8(0x3818), 0x00 },
> { CCI_REG8(0x3819), 0x00 },
> { CCI_REG8(0x381a), 0x00 },
> { CCI_REG8(0x381b), 0x01 },
> - { CCI_REG8(0x3820), 0x88 },
> - { CCI_REG8(0x3821), 0x00 },
> + { OS05B10_REG_FORMAT1, 0x88 },
> + { OS05B10_REG_FORMAT2, 0x00 },
> { CCI_REG8(0x3822), 0x12 },
> { CCI_REG8(0x3823), 0x08 },
> { CCI_REG8(0x3824), 0x00 },
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
--
Best wishes,
Vladimir
next prev parent reply other threads:[~2026-07-24 13:09 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-18 20:08 [PATCH v3 00/17] media: i2c: os05b10: Refactor driver and Add new features Tarang Raval
2026-07-18 20:08 ` [PATCH v3 01/17] media: i2c: os05b10: Use pm_runtime_get_if_active() when applying controls Tarang Raval
2026-07-22 5:58 ` Jai Luthra
2026-07-24 6:47 ` Tarang Raval
2026-07-18 20:08 ` [PATCH v3 02/17] media: i2c: os05b10: drop unused group-hold programming Tarang Raval
2026-07-18 20:08 ` [PATCH v3 03/17] media: i2c: os05b10: add register definitions and use them in init table Tarang Raval
2026-07-21 17:25 ` Mehdi Djait
2026-07-24 13:09 ` Vladimir Zapolskiy [this message]
2026-07-18 20:08 ` [PATCH v3 04/17] media: i2c: os05b10: split common and mode-specific init registers Tarang Raval
2026-07-21 17:40 ` Mehdi Djait
2026-07-24 13:15 ` Vladimir Zapolskiy
2026-07-18 20:08 ` [PATCH v3 05/17] media: i2c: os05b10: add V4L2 digital gain control Tarang Raval
2026-07-24 13:19 ` Vladimir Zapolskiy
2026-07-24 14:15 ` Tarang Raval
2026-07-18 20:08 ` [PATCH v3 06/17] media: i2c: os05b10: Add H/V flip support Tarang Raval
2026-07-21 17:43 ` Mehdi Djait
2026-07-22 5:51 ` Jai Luthra
2026-07-24 7:01 ` Tarang Raval
2026-07-24 13:27 ` Vladimir Zapolskiy
2026-07-18 20:08 ` [PATCH v3 07/17] media: i2c: os05b10: Add test pattern options Tarang Raval
2026-07-21 17:50 ` Mehdi Djait
2026-07-24 13:43 ` Vladimir Zapolskiy
2026-07-18 20:08 ` [PATCH v3 08/17] media: i2c: os05b10: add 12-bit RAW mode support Tarang Raval
2026-07-24 14:08 ` Vladimir Zapolskiy
2026-07-24 14:30 ` Tarang Raval
2026-07-24 15:03 ` Vladimir Zapolskiy
2026-07-18 20:09 ` [PATCH v3 09/17] media: i2c: os05b10: update pixel rate on 10/12-bit mode switch Tarang Raval
2026-07-22 6:17 ` Jai Luthra
2026-07-22 6:19 ` Jai Luthra
2026-07-24 9:08 ` Tarang Raval
2026-07-25 7:26 ` Vladimir Zapolskiy
2026-07-18 20:09 ` [PATCH v3 10/17] media: i2c: os05b10: Add 1080p and 2x2 binning 720p modes Tarang Raval
2026-07-22 6:05 ` Jai Luthra
2026-07-24 6:44 ` Tarang Raval
2026-07-18 20:09 ` [PATCH v3 11/17] media: i2c: os05b10: keep vblank and exposure range in sync on mode switch Tarang Raval
2026-07-25 7:30 ` Vladimir Zapolskiy
2026-07-18 20:09 ` [PATCH v3 12/17] media: i2c: os05b10: Update active format before adjusting framing controls Tarang Raval
2026-07-25 7:32 ` Vladimir Zapolskiy
2026-07-18 20:09 ` [PATCH v3 13/17] media: i2c: os05b10: Rename vmax variable in VBLANK control Tarang Raval
2026-07-25 7:33 ` Vladimir Zapolskiy
2026-07-18 20:09 ` [PATCH v3 14/17] media: i2c: os05b10: add 2-lane support Tarang Raval
2026-07-25 7:38 ` Vladimir Zapolskiy
2026-07-18 20:09 ` [PATCH v3 15/17] media: i2c: os05b10: fix negative hblank calculation Tarang Raval
2026-07-22 6:14 ` Jai Luthra
2026-07-24 8:14 ` Tarang Raval
2026-07-18 20:09 ` [PATCH v3 16/17] media: i2c: os05b10: Enable runtime PM autosuspend Tarang Raval
2026-07-22 6:08 ` Jai Luthra
2026-07-24 7:02 ` Tarang Raval
2026-07-24 9:58 ` Jai Luthra
2026-07-18 20:09 ` [PATCH v3 17/17] media: i2c: os05b10: remove unused control fields, simplify error handling Tarang Raval
2026-07-25 11:33 ` Vladimir Zapolskiy
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=3673f7ac-4a86-4576-9a27-9e736a69c45f@linaro.org \
--to=vladimir.zapolskiy@linaro.org \
--cc=elgin.perumbilly@siliconsignals.io \
--cc=himanshu.bhavani@siliconsignals.io \
--cc=hverkuil+cisco@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=mehdi.djait@linux.intel.com \
--cc=sakari.ailus@linux.intel.com \
--cc=tarang.raval@siliconsignals.io \
/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®