* Re: [PATCH 1/2] dt-bindings: reset: imx8mq: Add _N suffix to IMX8MQ_RESET_MIPI_CSI*_RESET
From: Robby Cai @ 2026-04-14 9:55 UTC (permalink / raw)
To: Philipp Zabel
Cc: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, festevam, devicetree,
kernel, imx, linux-arm-kernel, linux-kernel, aisheng.dong
In-Reply-To: <d6cdea5f1a4f2eb3f7b73e6136c77020f444d8f0.camel@pengutronix.de>
On Tue, Mar 31, 2026 at 02:45:07PM +0200, Philipp Zabel wrote:
> On Di, 2026-03-31 at 18:13 +0800, Robby Cai wrote:
> > The assert logic of the MIPI CSI reset signals is active-low on i.MX8MQ,
> > but the existing names do not indicate this explicitly. To improve
> > consistency and clarity, append the _N suffix to all
> > IMX8MQ_RESET_MIPI_CSI*_RESET definitions. The deprecated
> > IMX8MQ_RESET_MIPI_CSI*_RESET versions remain temporarily for DT ABI
> > compatibility and will be removed at an appropriate time in the future.
>
> The register description in the latest reference manual I can download,
> IMX8MDQLQRM Rev. 3.1 (06/2021), still call these bits
> MIPI_CSI1_CORE_RESET and so on (without _N). There is no mention of
> polarity in the bitfield description. Is a documentation update
> planned?
Yes. A documentation update is already planned to clarify the reset polarity
(active-low) and naming. The current RM description is incomplete and will be
corrected in a future revision.
>
> Right now I'd say this improves clarity, but reduces consistency with
> existing documentation.
>
> Are these bits self-clearing, or can the reset be asserted by writing
> 0? As it stands, the CSI driver using these resets, imx8mq-mipi-csi2.c,
> only calls reset_control_assert() in imx8mq_mipi_csi_sw_reset():
>
> /*
> * these are most likely self-clearing reset bits. to make it
> * more clear, the reset-imx7 driver should implement the
> * .reset() operation.
> */
> ret = reset_control_assert(state->rst);
>
> This will probably have to be turned into a deassert together with the
> reset driver change.
No, these reset bits are not self-clearing.
According to the design team, the MIPI CSI reset logic on i.MX8MQ is identical
to MIPI DSI: the reset is active-low and must be explicitly deasserted.
Writing ��0�� asserts reset and it will remain asserted until cleared by software.
The current assumption in the CSI driver that these bits are self-clearing is
therefore incorrect. This was exposed by the landing patch [1].
To fix this properly, the reset-imx7 driver needs to reflect the correct polarity,
and the CSI driver should use deassert instead of only calling
reset_control_assert(). The RM will also be updated to clarify this behavior.
[1] https://git.linuxtv.org/media.git/commit/?id=6d79bb8fd2aa25afccbd6aeec2821722fa0b5db5
Regards,
Robby
^ permalink raw reply
* Re: [PATCH v1 3/4] ASoC: qcom: q6dsp: Update bit format support for secondary i2s
From: Konrad Dybcio @ 2026-04-14 9:54 UTC (permalink / raw)
To: Kumar Anurag, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Srinivas Kandagatla,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: linux-arm-msm, devicetree, linux-kernel, linux-sound
In-Reply-To: <20260413091937.134469-4-kumar.singh@oss.qualcomm.com>
On 4/13/26 11:19 AM, Kumar Anurag wrote:
> Add 32bit for playback and capture over secondary mi2s.
>
> Signed-off-by: Kumar Anurag <kumar.singh@oss.qualcomm.com>
> ---
Would this apply to the other 5 I2S ports as well?
Konrad
^ permalink raw reply
* Re: [PATCH v2 2/2] media: i2c: add os02g10 image sensor driver
From: Sakari Ailus @ 2026-04-14 9:57 UTC (permalink / raw)
To: Elgin Perumbilly
Cc: tarang.raval, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, Hans de Goede,
Vladimir Zapolskiy, Mehdi Djait, Laurent Pinchart,
Benjamin Mugnier, Sylvain Petinot, Hardevsinh Palaniya,
Jingjing Xiong, linux-media, devicetree, linux-kernel
In-Reply-To: <20260414084952.217215-3-elgin.perumbilly@siliconsignals.io>
Hi Elgin,
Thanks for the update.
On Tue, Apr 14, 2026 at 02:19:45PM +0530, Elgin Perumbilly wrote:
> Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.
>
> The Omnivision os02g10 is a CMOS image sensor with an active array size of
> 1920 x 1080.
>
> The following features are supported:
> - Manual exposure an gain control support
> - vblank/hblank control support
> - vflip/hflip control support
> - Test pattern control support
> - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)
>
> Signed-off-by: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
> ---
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 10 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/os02g10.c | 1039 +++++++++++++++++++++++++++++++++++
> 4 files changed, 1051 insertions(+)
> create mode 100644 drivers/media/i2c/os02g10.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 13409c71a765..28827e77ea31 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19463,6 +19463,7 @@ M: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> L: linux-media@vger.kernel.org
> S: Maintained
> F: Documentation/devicetree/bindings/media/i2c/ovti,os02g10.yaml
> +F: drivers/media/i2c/os02g10.c
>
> OMNIVISION OS05B10 SENSOR DRIVER
> M: Himanshu Bhavani <himanshu.bhavani@siliconsignals.io>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 5eb1e0e0a87a..dd6e9562acf6 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -372,6 +372,16 @@ config VIDEO_OG0VE1B
> To compile this driver as a module, choose M here: the
> module will be called og0ve1b.
>
> +config VIDEO_OS02G10
> + tristate "OmniVision OS02G10 sensor support"
> + select V4L2_CCI_I2C
> + help
> + This is a Video4Linux2 sensor driver for Omnivision
> + OS02G10 camera sensor.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called os02g10.
> +
> config VIDEO_OS05B10
> tristate "OmniVision OS05B10 sensor support"
> select V4L2_CCI_I2C
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index a3a6396df3c4..a7554d2eb140 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -84,6 +84,7 @@ obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o
> obj-$(CONFIG_VIDEO_MT9V111) += mt9v111.o
> obj-$(CONFIG_VIDEO_OG01A1B) += og01a1b.o
> obj-$(CONFIG_VIDEO_OG0VE1B) += og0ve1b.o
> +obj-$(CONFIG_VIDEO_OS02G10) += os02g10.o
> obj-$(CONFIG_VIDEO_OS05B10) += os05b10.o
> obj-$(CONFIG_VIDEO_OV01A10) += ov01a10.o
> obj-$(CONFIG_VIDEO_OV02A10) += ov02a10.o
> diff --git a/drivers/media/i2c/os02g10.c b/drivers/media/i2c/os02g10.c
> new file mode 100644
> index 000000000000..b8df79162f88
> --- /dev/null
> +++ b/drivers/media/i2c/os02g10.c
> @@ -0,0 +1,1039 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * V4L2 Support for the OS02G10
> + *
> + * Copyright (C) 2026 Silicon Signals Pvt. Ltd.
> + *
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bitops.h>
> +#include <linux/cleanup.h>
> +#include <linux/clk.h>
> +#include <linux/container_of.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +#include <linux/types.h>
> +#include <linux/time.h>
> +#include <linux/regmap.h>
> +
> +#include <media/v4l2-cci.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mediabus.h>
> +
> +#define OS02G10_XCLK_FREQ (24 * HZ_PER_MHZ)
> +
> +/* Add page number in CCI private bits [31:28] of the register address */
> +#define OS02G10_PAGE_REG8(p, x) (((p) << CCI_REG_PRIVATE_SHIFT) | CCI_REG8(x))
> +#define OS02G10_PAGE_REG16(p, x) (((p) << CCI_REG_PRIVATE_SHIFT) | CCI_REG16(x))
> +#define OS02G10_PAGE_REG24(p, x) (((p) << CCI_REG_PRIVATE_SHIFT) | CCI_REG24(x))
I just noticed regmap supports paging through register ranges. Could this
work for the driver as-is?
> +
> +#define OS02G10_REG_PAGE_SELECT CCI_REG8(0xfd)
> +
> +/* Page 0 */
> +#define OS02G10_REG_CHIPID OS02G10_PAGE_REG24(0x00, 0x02)
> +#define OS02G10_CHIPID 0x560247
> +
> +#define OS02G10_REG_PLL_DIV_CTRL OS02G10_PAGE_REG8(0x00, 0x30)
> +#define OS02G10_REG_PLL_DCTL_BIAS_CTRL OS02G10_PAGE_REG8(0x00, 0x35)
> +#define OS02G10_REG_GATE_EN_CTRL OS02G10_PAGE_REG8(0x00, 0x38)
> +#define OS02G10_REG_DPLL_NC OS02G10_PAGE_REG8(0x00, 0x41)
> +#define OS02G10_REG_MP_PHASE_CTRL OS02G10_PAGE_REG8(0x00, 0x44)
> +
> +/* Page 1 */
> +#define OS02G10_REG_STREAM_CTRL OS02G10_PAGE_REG8(0x01, 0xb1)
> +#define OS02G10_STREAM_CTRL_ON 0x03
> +#define OS02G10_STREAM_CTRL_OFF 0x00
> +
> +#define OS02G10_REG_FRAME_SYNC OS02G10_PAGE_REG8(0x01, 0x01)
> +
> +#define OS02G10_REG_FRAME_LENGTH OS02G10_PAGE_REG16(0x01, 0x0e)
> +#define OS02G10_FRAME_LENGTH_MAX 0xffff
> +#define OS02G10_REG_HBLANK OS02G10_PAGE_REG16(0x01, 0x09)
> +
> +#define OS02G10_REG_FRAME_TEST_CTRL OS02G10_PAGE_REG8(0x01, 0x0d)
> +#define OS02G10_FRAME_EXP_SEPERATE_EN BIT(4)
> +#define OS02G10_TEST_PATTERN_ENABLE BIT(0)
> +
> +#define OS02G10_REG_ULP_PWD_DUMMY_CTRL OS02G10_PAGE_REG8(0x01, 0x3c)
> +#define OS02G10_REG_DC_LEVEL_LIMIT_EN OS02G10_PAGE_REG8(0x01, 0x46)
> +#define OS02G10_REG_DC_LEVEL_LIMIT_L OS02G10_PAGE_REG8(0x01, 0x47)
> +#define OS02G10_REG_BLC_DATA_LIMIT_L OS02G10_PAGE_REG8(0x01, 0x48)
> +#define OS02G10_REG_DC_BLC_LIMIT_H OS02G10_PAGE_REG8(0x01, 0x49)
> +
> +#define OS02G10_REG_HS_LP_CTRL OS02G10_PAGE_REG8(0x01, 0x92)
> +#define OS02G10_REG_HS_LEVEL OS02G10_PAGE_REG8(0x01, 0x9d)
> +#define OS02G10_REG_HS_DRV OS02G10_PAGE_REG8(0x01, 0x9e)
> +
> +#define OS02G10_REG_GB_SUBOFFSET OS02G10_PAGE_REG8(0x01, 0xf0)
> +#define OS02G10_REG_BLUE_SUBOFFSET OS02G10_PAGE_REG8(0x01, 0xf1)
> +#define OS02G10_REG_RED_SUBOFFSET OS02G10_PAGE_REG8(0x01, 0xf2)
> +#define OS02G10_REG_GR_SUBOFFSET OS02G10_PAGE_REG8(0x01, 0xf3)
> +
> +#define OS02G10_REG_ABL_TRIGGER OS02G10_PAGE_REG8(0x01, 0xfa)
> +#define OS02G10_REG_ABL OS02G10_PAGE_REG8(0x01, 0xfb)
> +
> +#define OS02G10_REG_H_SIZE_MIPI OS02G10_PAGE_REG16(0x01, 0x8e)
> +#define OS02G10_REG_V_SIZE_MIPI OS02G10_PAGE_REG16(0x01, 0x90)
> +#define OS02G10_REG_MIPI_TX_SPEED_CTRL OS02G10_PAGE_REG8(0x01, 0xa1)
> +
> +#define OS02G10_REG_LONG_EXPOSURE OS02G10_PAGE_REG16(0x01, 0x03)
> +#define OS02G10_EXPOSURE_MIN 4
> +#define OS02G10_EXPOSURE_STEP 1
> +#define OS02G10_EXPOSURE_MARGIN 9
> +
> +#define OS02G10_REG_ANALOG_GAIN OS02G10_PAGE_REG8(0x01, 0x24)
> +#define OS02G10_ANALOG_GAIN_MIN 0x10
> +#define OS02G10_ANALOG_GAIN_MAX 0xf8
> +#define OS02G10_ANALOG_GAIN_STEP 1
> +#define OS02G10_ANALOG_GAIN_DEFAULT 0x10
> +
> +#define OS02G10_REG_DIGITAL_GAIN_H OS02G10_PAGE_REG8(0x01, 0x37)
> +#define OS02G10_REG_DIGITAL_GAIN_L OS02G10_PAGE_REG8(0x01, 0x39)
> +#define OS02G10_DIGITAL_GAIN_MIN 0x40
> +#define OS02G10_DIGITAL_GAIN_MAX 0x800
> +#define OS02G10_DIGITAL_GAIN_STEP 64
> +#define OS02G10_DIGITAL_GAIN_DEFAULT 0x40
> +
> +#define OS02G10_REG_FLIP_MIRROR OS02G10_PAGE_REG8(0x01, 0x3f)
> +#define OS02G10_FLIP BIT(1)
> +#define OS02G10_MIRROR BIT(0)
> +
> +/* Page 2 */
> +#define OS02G10_REG_V_START OS02G10_PAGE_REG16(0x02, 0xa0)
> +#define OS02G10_REG_V_SIZE OS02G10_PAGE_REG16(0x02, 0xa2)
> +#define OS02G10_REG_H_START OS02G10_PAGE_REG16(0x02, 0xa4)
> +#define OS02G10_REG_H_SIZE OS02G10_PAGE_REG16(0x02, 0xa6)
> +
> +#define OS02G10_REG_SIF_CTRL OS02G10_PAGE_REG8(0x02, 0x5e)
> +#define OS02G10_ORIENTATION_BAYER_FIX 0x32
> +
> +#define OS02G10_LINK_FREQ_720MHZ (720 * HZ_PER_MHZ)
> +
> +/* OS02G10 native and active pixel array size */
> +static const struct v4l2_rect os02g10_native_area = {
> + .top = 0,
> + .left = 0,
> + .width = 1928,
> + .height = 1088,
> +};
> +
> +static const struct v4l2_rect os02g10_active_area = {
> + .top = 4,
> + .left = 4,
> + .width = 1920,
> + .height = 1080,
> +};
> +
> +static const char * const os02g10_supply_name[] = {
> + "avdd", /* Analog power */
> + "dovdd", /* Digital I/O power */
> + "dvdd", /* Digital core power */
> +};
> +
> +struct os02g10 {
> + struct device *dev;
> + struct regmap *cci;
> + struct v4l2_subdev sd;
> + struct media_pad pad;
> + struct clk *xclk;
> + struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data supplies[ARRAY_SIZE(os02g10_supply_name)];
> +
> + /* V4L2 Controls */
> + struct v4l2_ctrl_handler handler;
> + struct v4l2_ctrl *link_freq;
> + struct v4l2_ctrl *hblank;
> + struct v4l2_ctrl *vblank;
> + struct v4l2_ctrl *exposure;
> + struct v4l2_ctrl *vflip;
> + struct v4l2_ctrl *hflip;
> +
> + u32 link_freq_index;
> +
> + u8 current_page;
> + struct mutex page_lock;
> +};
> +
> +struct os02g10_mode {
> + u32 width;
> + u32 height;
> + u32 vts_def;
> + u32 hts_def;
> + u32 exp_def;
> + u32 x_start;
> + u32 y_start;
> +};
> +
> +static const struct cci_reg_sequence os02g10_common_regs[] = {
> + { OS02G10_REG_PLL_DIV_CTRL, 0x0a},
> + { OS02G10_REG_PLL_DCTL_BIAS_CTRL, 0x04},
> + { OS02G10_REG_GATE_EN_CTRL, 0x11},
> + { OS02G10_REG_DPLL_NC, 0x06},
> + { OS02G10_REG_MP_PHASE_CTRL, 0x20},
> + { OS02G10_PAGE_REG8(0x01, 0x19), 0x50},
> + { OS02G10_PAGE_REG8(0x01, 0x1a), 0x0c},
> + { OS02G10_PAGE_REG8(0x01, 0x1b), 0x0d},
> + { OS02G10_PAGE_REG8(0x01, 0x1c), 0x00},
> + { OS02G10_PAGE_REG8(0x01, 0x1d), 0x75},
> + { OS02G10_PAGE_REG8(0x01, 0x1e), 0x52},
> + { OS02G10_PAGE_REG8(0x01, 0x22), 0x14},
> + { OS02G10_PAGE_REG8(0x01, 0x25), 0x44},
> + { OS02G10_PAGE_REG8(0x01, 0x26), 0x0f},
> + { OS02G10_REG_ULP_PWD_DUMMY_CTRL, 0xca},
> + { OS02G10_PAGE_REG8(0x01, 0x3d), 0x4a},
> + { OS02G10_PAGE_REG8(0x01, 0x40), 0x0f},
> + { OS02G10_PAGE_REG8(0x01, 0x43), 0x38},
> + { OS02G10_REG_DC_LEVEL_LIMIT_EN, 0x01},
> + { OS02G10_REG_DC_LEVEL_LIMIT_L, 0x00},
> + { OS02G10_REG_DC_BLC_LIMIT_H, 0x32},
> + { OS02G10_PAGE_REG8(0x01, 0x50), 0x01},
> + { OS02G10_PAGE_REG8(0x01, 0x51), 0x28},
> + { OS02G10_PAGE_REG8(0x01, 0x52), 0x20},
> + { OS02G10_PAGE_REG8(0x01, 0x53), 0x03},
> + { OS02G10_PAGE_REG8(0x01, 0x57), 0x16},
> + { OS02G10_PAGE_REG8(0x01, 0x59), 0x01},
> + { OS02G10_PAGE_REG8(0x01, 0x5a), 0x01},
> + { OS02G10_PAGE_REG8(0x01, 0x5d), 0x04},
> + { OS02G10_PAGE_REG8(0x01, 0x6a), 0x04},
> + { OS02G10_PAGE_REG8(0x01, 0x6b), 0x03},
> + { OS02G10_PAGE_REG8(0x01, 0x6e), 0x28},
> + { OS02G10_PAGE_REG8(0x01, 0x71), 0xc2},
> + { OS02G10_PAGE_REG8(0x01, 0x72), 0x04},
> + { OS02G10_PAGE_REG8(0x01, 0x73), 0x38},
> + { OS02G10_PAGE_REG8(0x01, 0x74), 0x04},
> + { OS02G10_PAGE_REG8(0x01, 0x79), 0x00},
> + { OS02G10_PAGE_REG8(0x01, 0x7a), 0xb2},
> + { OS02G10_PAGE_REG8(0x01, 0x7b), 0x10},
> + { OS02G10_REG_HS_LP_CTRL, 0x02},
> + { OS02G10_REG_HS_LEVEL, 0x03},
> + { OS02G10_REG_HS_DRV, 0x55},
> + { OS02G10_PAGE_REG8(0x01, 0xb8), 0x70},
> + { OS02G10_PAGE_REG8(0x01, 0xb9), 0x70},
> + { OS02G10_PAGE_REG8(0x01, 0xba), 0x70},
> + { OS02G10_PAGE_REG8(0x01, 0xbb), 0x70},
> + { OS02G10_PAGE_REG8(0x01, 0xbc), 0x00},
> + { OS02G10_PAGE_REG8(0x01, 0xc4), 0x6d},
> + { OS02G10_PAGE_REG8(0x01, 0xc5), 0x6d},
> + { OS02G10_PAGE_REG8(0x01, 0xc6), 0x6d},
> + { OS02G10_PAGE_REG8(0x01, 0xc7), 0x6d},
> + { OS02G10_PAGE_REG8(0x01, 0xcc), 0x11},
> + { OS02G10_PAGE_REG8(0x01, 0xcd), 0xe0},
> + { OS02G10_PAGE_REG8(0x01, 0xd0), 0x1b},
> + { OS02G10_PAGE_REG8(0x01, 0xd2), 0x76},
> + { OS02G10_PAGE_REG8(0x01, 0xd3), 0x68},
> + { OS02G10_PAGE_REG8(0x01, 0xd4), 0x68},
> + { OS02G10_PAGE_REG8(0x01, 0xd5), 0x73},
> + { OS02G10_PAGE_REG8(0x01, 0xd6), 0x73},
> + { OS02G10_PAGE_REG8(0x01, 0xe8), 0x55},
> + { OS02G10_REG_GB_SUBOFFSET, 0x40},
> + { OS02G10_REG_BLUE_SUBOFFSET, 0x40},
> + { OS02G10_REG_RED_SUBOFFSET, 0x40},
> + { OS02G10_REG_GR_SUBOFFSET, 0x40},
> + { OS02G10_REG_ABL_TRIGGER, 0x1c},
> + { OS02G10_REG_ABL, 0x33},
> + { OS02G10_PAGE_REG8(0x01, 0xfc), 0x80},
> + { OS02G10_PAGE_REG8(0x01, 0xfe), 0x80},
> + { OS02G10_PAGE_REG8(0x03, 0x03), 0x67},
> + { OS02G10_PAGE_REG8(0x03, 0x00), 0x59},
> + { OS02G10_PAGE_REG8(0x03, 0x04), 0x11},
> + { OS02G10_PAGE_REG8(0x03, 0x05), 0x04},
> + { OS02G10_PAGE_REG8(0x03, 0x06), 0x0c},
> + { OS02G10_PAGE_REG8(0x03, 0x07), 0x08},
> + { OS02G10_PAGE_REG8(0x03, 0x08), 0x08},
> + { OS02G10_PAGE_REG8(0x03, 0x09), 0x4f},
> + { OS02G10_PAGE_REG8(0x03, 0x0b), 0x08},
> + { OS02G10_PAGE_REG8(0x03, 0x0d), 0x26},
> + { OS02G10_PAGE_REG8(0x03, 0x0f), 0x00},
> + { OS02G10_PAGE_REG8(0x02, 0x34), 0xfe},
> + { OS02G10_REG_MIPI_TX_SPEED_CTRL, 0x05},
> +};
> +
> +static const struct os02g10_mode supported_modes[] = {
> + {
> + .width = 1920,
> + .height = 1080,
> + .vts_def = 1246,
> + .hts_def = 1082,
> + .exp_def = 1100,
> + .x_start = 2,
> + .y_start = 6,
> + },
> +};
> +
> +static const s64 link_freq_menu_items[] = {
> + OS02G10_LINK_FREQ_720MHZ,
> +};
> +
> +static const char * const os02g10_test_pattern_menu[] = {
> + "Disabled",
> + "Colorbar",
> +};
> +
> +static int os02g10_page_access(struct os02g10 *os02g10, u32 reg, int *err)
> +{
> + u8 page = (reg & CCI_REG_PRIVATE_MASK) >> CCI_REG_PRIVATE_SHIFT;
> + int ret = 0;
> +
> + if (err && *err)
> + return *err;
> +
> + guard(mutex)(&os02g10->page_lock);
> +
> + /* Perform page access before read/write */
> + if (os02g10->current_page == page)
> + return ret;
> +
> + ret = cci_write(os02g10->cci, OS02G10_REG_PAGE_SELECT, page, err);
> + if (!ret)
> + os02g10->current_page = page;
> +
> + return ret;
> +}
> +
> +static int os02g10_read(struct os02g10 *os02g10, u32 reg, u64 *val, int *err)
> +{
> + u32 addr = reg & ~CCI_REG_PRIVATE_MASK;
> + int ret;
> +
> + ret = os02g10_page_access(os02g10, reg, err);
> + if (ret)
> + return ret;
> +
> + return cci_read(os02g10->cci, addr, val, err);
> +}
> +
> +static int os02g10_write(struct os02g10 *os02g10, u32 reg, u64 val, int *err)
> +{
> + u32 addr = reg & ~CCI_REG_PRIVATE_MASK;
> + int ret;
> +
> + ret = os02g10_page_access(os02g10, reg, err);
> + if (ret)
> + return ret;
> +
> + return cci_write(os02g10->cci, addr, val, err);
> +}
> +
> +static int os02g10_update_bits(struct os02g10 *os02g10, u32 reg, u64 mask,
> + u64 val, int *err)
> +{
> + u32 addr = reg & ~CCI_REG_PRIVATE_MASK;
> + int ret;
> +
> + ret = os02g10_page_access(os02g10, reg, err);
> + if (ret)
> + return ret;
> +
> + return cci_update_bits(os02g10->cci, addr, mask, val, err);
> +}
> +
> +static int os02g10_multi_reg_write(struct os02g10 *os02g10,
> + const struct cci_reg_sequence *regs,
> + unsigned int num_regs, int *err)
> +{
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < num_regs; i++) {
> + ret = os02g10_write(os02g10, regs[i].reg, regs[i].val, err);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static inline struct os02g10 *to_os02g10(struct v4l2_subdev *_sd)
> +{
> + return container_of_const(_sd, struct os02g10, sd);
You can call "_sd" "sd".
> +}
> +
> +static u32 os02g10_get_format_code(struct os02g10 *os02g10)
> +{
> + static const u32 codes[2][2] = {
> + { MEDIA_BUS_FMT_SBGGR10_1X10, MEDIA_BUS_FMT_SGBRG10_1X10, },
> + { MEDIA_BUS_FMT_SGRBG10_1X10, MEDIA_BUS_FMT_SRGGB10_1X10, },
> + };
> +
Extra newline.
> + u32 code = codes[os02g10->vflip->val][os02g10->hflip->val];
> +
> + return code;
> +}
> +
> +static int os02g10_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct os02g10 *os02g10 = container_of_const(ctrl->handler,
> + struct os02g10, handler);
> + struct v4l2_subdev_state *state;
> + struct v4l2_mbus_framefmt *fmt;
> + int ret = 0;
> +
> + state = v4l2_subdev_get_locked_active_state(&os02g10->sd);
> + fmt = v4l2_subdev_state_get_format(state, 0);
> +
> + if (ctrl->id == V4L2_CID_VBLANK) {
> + /* Honour the VBLANK limits when setting exposure */
> + s64 max = fmt->height + ctrl->val - OS02G10_EXPOSURE_MARGIN;
> +
> + ret = __v4l2_ctrl_modify_range(os02g10->exposure,
> + os02g10->exposure->minimum, max,
> + os02g10->exposure->step,
> + os02g10->exposure->default_value);
> + if (ret)
> + return ret;
> + }
> +
> + if (pm_runtime_get_if_active(os02g10->dev) == 0)
> + return 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_EXPOSURE:
> + os02g10_write(os02g10, OS02G10_REG_LONG_EXPOSURE, ctrl->val, &ret);
> + break;
> + case V4L2_CID_ANALOGUE_GAIN:
> + os02g10_write(os02g10, OS02G10_REG_ANALOG_GAIN, ctrl->val, &ret);
> + break;
> + case V4L2_CID_DIGITAL_GAIN:
> + os02g10_write(os02g10, OS02G10_REG_DIGITAL_GAIN_L,
> + (ctrl->val & 0xff), &ret);
> + os02g10_write(os02g10, OS02G10_REG_DIGITAL_GAIN_H,
> + ((ctrl->val >> 8) & 0x7), &ret);
> + break;
> + case V4L2_CID_VBLANK:
> + u64 vts = ctrl->val + fmt->height;
> +
> + os02g10_update_bits(os02g10, OS02G10_REG_FRAME_TEST_CTRL,
> + OS02G10_FRAME_EXP_SEPERATE_EN,
> + OS02G10_FRAME_EXP_SEPERATE_EN, &ret);
> + os02g10_write(os02g10, OS02G10_REG_FRAME_LENGTH, vts, &ret);
> + break;
> + case V4L2_CID_HFLIP:
> + case V4L2_CID_VFLIP:
> + os02g10_write(os02g10, OS02G10_REG_FLIP_MIRROR,
> + os02g10->hflip->val | os02g10->vflip->val << 1,
> + &ret);
> + os02g10_write(os02g10, OS02G10_REG_SIF_CTRL,
> + OS02G10_ORIENTATION_BAYER_FIX, &ret);
> + break;
> + case V4L2_CID_TEST_PATTERN:
> + os02g10_update_bits(os02g10,
> + OS02G10_REG_FRAME_TEST_CTRL,
> + OS02G10_TEST_PATTERN_ENABLE,
> + ctrl->val ? OS02G10_TEST_PATTERN_ENABLE : 0,
> + &ret);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + os02g10_write(os02g10, OS02G10_REG_FRAME_SYNC, 0x01, &ret);
> +
> + pm_runtime_put(os02g10->dev);
> +
> + return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops os02g10_ctrl_ops = {
> + .s_ctrl = os02g10_set_ctrl,
> +};
> +
> +static int os02g10_init_controls(struct os02g10 *os02g10)
> +{
> + const struct os02g10_mode *mode = &supported_modes[0];
> + u64 vblank_def, hblank_def, exp_max, pixel_rate;
> + struct v4l2_fwnode_device_properties props;
> + struct v4l2_ctrl_handler *ctrl_hdlr;
> + int ret;
> +
> + ctrl_hdlr = &os02g10->handler;
> + v4l2_ctrl_handler_init(ctrl_hdlr, 12);
> +
> + /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
> + pixel_rate = div_u64(OS02G10_LINK_FREQ_720MHZ * 2 * 2, 10);
> + v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops, V4L2_CID_PIXEL_RATE, 0,
> + pixel_rate, 1, pixel_rate);
> +
> + os02g10->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_LINK_FREQ,
> + os02g10->link_freq_index,
> + 0, link_freq_menu_items);
> + if (os02g10->link_freq)
> + os02g10->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + /*
> + * Multiply by 2 to ensure positive hblank.
V4L2 controls can also have negative values, there's no need to multiply
line length in pixels for this reason.
> + * Datasheet does not provide information about the unit of HTS.
> + */
> + hblank_def = (mode->hts_def * 2) - mode->width;
> + os02g10->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_HBLANK, hblank_def, hblank_def,
> + 1, hblank_def);
> + if (os02g10->hblank)
> + os02g10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + vblank_def = mode->vts_def - mode->height;
> + os02g10->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_VBLANK, vblank_def,
> + OS02G10_FRAME_LENGTH_MAX - mode->height,
> + 1, vblank_def);
> +
> + exp_max = mode->vts_def - OS02G10_EXPOSURE_MARGIN;
> + os02g10->exposure =
> + v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_EXPOSURE,
> + OS02G10_EXPOSURE_MIN, exp_max,
> + OS02G10_EXPOSURE_STEP, mode->exp_def);
> +
> + v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_ANALOGUE_GAIN, OS02G10_ANALOG_GAIN_MIN,
> + OS02G10_ANALOG_GAIN_MAX, OS02G10_ANALOG_GAIN_STEP,
> + OS02G10_ANALOG_GAIN_DEFAULT);
> +
> + v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_DIGITAL_GAIN, OS02G10_DIGITAL_GAIN_MIN,
> + OS02G10_DIGITAL_GAIN_MAX, OS02G10_DIGITAL_GAIN_STEP,
> + OS02G10_DIGITAL_GAIN_DEFAULT);
> +
> + os02g10->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_HFLIP, 0, 1, 1, 0);
> + if (os02g10->hflip)
> + os02g10->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> +
> + os02g10->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_VFLIP, 0, 1, 1, 0);
> + if (os02g10->vflip)
> + os02g10->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> +
> + v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &os02g10_ctrl_ops,
> + V4L2_CID_TEST_PATTERN,
> + ARRAY_SIZE(os02g10_test_pattern_menu) - 1,
> + 0, 0, os02g10_test_pattern_menu);
> + if (ctrl_hdlr->error) {
You can skip this error check; doing this after
v4l2_ctrl_new_fwnode_properties() is enough.
> + ret = ctrl_hdlr->error;
> + dev_err(os02g10->dev, "control init failed (%d)\n", ret);
> + goto err_handler_free;
> + }
> +
> + ret = v4l2_fwnode_device_parse(os02g10->dev, &props);
> + if (ret)
> + goto err_handler_free;
> +
> + ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr,
> + &os02g10_ctrl_ops, &props);
> + if (ret)
> + goto err_handler_free;
> +
> + os02g10->sd.ctrl_handler = ctrl_hdlr;
> +
> + return 0;
> +
> +err_handler_free:
> + v4l2_ctrl_handler_free(ctrl_hdlr);
> +
> + return ret;
> +}
> +
> +static int os02g10_set_framefmt(struct os02g10 *os02g10,
> + struct v4l2_subdev_state *state)
> +{
> + const struct v4l2_mbus_framefmt *format;
> + const struct os02g10_mode *mode;
> + int ret = 0;
> +
> + format = v4l2_subdev_state_get_format(state, 0);
> + mode = v4l2_find_nearest_size(supported_modes,
> + ARRAY_SIZE(supported_modes), width,
> + height, format->width, format->height);
> +
> + os02g10_write(os02g10, OS02G10_REG_V_START, mode->y_start, &ret);
> + os02g10_write(os02g10, OS02G10_REG_V_SIZE, mode->height, &ret);
> + os02g10_write(os02g10, OS02G10_REG_V_SIZE_MIPI, mode->height, &ret);
> + os02g10_write(os02g10, OS02G10_REG_H_START, mode->x_start, &ret);
> + os02g10_write(os02g10, OS02G10_REG_H_SIZE, mode->width, &ret);
> + os02g10_write(os02g10, OS02G10_REG_H_SIZE_MIPI, mode->width, &ret);
> +
> + return ret;
> +}
> +
> +static int os02g10_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(os02g10->dev);
> + if (ret < 0)
> + return ret;
> +
> + ret = os02g10_multi_reg_write(os02g10, os02g10_common_regs,
> + ARRAY_SIZE(os02g10_common_regs), NULL);
> + if (ret) {
> + dev_err(os02g10->dev, "failed to write common registers\n");
> + goto err_rpm_put;
> + }
> +
> + ret = os02g10_set_framefmt(os02g10, state);
> + if (ret) {
> + dev_err(os02g10->dev, "failed to set frame foramt\n");
> + goto err_rpm_put;
> + }
> +
> + /* Apply customized values from user */
> + ret = __v4l2_ctrl_handler_setup(os02g10->sd.ctrl_handler);
> + if (ret)
> + goto err_rpm_put;
> +
> + ret = os02g10_write(os02g10, OS02G10_REG_STREAM_CTRL,
> + OS02G10_STREAM_CTRL_ON, NULL);
> + if (ret)
> + goto err_rpm_put;
> +
> + /* vflip and hflip cannot change during streaming */
> + __v4l2_ctrl_grab(os02g10->vflip, true);
> + __v4l2_ctrl_grab(os02g10->hflip, true);
> +
> + return 0;
> +
> +err_rpm_put:
> + pm_runtime_put(os02g10->dev);
> + return ret;
> +}
> +
> +static int os02g10_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> + int ret;
> +
> + ret = os02g10_write(os02g10, OS02G10_REG_STREAM_CTRL,
> + OS02G10_STREAM_CTRL_OFF, NULL);
> + if (ret)
> + dev_err(os02g10->dev, "Failed to stop stream\n");
> +
> + __v4l2_ctrl_grab(os02g10->vflip, false);
> + __v4l2_ctrl_grab(os02g10->hflip, false);
> +
> + pm_runtime_put(os02g10->dev);
> +
> + return ret;
> +}
> +
> +static int os02g10_get_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_selection *sel)
> +{
> + switch (sel->target) {
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + case V4L2_SEL_TGT_NATIVE_SIZE:
> + sel->r = os02g10_native_area;
> + return 0;
> + case V4L2_SEL_TGT_CROP:
> + case V4L2_SEL_TGT_CROP_DEFAULT:
> + sel->r = os02g10_active_area;
> + return 0;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int os02g10_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> +
> + if (code->index)
> + return -EINVAL;
> +
> + code->code = os02g10_get_format_code(os02g10);
> +
> + return 0;
> +}
> +
> +static int os02g10_enum_frame_size(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> +
> + if (fse->index >= ARRAY_SIZE(supported_modes))
> + return -EINVAL;
> +
> + if (fse->code != os02g10_get_format_code(os02g10))
> + return -EINVAL;
> +
> + fse->min_width = supported_modes[fse->index].width;
> + fse->max_width = fse->min_width;
> + fse->min_height = supported_modes[fse->index].height;
> + fse->max_height = fse->min_height;
> +
> + return 0;
> +}
> +
> +static int os02g10_set_pad_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> + struct v4l2_mbus_framefmt *format;
> + const struct os02g10_mode *mode;
> + int ret;
> +
> + format = v4l2_subdev_state_get_format(sd_state, 0);
> +
> + mode = v4l2_find_nearest_size(supported_modes,
> + ARRAY_SIZE(supported_modes),
> + width, height,
> + fmt->format.width, fmt->format.height);
> +
> + fmt->format.code = os02g10_get_format_code(os02g10);
> + fmt->format.width = mode->width;
> + fmt->format.height = mode->height;
> + fmt->format.field = V4L2_FIELD_NONE;
> + fmt->format.colorspace = V4L2_COLORSPACE_RAW;
> + fmt->format.quantization = V4L2_QUANTIZATION_FULL_RANGE;
> + fmt->format.xfer_func = V4L2_XFER_FUNC_NONE;
> +
> + *format = fmt->format;
> +
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> + u32 vblank_def = mode->vts_def - mode->height;
You can declare ret here.
> +
> + ret = __v4l2_ctrl_modify_range(os02g10->vblank, vblank_def,
> + OS02G10_FRAME_LENGTH_MAX -
> + mode->height, 1, vblank_def);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int os02g10_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state)
> +{
> + struct os02g10 *os02g10 = to_os02g10(sd);
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_TRY,
> + .format = {
> + .code = os02g10_get_format_code(os02g10),
> + .width = supported_modes[0].width,
> + .height = supported_modes[0].height,
> + },
> + };
> +
> + os02g10_set_pad_format(sd, state, &fmt);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops os02g10_video_ops = {
> + .s_stream = v4l2_subdev_s_stream_helper,
> +};
> +
> +static const struct v4l2_subdev_pad_ops os02g10_pad_ops = {
> + .enum_mbus_code = os02g10_enum_mbus_code,
> + .get_fmt = v4l2_subdev_get_fmt,
> + .set_fmt = os02g10_set_pad_format,
> + .get_selection = os02g10_get_selection,
> + .enum_frame_size = os02g10_enum_frame_size,
> + .enable_streams = os02g10_enable_streams,
> + .disable_streams = os02g10_disable_streams,
> +};
> +
> +static const struct v4l2_subdev_ops os02g10_subdev_ops = {
> + .video = &os02g10_video_ops,
> + .pad = &os02g10_pad_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops os02g10_internal_ops = {
> + .init_state = os02g10_init_state,
> +};
> +
> +static int os02g10_power_on(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct os02g10 *os02g10 = to_os02g10(sd);
> + int ret;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(os02g10_supply_name),
> + os02g10->supplies);
> + if (ret) {
> + dev_err(os02g10->dev, "failed to enable regulators\n");
> + return ret;
> + }
> +
> + /* T4: delay from DOVDD stable to MCLK on */
> + fsleep(5 * USEC_PER_MSEC);
> +
> + ret = clk_prepare_enable(os02g10->xclk);
> + if (ret) {
> + dev_err(os02g10->dev, "failed to enable clock\n");
> + goto err_regulator_off;
> + }
> +
> + /* T3: delay from DVDD stable to sensor power up stable */
> + fsleep(5 * USEC_PER_MSEC);
> +
> + gpiod_set_value_cansleep(os02g10->reset_gpio, 0);
> +
> + /* T5: delay from sensor power up stable to SCCB initialization */
> + fsleep(5 * USEC_PER_MSEC);
> +
> + return 0;
> +
> +err_regulator_off:
> + regulator_bulk_disable(ARRAY_SIZE(os02g10_supply_name), os02g10->supplies);
> + return ret;
> +}
> +
> +static int os02g10_power_off(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct os02g10 *os02g10 = to_os02g10(sd);
> +
> + clk_disable_unprepare(os02g10->xclk);
> + gpiod_set_value_cansleep(os02g10->reset_gpio, 1);
> + regulator_bulk_disable(ARRAY_SIZE(os02g10_supply_name), os02g10->supplies);
> +
> + return 0;
> +}
> +
> +static int os02g10_identify_module(struct os02g10 *os02g10)
> +{
> + u64 chip_id;
> + int ret;
> +
> + ret = os02g10_read(os02g10, OS02G10_REG_CHIPID, &chip_id, NULL);
> + if (ret)
> + return dev_err_probe(os02g10->dev, ret,
> + "failed to read chip id %x\n",
> + OS02G10_CHIPID);
> +
> + if (chip_id != OS02G10_CHIPID)
> + return dev_err_probe(os02g10->dev, -EIO,
> + "chip id mismatch: %x!=%llx\n",
> + OS02G10_CHIPID, chip_id);
> +
> + return 0;
> +}
> +
> +static int os02g10_parse_endpoint(struct os02g10 *os02g10)
> +{
> + 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(os02g10->dev), NULL);
> + if (!ep)
You can omit this check.
> + return dev_err_probe(os02g10->dev, -ENXIO,
> + "Failed to get next endpoint\n");
> +
> + 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 != 2) {
> + ret = dev_err_probe(os02g10->dev, -EINVAL,
> + "only 2 data lanes are supported\n");
> + goto error_out;
> + }
> +
> + ret = v4l2_link_freq_to_bitmap(os02g10->dev, bus_cfg.link_frequencies,
> + bus_cfg.nr_of_link_frequencies,
> + link_freq_menu_items,
> + ARRAY_SIZE(link_freq_menu_items),
> + &link_freq_bitmap);
> + if (ret) {
> + ret = dev_err_probe(os02g10->dev, -EINVAL,
> + "only 720MHz frequency is available\n");
> + goto error_out;
> + }
> +
> + os02g10->link_freq_index = __ffs(link_freq_bitmap);
Could you rely on the value of the control?
> +
> +error_out:
> + v4l2_fwnode_endpoint_free(&bus_cfg);
> +
> + return ret;
> +};
> +
> +static int os02g10_probe(struct i2c_client *client)
> +{
> + struct os02g10 *os02g10;
> + unsigned int xclk_freq;
> + int ret;
> +
> + os02g10 = devm_kzalloc(&client->dev, sizeof(*os02g10), GFP_KERNEL);
> + if (!os02g10)
> + return -ENOMEM;
> +
> + os02g10->dev = &client->dev;
> +
> + v4l2_i2c_subdev_init(&os02g10->sd, client, &os02g10_subdev_ops);
> + os02g10->sd.internal_ops = &os02g10_internal_ops;
> +
> + os02g10->cci = devm_cci_regmap_init_i2c(client, 8);
> + if (IS_ERR(os02g10->cci))
> + return dev_err_probe(os02g10->dev, PTR_ERR(os02g10->cci),
> + "failed to initialize CCI\n");
> +
> + /* Set Current page to 0 */
> + os02g10->current_page = 0;
> +
> + ret = devm_mutex_init(os02g10->dev, &os02g10->page_lock);
> + if (ret)
> + return dev_err_probe(os02g10->dev, ret,
> + "Failed to initialize lock\n");
> +
> + /* Get system clock (xvclk) */
> + os02g10->xclk = devm_v4l2_sensor_clk_get(os02g10->dev, NULL);
> + if (IS_ERR(os02g10->xclk))
> + return dev_err_probe(os02g10->dev, PTR_ERR(os02g10->xclk),
> + "failed to get xclk\n");
> +
> + xclk_freq = clk_get_rate(os02g10->xclk);
> + if (xclk_freq != OS02G10_XCLK_FREQ)
> + return dev_err_probe(os02g10->dev, -EINVAL,
> + "xclk frequency not supported: %u Hz\n",
> + xclk_freq);
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(os02g10_supply_name); i++)
> + os02g10->supplies[i].supply = os02g10_supply_name[i];
> +
> + ret = devm_regulator_bulk_get(os02g10->dev,
> + ARRAY_SIZE(os02g10_supply_name),
> + os02g10->supplies);
> + if (ret)
> + return dev_err_probe(os02g10->dev, ret,
> + "failed to get regulators\n");
> +
> + ret = os02g10_parse_endpoint(os02g10);
I'd do this first: in some cases driver probing ends up being done a few
times before it succeeds so getting regulators, GPIOs and clock is better
left after that.
> + if (ret)
> + return dev_err_probe(os02g10->dev, ret,
> + "failed to parse endpoint configuration\n");
> +
> + os02g10->reset_gpio = devm_gpiod_get_optional(os02g10->dev,
> + "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(os02g10->reset_gpio))
> + return dev_err_probe(os02g10->dev, PTR_ERR(os02g10->reset_gpio),
> + "failed to get reset GPIO\n");
> +
> + ret = os02g10_power_on(os02g10->dev);
> + if (ret)
> + return ret;
> +
> + ret = os02g10_identify_module(os02g10);
> + if (ret)
> + goto error_power_off;
> +
> + ret = os02g10_init_controls(os02g10);
> + if (ret)
> + goto error_power_off;
> +
> + /* Initialize subdev */
> + os02g10->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + os02g10->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> + os02g10->pad.flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&os02g10->sd.entity, 1, &os02g10->pad);
> + if (ret) {
> + dev_err_probe(os02g10->dev, ret, "failed to init entity pads\n");
> + goto error_handler_free;
> + }
> +
> + os02g10->sd.state_lock = os02g10->handler.lock;
> + ret = v4l2_subdev_init_finalize(&os02g10->sd);
> + if (ret) {
> + dev_err_probe(os02g10->dev, ret, "subdev init error\n");
> + goto error_media_entity;
> + }
> +
> + pm_runtime_set_active(os02g10->dev);
> + pm_runtime_enable(os02g10->dev);
> +
> + ret = v4l2_async_register_subdev_sensor(&os02g10->sd);
> + if (ret) {
> + dev_err_probe(os02g10->dev, ret,
> + "failed to register os02g10 sub-device\n");
> + goto error_subdev_cleanup;
> + }
> +
> + pm_runtime_idle(os02g10->dev);
A newline here?
> + return 0;
> +
> +error_subdev_cleanup:
> + v4l2_subdev_cleanup(&os02g10->sd);
> + pm_runtime_disable(os02g10->dev);
> + pm_runtime_set_suspended(os02g10->dev);
> +
> +error_media_entity:
> + media_entity_cleanup(&os02g10->sd.entity);
> +
> +error_handler_free:
> + v4l2_ctrl_handler_free(os02g10->sd.ctrl_handler);
> +
> +error_power_off:
> + os02g10_power_off(os02g10->dev);
> +
> + return ret;
> +}
> +
> +static void os02g10_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct os02g10 *os02g10 = to_os02g10(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + v4l2_subdev_cleanup(&os02g10->sd);
> + media_entity_cleanup(&sd->entity);
> + v4l2_ctrl_handler_free(os02g10->sd.ctrl_handler);
> +
> + pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev)) {
> + os02g10_power_off(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> + }
> +}
> +
> +static DEFINE_RUNTIME_DEV_PM_OPS(os02g10_pm_ops,
> + os02g10_power_off, os02g10_power_on, NULL);
> +
> +static const struct of_device_id os02g10_id[] = {
> + { .compatible = "ovti,os02g10" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, os02g10_id);
> +
> +static struct i2c_driver os02g10_driver = {
> + .driver = {
> + .name = "os02g10",
> + .pm = pm_ptr(&os02g10_pm_ops),
> + .of_match_table = os02g10_id,
> + },
> + .probe = os02g10_probe,
> + .remove = os02g10_remove,
> +};
> +module_i2c_driver(os02g10_driver);
> +
> +MODULE_DESCRIPTION("OS02G10 Camera Sensor Driver");
> +MODULE_AUTHOR("Tarang Raval <tarang.raval@siliconsignals.io>");
> +MODULE_AUTHOR("Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>");
> +MODULE_LICENSE("GPL");
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH v2 2/2] media: i2c: add os02g10 image sensor driver
From: Laurent Pinchart @ 2026-04-14 9:57 UTC (permalink / raw)
To: Elgin Perumbilly
Cc: sakari.ailus@linux.intel.com, Tarang Raval, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans Verkuil,
Hans de Goede, Vladimir Zapolskiy, Mehdi Djait, Benjamin Mugnier,
Sylvain Petinot, Hardevsinh Palaniya, Jingjing Xiong,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <MA0P287MB2178FAA81D07B561FA68014988252@MA0P287MB2178.INDP287.PROD.OUTLOOK.COM>
On Tue, Apr 14, 2026 at 09:43:32AM +0000, Elgin Perumbilly wrote:
> Hi Laurent,
>
> > I sent a review comment on v1.
> >
> > On Tue, Apr 14, 2026 at 02:19:45PM +0530, Elgin Perumbilly wrote:
> > > Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.
> > >
> > > The Omnivision os02g10 is a CMOS image sensor with an active array size of
> > > 1920 x 1080.
> > >
> > > The following features are supported:
> > > - Manual exposure an gain control support
> > > - vblank/hblank control support
> > > - vflip/hflip control support
> > > - Test pattern control support
> > > - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)
> > >
> > > Signed-off-by: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> > > Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
> > > ---
> > > MAINTAINERS | 1 +
> > > drivers/media/i2c/Kconfig | 10 +
> > > drivers/media/i2c/Makefile | 1 +
> > > drivers/media/i2c/os02g10.c | 1039 +++++++++++++++++++++++++++++++++++
> > > 4 files changed, 1051 insertions(+)
> > > create mode 100644 drivers/media/i2c/os02g10.c
>
> I have added a new function, os02g10_set_framefmt, which dynamically sets
> the mode register.
>
> Please let me know if I have missed anything or if further changes are
> needed.
You also need to drop the supported_modes array, and implement support
for .set_selection().
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 1/7] arm64: dts: qcom: lemans: Add label properties to CoreSight devices
From: Konrad Dybcio @ 2026-04-14 9:59 UTC (permalink / raw)
To: Jie Gan, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260410-add-label-to-coresight-device-v1-1-d71a6759dbc2@oss.qualcomm.com>
On 4/10/26 5:08 AM, Jie Gan wrote:
> Add label properties to TPDM and CTI nodes in the lemans device tree to
> provide human-readable identifiers for each CoreSight device. These
> labels allow userspace tools and the CoreSight framework to identify
> devices by name rather than by base address.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 2/7] arm64: dts: qcom: talos: Add label properties to CoreSight devices
From: Konrad Dybcio @ 2026-04-14 10:04 UTC (permalink / raw)
To: Jie Gan, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260410-add-label-to-coresight-device-v1-2-d71a6759dbc2@oss.qualcomm.com>
On 4/10/26 5:08 AM, Jie Gan wrote:
> Add label properties to CTI and TPDM nodes in the talos device tree to
> provide human-readable identifiers for each CoreSight device. These
> labels allow userspace tools and the CoreSight framework to identify
> devices by name rather than by base address.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad3597,6 +3652,7 @@ tpdm@78b0000 {>
> clocks = <&aoss_qmp>;
> clock-names = "apb_pclk";
> + label = "tpdm_llm_gold";
>
> qcom,cmb-element-bits = <32>;
> qcom,cmb-msrs-num = <32>;
> @@ -3664,6 +3720,7 @@ cti@78e0000 {
>
> clocks = <&aoss_qmp>;
> clock-names = "apb_pclk";
> + label = "cti_apss";
> };
>
> cti@78f0000 {
> @@ -3672,6 +3729,7 @@ cti@78f0000 {
>
> clocks = <&aoss_qmp>;
> clock-names = "apb_pclk";
> + label = "cti_apss_1";
> };
>
> cti@7900000 {
> @@ -3680,6 +3738,7 @@ cti@7900000 {
>
> clocks = <&aoss_qmp>;
> clock-names = "apb_pclk";
> + label = "cti_apss_2";
> };
>
> remoteproc_cdsp: remoteproc@8300000 {
>
^ permalink raw reply
* Re: [PATCH v2 2/2] media: i2c: add os02g10 image sensor driver
From: sakari.ailus @ 2026-04-14 10:06 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Elgin Perumbilly, Tarang Raval, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans Verkuil,
Hans de Goede, Vladimir Zapolskiy, Mehdi Djait, Benjamin Mugnier,
Sylvain Petinot, Hardevsinh Palaniya, Jingjing Xiong,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20260414095727.GF4061@killaraus.ideasonboard.com>
Hi Laurent,
On Tue, Apr 14, 2026 at 12:57:27PM +0300, Laurent Pinchart wrote:
> On Tue, Apr 14, 2026 at 09:43:32AM +0000, Elgin Perumbilly wrote:
> > Hi Laurent,
> >
> > > I sent a review comment on v1.
> > >
> > > On Tue, Apr 14, 2026 at 02:19:45PM +0530, Elgin Perumbilly wrote:
> > > > Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.
> > > >
> > > > The Omnivision os02g10 is a CMOS image sensor with an active array size of
> > > > 1920 x 1080.
> > > >
> > > > The following features are supported:
> > > > - Manual exposure an gain control support
> > > > - vblank/hblank control support
> > > > - vflip/hflip control support
> > > > - Test pattern control support
> > > > - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)
> > > >
> > > > Signed-off-by: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> > > > Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
> > > > ---
> > > > MAINTAINERS | 1 +
> > > > drivers/media/i2c/Kconfig | 10 +
> > > > drivers/media/i2c/Makefile | 1 +
> > > > drivers/media/i2c/os02g10.c | 1039 +++++++++++++++++++++++++++++++++++
> > > > 4 files changed, 1051 insertions(+)
> > > > create mode 100644 drivers/media/i2c/os02g10.c
> >
> > I have added a new function, os02g10_set_framefmt, which dynamically sets
> > the mode register.
> >
> > Please let me know if I have missed anything or if further changes are
> > needed.
>
> You also need to drop the supported_modes array, and implement support
> for .set_selection().
I don't think we have a very well established behaviour for set_selections
on sensors before the common raw sensor model, do we? I guess it can be the
same as get_selections, though.
Hopefully we can merge the metadata series soon...
--
Regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH v3] dt-bindings: display: ti, am65x-dss: Fix AM62L DSS reg and clock constraints
From: Swamil Jain @ 2026-04-14 10:06 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: jyri.sarha, tomi.valkeinen, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, robh, krzk+dt, conor+dt, devarsht,
dri-devel, devicetree, linux-kernel, praneeth, vigneshr
In-Reply-To: <20260411-resilient-tireless-centipede-cd6fef@quoll>
Hi Krzysztof,
On 4/11/26 19:37, Krzysztof Kozlowski wrote:
> On Fri, Apr 10, 2026 at 04:29:55PM +0530, Swamil Jain wrote:
>> clocks:
>> + minItems: 2
>> items:
>> - description: fck DSS functional clock
>> - description: vp1 Video Port 1 pixel clock
>> - description: vp2 Video Port 2 pixel clock
>>
>> clock-names:
>> + minItems: 2
>> items:
>> - const: fck
>> - const: vp1
>> @@ -179,6 +195,20 @@ allOf:
>> ports:
>> properties:
>> port@1: false
>> + clock-names:
>> + maxItems: 2
>> + clocks:
>> + maxItems: 2
>> + reg:
>> + maxItems: 5
>
> Also constrain for reg-names,
>
Sure, will add in v4.
>> + else:
>> + properties:
>> + clock-names:
>> + minItems: 3
>> + clocks:
>> + minItems: 3
>> + reg:
>> + minItems: 8
>
> Same here, please.
>
> And if you are sending new version: they should be listed in the same
> order as in top-level properties, so reg, reg-names, clocks and
> clock-names. (juging by the diff)
Yeah, sure, will keep the order. Thanks for the feedback, will
re-spin the patch.
Regards,
Swamil.
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH 3/7] arm64: dts: qcom: monaco: Add label properties to CoreSight devices
From: Konrad Dybcio @ 2026-04-14 10:09 UTC (permalink / raw)
To: Jie Gan, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260410-add-label-to-coresight-device-v1-3-d71a6759dbc2@oss.qualcomm.com>
On 4/10/26 5:08 AM, Jie Gan wrote:
> Add label properties to TPDM and CTI nodes in the monaco device tree to
> provide human-readable identifiers for each CoreSight device. These
> labels allow userspace tools and the CoreSight framework to identify
> devices by name rather than by base address.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 4/7] arm64: dts: qcom: kodiak: Add label properties to CoreSight devices
From: Konrad Dybcio @ 2026-04-14 10:11 UTC (permalink / raw)
To: Jie Gan, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260410-add-label-to-coresight-device-v1-4-d71a6759dbc2@oss.qualcomm.com>
On 4/10/26 5:08 AM, Jie Gan wrote:
> Add label properties to TPDM and CTI nodes in the kodiak device tree to
> provide human-readable identifiers for each CoreSight device. These
> labels allow userspace tools and the CoreSight framework to identify
> devices by name rather than by base address.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v2 2/2] media: i2c: add os02g10 image sensor driver
From: Laurent Pinchart @ 2026-04-14 10:12 UTC (permalink / raw)
To: sakari.ailus@linux.intel.com
Cc: Elgin Perumbilly, Tarang Raval, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans Verkuil,
Hans de Goede, Vladimir Zapolskiy, Mehdi Djait, Benjamin Mugnier,
Sylvain Petinot, Hardevsinh Palaniya, Jingjing Xiong,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <ad4RrbNLa1EXYZM0@kekkonen.localdomain>
On Tue, Apr 14, 2026 at 01:06:37PM +0300, Sakari Ailus wrote:
> On Tue, Apr 14, 2026 at 12:57:27PM +0300, Laurent Pinchart wrote:
> > On Tue, Apr 14, 2026 at 09:43:32AM +0000, Elgin Perumbilly wrote:
> > > Hi Laurent,
> > >
> > > > I sent a review comment on v1.
> > > >
> > > > On Tue, Apr 14, 2026 at 02:19:45PM +0530, Elgin Perumbilly wrote:
> > > > > Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.
> > > > >
> > > > > The Omnivision os02g10 is a CMOS image sensor with an active array size of
> > > > > 1920 x 1080.
> > > > >
> > > > > The following features are supported:
> > > > > - Manual exposure an gain control support
> > > > > - vblank/hblank control support
> > > > > - vflip/hflip control support
> > > > > - Test pattern control support
> > > > > - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)
> > > > >
> > > > > Signed-off-by: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> > > > > Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
> > > > > ---
> > > > > MAINTAINERS | 1 +
> > > > > drivers/media/i2c/Kconfig | 10 +
> > > > > drivers/media/i2c/Makefile | 1 +
> > > > > drivers/media/i2c/os02g10.c | 1039 +++++++++++++++++++++++++++++++++++
> > > > > 4 files changed, 1051 insertions(+)
> > > > > create mode 100644 drivers/media/i2c/os02g10.c
> > >
> > > I have added a new function, os02g10_set_framefmt, which dynamically sets
> > > the mode register.
> > >
> > > Please let me know if I have missed anything or if further changes are
> > > needed.
> >
> > You also need to drop the supported_modes array, and implement support
> > for .set_selection().
>
> I don't think we have a very well established behaviour for set_selections
> on sensors before the common raw sensor model, do we? I guess it can be the
> same as get_selections, though.
>
> Hopefully we can merge the metadata series soon...
I've bumped that high on my list of priority tasks.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 5/7] arm64: dts: qcom: kaanapali: Add label properties to CoreSight devices
From: Konrad Dybcio @ 2026-04-14 10:15 UTC (permalink / raw)
To: Jie Gan, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260410-add-label-to-coresight-device-v1-5-d71a6759dbc2@oss.qualcomm.com>
On 4/10/26 5:08 AM, Jie Gan wrote:
> Add label properties to TPDM nodes in the kaanapali device tree to
> provide human-readable identifiers for each CoreSight device. These
> labels allow userspace tools and the CoreSight framework to identify
> devices by name rather than by base address.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 6/7] arm64: dts: qcom: sm8750: Add label properties to CoreSight devices
From: Konrad Dybcio @ 2026-04-14 10:19 UTC (permalink / raw)
To: Jie Gan, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260410-add-label-to-coresight-device-v1-6-d71a6759dbc2@oss.qualcomm.com>
On 4/10/26 5:08 AM, Jie Gan wrote:
> Add label properties to TPDM and CTI nodes in the sm8750 device tree to
> provide human-readable identifiers for each CoreSight device. These
> labels allow userspace tools and the CoreSight framework to identify
> devices by name rather than by base address.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
[...]
> tpdm-cdsp-llm {
> compatible = "qcom,coresight-static-tpdm";
> + label = "tpdm_cdsp_llm";
> qcom,cmb-element-bits = <32>;
>
> out-ports {
> @@ -6814,6 +6839,7 @@ tpdm_cdsp_llm_out: endpoint {
>
> tpdm-cdsp-llm2 {
> compatible = "qcom,coresight-static-tpdm";
> + label = "tpdm_cdsp_llm2";
> qcom,cmb-element-bits = <32>;
>
> out-ports {
> @@ -6827,6 +6853,7 @@ tpdm_cdsp_llm2_out: endpoint {
>
> tpdm-modem1 {
> compatible = "qcom,coresight-static-tpdm";
> + label = "tpdm_modem_1";
Please fix the extra \t
Konrad
^ permalink raw reply
* Re: [PATCH v2 2/2] media: i2c: add os02g10 image sensor driver
From: Tarang Raval @ 2026-04-14 10:19 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Elgin Perumbilly, sakari.ailus@linux.intel.com,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hans Verkuil, Hans de Goede, Vladimir Zapolskiy,
Mehdi Djait, Benjamin Mugnier, Sylvain Petinot,
Hardevsinh Palaniya, Jingjing Xiong, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20260414095727.GF4061@killaraus.ideasonboard.com>
Hi Laurent,
> On Tue, Apr 14, 2026 at 09:43:32AM +0000, Elgin Perumbilly wrote:
> > Hi Laurent,
> >
> > > I sent a review comment on v1.
> > >
> > > On Tue, Apr 14, 2026 at 02:19:45PM +0530, Elgin Perumbilly wrote:
> > > > Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.
> > > >
> > > > The Omnivision os02g10 is a CMOS image sensor with an active array size of
> > > > 1920 x 1080.
> > > >
> > > > The following features are supported:
> > > > - Manual exposure an gain control support
> > > > - vblank/hblank control support
> > > > - vflip/hflip control support
> > > > - Test pattern control support
> > > > - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)
> > > >
> > > > Signed-off-by: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> > > > Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
> > > > ---
> > > > MAINTAINERS | 1 +
> > > > drivers/media/i2c/Kconfig | 10 +
> > > > drivers/media/i2c/Makefile | 1 +
> > > > drivers/media/i2c/os02g10.c | 1039 +++++++++++++++++++++++++++++++++++
> > > > 4 files changed, 1051 insertions(+)
> > > > create mode 100644 drivers/media/i2c/os02g10.c
> >
> > I have added a new function, os02g10_set_framefmt, which dynamically sets
> > the mode register.
> >
> > Please let me know if I have missed anything or if further changes are
> > needed.
>
> You also need to drop the supported_modes array, and implement support
> for .set_selection().
Are you suggesting that we should drop the array below?
static const struct os02g10_mode supported_modes[] = {
{
.width = 1920,
.height = 1080,
.vts_def = 1246,
.hts_def = 1082,
.exp_def = 1100,
.x_start = 2,
.y_start = 6,
},
};
If we remove this, how would we provide mode-specific parameters such as VTS?
Best Regards,
Tarang
^ permalink raw reply
* Re: [PATCH] arm64: dts: exynos850: Add SRAM node
From: Tudor Ambarus @ 2026-04-14 10:20 UTC (permalink / raw)
To: Krzysztof Kozlowski, Alexey Klimov, Sam Protsenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alim Akhtar
Cc: linux-samsung-soc, linux-arm-kernel, devicetree, linux-kernel,
Juan Yescas, Peter Griffin, André Draszik
In-Reply-To: <4c6a92e0-15a1-4f82-afc9-542f5ad9d2df@kernel.org>
Hi!
On 4/14/26 12:08 PM, Krzysztof Kozlowski wrote:
> On 14/04/2026 11:00, Alexey Klimov wrote:
>> On Mon Apr 13, 2026 at 4:23 PM BST, Krzysztof Kozlowski wrote:
>>> On 13/04/2026 16:52, Alexey Klimov wrote:
>>>> SRAM is used by the ACPM protocol to retrieve the ACPM channels
>>>> information and configuration data. Add the SRAM node.
>>>>
>>>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>>>> ---
>>>> arch/arm64/boot/dts/exynos/exynos850.dtsi | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
>>>> index cb55015c8dce..cf4a6168846c 100644
>>>> --- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
>>>> +++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
>>>> @@ -910,6 +910,14 @@ spi_2: spi@11d20000 {
>>>> };
>>>> };
>>>> };
>>>> +
>>>> + apm_sram: sram@2039000 {
>>>> + compatible = "mmio-sram";
>>>> + reg = <0x0 0x2039000 0x40000>;
>>>> + #address-cells = <1>;
>>>> + #size-cells = <1>;
>>>> + ranges = <0x0 0x0 0x2039000 0x40000>;
>>>
>>> You miss here children.
>>
>> Thank you! I guess I should convert it to smth like this:
>>
>> apm_sram: sram@2039000 {
>> compatible = "mmio-sram";
>> reg = <0x0 0x2039000 0x40000>;
>> ranges = <0x0 0x0 0x2039000 0x40000>;
>> #address-cells = <1>;
>> #size-cells = <1>;
>>
>> acpm_sram_region: sram-section@0 {
>> reg = <0x0 0x40000>;
>
> This covers entire block, so feels pointless. Maybe requirement of
> children should be dropped. What's the point of having children? Why
> does the driver need them?
>
>> };
>> };
>>
>> And then later reference shmem = &acpm_sram_region from acpm node.
>>
>>> Also, 'ranges' should be after 'reg'.
>>
>> Thanks, will fix this.
>>
>> FWIW this commit is a copy of commit 48e7821b26904
>> https://lore.kernel.org/r/20250207-gs101-acpm-dt-v4-1-230ba8663a2d@linaro.org
>
>
> Huh, we should fix that one as well.
>
On gs101, likely on e850 too, ACPM parses the SRAM and discovers at runtime where
the TX/RX queue offsets are in SRAM. So we can't define static partitions in DT.
I remember that I thought about extending the SRAM driver to add dynamic
partitions (clients to request the mmio-sram driver to create partitions at
runtime), but it was just ACPM that's using SRAM, so I didn't need it.
Cheers,
ta
^ permalink raw reply
* Re: [PATCH 7/7] arm64: dts: qcom: hamoa: Add label properties to CoreSight devices
From: Konrad Dybcio @ 2026-04-14 10:22 UTC (permalink / raw)
To: Jie Gan, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260410-add-label-to-coresight-device-v1-7-d71a6759dbc2@oss.qualcomm.com>
On 4/10/26 5:08 AM, Jie Gan wrote:
> Add label properties to TPDM and CTI nodes in the hamoa device tree to
> provide human-readable identifiers for each CoreSight device. These
> labels allow userspace tools and the CoreSight framework to identify
> devices by name rather than by base address.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v11 2/4] crypto: spacc - Add SPAcc ahash support
From: Pavitrakumar Managutte @ 2026-04-14 10:28 UTC (permalink / raw)
To: Herbert Xu
Cc: linux-crypto, linux-kernel, devicetree, robh, conor+dt,
Ruud.Derwig, manjunath.hadli, adityak, navami.telsang, bhoomikak
In-Reply-To: <CALxtO0nFEG2Lm18Fnb=YVQfy4-Qjb5+WtOxsHNOwYTy2Kzyb4g@mail.gmail.com>
Hi Herbert,
If the above snip looks good, I can push that and some more code
clean-ups/improvements as part of V12 patchset. Do let me know.
Below are the code fixes and improvements
1. Multi-device safety handling - All packed up inside priv
2. Minor code polishes
3. memzero_explicit inside setkey, spacc_compute_xcbc_key etc.
4. Algo registration clean-ups
Warm regards,
PK
On Thu, Apr 2, 2026 at 1:30 PM Pavitrakumar Managutte
<pavitrakumarm@vayavyalabs.com> wrote:
>
> Hi Herbert,
> As per your inputs, I've replaced the do_shash switch to use
> lib/crypto single-shot calls for SHA-1, SHA-224, SHA-256, SHA-384,
> SHA-512, and MD5.
>
> However, SM3 does not have a single-shot library API in lib/crypto yet
> — include/crypto/sm3.h exposes sm3_init() and sm3_block_generic(),
> with no sm3()/sm3_update()/sm3_final() equivalents.
>
> For now, I've retained do_shash only for the SM3 case. Would this be
> acceptable, or would you prefer a different approach?
>
>
> Code snippet below for your reference
> ============== snip start ================
>
> switch (salg->mode->id) {
> case CRYPTO_MODE_HMAC_SHA224:
> sha224(key, keylen, tctx->ipad);
> break;
>
> case CRYPTO_MODE_HMAC_SHA256:
> sha256(key, keylen, tctx->ipad);
> break;
>
> case CRYPTO_MODE_HMAC_SHA384:
> sha384(key, keylen, tctx->ipad);
> break;
>
> case CRYPTO_MODE_HMAC_SHA512:
> sha512(key, keylen, tctx->ipad);
> break;
>
> case CRYPTO_MODE_HMAC_MD5:
> md5(key, keylen, tctx->ipad);
> break;
>
> case CRYPTO_MODE_HMAC_SHA1:
> sha1(key, keylen, tctx->ipad);
> break;
>
> case CRYPTO_MODE_HMAC_SM3:
> rc = do_shash(salg->dev, "sm3", tctx->ipad, key,
> keylen);
> if (rc < 0) {
> dev_err(salg->dev,
> "ERR: %d computing shash for sm3\n", rc);
> return -EIO;
> }
> break;
>
> default:
> return -EINVAL;
> }
>
> ============== snip end ================
>
> Warm Regards,
> PK
>
>
>
> On Fri, Mar 27, 2026 at 2:50 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
> >
> > On Wed, Mar 18, 2026 at 12:48:06PM +0530, Pavitrakumar Managutte wrote:
> > >
> > > + switch (salg->mode->id) {
> > > + case CRYPTO_MODE_HMAC_SHA224:
> > > + rc = do_shash(salg->dev, "sha224", tctx->ipad, key,
> > > + keylen);
> > > + break;
> >
> > Since you're doing a giant switch statement anyway, please convert
> > this to use lib/crypto instead of shash.
> >
> > Thanks,
> > --
> > Email: Herbert Xu <herbert@gondor.apana.org.au>
> > Home Page: http://gondor.apana.org.au/~herbert/
> > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v7 0/8] Add support for handling PCIe M.2 Key E connectors in devicetree
From: Chen-Yu Tsai @ 2026-04-14 10:29 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Manivannan Sadhasivam, Manivannan Sadhasivam, Rob Herring,
Greg Kroah-Hartman, Jiri Slaby, Nathan Chancellor, Nicolas Schier,
Hans de Goede, Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Krzysztof Kozlowski, Conor Dooley, Marcel Holtmann,
Luiz Augusto von Dentz, Bartosz Golaszewski, Bartosz Golaszewski,
linux-serial, linux-kernel, linux-kbuild, platform-driver-x86,
linux-pci, devicetree, linux-arm-msm, linux-bluetooth, linux-pm,
Stephan Gerhold, Dmitry Baryshkov, linux-acpi, Hans de Goede,
Bartosz Golaszewski
In-Reply-To: <ad36pIu-0dutL7Nk@ashevche-desk.local>
On Tue, Apr 14, 2026 at 4:28 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Apr 14, 2026 at 01:03:19PM +0800, Chen-Yu Tsai wrote:
> > On Tue, Apr 14, 2026 at 12:08 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > > On Mon, Apr 13, 2026 at 07:33:12PM +0530, Manivannan Sadhasivam wrote:
> > > > On Mon, Apr 13, 2026 at 03:54:59PM +0800, Chen-Yu Tsai wrote:
> > > > > On Thu, Mar 26, 2026 at 01:36:28PM +0530, Manivannan Sadhasivam wrote:
>
> ...
>
> > > > > - Given that this connector actually represents two devices, how do I
> > > > > say I want the BT part to be a wakeup source, but not the WiFi part?
> > > > > Does wakeup-source even work at this point?
> > > >
> > > > You can't use the DT property since the devices are not described in DT
> > > > statically. But you can still use the per-device 'wakeup' sysfs knob to enable
> > > > wakeup.
> >
> > I see. I think not being able to specify generic properties for the devices
> > on the connector is going to be a bit problematic.
>
> This is nature of the open-connectors, especially on the busses that are
> hotpluggable, like PCIe. We never know what is connected there _ahead_.
I believe what you mean by "hotpluggable" is "user replaceable".
> In other words you can't describe in DT something that may not exist.
But this is actually doable with the PCIe slot representation. The
properties are put in the device node for the slot. If no card is
actually inserted in the slot, then no device is created, and the
device node is left as not associated with anything.
It's just that for this new M.2 E-key connector, there aren't separate
nodes for each interface. And the system doesn't associate the device
node with the device, because it's no longer a child node of the
controller or hierarchy, but connected over the OF graph.
Moving over to the E-key connector representation seems like one step
forward and one step backward in descriptive ability. We gain proper
power sequencing, but lose generic properties.
The latter part is solvable, but we likely need child nodes under the
connector for the different interfaces. Properties that make sense for
one type might not make sense for another.
Thanks
ChenYu
P.S. We could also just add child device nodes under the controller to
put the generic properties, but that's splitting the description into
multiple parts. Let's not go there if at all possible.
^ permalink raw reply
* RE: [PATCH v7 4/6] iio: adc: ad4691: add SPI offload support
From: Sabau, Radu bogdan @ 2026-04-14 10:28 UTC (permalink / raw)
To: David Lechner, Lars-Peter Clausen, Hennerich, Michael,
Jonathan Cameron, Sa, Nuno, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
Philipp Zabel, Jonathan Corbet, Shuah Khan
Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <1170956f-da05-4280-990f-64306ca905c2@baylibre.com>
> -----Original Message-----
> From: David Lechner <dlechner@baylibre.com>
> Sent: Saturday, April 11, 2026 12:01 AM
...
> >
> > static const struct ad4691_chip_info ad4694_chip_info = {
> > .name = "ad4694",
> > .max_rate = 1 * HZ_PER_MHZ,
> > .sw_info = &ad4693_sw_info,
> > + .offload_info = &ad4693_offload_info,
> > +};
> > +
> > +struct ad4691_offload_state {
> > + struct spi_offload *spi;
>
> I would call this "offload" or "instance". "spi" is usally the SPI
> device handle.
I thought about this too, will implement it as offload then.
>
> > + struct spi_offload_trigger *trigger;
> > + u64 trigger_hz;
> > + u8 tx_cmd[17][2];
> > + u8 tx_reset[4];
> > };
> >
>
> ...
>
> > +
> > +static int ad4691_cnv_burst_offload_buffer_predisable(struct iio_dev
> *indio_dev)
> > +{
> > + struct ad4691_state *st = iio_priv(indio_dev);
> > + struct ad4691_offload_state *offload = st->offload;
> > + int ret;
> > +
> > + spi_offload_trigger_disable(offload->spi, offload->trigger);
> > +
> > + ret = ad4691_sampling_enable(st, false);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> > + AD4691_SEQ_ALL_CHANNELS_OFF);
>
> Why this extra step? We don't have it when unwinding in the
> error path of the postenable function.
This is a mistake from my end. Perhaps this could be removed since
the sequencer is over-written upon new buffers/raw readings anyway.
>
> > + if (ret)
> > + return ret;
> > +
> > + spi_unoptimize_message(&st->scan_msg);
> > +
> > + return ad4691_exit_conversion_mode(st);
> > +}
> > +
> > +static const struct iio_buffer_setup_ops
> ad4691_cnv_burst_offload_buffer_setup_ops = {
> > + .postenable = &ad4691_cnv_burst_offload_buffer_postenable,
> > + .predisable = &ad4691_cnv_burst_offload_buffer_predisable,
> > +};
> > +
> > static ssize_t sampling_frequency_show(struct device *dev,
> > struct device_attribute *attr,
> > char *buf)
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: monaco: Add iface clock and power domain for ice sdhc
From: Konrad Dybcio @ 2026-04-14 10:30 UTC (permalink / raw)
To: Kuldeep Singh, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260409-ice_emmc_clock_addition-v2-2-90bbcc057361@oss.qualcomm.com>
On 4/9/26 10:31 AM, Kuldeep Singh wrote:
> Qualcomm in-line crypto engine (ICE) platform driver specifies and votes
> for its own resources. Before accessing ICE hardware during probe, to
> avoid potential unclocked register access issues (when clk_ignore_unused
> is not passed on the kernel command line), in addition to the 'core'
> clock the 'iface' clock should also be turned on by the driver. This can
> only be done if power domain is enabled.
>
> Specify both power domain and the iface clock.
>
> Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v2 1/2] arm64: dts: qcom: kodiak: Add iface clock and power domain for ice sdhc
From: Konrad Dybcio @ 2026-04-14 10:30 UTC (permalink / raw)
To: Kuldeep Singh, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260409-ice_emmc_clock_addition-v2-1-90bbcc057361@oss.qualcomm.com>
On 4/9/26 10:31 AM, Kuldeep Singh wrote:
> Qualcomm in-line crypto engine (ICE) platform driver specifies and votes
> for its own resources. Before accessing ICE hardware during probe, to
> avoid potential unclocked register access issues (when clk_ignore_unused
> is not passed on the kernel command line), in addition to the 'core'
> clock the 'iface' clock should also be turned on by the driver. This can
> only be done if power domain is enabled.
>
> Specify both power domain and the iface clock.
>
> Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* RE: [PATCH v7 5/6] iio: adc: ad4691: add oversampling support
From: Sabau, Radu bogdan @ 2026-04-14 10:32 UTC (permalink / raw)
To: David Lechner, Lars-Peter Clausen, Hennerich, Michael,
Jonathan Cameron, Sa, Nuno, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
Philipp Zabel, Jonathan Corbet, Shuah Khan
Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <742b1821-9103-414e-a860-c2e8d5406e35@baylibre.com>
> -----Original Message-----
> From: David Lechner <dlechner@baylibre.com>
> Sent: Saturday, April 11, 2026 12:15 AM
...
> >
> > osc_idx = FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val);
> > - /* Wait 2 oscillator periods for the conversion to complete. */
> > - period_us = DIV_ROUND_UP(2UL * USEC_PER_SEC,
> ad4691_osc_freqs_Hz[osc_idx]);
> > + /* Wait osr oscillator periods for all accumulator samples to complete.
> */
>
> Why did we need to way 2 before and only 1 now when OSR == 1?
>
You are right, that extra period should exist when reading raw not dependent
on the OSR. If OSR = 4 then we should wait 5 just to make sure we are reading
a correct result, since the single_shot_read doesn’t use any interrupts as the
buffers do.
^ permalink raw reply
* Re: [PATCH v2 2/2] media: i2c: add os02g10 image sensor driver
From: Laurent Pinchart @ 2026-04-14 10:33 UTC (permalink / raw)
To: Tarang Raval
Cc: Elgin Perumbilly, sakari.ailus@linux.intel.com,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hans Verkuil, Hans de Goede, Vladimir Zapolskiy,
Mehdi Djait, Benjamin Mugnier, Sylvain Petinot,
Hardevsinh Palaniya, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <PN3P287MB1829155B216E557C7DF7B6778B252@PN3P287MB1829.INDP287.PROD.OUTLOOK.COM>
On Tue, Apr 14, 2026 at 10:19:23AM +0000, Tarang Raval wrote:
> > On Tue, Apr 14, 2026 at 09:43:32AM +0000, Elgin Perumbilly wrote:
> > > > On Tue, Apr 14, 2026 at 02:19:45PM +0530, Elgin Perumbilly wrote:
> > > > > Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.
> > > > >
> > > > > The Omnivision os02g10 is a CMOS image sensor with an active array size of
> > > > > 1920 x 1080.
> > > > >
> > > > > The following features are supported:
> > > > > - Manual exposure an gain control support
> > > > > - vblank/hblank control support
> > > > > - vflip/hflip control support
> > > > > - Test pattern control support
> > > > > - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)
> > > > >
> > > > > Signed-off-by: Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>
> > > > > Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
> > > > > ---
> > > > > MAINTAINERS | 1 +
> > > > > drivers/media/i2c/Kconfig | 10 +
> > > > > drivers/media/i2c/Makefile | 1 +
> > > > > drivers/media/i2c/os02g10.c | 1039 +++++++++++++++++++++++++++++++++++
> > > > > 4 files changed, 1051 insertions(+)
> > > > > create mode 100644 drivers/media/i2c/os02g10.c
> > >
> > > I have added a new function, os02g10_set_framefmt, which dynamically sets
> > > the mode register.
> > >
> > > Please let me know if I have missed anything or if further changes are
> > > needed.
> >
> > You also need to drop the supported_modes array, and implement support
> > for .set_selection().
>
> Are you suggesting that we should drop the array below?
Correct.
> static const struct os02g10_mode supported_modes[] = {
> {
> .width = 1920,
> .height = 1080,
> .vts_def = 1246,
> .hts_def = 1082,
> .exp_def = 1100,
> .x_start = 2,
> .y_start = 6,
> },
> };
>
> If we remove this, how would we provide mode-specific parameters such as VTS?
Those should be computed by the driver based on the format and crop
rectangle configured by userspace.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 0/3] Mediatek Genio 510/700-EVK: add CPU power supplies
From: Louis-Alexis Eyraud @ 2026-04-14 10:33 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno
Cc: kernel, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Louis-Alexis Eyraud
This series adds for the Mediatek Genio 510-EVK (MT8370) and 700-EVK
(MT8390) boards the CPU power supply definitions in their devicetree
that are missing for all their CPU cores.
On the boards, the big core power is supplied by a MT6319 (sub PMIC)
and little core power by a MT6365 (main PMIC).
Patch 1 adds the MT6319 PMIC support that was not yet enabled for these
boards.
Patch 2 adds the CPU power supplies definitions that are common to both
Genio 510 and 700 EVK boards, and patch 3 adds the Genio 700-EVK
specific ones.
The series has been tested on Genio 510-EVK board with a kernel based
on linux-next (tag: next-20260410).
Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
Louis-Alexis Eyraud (3):
arm64: dts: mediatek: mt8390-genio-common: add MT6319 PMIC support
arm64: dts: mediatek: mt8390-genio-common: add CPU power supplies
arm64: dts: mediatek: mt8390-genio-700-evk: add specific CPU power supplies
.../boot/dts/mediatek/mt8390-genio-700-evk.dts | 7 +++
.../boot/dts/mediatek/mt8390-genio-common.dtsi | 68 ++++++++++++++++++++++
2 files changed, 75 insertions(+)
---
base-commit: f244905cd8cff7a7249cd3dac8a366e02d61ad4f
change-id: 20260413-mtk-g510-700-cpu-supplies-41a78a6bf175
Best regards,
--
Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
^ permalink raw reply
* [PATCH 1/3] arm64: dts: mediatek: mt8390-genio-common: add MT6319 PMIC support
From: Louis-Alexis Eyraud @ 2026-04-14 10:33 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno
Cc: kernel, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Louis-Alexis Eyraud
In-Reply-To: <20260414-mtk-g510-700-cpu-supplies-v1-0-3b8313e5ca8d@collabora.com>
Mediatek Genio 510 and 700-EVK boards integrate a MT6319 PMIC, powered
by the board system power rail (VSYS) and connected to the SPMI
interface. It provides buck regulators for CPU core power supplies in
particular.
Add the needed nodes in the board common dtsi to enable its support.
Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
.../boot/dts/mediatek/mt8390-genio-common.dtsi | 44 ++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8390-genio-common.dtsi b/arch/arm64/boot/dts/mediatek/mt8390-genio-common.dtsi
index 2062506f6cc5..aab474d6c5f8 100644
--- a/arch/arm64/boot/dts/mediatek/mt8390-genio-common.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8390-genio-common.dtsi
@@ -1364,6 +1364,50 @@ &spi2 {
status = "okay";
};
+&spmi {
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ pmic@6 {
+ compatible = "mediatek,mt6319-regulator", "mediatek,mt6315-regulator";
+ reg = <0x6 SPMI_USID>;
+
+ pvdd1-supply = <®_vsys>;
+ pvdd2-supply = <®_vsys>;
+ pvdd3-supply = <®_vsys>;
+ pvdd4-supply = <®_vsys>;
+
+ regulators {
+ mt6319_vbuck1: vbuck1 {
+ regulator-name = "dvdd_proc_b";
+ regulator-min-microvolt = <300000>;
+ regulator-max-microvolt = <1193750>;
+ regulator-enable-ramp-delay = <256>;
+ regulator-allowed-modes = <0 1 2>;
+ regulator-always-on;
+ };
+
+ vbuck3 {
+ regulator-name = "avdd2_emi";
+ regulator-min-microvolt = <300000>;
+ regulator-max-microvolt = <1193750>;
+ regulator-enable-ramp-delay = <256>;
+ regulator-allowed-modes = <0 1 2>;
+ regulator-always-on;
+ };
+
+ vbuck4 {
+ regulator-name = "avddq_emi";
+ regulator-min-microvolt = <300000>;
+ regulator-max-microvolt = <1193750>;
+ regulator-enable-ramp-delay = <256>;
+ regulator-allowed-modes = <0 1 2>;
+ regulator-always-on;
+ };
+ };
+ };
+};
+
&uart0 {
pinctrl-0 = <&uart0_pins>;
pinctrl-names = "default";
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox