* [PATCH v3 0/2] Add IMX219 CMOS image sensor support
@ 2020-01-10 20:09 Andrey Konovalov
2020-01-10 20:09 ` [PATCH v3 1/2] dt-bindings: media: i2c: Add IMX219 CMOS sensor binding Andrey Konovalov
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Andrey Konovalov @ 2020-01-10 20:09 UTC (permalink / raw)
To: mchehab, robh+dt
Cc: linux-media, devicetree, peter.griffin, dave.stevenson, ezequiel,
sakari.ailus, Andrey Konovalov
This patchset adds support for IMX219 CMOS image sensor from Sony.
Sony IMX219 is an 8MPix, 1/4.0-inch CMOS active pixel digital image sensor
with an active array size of 3280H x 2464V. It is programmable through
I2C interface. Image data are sent through MIPI CSI-2, which can be configured
as either 2 or 4 data lanes, but this driver currently only supports 2 lanes.
The currently supported resolutions are 3280x2464 @ 15fps, 1920x1080 @ 30fps
(cropped FOV), and 1640x1232 (2x2 binned) @ 30fps.
The driver has been tested with Raspberry Pi Camera Module v2 connected to
Raspberry Pi Zero W.
Changes since v2 [1]:
dt-bindings:
- "clock-names" property removed
- "xclr-gpios" property renamed to "reset-gpios"
- the camera-clk mode moved out of sensor device node
- "clock-lanes" property removed (the sensor doesn't support lane reordering)
- "clock-noncontinuous" description made more clear (thanks Sakari)
- "data-lanes" property reworked: it is now optional, and if it is not
present the driver should assume four-lane operation. For two-lane
operation (the only mode supported by the current driver) this property
must be present and set to <1 2>
imx219 sensor driver:
- "xclr-gpios" property renamed to "reset-gpios", the corresponding
struct gpio_desc field in the imx219 structure is renamed to reset_gpio
- in the driver probe() a test to check that the number of CSI-2 data lanes
is supported by the driver was added
- devm_clk_get() is now called with NULL as the 2nd argument, as there is
just single clock, and there is no need to use clock ID
- error messages are added when the driver fails to get regulators, to init
media entity pads, or to register sensor sub-device
Thanks,
Andrey
[1] https://patchwork.kernel.org/cover/11311137/
Andrey Konovalov (1):
dt-bindings: media: i2c: Add IMX219 CMOS sensor binding
Dave Stevenson (1):
media: i2c: Add driver for Sony IMX219 sensor
.../devicetree/bindings/media/i2c/imx219.yaml | 104 ++
MAINTAINERS | 8 +
drivers/media/i2c/Kconfig | 12 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/imx219.c | 1253 +++++++++++++++++
5 files changed, 1378 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml
create mode 100644 drivers/media/i2c/imx219.c
--
2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/2] dt-bindings: media: i2c: Add IMX219 CMOS sensor binding
2020-01-10 20:09 [PATCH v3 0/2] Add IMX219 CMOS image sensor support Andrey Konovalov
@ 2020-01-10 20:09 ` Andrey Konovalov
2020-01-13 23:04 ` Rob Herring
2020-01-10 20:09 ` [PATCH v3 2/2] media: i2c: Add driver for Sony IMX219 sensor Andrey Konovalov
2020-01-11 4:45 ` [PATCH v3 0/2] Add IMX219 CMOS image sensor support Ezequiel Garcia
2 siblings, 1 reply; 10+ messages in thread
From: Andrey Konovalov @ 2020-01-10 20:09 UTC (permalink / raw)
To: mchehab, robh+dt
Cc: linux-media, devicetree, peter.griffin, dave.stevenson, ezequiel,
sakari.ailus, Andrey Konovalov
Add YAML device tree binding for IMX219 CMOS image sensor, and
the relevant MAINTAINERS entries.
Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
---
.../devicetree/bindings/media/i2c/imx219.yaml | 104 ++++++++++++++++++
MAINTAINERS | 8 ++
2 files changed, 112 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml
new file mode 100644
index 000000000000..536766b63e39
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/imx219.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony 1/4.0-Inch 8Mpixel CMOS Digital Image Sensor
+
+maintainers:
+ - Dave Stevenson <dave.stevenson@raspberrypi.com>
+
+description: |-
+ The Sony imx219 is a 1/4.0-inch CMOS active pixel digital image sensor
+ with an active array size of 3280H x 2464V. It is programmable through
+ I2C interface. The I2C address is fixed to 0x10 as per sensor data sheet.
+ Image data is sent through MIPI CSI-2, which is configured as either 2 or
+ 4 data lanes.
+
+properties:
+ compatible:
+ const: sony,imx219
+
+ reg:
+ description: I2C device address
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ VDIG-supply:
+ description:
+ Digital I/O voltage supply, 1.8 volts
+
+ VANA-supply:
+ description:
+ Analog voltage supply, 2.8 volts
+
+ VDDL-supply:
+ description:
+ Digital core voltage supply, 1.2 volts
+
+ reset-gpios:
+ description: |-
+ Reference to the GPIO connected to the xclr pin, if any.
+ Must be released (set high) after all supplies are applied.
+
+ # See ../video-interfaces.txt for more details
+ port:
+ type: object
+ properties:
+ endpoint:
+ type: object
+ properties:
+ data-lanes:
+ description: |-
+ The sensor supports either two-lane, or four-lane operation.
+ If this property is omitted four-lane operation is assumed.
+ For two-lane operation the property must be set to <1 2>.
+ items:
+ - const: 1
+ - const: 2
+
+ clock-noncontinuous:
+ type: boolean
+ description: |-
+ MIPI CSI-2 clock is non-continuous if this property is present,
+ otherwise it's continuous.
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - VANA-supply
+ - VDIG-supply
+ - VDDL-supply
+ - port
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ imx219: sensor@10 {
+ compatible = "sony,imx219";
+ reg = <0x10>;
+ clocks = <&imx219_clk>;
+ VANA-supply = <&imx219_vana>; /* 2.8v */
+ VDIG-supply = <&imx219_vdig>; /* 1.8v */
+ VDDL-supply = <&imx219_vddl>; /* 1.2v */
+
+ port {
+ imx219_0: endpoint {
+ remote-endpoint = <&csi1_ep>;
+ data-lanes = <1 2>;
+ clock-noncontinuous;
+ };
+ };
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 4017e6b760be..61b598f072e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15351,6 +15351,14 @@ S: Maintained
F: drivers/media/i2c/imx214.c
F: Documentation/devicetree/bindings/media/i2c/sony,imx214.txt
+SONY IMX219 SENSOR DRIVER
+M: Dave Stevenson <dave.stevenson@raspberrypi.com>
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/imx219.c
+F: Documentation/devicetree/bindings/media/i2c/imx219.yaml
+
SONY IMX258 SENSOR DRIVER
M: Sakari Ailus <sakari.ailus@linux.intel.com>
L: linux-media@vger.kernel.org
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] media: i2c: Add driver for Sony IMX219 sensor
2020-01-10 20:09 [PATCH v3 0/2] Add IMX219 CMOS image sensor support Andrey Konovalov
2020-01-10 20:09 ` [PATCH v3 1/2] dt-bindings: media: i2c: Add IMX219 CMOS sensor binding Andrey Konovalov
@ 2020-01-10 20:09 ` Andrey Konovalov
2025-01-20 10:59 ` Sakari Ailus
2020-01-11 4:45 ` [PATCH v3 0/2] Add IMX219 CMOS image sensor support Ezequiel Garcia
2 siblings, 1 reply; 10+ messages in thread
From: Andrey Konovalov @ 2020-01-10 20:09 UTC (permalink / raw)
To: mchehab, robh+dt
Cc: linux-media, devicetree, peter.griffin, dave.stevenson, ezequiel,
sakari.ailus, Andrey Konovalov
From: Dave Stevenson <dave.stevenson@raspberrypi.com>
Adds a driver for the 8MPix Sony IMX219 CSI2 sensor.
Whilst the sensor supports 2 or 4 CSI2 data lanes, this driver
currently only supports 2 lanes.
8MPix @ 15fps, 1080P @ 30fps (cropped FOV), and 1640x1232 (2x2 binned)
@ 30fps are currently supported.
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
---
drivers/media/i2c/Kconfig | 12 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/imx219.c | 1253 ++++++++++++++++++++++++++++++++++++
3 files changed, 1266 insertions(+)
create mode 100644 drivers/media/i2c/imx219.c
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index c68e002d26ea..6fa5af1f72b9 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -591,6 +591,18 @@ config VIDEO_IMX214
To compile this driver as a module, choose M here: the
module will be called imx214.
+config VIDEO_IMX219
+ tristate "Sony IMX219 sensor support"
+ depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+ depends on MEDIA_CAMERA_SUPPORT
+ select V4L2_FWNODE
+ help
+ This is a Video4Linux2 sensor driver for the Sony
+ IMX219 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx219.
+
config VIDEO_IMX258
tristate "Sony IMX258 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c147bb9d28db..77bf7d0b691f 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -111,6 +111,7 @@ obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
obj-$(CONFIG_VIDEO_HI556) += hi556.o
obj-$(CONFIG_VIDEO_IMX214) += imx214.o
+obj-$(CONFIG_VIDEO_IMX219) += imx219.o
obj-$(CONFIG_VIDEO_IMX258) += imx258.o
obj-$(CONFIG_VIDEO_IMX274) += imx274.o
obj-$(CONFIG_VIDEO_IMX290) += imx290.o
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
new file mode 100644
index 000000000000..263959fe851c
--- /dev/null
+++ b/drivers/media/i2c/imx219.c
@@ -0,0 +1,1253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A V4L2 driver for Sony IMX219 cameras.
+ * Copyright (C) 2019, Raspberry Pi (Trading) Ltd
+ *
+ * Based on Sony imx258 camera driver
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * DT / fwnode changes, and regulator / GPIO control taken from imx214 driver
+ * Copyright 2018 Qtechnology A/S
+ *
+ * Flip handling taken from the Sony IMX319 driver.
+ * Copyright (C) 2018 Intel Corporation
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.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-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mediabus.h>
+#include <asm/unaligned.h>
+
+#define IMX219_REG_VALUE_08BIT 1
+#define IMX219_REG_VALUE_16BIT 2
+
+#define IMX219_REG_MODE_SELECT 0x0100
+#define IMX219_MODE_STANDBY 0x00
+#define IMX219_MODE_STREAMING 0x01
+
+/* Chip ID */
+#define IMX219_REG_CHIP_ID 0x0000
+#define IMX219_CHIP_ID 0x0219
+
+/* External clock frequency is 24.0M */
+#define IMX219_XCLK_FREQ 24000000
+
+/* Pixel rate is fixed at 182.4M for all the modes */
+#define IMX219_PIXEL_RATE 182400000
+
+/* V_TIMING internal */
+#define IMX219_REG_VTS 0x0160
+#define IMX219_VTS_15FPS 0x0dc6
+#define IMX219_VTS_30FPS_1080P 0x06e3
+#define IMX219_VTS_30FPS_BINNED 0x06e3
+#define IMX219_VTS_MAX 0xffff
+
+#define IMX219_VBLANK_MIN 4
+
+/*Frame Length Line*/
+#define IMX219_FLL_MIN 0x08a6
+#define IMX219_FLL_MAX 0xffff
+#define IMX219_FLL_STEP 1
+#define IMX219_FLL_DEFAULT 0x0c98
+
+/* HBLANK control - read only */
+#define IMX219_PPL_DEFAULT 3448
+
+/* Exposure control */
+#define IMX219_REG_EXPOSURE 0x015a
+#define IMX219_EXPOSURE_MIN 4
+#define IMX219_EXPOSURE_STEP 1
+#define IMX219_EXPOSURE_DEFAULT 0x640
+#define IMX219_EXPOSURE_MAX 65535
+
+/* Analog gain control */
+#define IMX219_REG_ANALOG_GAIN 0x0157
+#define IMX219_ANA_GAIN_MIN 0
+#define IMX219_ANA_GAIN_MAX 232
+#define IMX219_ANA_GAIN_STEP 1
+#define IMX219_ANA_GAIN_DEFAULT 0x0
+
+/* Digital gain control */
+#define IMX219_REG_DIGITAL_GAIN 0x0158
+#define IMX219_DGTL_GAIN_MIN 0x0100
+#define IMX219_DGTL_GAIN_MAX 0x0fff
+#define IMX219_DGTL_GAIN_DEFAULT 0x0100
+#define IMX219_DGTL_GAIN_STEP 1
+
+#define IMX219_REG_ORIENTATION 0x0172
+
+/* Test Pattern Control */
+#define IMX219_REG_TEST_PATTERN 0x0600
+#define IMX219_TEST_PATTERN_DISABLE 0
+#define IMX219_TEST_PATTERN_SOLID_COLOR 1
+#define IMX219_TEST_PATTERN_COLOR_BARS 2
+#define IMX219_TEST_PATTERN_GREY_COLOR 3
+#define IMX219_TEST_PATTERN_PN9 4
+
+/* Test pattern colour components */
+#define IMX219_REG_TESTP_RED 0x0602
+#define IMX219_REG_TESTP_GREENR 0x0604
+#define IMX219_REG_TESTP_BLUE 0x0606
+#define IMX219_REG_TESTP_GREENB 0x0608
+#define IMX219_TESTP_COLOUR_MIN 0
+#define IMX219_TESTP_COLOUR_MAX 0x03ff
+#define IMX219_TESTP_COLOUR_STEP 1
+#define IMX219_TESTP_RED_DEFAULT IMX219_TESTP_COLOUR_MAX
+#define IMX219_TESTP_GREENR_DEFAULT 0
+#define IMX219_TESTP_BLUE_DEFAULT 0
+#define IMX219_TESTP_GREENB_DEFAULT 0
+
+struct imx219_reg {
+ u16 address;
+ u8 val;
+};
+
+struct imx219_reg_list {
+ unsigned int num_of_regs;
+ const struct imx219_reg *regs;
+};
+
+/* Mode : resolution and related config&values */
+struct imx219_mode {
+ /* Frame width */
+ unsigned int width;
+ /* Frame height */
+ unsigned int height;
+
+ /* V-timing */
+ unsigned int vts_def;
+
+ /* Default register values */
+ struct imx219_reg_list reg_list;
+};
+
+/*
+ * Register sets lifted off the i2C interface from the Raspberry Pi firmware
+ * driver.
+ * 3280x2464 = mode 2, 1920x1080 = mode 1, and 1640x1232 = mode 4.
+ */
+static const struct imx219_reg mode_3280x2464_regs[] = {
+ {0x0100, 0x00},
+ {0x30eb, 0x0c},
+ {0x30eb, 0x05},
+ {0x300a, 0xff},
+ {0x300b, 0xff},
+ {0x30eb, 0x05},
+ {0x30eb, 0x09},
+ {0x0114, 0x01},
+ {0x0128, 0x00},
+ {0x012a, 0x18},
+ {0x012b, 0x00},
+ {0x0164, 0x00},
+ {0x0165, 0x00},
+ {0x0166, 0x0c},
+ {0x0167, 0xcf},
+ {0x0168, 0x00},
+ {0x0169, 0x00},
+ {0x016a, 0x09},
+ {0x016b, 0x9f},
+ {0x016c, 0x0c},
+ {0x016d, 0xd0},
+ {0x016e, 0x09},
+ {0x016f, 0xa0},
+ {0x0170, 0x01},
+ {0x0171, 0x01},
+ {0x0174, 0x00},
+ {0x0175, 0x00},
+ {0x018c, 0x0a},
+ {0x018d, 0x0a},
+ {0x0301, 0x05},
+ {0x0303, 0x01},
+ {0x0304, 0x03},
+ {0x0305, 0x03},
+ {0x0306, 0x00},
+ {0x0307, 0x39},
+ {0x0309, 0x0a},
+ {0x030b, 0x01},
+ {0x030c, 0x00},
+ {0x030d, 0x72},
+ {0x0624, 0x0c},
+ {0x0625, 0xd0},
+ {0x0626, 0x09},
+ {0x0627, 0xa0},
+ {0x455e, 0x00},
+ {0x471e, 0x4b},
+ {0x4767, 0x0f},
+ {0x4750, 0x14},
+ {0x4540, 0x00},
+ {0x47b4, 0x14},
+ {0x4713, 0x30},
+ {0x478b, 0x10},
+ {0x478f, 0x10},
+ {0x4793, 0x10},
+ {0x4797, 0x0e},
+ {0x479b, 0x0e},
+ {0x0162, 0x0d},
+ {0x0163, 0x78},
+};
+
+static const struct imx219_reg mode_1920_1080_regs[] = {
+ {0x0100, 0x00},
+ {0x30eb, 0x05},
+ {0x30eb, 0x0c},
+ {0x300a, 0xff},
+ {0x300b, 0xff},
+ {0x30eb, 0x05},
+ {0x30eb, 0x09},
+ {0x0114, 0x01},
+ {0x0128, 0x00},
+ {0x012a, 0x18},
+ {0x012b, 0x00},
+ {0x0162, 0x0d},
+ {0x0163, 0x78},
+ {0x0164, 0x02},
+ {0x0165, 0xa8},
+ {0x0166, 0x0a},
+ {0x0167, 0x27},
+ {0x0168, 0x02},
+ {0x0169, 0xb4},
+ {0x016a, 0x06},
+ {0x016b, 0xeb},
+ {0x016c, 0x07},
+ {0x016d, 0x80},
+ {0x016e, 0x04},
+ {0x016f, 0x38},
+ {0x0170, 0x01},
+ {0x0171, 0x01},
+ {0x0174, 0x00},
+ {0x0175, 0x00},
+ {0x018c, 0x0a},
+ {0x018d, 0x0a},
+ {0x0301, 0x05},
+ {0x0303, 0x01},
+ {0x0304, 0x03},
+ {0x0305, 0x03},
+ {0x0306, 0x00},
+ {0x0307, 0x39},
+ {0x0309, 0x0a},
+ {0x030b, 0x01},
+ {0x030c, 0x00},
+ {0x030d, 0x72},
+ {0x0624, 0x07},
+ {0x0625, 0x80},
+ {0x0626, 0x04},
+ {0x0627, 0x38},
+ {0x455e, 0x00},
+ {0x471e, 0x4b},
+ {0x4767, 0x0f},
+ {0x4750, 0x14},
+ {0x4540, 0x00},
+ {0x47b4, 0x14},
+ {0x4713, 0x30},
+ {0x478b, 0x10},
+ {0x478f, 0x10},
+ {0x4793, 0x10},
+ {0x4797, 0x0e},
+ {0x479b, 0x0e},
+ {0x0162, 0x0d},
+ {0x0163, 0x78},
+};
+
+static const struct imx219_reg mode_1640_1232_regs[] = {
+ {0x0100, 0x00},
+ {0x30eb, 0x0c},
+ {0x30eb, 0x05},
+ {0x300a, 0xff},
+ {0x300b, 0xff},
+ {0x30eb, 0x05},
+ {0x30eb, 0x09},
+ {0x0114, 0x01},
+ {0x0128, 0x00},
+ {0x012a, 0x18},
+ {0x012b, 0x00},
+ {0x0164, 0x00},
+ {0x0165, 0x00},
+ {0x0166, 0x0c},
+ {0x0167, 0xcf},
+ {0x0168, 0x00},
+ {0x0169, 0x00},
+ {0x016a, 0x09},
+ {0x016b, 0x9f},
+ {0x016c, 0x06},
+ {0x016d, 0x68},
+ {0x016e, 0x04},
+ {0x016f, 0xd0},
+ {0x0170, 0x01},
+ {0x0171, 0x01},
+ {0x0174, 0x01},
+ {0x0175, 0x01},
+ {0x018c, 0x0a},
+ {0x018d, 0x0a},
+ {0x0301, 0x05},
+ {0x0303, 0x01},
+ {0x0304, 0x03},
+ {0x0305, 0x03},
+ {0x0306, 0x00},
+ {0x0307, 0x39},
+ {0x0309, 0x0a},
+ {0x030b, 0x01},
+ {0x030c, 0x00},
+ {0x030d, 0x72},
+ {0x0624, 0x06},
+ {0x0625, 0x68},
+ {0x0626, 0x04},
+ {0x0627, 0xd0},
+ {0x455e, 0x00},
+ {0x471e, 0x4b},
+ {0x4767, 0x0f},
+ {0x4750, 0x14},
+ {0x4540, 0x00},
+ {0x47b4, 0x14},
+ {0x4713, 0x30},
+ {0x478b, 0x10},
+ {0x478f, 0x10},
+ {0x4793, 0x10},
+ {0x4797, 0x0e},
+ {0x479b, 0x0e},
+ {0x0162, 0x0d},
+ {0x0163, 0x78},
+};
+
+static const char * const imx219_test_pattern_menu[] = {
+ "Disabled",
+ "Color Bars",
+ "Solid Color",
+ "Grey Color Bars",
+ "PN9"
+};
+
+static const int imx219_test_pattern_val[] = {
+ IMX219_TEST_PATTERN_DISABLE,
+ IMX219_TEST_PATTERN_COLOR_BARS,
+ IMX219_TEST_PATTERN_SOLID_COLOR,
+ IMX219_TEST_PATTERN_GREY_COLOR,
+ IMX219_TEST_PATTERN_PN9,
+};
+
+/* regulator supplies */
+static const char * const imx219_supply_name[] = {
+ /* Supplies can be enabled in any order */
+ "VANA", /* Analog (2.8V) supply */
+ "VDIG", /* Digital Core (1.8V) supply */
+ "VDDL", /* IF (1.2V) supply */
+};
+
+#define IMX219_NUM_SUPPLIES ARRAY_SIZE(imx219_supply_name)
+
+#define IMX219_XCLR_DELAY_MS 10 /* Initialisation delay after XCLR low->high */
+
+/* Mode configs */
+static const struct imx219_mode supported_modes[] = {
+ {
+ /* 8MPix 15fps mode */
+ .width = 3280,
+ .height = 2464,
+ .vts_def = IMX219_VTS_15FPS,
+ .reg_list = {
+ .num_of_regs = ARRAY_SIZE(mode_3280x2464_regs),
+ .regs = mode_3280x2464_regs,
+ },
+ },
+ {
+ /* 1080P 30fps cropped */
+ .width = 1920,
+ .height = 1080,
+ .vts_def = IMX219_VTS_30FPS_1080P,
+ .reg_list = {
+ .num_of_regs = ARRAY_SIZE(mode_1920_1080_regs),
+ .regs = mode_1920_1080_regs,
+ },
+ },
+ {
+ /* 2x2 binned 30fps mode */
+ .width = 1640,
+ .height = 1232,
+ .vts_def = IMX219_VTS_30FPS_BINNED,
+ .reg_list = {
+ .num_of_regs = ARRAY_SIZE(mode_1640_1232_regs),
+ .regs = mode_1640_1232_regs,
+ },
+ },
+};
+
+struct imx219 {
+ struct device *dev;
+
+ struct v4l2_subdev sd;
+ struct media_pad pad;
+
+ struct v4l2_fwnode_endpoint ep; /* the parsed DT endpoint info */
+ struct clk *xclk; /* system clock to IMX219 */
+ u32 xclk_freq;
+
+ struct gpio_desc *reset_gpio;
+ struct regulator_bulk_data supplies[IMX219_NUM_SUPPLIES];
+
+ struct v4l2_ctrl_handler ctrl_handler;
+ /* V4L2 Controls */
+ struct v4l2_ctrl *pixel_rate;
+ struct v4l2_ctrl *exposure;
+ struct v4l2_ctrl *vflip;
+ struct v4l2_ctrl *hflip;
+ struct v4l2_ctrl *vblank;
+ struct v4l2_ctrl *hblank;
+
+ /* Current mode */
+ const struct imx219_mode *mode;
+
+ /*
+ * Mutex for serialized access:
+ * Protect sensor module set pad format and start/stop streaming safely.
+ */
+ struct mutex mutex;
+
+ /* Streaming on/off */
+ bool streaming;
+};
+
+static inline struct imx219 *to_imx219(struct v4l2_subdev *_sd)
+{
+ return container_of(_sd, struct imx219, sd);
+}
+
+/* Read registers up to 2 at a time */
+static int imx219_read_reg(struct imx219 *imx219, u16 reg, u32 len, u32 *val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
+ struct i2c_msg msgs[2];
+ u8 addr_buf[2] = { reg >> 8, reg & 0xff };
+ u8 data_buf[4] = { 0, };
+ int ret;
+
+ if (len > 4)
+ return -EINVAL;
+
+ /* Write register address */
+ msgs[0].addr = client->addr;
+ msgs[0].flags = 0;
+ msgs[0].len = ARRAY_SIZE(addr_buf);
+ msgs[0].buf = addr_buf;
+
+ /* Read data from register */
+ msgs[1].addr = client->addr;
+ msgs[1].flags = I2C_M_RD;
+ msgs[1].len = len;
+ msgs[1].buf = &data_buf[4 - len];
+
+ ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+ if (ret != ARRAY_SIZE(msgs))
+ return -EIO;
+
+ *val = get_unaligned_be32(data_buf);
+
+ return 0;
+}
+
+/* Write registers up to 2 at a time */
+static int imx219_write_reg(struct imx219 *imx219, u16 reg, u32 len, u32 val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
+ u8 buf[6];
+
+ if (len > 4)
+ return -EINVAL;
+
+ put_unaligned_be16(reg, buf);
+ put_unaligned_be32(val << (8 * (4 - len)), buf + 2);
+ if (i2c_master_send(client, buf, len + 2) != len + 2)
+ return -EIO;
+
+ return 0;
+}
+
+/* Write a list of registers */
+static int imx219_write_regs(struct imx219 *imx219,
+ const struct imx219_reg *regs, u32 len)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < len; i++) {
+ ret = imx219_write_reg(imx219, regs[i].address, 1, regs[i].val);
+ if (ret) {
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ regs[i].address, ret);
+
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+/* Get bayer order based on flip setting. */
+static u32 imx219_get_format_code(struct imx219 *imx219)
+{
+ /*
+ * Only one bayer order is supported.
+ * It depends on the flip settings.
+ */
+ static const u32 codes[2][2] = {
+ { MEDIA_BUS_FMT_SRGGB10_1X10, MEDIA_BUS_FMT_SGRBG10_1X10, },
+ { MEDIA_BUS_FMT_SGBRG10_1X10, MEDIA_BUS_FMT_SBGGR10_1X10, },
+ };
+
+ lockdep_assert_held(&imx219->mutex);
+ return codes[imx219->vflip->val][imx219->hflip->val];
+}
+
+static int imx219_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+ struct imx219 *imx219 = to_imx219(sd);
+ struct v4l2_mbus_framefmt *try_fmt =
+ v4l2_subdev_get_try_format(sd, fh->pad, 0);
+
+ mutex_lock(&imx219->mutex);
+
+ /* Initialize try_fmt */
+ try_fmt->width = supported_modes[0].width;
+ try_fmt->height = supported_modes[0].height;
+ try_fmt->code = imx219_get_format_code(imx219);
+ try_fmt->field = V4L2_FIELD_NONE;
+
+ mutex_unlock(&imx219->mutex);
+
+ return 0;
+}
+
+static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct imx219 *imx219 =
+ container_of(ctrl->handler, struct imx219, ctrl_handler);
+ struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
+ int ret;
+
+ if (ctrl->id == V4L2_CID_VBLANK) {
+ int exposure_max, exposure_def;
+
+ /* Update max exposure while meeting expected vblanking */
+ exposure_max = imx219->mode->height + ctrl->val - 4;
+ exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ?
+ exposure_max : IMX219_EXPOSURE_DEFAULT;
+ __v4l2_ctrl_modify_range(imx219->exposure,
+ imx219->exposure->minimum,
+ exposure_max, imx219->exposure->step,
+ exposure_def);
+ }
+
+ /*
+ * Applying V4L2 control value only happens
+ * when power is up for streaming
+ */
+ if (pm_runtime_get_if_in_use(&client->dev) == 0)
+ return 0;
+
+ switch (ctrl->id) {
+ case V4L2_CID_ANALOGUE_GAIN:
+ ret = imx219_write_reg(imx219, IMX219_REG_ANALOG_GAIN,
+ IMX219_REG_VALUE_08BIT, ctrl->val);
+ break;
+ case V4L2_CID_EXPOSURE:
+ ret = imx219_write_reg(imx219, IMX219_REG_EXPOSURE,
+ IMX219_REG_VALUE_16BIT, ctrl->val);
+ break;
+ case V4L2_CID_DIGITAL_GAIN:
+ ret = imx219_write_reg(imx219, IMX219_REG_DIGITAL_GAIN,
+ IMX219_REG_VALUE_16BIT, ctrl->val);
+ break;
+ case V4L2_CID_TEST_PATTERN:
+ ret = imx219_write_reg(imx219, IMX219_REG_TEST_PATTERN,
+ IMX219_REG_VALUE_16BIT,
+ imx219_test_pattern_val[ctrl->val]);
+ break;
+ case V4L2_CID_HFLIP:
+ case V4L2_CID_VFLIP:
+ ret = imx219_write_reg(imx219, IMX219_REG_ORIENTATION, 1,
+ imx219->hflip->val |
+ imx219->vflip->val << 1);
+ break;
+ case V4L2_CID_VBLANK:
+ ret = imx219_write_reg(imx219, IMX219_REG_VTS,
+ IMX219_REG_VALUE_16BIT,
+ imx219->mode->height + ctrl->val);
+ break;
+ case V4L2_CID_TEST_PATTERN_RED:
+ ret = imx219_write_reg(imx219, IMX219_REG_TESTP_RED,
+ IMX219_REG_VALUE_16BIT, ctrl->val);
+ break;
+ case V4L2_CID_TEST_PATTERN_GREENR:
+ ret = imx219_write_reg(imx219, IMX219_REG_TESTP_GREENR,
+ IMX219_REG_VALUE_16BIT, ctrl->val);
+ break;
+ case V4L2_CID_TEST_PATTERN_BLUE:
+ ret = imx219_write_reg(imx219, IMX219_REG_TESTP_BLUE,
+ IMX219_REG_VALUE_16BIT, ctrl->val);
+ break;
+ case V4L2_CID_TEST_PATTERN_GREENB:
+ ret = imx219_write_reg(imx219, IMX219_REG_TESTP_GREENB,
+ IMX219_REG_VALUE_16BIT, ctrl->val);
+ break;
+ default:
+ dev_info(&client->dev,
+ "ctrl(id:0x%x,val:0x%x) is not handled\n",
+ ctrl->id, ctrl->val);
+ ret = -EINVAL;
+ break;
+ }
+
+ pm_runtime_put(&client->dev);
+
+ return ret;
+}
+
+static const struct v4l2_ctrl_ops imx219_ctrl_ops = {
+ .s_ctrl = imx219_set_ctrl,
+};
+
+static int imx219_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ struct imx219 *imx219 = to_imx219(sd);
+
+ /*
+ * Only one bayer order is supported (though it depends on the flip
+ * settings)
+ */
+ if (code->index > 0)
+ return -EINVAL;
+
+ code->code = imx219_get_format_code(imx219);
+
+ return 0;
+}
+
+static int imx219_enum_frame_size(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_frame_size_enum *fse)
+{
+ struct imx219 *imx219 = to_imx219(sd);
+
+ if (fse->index >= ARRAY_SIZE(supported_modes))
+ return -EINVAL;
+
+ if (fse->code != imx219_get_format_code(imx219))
+ 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 void imx219_reset_colorspace(struct v4l2_mbus_framefmt *fmt)
+{
+ fmt->colorspace = V4L2_COLORSPACE_SRGB;
+ fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
+ fmt->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
+ fmt->colorspace,
+ fmt->ycbcr_enc);
+ fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
+}
+
+static void imx219_update_pad_format(struct imx219 *imx219,
+ const struct imx219_mode *mode,
+ struct v4l2_subdev_format *fmt)
+{
+ fmt->format.width = mode->width;
+ fmt->format.height = mode->height;
+ fmt->format.code = imx219_get_format_code(imx219);
+ fmt->format.field = V4L2_FIELD_NONE;
+
+ imx219_reset_colorspace(&fmt->format);
+}
+
+static int __imx219_get_pad_format(struct imx219 *imx219,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *fmt)
+{
+ if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
+ fmt->format = *v4l2_subdev_get_try_format(&imx219->sd, cfg,
+ fmt->pad);
+ else
+ imx219_update_pad_format(imx219, imx219->mode, fmt);
+
+ return 0;
+}
+
+static int imx219_get_pad_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *fmt)
+{
+ struct imx219 *imx219 = to_imx219(sd);
+ int ret;
+
+ mutex_lock(&imx219->mutex);
+ ret = __imx219_get_pad_format(imx219, cfg, fmt);
+ mutex_unlock(&imx219->mutex);
+
+ return ret;
+}
+
+static int imx219_set_pad_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *fmt)
+{
+ struct imx219 *imx219 = to_imx219(sd);
+ const struct imx219_mode *mode;
+ struct v4l2_mbus_framefmt *framefmt;
+ int exposure_max, exposure_def, hblank;
+
+ mutex_lock(&imx219->mutex);
+
+ /* Bayer order varies with flips */
+ fmt->format.code = imx219_get_format_code(imx219);
+
+ mode = v4l2_find_nearest_size(supported_modes,
+ ARRAY_SIZE(supported_modes),
+ width, height,
+ fmt->format.width, fmt->format.height);
+ imx219_update_pad_format(imx219, mode, fmt);
+ if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+ framefmt = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
+ *framefmt = fmt->format;
+ } else if (imx219->mode != mode) {
+ imx219->mode = mode;
+ /* Update limits and set FPS to default */
+ __v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN,
+ IMX219_VTS_MAX - mode->height, 1,
+ mode->vts_def - mode->height);
+ __v4l2_ctrl_s_ctrl(imx219->vblank,
+ mode->vts_def - mode->height);
+ /* Update max exposure while meeting expected vblanking */
+ exposure_max = mode->vts_def - 4;
+ exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ?
+ exposure_max : IMX219_EXPOSURE_DEFAULT;
+ __v4l2_ctrl_modify_range(imx219->exposure,
+ imx219->exposure->minimum,
+ exposure_max, imx219->exposure->step,
+ exposure_def);
+ /*
+ * Currently PPL is fixed to IMX219_PPL_DEFAULT, so hblank
+ * depends on mode->width only, and is not changeble in any
+ * way other than changing the mode.
+ */
+ hblank = IMX219_PPL_DEFAULT - mode->width;
+ __v4l2_ctrl_modify_range(imx219->hblank, hblank, hblank, 1,
+ hblank);
+ }
+
+ mutex_unlock(&imx219->mutex);
+
+ return 0;
+}
+
+static int imx219_start_streaming(struct imx219 *imx219)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
+ const struct imx219_reg_list *reg_list;
+ int ret;
+
+ /* Apply default values of current mode */
+ reg_list = &imx219->mode->reg_list;
+ ret = imx219_write_regs(imx219, reg_list->regs, reg_list->num_of_regs);
+ if (ret) {
+ dev_err(&client->dev, "%s failed to set mode\n", __func__);
+ return ret;
+ }
+
+ /* Apply customized values from user */
+ ret = __v4l2_ctrl_handler_setup(imx219->sd.ctrl_handler);
+ if (ret)
+ return ret;
+
+ /* set stream on register */
+ return imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
+ IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING);
+}
+
+static void imx219_stop_streaming(struct imx219 *imx219)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
+ int ret;
+
+ /* set stream off register */
+ ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
+ IMX219_REG_VALUE_08BIT, IMX219_MODE_STANDBY);
+ if (ret)
+ dev_err(&client->dev, "%s failed to set stream\n", __func__);
+}
+
+static int imx219_set_stream(struct v4l2_subdev *sd, int enable)
+{
+ struct imx219 *imx219 = to_imx219(sd);
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ int ret = 0;
+
+ mutex_lock(&imx219->mutex);
+ if (imx219->streaming == enable) {
+ mutex_unlock(&imx219->mutex);
+ return 0;
+ }
+
+ if (enable) {
+ ret = pm_runtime_get_sync(&client->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(&client->dev);
+ goto err_unlock;
+ }
+
+ /*
+ * Apply default & customized values
+ * and then start streaming.
+ */
+ ret = imx219_start_streaming(imx219);
+ if (ret)
+ goto err_rpm_put;
+ } else {
+ imx219_stop_streaming(imx219);
+ pm_runtime_put(&client->dev);
+ }
+
+ imx219->streaming = enable;
+
+ /* vflip and hflip cannot change during streaming */
+ __v4l2_ctrl_grab(imx219->vflip, enable);
+ __v4l2_ctrl_grab(imx219->hflip, enable);
+
+ mutex_unlock(&imx219->mutex);
+
+ return ret;
+
+err_rpm_put:
+ pm_runtime_put(&client->dev);
+err_unlock:
+ mutex_unlock(&imx219->mutex);
+
+ return ret;
+}
+
+/* Power/clock management functions */
+static int imx219_power_on(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx219 *imx219 = to_imx219(sd);
+ int ret;
+
+ ret = regulator_bulk_enable(IMX219_NUM_SUPPLIES,
+ imx219->supplies);
+ if (ret) {
+ dev_err(&client->dev, "%s: failed to enable regulators\n",
+ __func__);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(imx219->xclk);
+ if (ret) {
+ dev_err(&client->dev, "%s: failed to enable clock\n",
+ __func__);
+ goto reg_off;
+ }
+
+ gpiod_set_value_cansleep(imx219->reset_gpio, 1);
+ msleep(IMX219_XCLR_DELAY_MS);
+
+ return 0;
+reg_off:
+ regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies);
+ return ret;
+}
+
+static int imx219_power_off(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx219 *imx219 = to_imx219(sd);
+
+ gpiod_set_value_cansleep(imx219->reset_gpio, 0);
+ regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies);
+ clk_disable_unprepare(imx219->xclk);
+
+ return 0;
+}
+
+static int __maybe_unused imx219_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx219 *imx219 = to_imx219(sd);
+
+ if (imx219->streaming)
+ imx219_stop_streaming(imx219);
+
+ return 0;
+}
+
+static int __maybe_unused imx219_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx219 *imx219 = to_imx219(sd);
+ int ret;
+
+ if (imx219->streaming) {
+ ret = imx219_start_streaming(imx219);
+ if (ret)
+ goto error;
+ }
+
+ return 0;
+
+error:
+ imx219_stop_streaming(imx219);
+ imx219->streaming = 0;
+ return ret;
+}
+
+static int imx219_get_regulators(struct imx219 *imx219)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
+ int i;
+
+ for (i = 0; i < IMX219_NUM_SUPPLIES; i++)
+ imx219->supplies[i].supply = imx219_supply_name[i];
+
+ return devm_regulator_bulk_get(&client->dev,
+ IMX219_NUM_SUPPLIES,
+ imx219->supplies);
+}
+
+/* Verify chip ID */
+static int imx219_identify_module(struct imx219 *imx219)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
+ int ret;
+ u32 val;
+
+ ret = imx219_read_reg(imx219, IMX219_REG_CHIP_ID,
+ IMX219_REG_VALUE_16BIT, &val);
+ if (ret) {
+ dev_err(&client->dev, "failed to read chip id %x\n",
+ IMX219_CHIP_ID);
+ return ret;
+ }
+
+ if (val != IMX219_CHIP_ID) {
+ dev_err(&client->dev, "chip id mismatch: %x!=%x\n",
+ IMX219_CHIP_ID, val);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static const struct v4l2_subdev_core_ops imx219_core_ops = {
+ .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+ .unsubscribe_event = v4l2_event_subdev_unsubscribe,
+};
+
+static const struct v4l2_subdev_video_ops imx219_video_ops = {
+ .s_stream = imx219_set_stream,
+};
+
+static const struct v4l2_subdev_pad_ops imx219_pad_ops = {
+ .enum_mbus_code = imx219_enum_mbus_code,
+ .get_fmt = imx219_get_pad_format,
+ .set_fmt = imx219_set_pad_format,
+ .enum_frame_size = imx219_enum_frame_size,
+};
+
+static const struct v4l2_subdev_ops imx219_subdev_ops = {
+ .core = &imx219_core_ops,
+ .video = &imx219_video_ops,
+ .pad = &imx219_pad_ops,
+};
+
+static const struct v4l2_subdev_internal_ops imx219_internal_ops = {
+ .open = imx219_open,
+};
+
+/* Initialize control handlers */
+static int imx219_init_controls(struct imx219 *imx219)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
+ struct v4l2_ctrl_handler *ctrl_hdlr;
+ unsigned int height = imx219->mode->height;
+ int exposure_max, exposure_def, hblank;
+ int i, ret;
+
+ ctrl_hdlr = &imx219->ctrl_handler;
+ ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9);
+ if (ret)
+ return ret;
+
+ mutex_init(&imx219->mutex);
+ ctrl_hdlr->lock = &imx219->mutex;
+
+ /* By default, PIXEL_RATE is read only */
+ imx219->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
+ V4L2_CID_PIXEL_RATE,
+ IMX219_PIXEL_RATE,
+ IMX219_PIXEL_RATE, 1,
+ IMX219_PIXEL_RATE);
+
+ /* Initial vblank/hblank/exposure parameters based on current mode */
+ imx219->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
+ V4L2_CID_VBLANK, IMX219_VBLANK_MIN,
+ IMX219_VTS_MAX - height, 1,
+ imx219->mode->vts_def - height);
+ hblank = IMX219_PPL_DEFAULT - imx219->mode->width;
+ imx219->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
+ V4L2_CID_HBLANK, hblank, hblank,
+ 1, hblank);
+ if (imx219->hblank)
+ imx219->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+ exposure_max = imx219->mode->vts_def - 4;
+ exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ?
+ exposure_max : IMX219_EXPOSURE_DEFAULT;
+ imx219->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
+ V4L2_CID_EXPOSURE,
+ IMX219_EXPOSURE_MIN, exposure_max,
+ IMX219_EXPOSURE_STEP,
+ exposure_def);
+
+ v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, V4L2_CID_ANALOGUE_GAIN,
+ IMX219_ANA_GAIN_MIN, IMX219_ANA_GAIN_MAX,
+ IMX219_ANA_GAIN_STEP, IMX219_ANA_GAIN_DEFAULT);
+
+ v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, V4L2_CID_DIGITAL_GAIN,
+ IMX219_DGTL_GAIN_MIN, IMX219_DGTL_GAIN_MAX,
+ IMX219_DGTL_GAIN_STEP, IMX219_DGTL_GAIN_DEFAULT);
+
+ imx219->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ if (imx219->hflip)
+ imx219->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
+
+ imx219->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ if (imx219->vflip)
+ imx219->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
+
+ v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &imx219_ctrl_ops,
+ V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(imx219_test_pattern_menu) - 1,
+ 0, 0, imx219_test_pattern_menu);
+ for (i = 0; i < 4; i++) {
+ /*
+ * The assumption is that
+ * V4L2_CID_TEST_PATTERN_GREENR == V4L2_CID_TEST_PATTERN_RED + 1
+ * V4L2_CID_TEST_PATTERN_BLUE == V4L2_CID_TEST_PATTERN_RED + 2
+ * V4L2_CID_TEST_PATTERN_GREENB == V4L2_CID_TEST_PATTERN_RED + 3
+ */
+ v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
+ V4L2_CID_TEST_PATTERN_RED + i,
+ IMX219_TESTP_COLOUR_MIN,
+ IMX219_TESTP_COLOUR_MAX,
+ IMX219_TESTP_COLOUR_STEP,
+ IMX219_TESTP_COLOUR_MAX);
+ /* The "Solid color" pattern is white by default */
+ }
+
+ if (ctrl_hdlr->error) {
+ ret = ctrl_hdlr->error;
+ dev_err(&client->dev, "%s control init failed (%d)\n",
+ __func__, ret);
+ goto error;
+ }
+
+ imx219->sd.ctrl_handler = ctrl_hdlr;
+
+ return 0;
+
+error:
+ v4l2_ctrl_handler_free(ctrl_hdlr);
+ mutex_destroy(&imx219->mutex);
+
+ return ret;
+}
+
+static void imx219_free_controls(struct imx219 *imx219)
+{
+ v4l2_ctrl_handler_free(imx219->sd.ctrl_handler);
+ mutex_destroy(&imx219->mutex);
+}
+
+static int imx219_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct fwnode_handle *endpoint;
+ struct imx219 *imx219;
+ int ret;
+
+ imx219 = devm_kzalloc(&client->dev, sizeof(*imx219), GFP_KERNEL);
+ if (!imx219)
+ return -ENOMEM;
+
+ imx219->dev = dev;
+
+ v4l2_i2c_subdev_init(&imx219->sd, client, &imx219_subdev_ops);
+
+ /* Get CSI2 bus config */
+ endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
+ NULL);
+ if (!endpoint) {
+ dev_err(dev, "endpoint node not found\n");
+ return -EINVAL;
+ }
+
+ ret = v4l2_fwnode_endpoint_parse(endpoint, &imx219->ep);
+ fwnode_handle_put(endpoint);
+ if (ret) {
+ dev_err(dev, "could not parse endpoint\n");
+ return ret;
+ }
+
+ /* Check the number of MIPI CSI2 data lanes */
+ if (imx219->ep.bus_type != V4L2_MBUS_CSI2_DPHY ||
+ imx219->ep.bus.mipi_csi2.num_data_lanes != 2) {
+ dev_err(dev, "only 2 data lanes are currently supported\n");
+ return -EINVAL;
+ }
+
+ /* Get system clock (xclk) */
+ imx219->xclk = devm_clk_get(dev, NULL);
+ if (IS_ERR(imx219->xclk)) {
+ dev_err(dev, "failed to get xclk\n");
+ return PTR_ERR(imx219->xclk);
+ }
+
+ imx219->xclk_freq = clk_get_rate(imx219->xclk);
+ if (imx219->xclk_freq != IMX219_XCLK_FREQ) {
+ dev_err(dev, "xclk frequency not supported: %d Hz\n",
+ imx219->xclk_freq);
+ return -EINVAL;
+ }
+
+ ret = imx219_get_regulators(imx219);
+ if (ret) {
+ dev_err(dev, "failed to get regulators\n");
+ return ret;
+ }
+
+ /* Request optional enable pin */
+ imx219->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+ GPIOD_OUT_HIGH);
+
+ /*
+ * The sensor must be powered for imx219_identify_module()
+ * to be able to read the CHIP_ID register
+ */
+ ret = imx219_power_on(dev);
+ if (ret)
+ return ret;
+
+ ret = imx219_identify_module(imx219);
+ if (ret)
+ goto error_power_off;
+
+ /* Set default mode to max resolution */
+ imx219->mode = &supported_modes[0];
+
+ ret = imx219_init_controls(imx219);
+ if (ret)
+ goto error_power_off;
+
+ /* Initialize subdev */
+ imx219->sd.internal_ops = &imx219_internal_ops;
+ imx219->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ imx219->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+
+ /* Initialize source pad */
+ imx219->pad.flags = MEDIA_PAD_FL_SOURCE;
+
+ ret = media_entity_pads_init(&imx219->sd.entity, 1, &imx219->pad);
+ if (ret) {
+ dev_err(dev, "failed to init entity pads: %d\n", ret);
+ goto error_handler_free;
+ }
+
+ ret = v4l2_async_register_subdev_sensor_common(&imx219->sd);
+ if (ret < 0) {
+ dev_err(dev, "failed to register sensor sub-device: %d\n", ret);
+ goto error_media_entity;
+ }
+
+ /* Enable runtime PM and turn off the device */
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ pm_runtime_idle(dev);
+
+ return 0;
+
+error_media_entity:
+ media_entity_cleanup(&imx219->sd.entity);
+
+error_handler_free:
+ imx219_free_controls(imx219);
+
+error_power_off:
+ imx219_power_off(dev);
+
+ return ret;
+}
+
+static int imx219_remove(struct i2c_client *client)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx219 *imx219 = to_imx219(sd);
+
+ v4l2_async_unregister_subdev(sd);
+ media_entity_cleanup(&sd->entity);
+ imx219_free_controls(imx219);
+
+ pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+
+ return 0;
+}
+
+static const struct of_device_id imx219_dt_ids[] = {
+ { .compatible = "sony,imx219" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx219_dt_ids);
+
+static const struct dev_pm_ops imx219_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(imx219_suspend, imx219_resume)
+ SET_RUNTIME_PM_OPS(imx219_power_off, imx219_power_on, NULL)
+};
+
+static struct i2c_driver imx219_i2c_driver = {
+ .driver = {
+ .name = "imx219",
+ .of_match_table = imx219_dt_ids,
+ .pm = &imx219_pm_ops,
+ },
+ .probe = imx219_probe,
+ .remove = imx219_remove,
+};
+
+module_i2c_driver(imx219_i2c_driver);
+
+MODULE_AUTHOR("Dave Stevenson <dave.stevenson@raspberrypi.com");
+MODULE_DESCRIPTION("Sony IMX219 sensor driver");
+MODULE_LICENSE("GPL v2");
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] Add IMX219 CMOS image sensor support
2020-01-10 20:09 [PATCH v3 0/2] Add IMX219 CMOS image sensor support Andrey Konovalov
2020-01-10 20:09 ` [PATCH v3 1/2] dt-bindings: media: i2c: Add IMX219 CMOS sensor binding Andrey Konovalov
2020-01-10 20:09 ` [PATCH v3 2/2] media: i2c: Add driver for Sony IMX219 sensor Andrey Konovalov
@ 2020-01-11 4:45 ` Ezequiel Garcia
2020-01-11 17:46 ` Andrey Konovalov
2 siblings, 1 reply; 10+ messages in thread
From: Ezequiel Garcia @ 2020-01-11 4:45 UTC (permalink / raw)
To: Andrey Konovalov, mchehab, robh+dt
Cc: linux-media, devicetree, peter.griffin, dave.stevenson,
sakari.ailus
Hello Andrey,
Thanks for submitting a new version.
On Fri, 2020-01-10 at 23:09 +0300, Andrey Konovalov wrote:
> This patchset adds support for IMX219 CMOS image sensor from Sony.
> Sony IMX219 is an 8MPix, 1/4.0-inch CMOS active pixel digital image sensor
> with an active array size of 3280H x 2464V. It is programmable through
> I2C interface. Image data are sent through MIPI CSI-2, which can be configured
> as either 2 or 4 data lanes, but this driver currently only supports 2 lanes.
> The currently supported resolutions are 3280x2464 @ 15fps, 1920x1080 @ 30fps
> (cropped FOV), and 1640x1232 (2x2 binned) @ 30fps.
>
> The driver has been tested with Raspberry Pi Camera Module v2 connected to
> Raspberry Pi Zero W.
>
> Changes since v2 [1]:
>
> dt-bindings:
> - "clock-names" property removed
> - "xclr-gpios" property renamed to "reset-gpios"
> - the camera-clk mode moved out of sensor device node
> - "clock-lanes" property removed (the sensor doesn't support lane reordering)
> - "clock-noncontinuous" description made more clear (thanks Sakari)
> - "data-lanes" property reworked: it is now optional, and if it is not
> present the driver should assume four-lane operation. For two-lane
> operation (the only mode supported by the current driver) this property
> must be present and set to <1 2>
>
> imx219 sensor driver:
> - "xclr-gpios" property renamed to "reset-gpios", the corresponding
> struct gpio_desc field in the imx219 structure is renamed to reset_gpio
> - in the driver probe() a test to check that the number of CSI-2 data lanes
> is supported by the driver was added
> - devm_clk_get() is now called with NULL as the 2nd argument, as there is
> just single clock, and there is no need to use clock ID
> - error messages are added when the driver fails to get regulators, to init
> media entity pads, or to register sensor sub-device
>
It seems you missed Sakari's review comments on v2,
see https://patchwork.linuxtv.org/patch/60925/#114407
Thanks!
Eze
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] Add IMX219 CMOS image sensor support
2020-01-11 4:45 ` [PATCH v3 0/2] Add IMX219 CMOS image sensor support Ezequiel Garcia
@ 2020-01-11 17:46 ` Andrey Konovalov
0 siblings, 0 replies; 10+ messages in thread
From: Andrey Konovalov @ 2020-01-11 17:46 UTC (permalink / raw)
To: Ezequiel Garcia, mchehab, robh+dt
Cc: linux-media, devicetree, peter.griffin, dave.stevenson,
sakari.ailus
Hi Ezequiel,
On 11.01.2020 07:45, Ezequiel Garcia wrote:
> Hello Andrey,
>
> Thanks for submitting a new version.
>
> On Fri, 2020-01-10 at 23:09 +0300, Andrey Konovalov wrote:
>> This patchset adds support for IMX219 CMOS image sensor from Sony.
>> Sony IMX219 is an 8MPix, 1/4.0-inch CMOS active pixel digital image sensor
>> with an active array size of 3280H x 2464V. It is programmable through
>> I2C interface. Image data are sent through MIPI CSI-2, which can be configured
>> as either 2 or 4 data lanes, but this driver currently only supports 2 lanes.
>> The currently supported resolutions are 3280x2464 @ 15fps, 1920x1080 @ 30fps
>> (cropped FOV), and 1640x1232 (2x2 binned) @ 30fps.
>>
>> The driver has been tested with Raspberry Pi Camera Module v2 connected to
>> Raspberry Pi Zero W.
>>
>> Changes since v2 [1]:
>>
>> dt-bindings:
>> - "clock-names" property removed
>> - "xclr-gpios" property renamed to "reset-gpios"
>> - the camera-clk mode moved out of sensor device node
>> - "clock-lanes" property removed (the sensor doesn't support lane reordering)
>> - "clock-noncontinuous" description made more clear (thanks Sakari)
>> - "data-lanes" property reworked: it is now optional, and if it is not
>> present the driver should assume four-lane operation. For two-lane
>> operation (the only mode supported by the current driver) this property
>> must be present and set to <1 2>
>>
>> imx219 sensor driver:
>> - "xclr-gpios" property renamed to "reset-gpios", the corresponding
>> struct gpio_desc field in the imx219 structure is renamed to reset_gpio
>> - in the driver probe() a test to check that the number of CSI-2 data lanes
>> is supported by the driver was added
>> - devm_clk_get() is now called with NULL as the 2nd argument, as there is
>> just single clock, and there is no need to use clock ID
>> - error messages are added when the driver fails to get regulators, to init
>> media entity pads, or to register sensor sub-device
>>
>
> It seems you missed Sakari's review comments on v2,
> see https://patchwork.linuxtv.org/patch/60925/#114407
Hm.. Indeed. I missed Sakari's comments on the imx219.c v2 somehow.
I'll post an updated patchset shortly.
Thanks!
Andrey
> Thanks!
> Eze
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: media: i2c: Add IMX219 CMOS sensor binding
2020-01-10 20:09 ` [PATCH v3 1/2] dt-bindings: media: i2c: Add IMX219 CMOS sensor binding Andrey Konovalov
@ 2020-01-13 23:04 ` Rob Herring
0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2020-01-13 23:04 UTC (permalink / raw)
To: Andrey Konovalov
Cc: mchehab, robh+dt, linux-media, devicetree, peter.griffin,
dave.stevenson, ezequiel, sakari.ailus, Andrey Konovalov
On Fri, 10 Jan 2020 23:09:14 +0300, Andrey Konovalov wrote:
> Add YAML device tree binding for IMX219 CMOS image sensor, and
> the relevant MAINTAINERS entries.
>
> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
> ---
> .../devicetree/bindings/media/i2c/imx219.yaml | 104 ++++++++++++++++++
> MAINTAINERS | 8 ++
> 2 files changed, 112 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] media: i2c: Add driver for Sony IMX219 sensor
2020-01-10 20:09 ` [PATCH v3 2/2] media: i2c: Add driver for Sony IMX219 sensor Andrey Konovalov
@ 2025-01-20 10:59 ` Sakari Ailus
2025-01-20 12:28 ` Dave Stevenson
0 siblings, 1 reply; 10+ messages in thread
From: Sakari Ailus @ 2025-01-20 10:59 UTC (permalink / raw)
To: robh+dt, linux-media, devicetree, peter.griffin, dave.stevenson,
ezequiel
Hi Dave,
On Fri, Jan 10, 2020 at 11:09:15PM +0300, Andrey Konovalov wrote:
> +/* Power/clock management functions */
> +static int imx219_power_on(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx219 *imx219 = to_imx219(sd);
> + int ret;
> +
> + ret = regulator_bulk_enable(IMX219_NUM_SUPPLIES,
> + imx219->supplies);
> + if (ret) {
> + dev_err(&client->dev, "%s: failed to enable regulators\n",
> + __func__);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(imx219->xclk);
> + if (ret) {
> + dev_err(&client->dev, "%s: failed to enable clock\n",
> + __func__);
> + goto reg_off;
> + }
> +
> + gpiod_set_value_cansleep(imx219->reset_gpio, 1);
> + msleep(IMX219_XCLR_DELAY_MS);
> +
> + return 0;
> +reg_off:
> + regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies);
> + return ret;
> +}
> +
> +static int imx219_power_off(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx219 *imx219 = to_imx219(sd);
> +
> + gpiod_set_value_cansleep(imx219->reset_gpio, 0);
The polarity of the reset GPIO appears to be wrong above. Given it works
somewhere (arch/arm64/boot/dts/ti/k3-am62x-sk-csi2-imx219.dtso), the
existing DTS files have it wrong, too. The bindings still appear to
document it correctly.
Laurent confirmed xcrl isn't controllable in the RPi imx219 camera module.
How about fixing this? Currently correctly written DTBs including imx219
won't work.
I noticed this while fixing the power sequences in this and a few other
drivers.
> + regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies);
> + clk_disable_unprepare(imx219->xclk);
> +
> + return 0;
> +}
...
> +static int imx219_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct fwnode_handle *endpoint;
> + struct imx219 *imx219;
> + int ret;
> +
> + imx219 = devm_kzalloc(&client->dev, sizeof(*imx219), GFP_KERNEL);
> + if (!imx219)
> + return -ENOMEM;
> +
> + imx219->dev = dev;
> +
> + v4l2_i2c_subdev_init(&imx219->sd, client, &imx219_subdev_ops);
> +
> + /* Get CSI2 bus config */
> + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
> + NULL);
> + if (!endpoint) {
> + dev_err(dev, "endpoint node not found\n");
> + return -EINVAL;
> + }
> +
> + ret = v4l2_fwnode_endpoint_parse(endpoint, &imx219->ep);
> + fwnode_handle_put(endpoint);
> + if (ret) {
> + dev_err(dev, "could not parse endpoint\n");
> + return ret;
> + }
> +
> + /* Check the number of MIPI CSI2 data lanes */
> + if (imx219->ep.bus_type != V4L2_MBUS_CSI2_DPHY ||
> + imx219->ep.bus.mipi_csi2.num_data_lanes != 2) {
> + dev_err(dev, "only 2 data lanes are currently supported\n");
> + return -EINVAL;
> + }
> +
> + /* Get system clock (xclk) */
> + imx219->xclk = devm_clk_get(dev, NULL);
> + if (IS_ERR(imx219->xclk)) {
> + dev_err(dev, "failed to get xclk\n");
> + return PTR_ERR(imx219->xclk);
> + }
> +
> + imx219->xclk_freq = clk_get_rate(imx219->xclk);
> + if (imx219->xclk_freq != IMX219_XCLK_FREQ) {
> + dev_err(dev, "xclk frequency not supported: %d Hz\n",
> + imx219->xclk_freq);
> + return -EINVAL;
> + }
> +
> + ret = imx219_get_regulators(imx219);
> + if (ret) {
> + dev_err(dev, "failed to get regulators\n");
> + return ret;
> + }
> +
> + /* Request optional enable pin */
> + imx219->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> + GPIOD_OUT_HIGH);
> +
> + /*
> + * The sensor must be powered for imx219_identify_module()
> + * to be able to read the CHIP_ID register
> + */
> + ret = imx219_power_on(dev);
> + if (ret)
> + return ret;
> +
> + ret = imx219_identify_module(imx219);
> + if (ret)
> + goto error_power_off;
> +
> + /* Set default mode to max resolution */
> + imx219->mode = &supported_modes[0];
> +
> + ret = imx219_init_controls(imx219);
> + if (ret)
> + goto error_power_off;
> +
> + /* Initialize subdev */
> + imx219->sd.internal_ops = &imx219_internal_ops;
> + imx219->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + imx219->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> +
> + /* Initialize source pad */
> + imx219->pad.flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&imx219->sd.entity, 1, &imx219->pad);
> + if (ret) {
> + dev_err(dev, "failed to init entity pads: %d\n", ret);
> + goto error_handler_free;
> + }
> +
> + ret = v4l2_async_register_subdev_sensor_common(&imx219->sd);
> + if (ret < 0) {
> + dev_err(dev, "failed to register sensor sub-device: %d\n", ret);
> + goto error_media_entity;
> + }
> +
> + /* Enable runtime PM and turn off the device */
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> + pm_runtime_idle(dev);
> +
> + return 0;
> +
> +error_media_entity:
> + media_entity_cleanup(&imx219->sd.entity);
> +
> +error_handler_free:
> + imx219_free_controls(imx219);
> +
> +error_power_off:
> + imx219_power_off(dev);
> +
> + return ret;
> +}
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] media: i2c: Add driver for Sony IMX219 sensor
2025-01-20 10:59 ` Sakari Ailus
@ 2025-01-20 12:28 ` Dave Stevenson
2025-01-20 13:35 ` Sakari Ailus
0 siblings, 1 reply; 10+ messages in thread
From: Dave Stevenson @ 2025-01-20 12:28 UTC (permalink / raw)
To: Sakari Ailus; +Cc: robh+dt, linux-media, devicetree, peter.griffin, ezequiel
Hi Sakari
On Mon, 20 Jan 2025 at 10:59, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> Hi Dave,
>
> On Fri, Jan 10, 2020 at 11:09:15PM +0300, Andrey Konovalov wrote:
> > +/* Power/clock management functions */
> > +static int imx219_power_on(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct imx219 *imx219 = to_imx219(sd);
> > + int ret;
> > +
> > + ret = regulator_bulk_enable(IMX219_NUM_SUPPLIES,
> > + imx219->supplies);
> > + if (ret) {
> > + dev_err(&client->dev, "%s: failed to enable regulators\n",
> > + __func__);
> > + return ret;
> > + }
> > +
> > + ret = clk_prepare_enable(imx219->xclk);
> > + if (ret) {
> > + dev_err(&client->dev, "%s: failed to enable clock\n",
> > + __func__);
> > + goto reg_off;
> > + }
> > +
> > + gpiod_set_value_cansleep(imx219->reset_gpio, 1);
> > + msleep(IMX219_XCLR_DELAY_MS);
> > +
> > + return 0;
> > +reg_off:
> > + regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies);
> > + return ret;
> > +}
> > +
> > +static int imx219_power_off(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct imx219 *imx219 = to_imx219(sd);
> > +
> > + gpiod_set_value_cansleep(imx219->reset_gpio, 0);
>
> The polarity of the reset GPIO appears to be wrong above. Given it works
> somewhere (arch/arm64/boot/dts/ti/k3-am62x-sk-csi2-imx219.dtso), the
> existing DTS files have it wrong, too. The bindings still appear to
> document it correctly.
Why do you say it is wrong?
I don't recall why this got called reset-gpio - the signal on the
sensor is XCLR, and that is documented in the binding.
The datasheet says low is standby and high is active, which matches
what the driver does.
> Laurent confirmed xcrl isn't controllable in the RPi imx219 camera module.
>
> How about fixing this? Currently correctly written DTBs including imx219
> won't work.
Seeing as the DTB is ABI, the only improvement I can see is to rename
"imx219->reset_gpio" to "imx219->xclr_gpio".
What else would you be proposing?
Dave
> I noticed this while fixing the power sequences in this and a few other
> drivers.
>
> > + regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies);
> > + clk_disable_unprepare(imx219->xclk);
> > +
> > + return 0;
> > +}
>
> ...
>
> > +static int imx219_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct device *dev = &client->dev;
> > + struct fwnode_handle *endpoint;
> > + struct imx219 *imx219;
> > + int ret;
> > +
> > + imx219 = devm_kzalloc(&client->dev, sizeof(*imx219), GFP_KERNEL);
> > + if (!imx219)
> > + return -ENOMEM;
> > +
> > + imx219->dev = dev;
> > +
> > + v4l2_i2c_subdev_init(&imx219->sd, client, &imx219_subdev_ops);
> > +
> > + /* Get CSI2 bus config */
> > + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
> > + NULL);
> > + if (!endpoint) {
> > + dev_err(dev, "endpoint node not found\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = v4l2_fwnode_endpoint_parse(endpoint, &imx219->ep);
> > + fwnode_handle_put(endpoint);
> > + if (ret) {
> > + dev_err(dev, "could not parse endpoint\n");
> > + return ret;
> > + }
> > +
> > + /* Check the number of MIPI CSI2 data lanes */
> > + if (imx219->ep.bus_type != V4L2_MBUS_CSI2_DPHY ||
> > + imx219->ep.bus.mipi_csi2.num_data_lanes != 2) {
> > + dev_err(dev, "only 2 data lanes are currently supported\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* Get system clock (xclk) */
> > + imx219->xclk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(imx219->xclk)) {
> > + dev_err(dev, "failed to get xclk\n");
> > + return PTR_ERR(imx219->xclk);
> > + }
> > +
> > + imx219->xclk_freq = clk_get_rate(imx219->xclk);
> > + if (imx219->xclk_freq != IMX219_XCLK_FREQ) {
> > + dev_err(dev, "xclk frequency not supported: %d Hz\n",
> > + imx219->xclk_freq);
> > + return -EINVAL;
> > + }
> > +
> > + ret = imx219_get_regulators(imx219);
> > + if (ret) {
> > + dev_err(dev, "failed to get regulators\n");
> > + return ret;
> > + }
> > +
> > + /* Request optional enable pin */
> > + imx219->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > + GPIOD_OUT_HIGH);
> > +
> > + /*
> > + * The sensor must be powered for imx219_identify_module()
> > + * to be able to read the CHIP_ID register
> > + */
> > + ret = imx219_power_on(dev);
> > + if (ret)
> > + return ret;
> > +
> > + ret = imx219_identify_module(imx219);
> > + if (ret)
> > + goto error_power_off;
> > +
> > + /* Set default mode to max resolution */
> > + imx219->mode = &supported_modes[0];
> > +
> > + ret = imx219_init_controls(imx219);
> > + if (ret)
> > + goto error_power_off;
> > +
> > + /* Initialize subdev */
> > + imx219->sd.internal_ops = &imx219_internal_ops;
> > + imx219->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > + imx219->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > + /* Initialize source pad */
> > + imx219->pad.flags = MEDIA_PAD_FL_SOURCE;
> > +
> > + ret = media_entity_pads_init(&imx219->sd.entity, 1, &imx219->pad);
> > + if (ret) {
> > + dev_err(dev, "failed to init entity pads: %d\n", ret);
> > + goto error_handler_free;
> > + }
> > +
> > + ret = v4l2_async_register_subdev_sensor_common(&imx219->sd);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to register sensor sub-device: %d\n", ret);
> > + goto error_media_entity;
> > + }
> > +
> > + /* Enable runtime PM and turn off the device */
> > + pm_runtime_set_active(dev);
> > + pm_runtime_enable(dev);
> > + pm_runtime_idle(dev);
> > +
> > + return 0;
> > +
> > +error_media_entity:
> > + media_entity_cleanup(&imx219->sd.entity);
> > +
> > +error_handler_free:
> > + imx219_free_controls(imx219);
> > +
> > +error_power_off:
> > + imx219_power_off(dev);
> > +
> > + return ret;
> > +}
>
> --
> Kind regards,
>
> Sakari Ailus
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] media: i2c: Add driver for Sony IMX219 sensor
2025-01-20 12:28 ` Dave Stevenson
@ 2025-01-20 13:35 ` Sakari Ailus
2025-01-20 13:59 ` Laurent Pinchart
0 siblings, 1 reply; 10+ messages in thread
From: Sakari Ailus @ 2025-01-20 13:35 UTC (permalink / raw)
To: Dave Stevenson
Cc: robh+dt, linux-media, devicetree, peter.griffin, ezequiel,
laurent.pinchart
Hi Dave,
On Mon, Jan 20, 2025 at 12:28:04PM +0000, Dave Stevenson wrote:
> Hi Sakari
>
> On Mon, 20 Jan 2025 at 10:59, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Dave,
> >
> > On Fri, Jan 10, 2020 at 11:09:15PM +0300, Andrey Konovalov wrote:
> > > +/* Power/clock management functions */
> > > +static int imx219_power_on(struct device *dev)
> > > +{
> > > + struct i2c_client *client = to_i2c_client(dev);
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct imx219 *imx219 = to_imx219(sd);
> > > + int ret;
> > > +
> > > + ret = regulator_bulk_enable(IMX219_NUM_SUPPLIES,
> > > + imx219->supplies);
> > > + if (ret) {
> > > + dev_err(&client->dev, "%s: failed to enable regulators\n",
> > > + __func__);
> > > + return ret;
> > > + }
> > > +
> > > + ret = clk_prepare_enable(imx219->xclk);
> > > + if (ret) {
> > > + dev_err(&client->dev, "%s: failed to enable clock\n",
> > > + __func__);
> > > + goto reg_off;
> > > + }
> > > +
> > > + gpiod_set_value_cansleep(imx219->reset_gpio, 1);
> > > + msleep(IMX219_XCLR_DELAY_MS);
> > > +
> > > + return 0;
> > > +reg_off:
> > > + regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies);
> > > + return ret;
> > > +}
> > > +
> > > +static int imx219_power_off(struct device *dev)
> > > +{
> > > + struct i2c_client *client = to_i2c_client(dev);
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct imx219 *imx219 = to_imx219(sd);
> > > +
> > > + gpiod_set_value_cansleep(imx219->reset_gpio, 0);
> >
> > The polarity of the reset GPIO appears to be wrong above. Given it works
> > somewhere (arch/arm64/boot/dts/ti/k3-am62x-sk-csi2-imx219.dtso), the
> > existing DTS files have it wrong, too. The bindings still appear to
> > document it correctly.
>
> Why do you say it is wrong?
> I don't recall why this got called reset-gpio - the signal on the
> sensor is XCLR, and that is documented in the binding.
> The datasheet says low is standby and high is active, which matches
> what the driver does.
Documentation/driver-api/gpio/consumer.rst:
As a consumer should not have to care about the physical line
level, all of the gpiod_set_value_xxx() or
gpiod_set_array_value_xxx() functions operate with the *logical*
value. With this they take the active low property into account.
This means that they check whether the GPIO is configured to be
active low, and if so, they manipulate the passed value before the
physical line level is driven.
In other words, if the driver calls gpiod_set_value_cansleep(, 0), it
enables reset signal on the device, bringing the device to hard reset. This
is the opposite of what the driver does in its resume callback.
So in order to function, the DT must have wrong polarity as well.
Similarly, if someone writes a correct DT for a board connecting the reset
GPIO, the imx219 driver simply won't work.
Looking at the arch/arm64/boot/dts/ti/k3-am62x-sk-csi2-imx219.dtso overlay,
it is for the RPi IMX219 module that does not connect the reset GPIO at
all. In other words, there doesn't seem to be a upstream user of the reset
GPIO in IMX219.
>
> > Laurent confirmed xcrl isn't controllable in the RPi imx219 camera module.
> >
> > How about fixing this? Currently correctly written DTBs including imx219
> > won't work.
>
> Seeing as the DTB is ABI, the only improvement I can see is to rename
> "imx219->reset_gpio" to "imx219->xclr_gpio".
> What else would you be proposing?
If there's a concern this could break existing DTs, the driver could try
inverting the polarity of the GPIO if the sensor isn't immediately
accessible.
I wonder what others think.
Cc Laurent.
>
> Dave
>
> > I noticed this while fixing the power sequences in this and a few other
> > drivers.
> >
> > > + regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies);
> > > + clk_disable_unprepare(imx219->xclk);
> > > +
> > > + return 0;
> > > +}
> >
> > ...
> >
> > > +static int imx219_probe(struct i2c_client *client,
> > > + const struct i2c_device_id *id)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct fwnode_handle *endpoint;
> > > + struct imx219 *imx219;
> > > + int ret;
> > > +
> > > + imx219 = devm_kzalloc(&client->dev, sizeof(*imx219), GFP_KERNEL);
> > > + if (!imx219)
> > > + return -ENOMEM;
> > > +
> > > + imx219->dev = dev;
> > > +
> > > + v4l2_i2c_subdev_init(&imx219->sd, client, &imx219_subdev_ops);
> > > +
> > > + /* Get CSI2 bus config */
> > > + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
> > > + NULL);
> > > + if (!endpoint) {
> > > + dev_err(dev, "endpoint node not found\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = v4l2_fwnode_endpoint_parse(endpoint, &imx219->ep);
> > > + fwnode_handle_put(endpoint);
> > > + if (ret) {
> > > + dev_err(dev, "could not parse endpoint\n");
> > > + return ret;
> > > + }
> > > +
> > > + /* Check the number of MIPI CSI2 data lanes */
> > > + if (imx219->ep.bus_type != V4L2_MBUS_CSI2_DPHY ||
> > > + imx219->ep.bus.mipi_csi2.num_data_lanes != 2) {
> > > + dev_err(dev, "only 2 data lanes are currently supported\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + /* Get system clock (xclk) */
> > > + imx219->xclk = devm_clk_get(dev, NULL);
> > > + if (IS_ERR(imx219->xclk)) {
> > > + dev_err(dev, "failed to get xclk\n");
> > > + return PTR_ERR(imx219->xclk);
> > > + }
> > > +
> > > + imx219->xclk_freq = clk_get_rate(imx219->xclk);
> > > + if (imx219->xclk_freq != IMX219_XCLK_FREQ) {
> > > + dev_err(dev, "xclk frequency not supported: %d Hz\n",
> > > + imx219->xclk_freq);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = imx219_get_regulators(imx219);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get regulators\n");
> > > + return ret;
> > > + }
> > > +
> > > + /* Request optional enable pin */
> > > + imx219->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > > + GPIOD_OUT_HIGH);
> > > +
> > > + /*
> > > + * The sensor must be powered for imx219_identify_module()
> > > + * to be able to read the CHIP_ID register
> > > + */
> > > + ret = imx219_power_on(dev);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = imx219_identify_module(imx219);
> > > + if (ret)
> > > + goto error_power_off;
> > > +
> > > + /* Set default mode to max resolution */
> > > + imx219->mode = &supported_modes[0];
> > > +
> > > + ret = imx219_init_controls(imx219);
> > > + if (ret)
> > > + goto error_power_off;
> > > +
> > > + /* Initialize subdev */
> > > + imx219->sd.internal_ops = &imx219_internal_ops;
> > > + imx219->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > + imx219->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > +
> > > + /* Initialize source pad */
> > > + imx219->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > +
> > > + ret = media_entity_pads_init(&imx219->sd.entity, 1, &imx219->pad);
> > > + if (ret) {
> > > + dev_err(dev, "failed to init entity pads: %d\n", ret);
> > > + goto error_handler_free;
> > > + }
> > > +
> > > + ret = v4l2_async_register_subdev_sensor_common(&imx219->sd);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to register sensor sub-device: %d\n", ret);
> > > + goto error_media_entity;
> > > + }
> > > +
> > > + /* Enable runtime PM and turn off the device */
> > > + pm_runtime_set_active(dev);
> > > + pm_runtime_enable(dev);
> > > + pm_runtime_idle(dev);
> > > +
> > > + return 0;
> > > +
> > > +error_media_entity:
> > > + media_entity_cleanup(&imx219->sd.entity);
> > > +
> > > +error_handler_free:
> > > + imx219_free_controls(imx219);
> > > +
> > > +error_power_off:
> > > + imx219_power_off(dev);
> > > +
> > > + return ret;
> > > +}
> >
> > --
> > Kind regards,
> >
> > Sakari Ailus
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] media: i2c: Add driver for Sony IMX219 sensor
2025-01-20 13:35 ` Sakari Ailus
@ 2025-01-20 13:59 ` Laurent Pinchart
0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2025-01-20 13:59 UTC (permalink / raw)
To: Sakari Ailus
Cc: Dave Stevenson, robh+dt, linux-media, devicetree, peter.griffin,
ezequiel
On Mon, Jan 20, 2025 at 01:35:50PM +0000, Sakari Ailus wrote:
> On Mon, Jan 20, 2025 at 12:28:04PM +0000, Dave Stevenson wrote:
> > On Mon, 20 Jan 2025 at 10:59, Sakari Ailus wrote:
> > > On Fri, Jan 10, 2020 at 11:09:15PM +0300, Andrey Konovalov wrote:
> > > > +/* Power/clock management functions */
> > > > +static int imx219_power_on(struct device *dev)
> > > > +{
> > > > + struct i2c_client *client = to_i2c_client(dev);
> > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > + struct imx219 *imx219 = to_imx219(sd);
> > > > + int ret;
> > > > +
> > > > + ret = regulator_bulk_enable(IMX219_NUM_SUPPLIES,
> > > > + imx219->supplies);
> > > > + if (ret) {
> > > > + dev_err(&client->dev, "%s: failed to enable regulators\n",
> > > > + __func__);
> > > > + return ret;
> > > > + }
> > > > +
> > > > + ret = clk_prepare_enable(imx219->xclk);
> > > > + if (ret) {
> > > > + dev_err(&client->dev, "%s: failed to enable clock\n",
> > > > + __func__);
> > > > + goto reg_off;
> > > > + }
> > > > +
> > > > + gpiod_set_value_cansleep(imx219->reset_gpio, 1);
> > > > + msleep(IMX219_XCLR_DELAY_MS);
> > > > +
> > > > + return 0;
> > > > +reg_off:
> > > > + regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int imx219_power_off(struct device *dev)
> > > > +{
> > > > + struct i2c_client *client = to_i2c_client(dev);
> > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > + struct imx219 *imx219 = to_imx219(sd);
> > > > +
> > > > + gpiod_set_value_cansleep(imx219->reset_gpio, 0);
> > >
> > > The polarity of the reset GPIO appears to be wrong above. Given it works
> > > somewhere (arch/arm64/boot/dts/ti/k3-am62x-sk-csi2-imx219.dtso), the
> > > existing DTS files have it wrong, too. The bindings still appear to
> > > document it correctly.
> >
> > Why do you say it is wrong?
> > I don't recall why this got called reset-gpio - the signal on the
> > sensor is XCLR, and that is documented in the binding.
> > The datasheet says low is standby and high is active, which matches
> > what the driver does.
>
> Documentation/driver-api/gpio/consumer.rst:
>
> As a consumer should not have to care about the physical line
> level, all of the gpiod_set_value_xxx() or
> gpiod_set_array_value_xxx() functions operate with the *logical*
> value. With this they take the active low property into account.
> This means that they check whether the GPIO is configured to be
> active low, and if so, they manipulate the passed value before the
> physical line level is driven.
>
> In other words, if the driver calls gpiod_set_value_cansleep(, 0), it
> enables reset signal on the device, bringing the device to hard reset. This
> is the opposite of what the driver does in its resume callback.
>
> So in order to function, the DT must have wrong polarity as well.
> Similarly, if someone writes a correct DT for a board connecting the reset
> GPIO, the imx219 driver simply won't work.
>
> Looking at the arch/arm64/boot/dts/ti/k3-am62x-sk-csi2-imx219.dtso overlay,
> it is for the RPi IMX219 module that does not connect the reset GPIO at
> all. In other words, there doesn't seem to be a upstream user of the reset
> GPIO in IMX219.
>
> > > Laurent confirmed xcrl isn't controllable in the RPi imx219 camera module.
> > >
> > > How about fixing this? Currently correctly written DTBs including imx219
> > > won't work.
> >
> > Seeing as the DTB is ABI, the only improvement I can see is to rename
> > "imx219->reset_gpio" to "imx219->xclr_gpio".
> > What else would you be proposing?
>
> If there's a concern this could break existing DTs, the driver could try
> inverting the polarity of the GPIO if the sensor isn't immediately
> accessible.
I don't think that's a good idea.
> I wonder what others think.
>
> Cc Laurent.
The real question is whether or not this could cause regressions.
Technically speaking, you're not changing the bindings, just the
driver. One could argue that it's not a DT ABI issue, but a driver
regression issue. The kernel still doesn't allow regressions. If you
can't be reasonably sure that nobody will be affected by a driver
change, you'll need to change the bindings to document the incorrect
polarity.
One could also argue that drivers for other operating systems implement
the correct behaviour, as documented in the bindings. This would mean
that the bindings can't be changed. It sounds more of a theoritical
situation though.
All things considered, this ship has probably sailed, and we'll likely
have to live with the incorrect polarity.
> > > I noticed this while fixing the power sequences in this and a few other
> > > drivers.
> > >
> > > > + regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies);
> > > > + clk_disable_unprepare(imx219->xclk);
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > ...
> > >
> > > > +static int imx219_probe(struct i2c_client *client,
> > > > + const struct i2c_device_id *id)
> > > > +{
> > > > + struct device *dev = &client->dev;
> > > > + struct fwnode_handle *endpoint;
> > > > + struct imx219 *imx219;
> > > > + int ret;
> > > > +
> > > > + imx219 = devm_kzalloc(&client->dev, sizeof(*imx219), GFP_KERNEL);
> > > > + if (!imx219)
> > > > + return -ENOMEM;
> > > > +
> > > > + imx219->dev = dev;
> > > > +
> > > > + v4l2_i2c_subdev_init(&imx219->sd, client, &imx219_subdev_ops);
> > > > +
> > > > + /* Get CSI2 bus config */
> > > > + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
> > > > + NULL);
> > > > + if (!endpoint) {
> > > > + dev_err(dev, "endpoint node not found\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + ret = v4l2_fwnode_endpoint_parse(endpoint, &imx219->ep);
> > > > + fwnode_handle_put(endpoint);
> > > > + if (ret) {
> > > > + dev_err(dev, "could not parse endpoint\n");
> > > > + return ret;
> > > > + }
> > > > +
> > > > + /* Check the number of MIPI CSI2 data lanes */
> > > > + if (imx219->ep.bus_type != V4L2_MBUS_CSI2_DPHY ||
> > > > + imx219->ep.bus.mipi_csi2.num_data_lanes != 2) {
> > > > + dev_err(dev, "only 2 data lanes are currently supported\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + /* Get system clock (xclk) */
> > > > + imx219->xclk = devm_clk_get(dev, NULL);
> > > > + if (IS_ERR(imx219->xclk)) {
> > > > + dev_err(dev, "failed to get xclk\n");
> > > > + return PTR_ERR(imx219->xclk);
> > > > + }
> > > > +
> > > > + imx219->xclk_freq = clk_get_rate(imx219->xclk);
> > > > + if (imx219->xclk_freq != IMX219_XCLK_FREQ) {
> > > > + dev_err(dev, "xclk frequency not supported: %d Hz\n",
> > > > + imx219->xclk_freq);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + ret = imx219_get_regulators(imx219);
> > > > + if (ret) {
> > > > + dev_err(dev, "failed to get regulators\n");
> > > > + return ret;
> > > > + }
> > > > +
> > > > + /* Request optional enable pin */
> > > > + imx219->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > > > + GPIOD_OUT_HIGH);
> > > > +
> > > > + /*
> > > > + * The sensor must be powered for imx219_identify_module()
> > > > + * to be able to read the CHIP_ID register
> > > > + */
> > > > + ret = imx219_power_on(dev);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + ret = imx219_identify_module(imx219);
> > > > + if (ret)
> > > > + goto error_power_off;
> > > > +
> > > > + /* Set default mode to max resolution */
> > > > + imx219->mode = &supported_modes[0];
> > > > +
> > > > + ret = imx219_init_controls(imx219);
> > > > + if (ret)
> > > > + goto error_power_off;
> > > > +
> > > > + /* Initialize subdev */
> > > > + imx219->sd.internal_ops = &imx219_internal_ops;
> > > > + imx219->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > > + imx219->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > > +
> > > > + /* Initialize source pad */
> > > > + imx219->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > > +
> > > > + ret = media_entity_pads_init(&imx219->sd.entity, 1, &imx219->pad);
> > > > + if (ret) {
> > > > + dev_err(dev, "failed to init entity pads: %d\n", ret);
> > > > + goto error_handler_free;
> > > > + }
> > > > +
> > > > + ret = v4l2_async_register_subdev_sensor_common(&imx219->sd);
> > > > + if (ret < 0) {
> > > > + dev_err(dev, "failed to register sensor sub-device: %d\n", ret);
> > > > + goto error_media_entity;
> > > > + }
> > > > +
> > > > + /* Enable runtime PM and turn off the device */
> > > > + pm_runtime_set_active(dev);
> > > > + pm_runtime_enable(dev);
> > > > + pm_runtime_idle(dev);
> > > > +
> > > > + return 0;
> > > > +
> > > > +error_media_entity:
> > > > + media_entity_cleanup(&imx219->sd.entity);
> > > > +
> > > > +error_handler_free:
> > > > + imx219_free_controls(imx219);
> > > > +
> > > > +error_power_off:
> > > > + imx219_power_off(dev);
> > > > +
> > > > + return ret;
> > > > +}
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-01-20 13:59 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-10 20:09 [PATCH v3 0/2] Add IMX219 CMOS image sensor support Andrey Konovalov
2020-01-10 20:09 ` [PATCH v3 1/2] dt-bindings: media: i2c: Add IMX219 CMOS sensor binding Andrey Konovalov
2020-01-13 23:04 ` Rob Herring
2020-01-10 20:09 ` [PATCH v3 2/2] media: i2c: Add driver for Sony IMX219 sensor Andrey Konovalov
2025-01-20 10:59 ` Sakari Ailus
2025-01-20 12:28 ` Dave Stevenson
2025-01-20 13:35 ` Sakari Ailus
2025-01-20 13:59 ` Laurent Pinchart
2020-01-11 4:45 ` [PATCH v3 0/2] Add IMX219 CMOS image sensor support Ezequiel Garcia
2020-01-11 17:46 ` Andrey Konovalov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).