* [PATCH v2 0/2] media: i2c: Add OmniVision OV6211 image sensor driver @ 2025-07-29 23:14 Vladimir Zapolskiy 2025-07-29 23:14 ` [PATCH v2 1/2] dt-bindings: media: i2c: Add OmniVision OV6211 image sensor Vladimir Zapolskiy 2025-07-29 23:14 ` [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver Vladimir Zapolskiy 0 siblings, 2 replies; 7+ messages in thread From: Vladimir Zapolskiy @ 2025-07-29 23:14 UTC (permalink / raw) To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus, Hans Verkuil, Dave Stevenson, Mehdi Djait Cc: linux-media, devicetree OmniVision OV6211 is a monochrome image sensor, which produces frames in 8/10-bit raw output format and supports 400x400, 200x200 and 100x100 output image resolution modes. At the moment the only supported resolution in the device driver is 400x400@120fps (Y8). The driver version is based on top of the series, which adds a new devm_v4l2_sensor_clk_get() helper [1]. Link to v1 of the OV6211 camera sensor device driver: * https://lore.kernel.org/linux-media/20250717124001.108486-1-vladimir.zapolskiy@linaro.org/ Changes from v1 to v2: 1. restrict endpoint unevaluated properties (Krzysztof), 2. changed dev_err() to dev_err_probe() whenever it's applicable (Krzysztof), 3. removed in-driver I2C operations in favour of V4L2 CCI interface (Kieran), 4. added hblank, vblank, pixel rate and rotation/orientation V4L2 ro controls (Kieran, Dave), 5. due to unselectable data lanes property removed data_lanes handling (Dave), 6. replaced devm_clk_get_optional() with devm_v4l2_sensor_clk_get() (Dave, Mehdi), 7. minor cosmetic updates (reported error messages, goto label names etc.). [1] https://lore.kernel.org/linux-media/8ecbcafbd91b25ad5e188dbe127b921a1643027e.1750942967.git.mehdi.djait@linux.intel.com/ Vladimir Zapolskiy (2): dt-bindings: media: i2c: Add OmniVision OV6211 image sensor media: i2c: Add OmniVision OV6211 image sensor driver .../bindings/media/i2c/ovti,ov6211.yaml | 96 ++ MAINTAINERS | 8 + drivers/media/i2c/Kconfig | 10 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov6211.c | 821 ++++++++++++++++++ 5 files changed, 936 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov6211.yaml create mode 100644 drivers/media/i2c/ov6211.c -- 2.49.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] dt-bindings: media: i2c: Add OmniVision OV6211 image sensor 2025-07-29 23:14 [PATCH v2 0/2] media: i2c: Add OmniVision OV6211 image sensor driver Vladimir Zapolskiy @ 2025-07-29 23:14 ` Vladimir Zapolskiy 2025-07-30 22:32 ` Rob Herring (Arm) 2025-07-29 23:14 ` [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver Vladimir Zapolskiy 1 sibling, 1 reply; 7+ messages in thread From: Vladimir Zapolskiy @ 2025-07-29 23:14 UTC (permalink / raw) To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus, Hans Verkuil, Dave Stevenson, Mehdi Djait Cc: linux-media, devicetree Add device tree bindings documentation for OmniVision OV6211 image sensor. Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> --- .../bindings/media/i2c/ovti,ov6211.yaml | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov6211.yaml diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov6211.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov6211.yaml new file mode 100644 index 000000000000..0c552421eea5 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov6211.yaml @@ -0,0 +1,96 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/i2c/ovti,ov6211.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: OmniVision OV6211 Image Sensor + +description: + OmniVision OV6211 image sensor is a high performance monochrome image + sensor. The sensor is controlled over a serial camera control bus + protocol (SCCB), the widest supported output image frame size is 400x400 + at 120 frames per second rate, data output format is 8/10-bit RAW + transferred over one-lane MIPI D-PHY interface. + +maintainers: + - Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> + +allOf: + - $ref: /schemas/media/video-interface-devices.yaml# + +properties: + compatible: + const: ovti,ov6211 + + reg: + maxItems: 1 + + clocks: + description: XVCLK supply clock, 6MHz to 27MHz frequency. + maxItems: 1 + + reset-gpios: + description: Active low GPIO connected to XSHUTDOWN pad of the sensor. + maxItems: 1 + + strobe-gpios: + description: Input GPIO connected to strobe pad of the sensor. + maxItems: 1 + + avdd-supply: + description: Analogue voltage supply, 2.6 to 3.0 volts. + + dovdd-supply: + description: Digital I/O voltage supply, 1.7 to 3.0 volts. + + dvdd-supply: + description: Digital core voltage supply, 1.2 volts. + + port: + $ref: /schemas/graph.yaml#/$defs/port-base + additionalProperties: false + + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + required: + - link-frequencies + +required: + - compatible + - reg + - port + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + camera@60 { + compatible = "ovti,ov6211"; + reg = <0x60>; + clocks = <&camera_clk 0>; + assigned-clocks = <&camera_clk 0>; + assigned-clock-rates = <24000000>; + reset-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>; + avdd-supply = <&vreg_2p8>; + dovdd-supply = <&vreg_1p8>; + dvdd-supply = <&vreg_1p2>; + + port { + endpoint { + link-frequencies = /bits/ 64 <240000000>; + remote-endpoint = <&mipi_csi2_ep>; + }; + }; + }; + }; +... -- 2.49.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add OmniVision OV6211 image sensor 2025-07-29 23:14 ` [PATCH v2 1/2] dt-bindings: media: i2c: Add OmniVision OV6211 image sensor Vladimir Zapolskiy @ 2025-07-30 22:32 ` Rob Herring (Arm) 0 siblings, 0 replies; 7+ messages in thread From: Rob Herring (Arm) @ 2025-07-30 22:32 UTC (permalink / raw) To: Vladimir Zapolskiy Cc: Krzysztof Kozlowski, Hans Verkuil, linux-media, Sakari Ailus, Mehdi Djait, Mauro Carvalho Chehab, devicetree, Dave Stevenson, Conor Dooley On Wed, 30 Jul 2025 02:14:53 +0300, Vladimir Zapolskiy wrote: > Add device tree bindings documentation for OmniVision OV6211 image > sensor. > > Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > --- > .../bindings/media/i2c/ovti,ov6211.yaml | 96 +++++++++++++++++++ > 1 file changed, 96 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov6211.yaml > Reviewed-by: Rob Herring (Arm) <robh@kernel.org> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver 2025-07-29 23:14 [PATCH v2 0/2] media: i2c: Add OmniVision OV6211 image sensor driver Vladimir Zapolskiy 2025-07-29 23:14 ` [PATCH v2 1/2] dt-bindings: media: i2c: Add OmniVision OV6211 image sensor Vladimir Zapolskiy @ 2025-07-29 23:14 ` Vladimir Zapolskiy 2025-07-30 7:17 ` Tarang Raval ` (2 more replies) 1 sibling, 3 replies; 7+ messages in thread From: Vladimir Zapolskiy @ 2025-07-29 23:14 UTC (permalink / raw) To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus, Hans Verkuil, Dave Stevenson, Mehdi Djait Cc: linux-media, devicetree OmniVision OV6211 is a monochrome image sensor, which produces frames in 8/10-bit raw output format and supports 400x400, 200x200 and 100x100 output image resolution modes. Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> --- MAINTAINERS | 8 + drivers/media/i2c/Kconfig | 10 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov6211.c | 821 +++++++++++++++++++++++++++++++++++++ 4 files changed, 840 insertions(+) create mode 100644 drivers/media/i2c/ov6211.c diff --git a/MAINTAINERS b/MAINTAINERS index 658543062bba..df6cec828bf7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -18423,6 +18423,14 @@ S: Maintained T: git git://linuxtv.org/media.git F: drivers/media/i2c/ov5695.c +OMNIVISION OV6211 SENSOR DRIVER +M: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> +L: linux-media@vger.kernel.org +S: Maintained +T: git git://linuxtv.org/media_tree.git +F: Documentation/devicetree/bindings/media/i2c/ovti,ov6211.yaml +F: drivers/media/i2c/ov6211.c + OMNIVISION OV64A40 SENSOR DRIVER M: Jacopo Mondi <jacopo.mondi@ideasonboard.com> L: linux-media@vger.kernel.org diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 4b4c199da6ea..31f0c2fada03 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -542,6 +542,16 @@ config VIDEO_OV5695 To compile this driver as a module, choose M here: the module will be called ov5695. +config VIDEO_OV6211 + tristate "OmniVision OV6211 sensor support" + select V4L2_CCI_I2C + help + This is a Video4Linux2 sensor driver for the OmniVision + OV6211 camera. + + To compile this driver as a module, choose M here: the + module will be called ov6211. + config VIDEO_OV64A40 tristate "OmniVision OV64A40 sensor support" select V4L2_CCI_I2C diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 5873d29433ee..03814fc8e2f7 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -103,6 +103,7 @@ obj-$(CONFIG_VIDEO_OV5670) += ov5670.o obj-$(CONFIG_VIDEO_OV5675) += ov5675.o obj-$(CONFIG_VIDEO_OV5693) += ov5693.o obj-$(CONFIG_VIDEO_OV5695) += ov5695.o +obj-$(CONFIG_VIDEO_OV6211) += ov6211.o obj-$(CONFIG_VIDEO_OV64A40) += ov64a40.o obj-$(CONFIG_VIDEO_OV6650) += ov6650.o obj-$(CONFIG_VIDEO_OV7251) += ov7251.o diff --git a/drivers/media/i2c/ov6211.c b/drivers/media/i2c/ov6211.c new file mode 100644 index 000000000000..bbe6fa36bff7 --- /dev/null +++ b/drivers/media/i2c/ov6211.c @@ -0,0 +1,821 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2024-2025 Linaro Ltd + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/pm_runtime.h> +#include <linux/regulator/consumer.h> +#include <media/v4l2-cci.h> +#include <media/v4l2-ctrls.h> +#include <media/v4l2-device.h> +#include <media/v4l2-fwnode.h> + +#define OV6211_LINK_FREQ_240MHZ 240000000ULL +#define OV6211_MCLK_FREQ_24MHZ 24000000 + +#define OV6211_REG_CHIP_ID CCI_REG16(0x300a) +#define OV6211_CHIP_ID 0x6710 + +#define OV6211_REG_MODE_SELECT CCI_REG8(0x0100) +#define OV6211_MODE_STANDBY 0x00 +#define OV6211_MODE_STREAMING 0x01 + +/* Exposure controls from sensor */ +#define OV6211_REG_EXPOSURE CCI_REG24(0x3500) +#define OV6211_EXPOSURE_MIN 1 +#define OV6211_EXPOSURE_MAX_MARGIN 4 +#define OV6211_EXPOSURE_STEP 1 +#define OV6211_EXPOSURE_DEFAULT 210 + +/* Analogue gain controls from sensor */ +#define OV6211_REG_ANALOGUE_GAIN CCI_REG16(0x350a) +#define OV6211_ANALOGUE_GAIN_MIN 1 +#define OV6211_ANALOGUE_GAIN_MAX 0x3ff +#define OV6211_ANALOGUE_GAIN_STEP 1 +#define OV6211_ANALOGUE_GAIN_DEFAULT 0xa0 + +#define to_ov6211(_sd) container_of(_sd, struct ov6211, sd) + +struct ov6211_reg_list { + const struct cci_reg_sequence *regs; + unsigned int num_regs; +}; + +struct ov6211_mode { + u32 width; /* Frame width in pixels */ + u32 height; /* Frame height in pixels */ + u32 hts; /* Horizontal timing size */ + u32 vts; /* Default vertical timing size */ + u32 bpp; /* Bits per pixel */ + + const struct ov6211_reg_list reg_list; /* Sensor register setting */ +}; + +static const s64 link_freq_menu_items[] = { + OV6211_LINK_FREQ_240MHZ, +}; + +static const struct cci_reg_sequence ov6211_400x400_mode[] = { + { CCI_REG8(0x0103), 0x01 }, + { CCI_REG8(0x0100), 0x00 }, + { CCI_REG8(0x3005), 0x08 }, + { CCI_REG8(0x3013), 0x12 }, + { CCI_REG8(0x3014), 0x04 }, + { CCI_REG8(0x3016), 0x10 }, + { CCI_REG8(0x3017), 0x00 }, + { CCI_REG8(0x3018), 0x00 }, + { CCI_REG8(0x301a), 0x00 }, + { CCI_REG8(0x301b), 0x00 }, + { CCI_REG8(0x301c), 0x00 }, + { CCI_REG8(0x3037), 0xf0 }, + { CCI_REG8(0x3080), 0x01 }, + { CCI_REG8(0x3081), 0x00 }, + { CCI_REG8(0x3082), 0x01 }, + { CCI_REG8(0x3098), 0x04 }, + { CCI_REG8(0x3099), 0x28 }, + { CCI_REG8(0x309a), 0x06 }, + { CCI_REG8(0x309b), 0x04 }, + { CCI_REG8(0x309c), 0x00 }, + { CCI_REG8(0x309d), 0x00 }, + { CCI_REG8(0x309e), 0x01 }, + { CCI_REG8(0x309f), 0x00 }, + { CCI_REG8(0x30b0), 0x08 }, + { CCI_REG8(0x30b1), 0x02 }, + { CCI_REG8(0x30b2), 0x00 }, + { CCI_REG8(0x30b3), 0x28 }, + { CCI_REG8(0x30b4), 0x02 }, + { CCI_REG8(0x30b5), 0x00 }, + { CCI_REG8(0x3106), 0xd9 }, + { CCI_REG8(0x3500), 0x00 }, + { CCI_REG8(0x3501), 0x0d }, + { CCI_REG8(0x3502), 0x20 }, + { CCI_REG8(0x3503), 0x07 }, + { CCI_REG8(0x3509), 0x10 }, + { CCI_REG8(0x350b), 0xa0 }, + { CCI_REG8(0x3600), 0xfc }, + { CCI_REG8(0x3620), 0xb7 }, + { CCI_REG8(0x3621), 0x05 }, + { CCI_REG8(0x3626), 0x31 }, + { CCI_REG8(0x3627), 0x40 }, + { CCI_REG8(0x3632), 0xa3 }, + { CCI_REG8(0x3633), 0x34 }, + { CCI_REG8(0x3634), 0x40 }, + { CCI_REG8(0x3636), 0x00 }, + { CCI_REG8(0x3660), 0x80 }, + { CCI_REG8(0x3662), 0x03 }, + { CCI_REG8(0x3664), 0xf0 }, + { CCI_REG8(0x366a), 0x10 }, + { CCI_REG8(0x366b), 0x06 }, + { CCI_REG8(0x3680), 0xf4 }, + { CCI_REG8(0x3681), 0x50 }, + { CCI_REG8(0x3682), 0x00 }, + { CCI_REG8(0x3708), 0x20 }, + { CCI_REG8(0x3709), 0x40 }, + { CCI_REG8(0x370d), 0x03 }, + { CCI_REG8(0x373b), 0x02 }, + { CCI_REG8(0x373c), 0x08 }, + { CCI_REG8(0x3742), 0x00 }, + { CCI_REG8(0x3744), 0x16 }, + { CCI_REG8(0x3745), 0x08 }, + { CCI_REG8(0x3781), 0xfc }, + { CCI_REG8(0x3788), 0x00 }, + { CCI_REG8(0x3800), 0x00 }, + { CCI_REG8(0x3801), 0x04 }, + { CCI_REG8(0x3802), 0x00 }, + { CCI_REG8(0x3803), 0x04 }, + { CCI_REG8(0x3804), 0x01 }, + { CCI_REG8(0x3805), 0x9b }, + { CCI_REG8(0x3806), 0x01 }, + { CCI_REG8(0x3807), 0x9b }, + { CCI_REG8(0x3808), 0x01 }, + { CCI_REG8(0x3809), 0x90 }, + { CCI_REG8(0x380a), 0x01 }, + { CCI_REG8(0x380b), 0x90 }, + { CCI_REG8(0x380c), 0x05 }, + { CCI_REG8(0x380d), 0xf2 }, + { CCI_REG8(0x380e), 0x01 }, + { CCI_REG8(0x380f), 0xb6 }, + { CCI_REG8(0x3810), 0x00 }, + { CCI_REG8(0x3811), 0x04 }, + { CCI_REG8(0x3812), 0x00 }, + { CCI_REG8(0x3813), 0x04 }, + { CCI_REG8(0x3814), 0x11 }, + { CCI_REG8(0x3815), 0x11 }, + { CCI_REG8(0x3820), 0x00 }, + { CCI_REG8(0x3821), 0x00 }, + { CCI_REG8(0x382b), 0xfa }, + { CCI_REG8(0x382f), 0x04 }, + { CCI_REG8(0x3832), 0x00 }, + { CCI_REG8(0x3833), 0x05 }, + { CCI_REG8(0x3834), 0x00 }, + { CCI_REG8(0x3835), 0x05 }, + { CCI_REG8(0x3882), 0x04 }, + { CCI_REG8(0x3883), 0x00 }, + { CCI_REG8(0x38a4), 0x10 }, + { CCI_REG8(0x38a5), 0x00 }, + { CCI_REG8(0x38b1), 0x03 }, + { CCI_REG8(0x3b80), 0x00 }, + { CCI_REG8(0x3b81), 0xff }, + { CCI_REG8(0x3b82), 0x10 }, + { CCI_REG8(0x3b83), 0x00 }, + { CCI_REG8(0x3b84), 0x08 }, + { CCI_REG8(0x3b85), 0x00 }, + { CCI_REG8(0x3b86), 0x01 }, + { CCI_REG8(0x3b87), 0x00 }, + { CCI_REG8(0x3b88), 0x00 }, + { CCI_REG8(0x3b89), 0x00 }, + { CCI_REG8(0x3b8a), 0x00 }, + { CCI_REG8(0x3b8b), 0x05 }, + { CCI_REG8(0x3b8c), 0x00 }, + { CCI_REG8(0x3b8d), 0x00 }, + { CCI_REG8(0x3b8e), 0x01 }, + { CCI_REG8(0x3b8f), 0xb2 }, + { CCI_REG8(0x3b94), 0x05 }, + { CCI_REG8(0x3b95), 0xf2 }, + { CCI_REG8(0x3b96), 0xc0 }, + { CCI_REG8(0x4004), 0x04 }, + { CCI_REG8(0x404e), 0x01 }, + { CCI_REG8(0x4801), 0x0f }, + { CCI_REG8(0x4806), 0x0f }, + { CCI_REG8(0x4837), 0x43 }, + { CCI_REG8(0x5a08), 0x00 }, + { CCI_REG8(0x5a01), 0x00 }, + { CCI_REG8(0x5a03), 0x00 }, + { CCI_REG8(0x5a04), 0x10 }, + { CCI_REG8(0x5a05), 0xa0 }, + { CCI_REG8(0x5a06), 0x0c }, + { CCI_REG8(0x5a07), 0x78 }, +}; + +static const struct ov6211_mode supported_modes[] = { + { + .width = 400, + .height = 400, + .hts = 1522, + .vts = 438, + .bpp = 8, + .reg_list = { + .regs = ov6211_400x400_mode, + .num_regs = ARRAY_SIZE(ov6211_400x400_mode), + }, + }, +}; + +struct ov6211 { + struct regmap *regmap; + struct clk *xvclk; + struct gpio_desc *reset_gpio; + struct regulator *avdd; + struct regulator *dovdd; + struct regulator *dvdd; + + struct v4l2_subdev sd; + struct media_pad pad; + + struct v4l2_ctrl_handler ctrl_handler; + + const struct ov6211_mode *cur_mode; + + /* To serialize asynchronous callbacks */ + struct mutex mutex; +}; + +static int ov6211_set_ctrl(struct v4l2_ctrl *ctrl) +{ + struct ov6211 *ov6211 = container_of(ctrl->handler, struct ov6211, + ctrl_handler); + struct i2c_client *client = v4l2_get_subdevdata(&ov6211->sd); + int ret; + + /* V4L2 controls values will be applied only when power is already up */ + if (!pm_runtime_get_if_in_use(&client->dev)) + return 0; + + switch (ctrl->id) { + case V4L2_CID_ANALOGUE_GAIN: + ret = cci_write(ov6211->regmap, OV6211_REG_ANALOGUE_GAIN, + ctrl->val, NULL); + break; + case V4L2_CID_EXPOSURE: + ret = cci_write(ov6211->regmap, OV6211_REG_EXPOSURE, + ctrl->val << 4, NULL); + break; + default: + ret = -EINVAL; + break; + } + + pm_runtime_put(&client->dev); + + return ret; +} + +static const struct v4l2_ctrl_ops ov6211_ctrl_ops = { + .s_ctrl = ov6211_set_ctrl, +}; + +static int ov6211_init_controls(struct ov6211 *ov6211) +{ + struct i2c_client *client = v4l2_get_subdevdata(&ov6211->sd); + struct v4l2_ctrl_handler *ctrl_hdlr = &ov6211->ctrl_handler; + const struct ov6211_mode *cur_mode = ov6211->cur_mode; + struct v4l2_fwnode_device_properties props; + s64 exposure_max, pixel_rate, h_blank; + struct v4l2_ctrl *ctrl; + int ret; + + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8); + if (ret) + return ret; + + ctrl_hdlr->lock = &ov6211->mutex; + + ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, &ov6211_ctrl_ops, + V4L2_CID_LINK_FREQ, + ARRAY_SIZE(link_freq_menu_items) - 1, + 0, link_freq_menu_items); + if (ctrl) + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; + + pixel_rate = link_freq_menu_items[0] * 2 / cur_mode->bpp; + v4l2_ctrl_new_std(ctrl_hdlr, &ov6211_ctrl_ops, V4L2_CID_PIXEL_RATE, + 0, pixel_rate, 1, pixel_rate); + + h_blank = cur_mode->hts - cur_mode->width; + ctrl = v4l2_ctrl_new_std(ctrl_hdlr, &ov6211_ctrl_ops, V4L2_CID_HBLANK, + h_blank, h_blank, 1, h_blank); + if (ctrl) + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; + + ctrl = v4l2_ctrl_new_std(ctrl_hdlr, &ov6211_ctrl_ops, V4L2_CID_VBLANK, + cur_mode->vts - cur_mode->height, + cur_mode->vts - cur_mode->height, 1, + cur_mode->vts - cur_mode->height); + if (ctrl) + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; + + v4l2_ctrl_new_std(ctrl_hdlr, &ov6211_ctrl_ops, V4L2_CID_ANALOGUE_GAIN, + OV6211_ANALOGUE_GAIN_MIN, OV6211_ANALOGUE_GAIN_MAX, + OV6211_ANALOGUE_GAIN_STEP, + OV6211_ANALOGUE_GAIN_DEFAULT); + + exposure_max = (cur_mode->vts - OV6211_EXPOSURE_MAX_MARGIN); + ctrl = v4l2_ctrl_new_std(ctrl_hdlr, &ov6211_ctrl_ops, + V4L2_CID_EXPOSURE, + OV6211_EXPOSURE_MIN, exposure_max, + OV6211_EXPOSURE_STEP, + OV6211_EXPOSURE_DEFAULT); + + if (ctrl_hdlr->error) + return ctrl_hdlr->error; + + ret = v4l2_fwnode_device_parse(&client->dev, &props); + if (ret) + goto error_free_hdlr; + + ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ov6211_ctrl_ops, + &props); + if (ret) + goto error_free_hdlr; + + ov6211->sd.ctrl_handler = ctrl_hdlr; + + return 0; + +error_free_hdlr: + v4l2_ctrl_handler_free(ctrl_hdlr); + + return ret; +} + +static void ov6211_update_pad_format(const struct ov6211_mode *mode, + struct v4l2_mbus_framefmt *fmt) +{ + fmt->width = mode->width; + fmt->height = mode->height; + fmt->code = MEDIA_BUS_FMT_Y8_1X8; + fmt->field = V4L2_FIELD_NONE; +} + +static int ov6211_start_streaming(struct ov6211 *ov6211) +{ + struct i2c_client *client = v4l2_get_subdevdata(&ov6211->sd); + const struct ov6211_reg_list *reg_list; + int ret; + + reg_list = &ov6211->cur_mode->reg_list; + ret = cci_multi_reg_write(ov6211->regmap, reg_list->regs, + reg_list->num_regs, NULL); + if (ret) { + dev_err(&client->dev, "failed to set mode: %d\n", ret); + return ret; + } + + ret = __v4l2_ctrl_handler_setup(ov6211->sd.ctrl_handler); + if (ret) + return ret; + + ret = cci_write(ov6211->regmap, OV6211_REG_MODE_SELECT, + OV6211_MODE_STREAMING, NULL); + if (ret) { + dev_err(&client->dev, "failed to start streaming: %d\n", ret); + return ret; + } + + return 0; +} + +static void ov6211_stop_streaming(struct ov6211 *ov6211) +{ + struct i2c_client *client = v4l2_get_subdevdata(&ov6211->sd); + int ret; + + ret = cci_write(ov6211->regmap, OV6211_REG_MODE_SELECT, + OV6211_MODE_STANDBY, NULL); + if (ret) + dev_err(&client->dev, "failed to stop streaming: %d\n", ret); +} + +static int ov6211_set_stream(struct v4l2_subdev *sd, int enable) +{ + struct i2c_client *client = v4l2_get_subdevdata(sd); + struct ov6211 *ov6211 = to_ov6211(sd); + int ret = 0; + + mutex_lock(&ov6211->mutex); + + if (enable) { + ret = pm_runtime_resume_and_get(&client->dev); + if (ret) { + mutex_unlock(&ov6211->mutex); + return ret; + } + + ret = ov6211_start_streaming(ov6211); + if (!ret) { + mutex_unlock(&ov6211->mutex); + return 0; + } + } + + ov6211_stop_streaming(ov6211); + pm_runtime_put(&client->dev); + + mutex_unlock(&ov6211->mutex); + + return ret; +} + +static int ov6211_set_format(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_format *fmt) +{ + struct ov6211 *ov6211 = to_ov6211(sd); + const struct ov6211_mode *mode; + + mode = v4l2_find_nearest_size(supported_modes, + ARRAY_SIZE(supported_modes), + width, height, + fmt->format.width, + fmt->format.height); + + mutex_lock(&ov6211->mutex); + + ov6211_update_pad_format(mode, &fmt->format); + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) + *v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format; + else + ov6211->cur_mode = mode; + + mutex_unlock(&ov6211->mutex); + + return 0; +} + +static int ov6211_get_format(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_format *fmt) +{ + struct ov6211 *ov6211 = to_ov6211(sd); + + mutex_lock(&ov6211->mutex); + + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) + fmt->format = *v4l2_subdev_state_get_format(sd_state, fmt->pad); + else + ov6211_update_pad_format(ov6211->cur_mode, &fmt->format); + + mutex_unlock(&ov6211->mutex); + + return 0; +} + +static int ov6211_enum_mbus_code(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_mbus_code_enum *code) +{ + if (code->index > 0) + return -EINVAL; + + code->code = MEDIA_BUS_FMT_Y8_1X8; + + return 0; +} + +static int ov6211_enum_frame_size(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_frame_size_enum *fse) +{ + if (fse->index >= ARRAY_SIZE(supported_modes)) + return -EINVAL; + + if (fse->code != MEDIA_BUS_FMT_Y8_1X8) + 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 ov6211_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) +{ + struct ov6211 *ov6211 = to_ov6211(sd); + + mutex_lock(&ov6211->mutex); + + ov6211_update_pad_format(&supported_modes[0], + v4l2_subdev_state_get_format(fh->state, 0)); + + mutex_unlock(&ov6211->mutex); + + return 0; +} + +static const struct v4l2_subdev_video_ops ov6211_video_ops = { + .s_stream = ov6211_set_stream, +}; + +static const struct v4l2_subdev_pad_ops ov6211_pad_ops = { + .set_fmt = ov6211_set_format, + .get_fmt = ov6211_get_format, + .enum_mbus_code = ov6211_enum_mbus_code, + .enum_frame_size = ov6211_enum_frame_size, +}; + +static const struct v4l2_subdev_ops ov6211_subdev_ops = { + .video = &ov6211_video_ops, + .pad = &ov6211_pad_ops, +}; + +static const struct media_entity_operations ov6211_subdev_entity_ops = { + .link_validate = v4l2_subdev_link_validate, +}; + +static const struct v4l2_subdev_internal_ops ov6211_internal_ops = { + .open = ov6211_open, +}; + +static int ov6211_identify_module(struct ov6211 *ov6211) +{ + struct i2c_client *client = v4l2_get_subdevdata(&ov6211->sd); + u64 val; + int ret; + + ret = cci_read(ov6211->regmap, OV6211_REG_CHIP_ID, &val, NULL); + if (ret) { + dev_err(&client->dev, "failed to read chip id: %d\n", ret); + return ret; + } + + if (val != OV6211_CHIP_ID) { + dev_err(&client->dev, "chip id mismatch: %x!=%llx\n", + OV6211_CHIP_ID, val); + return -ENODEV; + } + + return 0; +} + +static int ov6211_check_hwcfg(struct ov6211 *ov6211) +{ + struct i2c_client *client = v4l2_get_subdevdata(&ov6211->sd); + struct device *dev = &client->dev; + struct fwnode_handle *fwnode = dev_fwnode(dev), *ep; + struct v4l2_fwnode_endpoint bus_cfg = { + .bus_type = V4L2_MBUS_CSI2_DPHY, + }; + int ret; + + if (!fwnode) + return -ENODEV; + + ep = fwnode_graph_get_next_endpoint(fwnode, NULL); + if (!ep) + return -EINVAL; + + ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg); + fwnode_handle_put(ep); + if (ret) + return ret; + + if (!bus_cfg.nr_of_link_frequencies) { + dev_err(dev, "link-frequency property is not found\n"); + ret = -EINVAL; + goto error; + } + + if (bus_cfg.nr_of_link_frequencies != 1 || + bus_cfg.link_frequencies[0] != link_freq_menu_items[0]) { + dev_err(dev, "Unsupported MIPI CSI2 link frequency\n"); + ret = -EINVAL; + goto error; + } + +error: + v4l2_fwnode_endpoint_free(&bus_cfg); + + return ret; +} + +static int ov6211_power_on(struct device *dev) +{ + struct v4l2_subdev *sd = dev_get_drvdata(dev); + struct ov6211 *ov6211 = to_ov6211(sd); + int ret; + + if (ov6211->avdd) { + ret = regulator_enable(ov6211->avdd); + if (ret) + return ret; + } + + if (ov6211->dovdd) { + ret = regulator_enable(ov6211->dovdd); + if (ret) + goto avdd_disable; + } + + if (ov6211->dvdd) { + ret = regulator_enable(ov6211->dvdd); + if (ret) + goto dovdd_disable; + } + + gpiod_set_value_cansleep(ov6211->reset_gpio, 0); + usleep_range(10 * USEC_PER_MSEC, 15 * USEC_PER_MSEC); + + ret = clk_prepare_enable(ov6211->xvclk); + if (ret) + goto reset_gpio; + + return 0; + +reset_gpio: + gpiod_set_value_cansleep(ov6211->reset_gpio, 1); + + if (ov6211->dvdd) + regulator_disable(ov6211->dvdd); +dovdd_disable: + if (ov6211->dovdd) + regulator_disable(ov6211->dovdd); +avdd_disable: + if (ov6211->avdd) + regulator_disable(ov6211->avdd); + + return ret; +} + +static int ov6211_power_off(struct device *dev) +{ + struct v4l2_subdev *sd = dev_get_drvdata(dev); + struct ov6211 *ov6211 = to_ov6211(sd); + + clk_disable_unprepare(ov6211->xvclk); + + gpiod_set_value_cansleep(ov6211->reset_gpio, 1); + + if (ov6211->dvdd) + regulator_disable(ov6211->dvdd); + + if (ov6211->dovdd) + regulator_disable(ov6211->dovdd); + + if (ov6211->avdd) + regulator_disable(ov6211->avdd); + + return 0; +} + +static int ov6211_probe(struct i2c_client *client) +{ + struct ov6211 *ov6211; + unsigned long freq; + int ret; + + ov6211 = devm_kzalloc(&client->dev, sizeof(*ov6211), GFP_KERNEL); + if (!ov6211) + return -ENOMEM; + + ov6211->regmap = devm_cci_regmap_init_i2c(client, 16); + if (IS_ERR(ov6211->regmap)) + return dev_err_probe(&client->dev, PTR_ERR(ov6211->regmap), + "failed to init CCI\n"); + + v4l2_i2c_subdev_init(&ov6211->sd, client, &ov6211_subdev_ops); + + ov6211->xvclk = devm_v4l2_sensor_clk_get(&client->dev, NULL); + if (IS_ERR(ov6211->xvclk)) + return dev_err_probe(&client->dev, PTR_ERR(ov6211->xvclk), + "failed to get XVCLK clock\n"); + + freq = clk_get_rate(ov6211->xvclk); + if (freq && freq != OV6211_MCLK_FREQ_24MHZ) + return dev_err_probe(&client->dev, -EINVAL, + "XVCLK clock frequency %lu is not supported\n", + freq); + + ret = ov6211_check_hwcfg(ov6211); + if (ret) + return dev_err_probe(&client->dev, ret, + "failed to check HW configuration\n"); + + ov6211->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", + GPIOD_OUT_HIGH); + if (IS_ERR(ov6211->reset_gpio)) + return dev_err_probe(&client->dev, PTR_ERR(ov6211->reset_gpio), + "cannot get reset GPIO\n"); + + ov6211->avdd = devm_regulator_get_optional(&client->dev, "avdd"); + if (IS_ERR(ov6211->avdd)) { + ret = PTR_ERR(ov6211->avdd); + if (ret != -ENODEV) + return dev_err_probe(&client->dev, ret, + "Failed to get avdd regulator\n"); + + ov6211->avdd = NULL; + } + + ov6211->dovdd = devm_regulator_get_optional(&client->dev, "dovdd"); + if (IS_ERR(ov6211->dovdd)) { + ret = PTR_ERR(ov6211->dovdd); + if (ret != -ENODEV) + return dev_err_probe(&client->dev, ret, + "Failed to get dovdd regulator\n"); + + ov6211->dovdd = NULL; + } + + ov6211->dvdd = devm_regulator_get_optional(&client->dev, "dvdd"); + if (IS_ERR(ov6211->dvdd)) { + ret = PTR_ERR(ov6211->dvdd); + if (ret != -ENODEV) + return dev_err_probe(&client->dev, ret, + "Failed to get dvdd regulator\n"); + + ov6211->dvdd = NULL; + } + + /* The sensor must be powered on to read the CHIP_ID register */ + ret = ov6211_power_on(&client->dev); + if (ret) + return ret; + + ret = ov6211_identify_module(ov6211); + if (ret) { + dev_err_probe(&client->dev, ret, "failed to find sensor\n"); + goto power_off; + } + + mutex_init(&ov6211->mutex); + ov6211->cur_mode = &supported_modes[0]; + + ret = ov6211_init_controls(ov6211); + if (ret) { + dev_err_probe(&client->dev, ret, "failed to init controls\n"); + goto mutex_destroy; + } + + ov6211->sd.internal_ops = &ov6211_internal_ops; + ov6211->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + ov6211->sd.entity.ops = &ov6211_subdev_entity_ops; + ov6211->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; + ov6211->pad.flags = MEDIA_PAD_FL_SOURCE; + ret = media_entity_pads_init(&ov6211->sd.entity, 1, &ov6211->pad); + if (ret) { + dev_err_probe(&client->dev, ret, + "failed to init media entity pads\n"); + goto v4l2_ctrl_handler_free; + } + + ret = v4l2_async_register_subdev_sensor(&ov6211->sd); + if (ret < 0) { + dev_err_probe(&client->dev, ret, + "failed to register V4L2 subdev\n"); + goto media_entity_cleanup; + } + + /* Enable runtime PM and turn off the device */ + pm_runtime_set_active(&client->dev); + pm_runtime_enable(&client->dev); + pm_runtime_idle(&client->dev); + + return 0; + +media_entity_cleanup: + media_entity_cleanup(&ov6211->sd.entity); + +v4l2_ctrl_handler_free: + v4l2_ctrl_handler_free(ov6211->sd.ctrl_handler); + +mutex_destroy: + mutex_destroy(&ov6211->mutex); + +power_off: + ov6211_power_off(&client->dev); + + return ret; +} + +static void ov6211_remove(struct i2c_client *client) +{ + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct ov6211 *ov6211 = to_ov6211(sd); + + v4l2_async_unregister_subdev(sd); + media_entity_cleanup(&sd->entity); + v4l2_ctrl_handler_free(sd->ctrl_handler); + pm_runtime_disable(&client->dev); + mutex_destroy(&ov6211->mutex); +} + +static const struct dev_pm_ops ov6211_pm_ops = { + SET_RUNTIME_PM_OPS(ov6211_power_off, ov6211_power_on, NULL) +}; + +static const struct of_device_id ov6211_of_match[] = { + { .compatible = "ovti,ov6211" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, ov6211_of_match); + +static struct i2c_driver ov6211_i2c_driver = { + .driver = { + .name = "ov6211", + .pm = &ov6211_pm_ops, + .of_match_table = ov6211_of_match, + }, + .probe = ov6211_probe, + .remove = ov6211_remove, +}; + +module_i2c_driver(ov6211_i2c_driver); + +MODULE_AUTHOR("Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>"); +MODULE_DESCRIPTION("OmniVision OV6211 sensor driver"); +MODULE_LICENSE("GPL"); -- 2.49.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver 2025-07-29 23:14 ` [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver Vladimir Zapolskiy @ 2025-07-30 7:17 ` Tarang Raval 2025-07-30 15:34 ` kernel test robot 2025-07-30 18:11 ` kernel test robot 2 siblings, 0 replies; 7+ messages in thread From: Tarang Raval @ 2025-07-30 7:17 UTC (permalink / raw) To: Vladimir Zapolskiy Cc: Sakari Ailus, Hans Verkuil, Conor Dooley, linux-media@vger.kernel.org, devicetree@vger.kernel.org, Mauro Carvalho Chehab, Rob Herring, Dave Stevenson, Krzysztof Kozlowski, Mehdi Djait Hi Vladimir, > OmniVision OV6211 is a monochrome image sensor, which produces frames in > 8/10-bit raw output format and supports 400x400, 200x200 and 100x100 > output image resolution modes. > > Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > --- > MAINTAINERS | 8 + > drivers/media/i2c/Kconfig | 10 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/ov6211.c | 821 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 840 insertions(+) > create mode 100644 drivers/media/i2c/ov6211.c ... > +struct ov6211 { > + struct regmap *regmap; > + struct clk *xvclk; > + struct gpio_desc *reset_gpio; > + struct regulator *avdd; > + struct regulator *dovdd; > + struct regulator *dvdd; > + > + struct v4l2_subdev sd; > + struct media_pad pad; > + > + struct v4l2_ctrl_handler ctrl_handler; > + > + const struct ov6211_mode *cur_mode; > + > + /* To serialize asynchronous callbacks */ > + struct mutex mutex; > +}; > + > +static int ov6211_set_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct ov6211 *ov6211 = container_of(ctrl->handler, struct ov6211, > + ctrl_handler); > + struct i2c_client *client = v4l2_get_subdevdata(&ov6211->sd); > + int ret; Consider adding struct i2c_client *client to struct ov6211 to avoid repeatedly retrieving it via v4l2_get_subdevdata(). or i think adding struct device *dev might be more generic > + > + /* V4L2 controls values will be applied only when power is already up */ > + if (!pm_runtime_get_if_in_use(&client->dev)) > + return 0; > + > + switch (ctrl->id) { > + case V4L2_CID_ANALOGUE_GAIN: > + ret = cci_write(ov6211->regmap, OV6211_REG_ANALOGUE_GAIN, > + ctrl->val, NULL); > + break; > + case V4L2_CID_EXPOSURE: > + ret = cci_write(ov6211->regmap, OV6211_REG_EXPOSURE, > + ctrl->val << 4, NULL); > + break; > + default: > + ret = -EINVAL; > + break; > + } > + > + pm_runtime_put(&client->dev); > + > + return ret; > +} > + ... > +static void ov6211_stop_streaming(struct ov6211 *ov6211) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&ov6211->sd); > + int ret; > + > + ret = cci_write(ov6211->regmap, OV6211_REG_MODE_SELECT, > + OV6211_MODE_STANDBY, NULL); > + if (ret) > + dev_err(&client->dev, "failed to stop streaming: %d\n", ret); > +} > + > +static int ov6211_set_stream(struct v4l2_subdev *sd, int enable) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(sd); > + struct ov6211 *ov6211 = to_ov6211(sd); > + int ret = 0; > + > + mutex_lock(&ov6211->mutex); Instead of using a custom mutex, you can switch to the subdev state lock. This requires calling v4l2_subdev_init_finalize() in probe. You can take referance from imx219. > + if (enable) { > + ret = pm_runtime_resume_and_get(&client->dev); > + if (ret) { > + mutex_unlock(&ov6211->mutex); > + return ret; > + } > + > + ret = ov6211_start_streaming(ov6211); > + if (!ret) { > + mutex_unlock(&ov6211->mutex); > + return 0; > + } > + } > + > + ov6211_stop_streaming(ov6211); > + pm_runtime_put(&client->dev); > + > + mutex_unlock(&ov6211->mutex); > + > + return ret; > +} > + > +static int ov6211_set_format(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_format *fmt) > +{ > + struct ov6211 *ov6211 = to_ov6211(sd); > + const struct ov6211_mode *mode; > + > + mode = v4l2_find_nearest_size(supported_modes, > + ARRAY_SIZE(supported_modes), > + width, height, > + fmt->format.width, > + fmt->format.height); > + > + mutex_lock(&ov6211->mutex); > + > + ov6211_update_pad_format(mode, &fmt->format); > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) > + *v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format; > + else > + ov6211->cur_mode = mode; > + > + mutex_unlock(&ov6211->mutex); > + > + return 0; > +} > + > +static int ov6211_get_format(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_format *fmt) You should use v4l2_subdev_get_fmt(). > +{ > + struct ov6211 *ov6211 = to_ov6211(sd); > + > + mutex_lock(&ov6211->mutex); > + > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) > + fmt->format = *v4l2_subdev_state_get_format(sd_state, fmt->pad); > + else > + ov6211_update_pad_format(ov6211->cur_mode, &fmt->format); > + > + mutex_unlock(&ov6211->mutex); > + > + return 0; > +} > + ... > +static int ov6211_probe(struct i2c_client *client) > +{ > + struct ov6211 *ov6211; > + unsigned long freq; > + int ret; > + > + ov6211 = devm_kzalloc(&client->dev, sizeof(*ov6211), GFP_KERNEL); > + if (!ov6211) > + return -ENOMEM; > + > + ov6211->regmap = devm_cci_regmap_init_i2c(client, 16); > + if (IS_ERR(ov6211->regmap)) > + return dev_err_probe(&client->dev, PTR_ERR(ov6211->regmap), > + "failed to init CCI\n"); > + > + v4l2_i2c_subdev_init(&ov6211->sd, client, &ov6211_subdev_ops); > + > + ov6211->xvclk = devm_v4l2_sensor_clk_get(&client->dev, NULL); > + if (IS_ERR(ov6211->xvclk)) > + return dev_err_probe(&client->dev, PTR_ERR(ov6211->xvclk), > + "failed to get XVCLK clock\n"); > + > + freq = clk_get_rate(ov6211->xvclk); > + if (freq && freq != OV6211_MCLK_FREQ_24MHZ) > + return dev_err_probe(&client->dev, -EINVAL, > + "XVCLK clock frequency %lu is not supported\n", > + freq); Wrong indentation > + ret = ov6211_check_hwcfg(ov6211); > + if (ret) > + return dev_err_probe(&client->dev, ret, > + "failed to check HW configuration\n"); > + > + ov6211->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", > + GPIOD_OUT_HIGH); > + if (IS_ERR(ov6211->reset_gpio)) > + return dev_err_probe(&client->dev, PTR_ERR(ov6211->reset_gpio), > + "cannot get reset GPIO\n"); > + > + ov6211->avdd = devm_regulator_get_optional(&client->dev, "avdd"); > + if (IS_ERR(ov6211->avdd)) { > + ret = PTR_ERR(ov6211->avdd); > + if (ret != -ENODEV) > + return dev_err_probe(&client->dev, ret, > + "Failed to get avdd regulator\n"); > + > + ov6211->avdd = NULL; > + } > + > + ov6211->dovdd = devm_regulator_get_optional(&client->dev, "dovdd"); > + if (IS_ERR(ov6211->dovdd)) { > + ret = PTR_ERR(ov6211->dovdd); > + if (ret != -ENODEV) > + return dev_err_probe(&client->dev, ret, > + "Failed to get dovdd regulator\n"); > + > + ov6211->dovdd = NULL; > + } > + > + ov6211->dvdd = devm_regulator_get_optional(&client->dev, "dvdd"); > + if (IS_ERR(ov6211->dvdd)) { > + ret = PTR_ERR(ov6211->dvdd); > + if (ret != -ENODEV) > + return dev_err_probe(&client->dev, ret, > + "Failed to get dvdd regulator\n"); > + > + ov6211->dvdd = NULL; > + } > + > + /* The sensor must be powered on to read the CHIP_ID register */ > + ret = ov6211_power_on(&client->dev); > + if (ret) > + return ret; > + > + ret = ov6211_identify_module(ov6211); > + if (ret) { > + dev_err_probe(&client->dev, ret, "failed to find sensor\n"); > + goto power_off; > + } > + > + mutex_init(&ov6211->mutex); > + ov6211->cur_mode = &supported_modes[0]; Since only one mode is supported, you can remove cur_mode from struct ov6211. > + > + ret = ov6211_init_controls(ov6211); > + if (ret) { > + dev_err_probe(&client->dev, ret, "failed to init controls\n"); > + goto mutex_destroy; > + } > + > + ov6211->sd.internal_ops = &ov6211_internal_ops; > + ov6211->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > + ov6211->sd.entity.ops = &ov6211_subdev_entity_ops; > + ov6211->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; > + ov6211->pad.flags = MEDIA_PAD_FL_SOURCE; > + ret = media_entity_pads_init(&ov6211->sd.entity, 1, &ov6211->pad); > + if (ret) { > + dev_err_probe(&client->dev, ret, > + "failed to init media entity pads\n"); > + goto v4l2_ctrl_handler_free; > + } > + > + ret = v4l2_async_register_subdev_sensor(&ov6211->sd); > + if (ret < 0) { > + dev_err_probe(&client->dev, ret, > + "failed to register V4L2 subdev\n"); > + goto media_entity_cleanup; > + } > + > + /* Enable runtime PM and turn off the device */ > + pm_runtime_set_active(&client->dev); > + pm_runtime_enable(&client->dev); Runtime PM should fully initialized before calling v4l2_async_register_subdev_sensor(), so you need to move both lines above the subdevice registration. > + pm_runtime_idle(&client->dev); > + > + return 0; > + > +media_entity_cleanup: > + media_entity_cleanup(&ov6211->sd.entity); > + > +v4l2_ctrl_handler_free: > + v4l2_ctrl_handler_free(ov6211->sd.ctrl_handler); > + > +mutex_destroy: > + mutex_destroy(&ov6211->mutex); > + > +power_off: > + ov6211_power_off(&client->dev); > + > + return ret; > +} > + > +static void ov6211_remove(struct i2c_client *client) > +{ > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct ov6211 *ov6211 = to_ov6211(sd); > + > + v4l2_async_unregister_subdev(sd); > + media_entity_cleanup(&sd->entity); > + v4l2_ctrl_handler_free(sd->ctrl_handler); > + pm_runtime_disable(&client->dev); Runtime PM usage here is incomplete. You should also set the device status to suspended. Refer to the imx219 implementation. Best Regards, Tarang ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver 2025-07-29 23:14 ` [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver Vladimir Zapolskiy 2025-07-30 7:17 ` Tarang Raval @ 2025-07-30 15:34 ` kernel test robot 2025-07-30 18:11 ` kernel test robot 2 siblings, 0 replies; 7+ messages in thread From: kernel test robot @ 2025-07-30 15:34 UTC (permalink / raw) To: Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus, Hans Verkuil, Dave Stevenson, Mehdi Djait Cc: llvm, oe-kbuild-all, linux-media, devicetree Hi Vladimir, kernel test robot noticed the following build errors: [auto build test ERROR on robh/for-next] [also build test ERROR on linuxtv-media-pending/master linus/master v6.16 next-20250730] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Vladimir-Zapolskiy/dt-bindings-media-i2c-Add-OmniVision-OV6211-image-sensor/20250730-071618 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/20250729231454.242748-3-vladimir.zapolskiy%40linaro.org patch subject: [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20250730/202507302325.nnfzHwrF-lkp@intel.com/config) compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250730/202507302325.nnfzHwrF-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202507302325.nnfzHwrF-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/media/i2c/ov6211.c:672:18: error: call to undeclared function 'devm_v4l2_sensor_clk_get'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] 672 | ov6211->xvclk = devm_v4l2_sensor_clk_get(&client->dev, NULL); | ^ >> drivers/media/i2c/ov6211.c:672:16: error: incompatible integer to pointer conversion assigning to 'struct clk *' from 'int' [-Wint-conversion] 672 | ov6211->xvclk = devm_v4l2_sensor_clk_get(&client->dev, NULL); | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2 errors generated. vim +/devm_v4l2_sensor_clk_get +672 drivers/media/i2c/ov6211.c 654 655 static int ov6211_probe(struct i2c_client *client) 656 { 657 struct ov6211 *ov6211; 658 unsigned long freq; 659 int ret; 660 661 ov6211 = devm_kzalloc(&client->dev, sizeof(*ov6211), GFP_KERNEL); 662 if (!ov6211) 663 return -ENOMEM; 664 665 ov6211->regmap = devm_cci_regmap_init_i2c(client, 16); 666 if (IS_ERR(ov6211->regmap)) 667 return dev_err_probe(&client->dev, PTR_ERR(ov6211->regmap), 668 "failed to init CCI\n"); 669 670 v4l2_i2c_subdev_init(&ov6211->sd, client, &ov6211_subdev_ops); 671 > 672 ov6211->xvclk = devm_v4l2_sensor_clk_get(&client->dev, NULL); 673 if (IS_ERR(ov6211->xvclk)) 674 return dev_err_probe(&client->dev, PTR_ERR(ov6211->xvclk), 675 "failed to get XVCLK clock\n"); 676 677 freq = clk_get_rate(ov6211->xvclk); 678 if (freq && freq != OV6211_MCLK_FREQ_24MHZ) 679 return dev_err_probe(&client->dev, -EINVAL, 680 "XVCLK clock frequency %lu is not supported\n", 681 freq); 682 683 ret = ov6211_check_hwcfg(ov6211); 684 if (ret) 685 return dev_err_probe(&client->dev, ret, 686 "failed to check HW configuration\n"); 687 688 ov6211->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", 689 GPIOD_OUT_HIGH); 690 if (IS_ERR(ov6211->reset_gpio)) 691 return dev_err_probe(&client->dev, PTR_ERR(ov6211->reset_gpio), 692 "cannot get reset GPIO\n"); 693 694 ov6211->avdd = devm_regulator_get_optional(&client->dev, "avdd"); 695 if (IS_ERR(ov6211->avdd)) { 696 ret = PTR_ERR(ov6211->avdd); 697 if (ret != -ENODEV) 698 return dev_err_probe(&client->dev, ret, 699 "Failed to get avdd regulator\n"); 700 701 ov6211->avdd = NULL; 702 } 703 704 ov6211->dovdd = devm_regulator_get_optional(&client->dev, "dovdd"); 705 if (IS_ERR(ov6211->dovdd)) { 706 ret = PTR_ERR(ov6211->dovdd); 707 if (ret != -ENODEV) 708 return dev_err_probe(&client->dev, ret, 709 "Failed to get dovdd regulator\n"); 710 711 ov6211->dovdd = NULL; 712 } 713 714 ov6211->dvdd = devm_regulator_get_optional(&client->dev, "dvdd"); 715 if (IS_ERR(ov6211->dvdd)) { 716 ret = PTR_ERR(ov6211->dvdd); 717 if (ret != -ENODEV) 718 return dev_err_probe(&client->dev, ret, 719 "Failed to get dvdd regulator\n"); 720 721 ov6211->dvdd = NULL; 722 } 723 724 /* The sensor must be powered on to read the CHIP_ID register */ 725 ret = ov6211_power_on(&client->dev); 726 if (ret) 727 return ret; 728 729 ret = ov6211_identify_module(ov6211); 730 if (ret) { 731 dev_err_probe(&client->dev, ret, "failed to find sensor\n"); 732 goto power_off; 733 } 734 735 mutex_init(&ov6211->mutex); 736 ov6211->cur_mode = &supported_modes[0]; 737 738 ret = ov6211_init_controls(ov6211); 739 if (ret) { 740 dev_err_probe(&client->dev, ret, "failed to init controls\n"); 741 goto mutex_destroy; 742 } 743 744 ov6211->sd.internal_ops = &ov6211_internal_ops; 745 ov6211->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; 746 ov6211->sd.entity.ops = &ov6211_subdev_entity_ops; 747 ov6211->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; 748 ov6211->pad.flags = MEDIA_PAD_FL_SOURCE; 749 ret = media_entity_pads_init(&ov6211->sd.entity, 1, &ov6211->pad); 750 if (ret) { 751 dev_err_probe(&client->dev, ret, 752 "failed to init media entity pads\n"); 753 goto v4l2_ctrl_handler_free; 754 } 755 756 ret = v4l2_async_register_subdev_sensor(&ov6211->sd); 757 if (ret < 0) { 758 dev_err_probe(&client->dev, ret, 759 "failed to register V4L2 subdev\n"); 760 goto media_entity_cleanup; 761 } 762 763 /* Enable runtime PM and turn off the device */ 764 pm_runtime_set_active(&client->dev); 765 pm_runtime_enable(&client->dev); 766 pm_runtime_idle(&client->dev); 767 768 return 0; 769 770 media_entity_cleanup: 771 media_entity_cleanup(&ov6211->sd.entity); 772 773 v4l2_ctrl_handler_free: 774 v4l2_ctrl_handler_free(ov6211->sd.ctrl_handler); 775 776 mutex_destroy: 777 mutex_destroy(&ov6211->mutex); 778 779 power_off: 780 ov6211_power_off(&client->dev); 781 782 return ret; 783 } 784 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver 2025-07-29 23:14 ` [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver Vladimir Zapolskiy 2025-07-30 7:17 ` Tarang Raval 2025-07-30 15:34 ` kernel test robot @ 2025-07-30 18:11 ` kernel test robot 2 siblings, 0 replies; 7+ messages in thread From: kernel test robot @ 2025-07-30 18:11 UTC (permalink / raw) To: Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus, Hans Verkuil, Dave Stevenson, Mehdi Djait Cc: oe-kbuild-all, linux-media, devicetree Hi Vladimir, kernel test robot noticed the following build warnings: [auto build test WARNING on robh/for-next] [also build test WARNING on linuxtv-media-pending/master linus/master media-tree/master v6.16 next-20250730] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Vladimir-Zapolskiy/dt-bindings-media-i2c-Add-OmniVision-OV6211-image-sensor/20250730-071618 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/20250729231454.242748-3-vladimir.zapolskiy%40linaro.org patch subject: [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver config: i386-randconfig-001-20250731 (https://download.01.org/0day-ci/archive/20250731/202507310131.4CfC6qil-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250731/202507310131.4CfC6qil-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202507310131.4CfC6qil-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/media/i2c/ov6211.c: In function 'ov6211_probe': drivers/media/i2c/ov6211.c:672:25: error: implicit declaration of function 'devm_v4l2_sensor_clk_get' [-Werror=implicit-function-declaration] 672 | ov6211->xvclk = devm_v4l2_sensor_clk_get(&client->dev, NULL); | ^~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/media/i2c/ov6211.c:672:23: warning: assignment to 'struct clk *' from 'int' makes pointer from integer without a cast [-Wint-conversion] 672 | ov6211->xvclk = devm_v4l2_sensor_clk_get(&client->dev, NULL); | ^ cc1: some warnings being treated as errors vim +672 drivers/media/i2c/ov6211.c 654 655 static int ov6211_probe(struct i2c_client *client) 656 { 657 struct ov6211 *ov6211; 658 unsigned long freq; 659 int ret; 660 661 ov6211 = devm_kzalloc(&client->dev, sizeof(*ov6211), GFP_KERNEL); 662 if (!ov6211) 663 return -ENOMEM; 664 665 ov6211->regmap = devm_cci_regmap_init_i2c(client, 16); 666 if (IS_ERR(ov6211->regmap)) 667 return dev_err_probe(&client->dev, PTR_ERR(ov6211->regmap), 668 "failed to init CCI\n"); 669 670 v4l2_i2c_subdev_init(&ov6211->sd, client, &ov6211_subdev_ops); 671 > 672 ov6211->xvclk = devm_v4l2_sensor_clk_get(&client->dev, NULL); 673 if (IS_ERR(ov6211->xvclk)) 674 return dev_err_probe(&client->dev, PTR_ERR(ov6211->xvclk), 675 "failed to get XVCLK clock\n"); 676 677 freq = clk_get_rate(ov6211->xvclk); 678 if (freq && freq != OV6211_MCLK_FREQ_24MHZ) 679 return dev_err_probe(&client->dev, -EINVAL, 680 "XVCLK clock frequency %lu is not supported\n", 681 freq); 682 683 ret = ov6211_check_hwcfg(ov6211); 684 if (ret) 685 return dev_err_probe(&client->dev, ret, 686 "failed to check HW configuration\n"); 687 688 ov6211->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", 689 GPIOD_OUT_HIGH); 690 if (IS_ERR(ov6211->reset_gpio)) 691 return dev_err_probe(&client->dev, PTR_ERR(ov6211->reset_gpio), 692 "cannot get reset GPIO\n"); 693 694 ov6211->avdd = devm_regulator_get_optional(&client->dev, "avdd"); 695 if (IS_ERR(ov6211->avdd)) { 696 ret = PTR_ERR(ov6211->avdd); 697 if (ret != -ENODEV) 698 return dev_err_probe(&client->dev, ret, 699 "Failed to get avdd regulator\n"); 700 701 ov6211->avdd = NULL; 702 } 703 704 ov6211->dovdd = devm_regulator_get_optional(&client->dev, "dovdd"); 705 if (IS_ERR(ov6211->dovdd)) { 706 ret = PTR_ERR(ov6211->dovdd); 707 if (ret != -ENODEV) 708 return dev_err_probe(&client->dev, ret, 709 "Failed to get dovdd regulator\n"); 710 711 ov6211->dovdd = NULL; 712 } 713 714 ov6211->dvdd = devm_regulator_get_optional(&client->dev, "dvdd"); 715 if (IS_ERR(ov6211->dvdd)) { 716 ret = PTR_ERR(ov6211->dvdd); 717 if (ret != -ENODEV) 718 return dev_err_probe(&client->dev, ret, 719 "Failed to get dvdd regulator\n"); 720 721 ov6211->dvdd = NULL; 722 } 723 724 /* The sensor must be powered on to read the CHIP_ID register */ 725 ret = ov6211_power_on(&client->dev); 726 if (ret) 727 return ret; 728 729 ret = ov6211_identify_module(ov6211); 730 if (ret) { 731 dev_err_probe(&client->dev, ret, "failed to find sensor\n"); 732 goto power_off; 733 } 734 735 mutex_init(&ov6211->mutex); 736 ov6211->cur_mode = &supported_modes[0]; 737 738 ret = ov6211_init_controls(ov6211); 739 if (ret) { 740 dev_err_probe(&client->dev, ret, "failed to init controls\n"); 741 goto mutex_destroy; 742 } 743 744 ov6211->sd.internal_ops = &ov6211_internal_ops; 745 ov6211->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; 746 ov6211->sd.entity.ops = &ov6211_subdev_entity_ops; 747 ov6211->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; 748 ov6211->pad.flags = MEDIA_PAD_FL_SOURCE; 749 ret = media_entity_pads_init(&ov6211->sd.entity, 1, &ov6211->pad); 750 if (ret) { 751 dev_err_probe(&client->dev, ret, 752 "failed to init media entity pads\n"); 753 goto v4l2_ctrl_handler_free; 754 } 755 756 ret = v4l2_async_register_subdev_sensor(&ov6211->sd); 757 if (ret < 0) { 758 dev_err_probe(&client->dev, ret, 759 "failed to register V4L2 subdev\n"); 760 goto media_entity_cleanup; 761 } 762 763 /* Enable runtime PM and turn off the device */ 764 pm_runtime_set_active(&client->dev); 765 pm_runtime_enable(&client->dev); 766 pm_runtime_idle(&client->dev); 767 768 return 0; 769 770 media_entity_cleanup: 771 media_entity_cleanup(&ov6211->sd.entity); 772 773 v4l2_ctrl_handler_free: 774 v4l2_ctrl_handler_free(ov6211->sd.ctrl_handler); 775 776 mutex_destroy: 777 mutex_destroy(&ov6211->mutex); 778 779 power_off: 780 ov6211_power_off(&client->dev); 781 782 return ret; 783 } 784 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-07-30 22:32 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-29 23:14 [PATCH v2 0/2] media: i2c: Add OmniVision OV6211 image sensor driver Vladimir Zapolskiy 2025-07-29 23:14 ` [PATCH v2 1/2] dt-bindings: media: i2c: Add OmniVision OV6211 image sensor Vladimir Zapolskiy 2025-07-30 22:32 ` Rob Herring (Arm) 2025-07-29 23:14 ` [PATCH v2 2/2] media: i2c: Add OmniVision OV6211 image sensor driver Vladimir Zapolskiy 2025-07-30 7:17 ` Tarang Raval 2025-07-30 15:34 ` kernel test robot 2025-07-30 18:11 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox