* [PATCH v4 1/5] media: Add MIPI CCI register access helper functions
2023-06-27 12:51 [PATCH v4 0/5] media: Add MIPI CCI register access helper functions Hans de Goede
@ 2023-06-27 12:51 ` Hans de Goede
2023-06-27 12:51 ` [PATCH v4 2/5] media: ov5693: Convert to new CCI register access helpers Hans de Goede
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2023-06-27 12:51 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart, Rui Miguel Silva, Daniel Scally
Cc: Hans de Goede, Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media, Andy Shevchenko
The CSI2 specification specifies a standard method to access camera sensor
registers called "Camera Control Interface (CCI)".
This uses either 8 or 16 bit (big-endian wire order) register addresses
and supports 8, 16, 24 or 32 bit (big-endian wire order) register widths.
Currently a lot of Linux camera sensor drivers all have their own custom
helpers for this, often copy and pasted from other drivers.
Add a set of generic helpers for this so that all sensor drivers can
switch to a single common implementation.
These helpers take an extra optional "int *err" function parameter,
this can be used to chain a bunch of register accesses together with
only a single error check at the end, rather than needing to error
check each individual register access. The first failing call will
set the contents of err to a non 0 value and all other calls will
then become no-ops.
Link: https://lore.kernel.org/linux-media/59aefa7f-7bf9-6736-6040-39551329cd0a@redhat.com/
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
Reviewed-by: Tommaso Merciai <tomm.merciai@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v4:
- Add V4L2_CCI_I2C Kconfig and wrap devm_cci_regmap_init_i2c()
with if IS_ENABLED(CONFIG_V4L2_CCI_I2C)
Changes in v3:
- Use unsigned int instead of int in a couple of places
- Run + fix checkpatch.pl --strict --max-line-length=80
- Rename cci_regmap_init_i2c() to devm_cci_regmap_init_i2c()
- Some small kerneldoc text tweaks
Changes in v2:
- Drop cci_reg_type enum
- Make having an encoded reg-width mandatory rather then using 0 to encode
8 bit width making reg-addresses without an encoded width default to
a width of 8
- Add support for 64 bit wide registers
- Introduce a new cci_reg_sequence struct with 64 bit reg values for 64 bit
support and without the delay_us field
- Various kerneldoc updates
- Stop supporting delays in cci_multi_reg_write()
- Some includes cleanups
- Disable regmap locking
---
Documentation/driver-api/media/v4l2-cci.rst | 5 +
Documentation/driver-api/media/v4l2-core.rst | 1 +
drivers/media/v4l2-core/Kconfig | 9 +
drivers/media/v4l2-core/Makefile | 1 +
drivers/media/v4l2-core/v4l2-cci.c | 166 +++++++++++++++++++
include/media/v4l2-cci.h | 125 ++++++++++++++
6 files changed, 307 insertions(+)
create mode 100644 Documentation/driver-api/media/v4l2-cci.rst
create mode 100644 drivers/media/v4l2-core/v4l2-cci.c
create mode 100644 include/media/v4l2-cci.h
diff --git a/Documentation/driver-api/media/v4l2-cci.rst b/Documentation/driver-api/media/v4l2-cci.rst
new file mode 100644
index 000000000000..dd297a40ed20
--- /dev/null
+++ b/Documentation/driver-api/media/v4l2-cci.rst
@@ -0,0 +1,5 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+V4L2 CCI kAPI
+^^^^^^^^^^^^^
+.. kernel-doc:: include/media/v4l2-cci.h
diff --git a/Documentation/driver-api/media/v4l2-core.rst b/Documentation/driver-api/media/v4l2-core.rst
index 1a8c4a5f256b..239045ecc8f4 100644
--- a/Documentation/driver-api/media/v4l2-core.rst
+++ b/Documentation/driver-api/media/v4l2-core.rst
@@ -22,6 +22,7 @@ Video4Linux devices
v4l2-mem2mem
v4l2-async
v4l2-fwnode
+ v4l2-cci
v4l2-rect
v4l2-tuner
v4l2-common
diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index 348559bc2468..f77ebd688cde 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -74,6 +74,15 @@ config V4L2_FWNODE
config V4L2_ASYNC
tristate
+config V4L2_CCI
+ tristate
+
+config V4L2_CCI_I2C
+ tristate
+ depends on I2C
+ select REGMAP_I2C
+ select V4L2_CCI
+
# Used by drivers that need Videobuf modules
config VIDEOBUF_GEN
tristate
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index 41d91bd10cf2..be2551705755 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -25,6 +25,7 @@ videodev-$(CONFIG_VIDEO_V4L2_I2C) += v4l2-i2c.o
# (e. g. LC_ALL=C sort Makefile)
obj-$(CONFIG_V4L2_ASYNC) += v4l2-async.o
+obj-$(CONFIG_V4L2_CCI) += v4l2-cci.o
obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash-led-class.o
obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o
obj-$(CONFIG_V4L2_H264) += v4l2-h264.o
diff --git a/drivers/media/v4l2-core/v4l2-cci.c b/drivers/media/v4l2-core/v4l2-cci.c
new file mode 100644
index 000000000000..bc2dbec019b0
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-cci.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MIPI Camera Control Interface (CCI) register access helpers.
+ *
+ * Copyright (C) 2023 Hans de Goede <hansg@kernel.org>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#include <asm/unaligned.h>
+
+#include <media/v4l2-cci.h>
+
+int cci_read(struct regmap *map, u32 reg, u64 *val, int *err)
+{
+ unsigned int len;
+ u8 buf[8];
+ int ret;
+
+ if (err && *err)
+ return *err;
+
+ len = FIELD_GET(CCI_REG_WIDTH_MASK, reg);
+ reg = FIELD_GET(CCI_REG_ADDR_MASK, reg);
+
+ ret = regmap_bulk_read(map, reg, buf, len);
+ if (ret) {
+ dev_err(regmap_get_device(map), "Error reading reg 0x%4x: %d\n",
+ reg, ret);
+ goto out;
+ }
+
+ switch (len) {
+ case 1:
+ *val = buf[0];
+ break;
+ case 2:
+ *val = get_unaligned_be16(buf);
+ break;
+ case 3:
+ *val = get_unaligned_be24(buf);
+ break;
+ case 4:
+ *val = get_unaligned_be32(buf);
+ break;
+ case 8:
+ *val = get_unaligned_be64(buf);
+ break;
+ default:
+ dev_err(regmap_get_device(map), "Error invalid reg-width %u for reg 0x%04x\n",
+ len, reg);
+ ret = -EINVAL;
+ break;
+ }
+
+out:
+ if (ret && err)
+ *err = ret;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(cci_read);
+
+int cci_write(struct regmap *map, u32 reg, u64 val, int *err)
+{
+ unsigned int len;
+ u8 buf[8];
+ int ret;
+
+ if (err && *err)
+ return *err;
+
+ len = FIELD_GET(CCI_REG_WIDTH_MASK, reg);
+ reg = FIELD_GET(CCI_REG_ADDR_MASK, reg);
+
+ switch (len) {
+ case 1:
+ buf[0] = val;
+ break;
+ case 2:
+ put_unaligned_be16(val, buf);
+ break;
+ case 3:
+ put_unaligned_be24(val, buf);
+ break;
+ case 4:
+ put_unaligned_be32(val, buf);
+ break;
+ case 8:
+ put_unaligned_be64(val, buf);
+ break;
+ default:
+ dev_err(regmap_get_device(map), "Error invalid reg-width %u for reg 0x%04x\n",
+ len, reg);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = regmap_bulk_write(map, reg, buf, len);
+ if (ret)
+ dev_err(regmap_get_device(map), "Error writing reg 0x%4x: %d\n",
+ reg, ret);
+
+out:
+ if (ret && err)
+ *err = ret;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(cci_write);
+
+int cci_update_bits(struct regmap *map, u32 reg, u64 mask, u64 val, int *err)
+{
+ u64 readval;
+ int ret;
+
+ ret = cci_read(map, reg, &readval, err);
+ if (ret)
+ return ret;
+
+ val = (readval & ~mask) | (val & mask);
+
+ return cci_write(map, reg, val, err);
+}
+EXPORT_SYMBOL_GPL(cci_update_bits);
+
+int cci_multi_reg_write(struct regmap *map, const struct cci_reg_sequence *regs,
+ unsigned int num_regs, int *err)
+{
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < num_regs; i++) {
+ ret = cci_write(map, regs[i].reg, regs[i].val, err);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cci_multi_reg_write);
+
+#if IS_ENABLED(CONFIG_V4L2_CCI_I2C)
+struct regmap *devm_cci_regmap_init_i2c(struct i2c_client *client,
+ int reg_addr_bits)
+{
+ struct regmap_config config = {
+ .reg_bits = reg_addr_bits,
+ .val_bits = 8,
+ .reg_format_endian = REGMAP_ENDIAN_BIG,
+ .disable_locking = true,
+ };
+
+ return devm_regmap_init_i2c(client, &config);
+}
+EXPORT_SYMBOL_GPL(devm_cci_regmap_init_i2c);
+#endif
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Hans de Goede <hansg@kernel.org>");
+MODULE_DESCRIPTION("MIPI Camera Control Interface (CCI) support");
diff --git a/include/media/v4l2-cci.h b/include/media/v4l2-cci.h
new file mode 100644
index 000000000000..0f6803e4b17e
--- /dev/null
+++ b/include/media/v4l2-cci.h
@@ -0,0 +1,125 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * MIPI Camera Control Interface (CCI) register access helpers.
+ *
+ * Copyright (C) 2023 Hans de Goede <hansg@kernel.org>
+ */
+#ifndef _V4L2_CCI_H
+#define _V4L2_CCI_H
+
+#include <linux/types.h>
+
+struct i2c_client;
+struct regmap;
+
+/**
+ * struct cci_reg_sequence - An individual write from a sequence of CCI writes
+ *
+ * @reg: Register address, use CCI_REG#() macros to encode reg width
+ * @val: Register value
+ *
+ * Register/value pairs for sequences of writes.
+ */
+struct cci_reg_sequence {
+ u32 reg;
+ u64 val;
+};
+
+/*
+ * Macros to define register address with the register width encoded
+ * into the higher bits.
+ */
+#define CCI_REG_ADDR_MASK GENMASK(15, 0)
+#define CCI_REG_WIDTH_SHIFT 16
+#define CCI_REG_WIDTH_MASK GENMASK(19, 16)
+
+#define CCI_REG8(x) ((1 << CCI_REG_WIDTH_SHIFT) | (x))
+#define CCI_REG16(x) ((2 << CCI_REG_WIDTH_SHIFT) | (x))
+#define CCI_REG24(x) ((3 << CCI_REG_WIDTH_SHIFT) | (x))
+#define CCI_REG32(x) ((4 << CCI_REG_WIDTH_SHIFT) | (x))
+#define CCI_REG64(x) ((8 << CCI_REG_WIDTH_SHIFT) | (x))
+
+/**
+ * cci_read() - Read a value from a single CCI register
+ *
+ * @map: Register map to read from
+ * @reg: Register address to read, use CCI_REG#() macros to encode reg width
+ * @val: Pointer to store read value
+ * @err: Optional pointer to store errors, if a previous error is set
+ * then the read will be skipped
+ *
+ * Return: %0 on success or a negative error code on failure.
+ */
+int cci_read(struct regmap *map, u32 reg, u64 *val, int *err);
+
+/**
+ * cci_write() - Write a value to a single CCI register
+ *
+ * @map: Register map to write to
+ * @reg: Register address to write, use CCI_REG#() macros to encode reg width
+ * @val: Value to be written
+ * @err: Optional pointer to store errors, if a previous error is set
+ * then the write will be skipped
+ *
+ * Return: %0 on success or a negative error code on failure.
+ */
+int cci_write(struct regmap *map, u32 reg, u64 val, int *err);
+
+/**
+ * cci_update_bits() - Perform a read/modify/write cycle on
+ * a single CCI register
+ *
+ * @map: Register map to update
+ * @reg: Register address to update, use CCI_REG#() macros to encode reg width
+ * @mask: Bitmask to change
+ * @val: New value for bitmask
+ * @err: Optional pointer to store errors, if a previous error is set
+ * then the update will be skipped
+ *
+ * Note this uses read-modify-write to update the bits, atomicity with regards
+ * to other cci_*() register access functions is NOT guaranteed.
+ *
+ * Return: %0 on success or a negative error code on failure.
+ */
+int cci_update_bits(struct regmap *map, u32 reg, u64 mask, u64 val, int *err);
+
+/**
+ * cci_multi_reg_write() - Write multiple registers to the device
+ *
+ * @map: Register map to write to
+ * @regs: Array of structures containing register-address, -value pairs to be
+ * written, register-addresses use CCI_REG#() macros to encode reg width
+ * @num_regs: Number of registers to write
+ * @err: Optional pointer to store errors, if a previous error is set
+ * then the write will be skipped
+ *
+ * Write multiple registers to the device where the set of register, value
+ * pairs are supplied in any order, possibly not all in a single range.
+ *
+ * Use of the CCI_REG#() macros to encode reg width is mandatory.
+ *
+ * For raw lists of register-address, -value pairs with only 8 bit
+ * wide writes regmap_multi_reg_write() can be used instead.
+ *
+ * Return: %0 on success or a negative error code on failure.
+ */
+int cci_multi_reg_write(struct regmap *map, const struct cci_reg_sequence *regs,
+ unsigned int num_regs, int *err);
+
+#if IS_ENABLED(CONFIG_V4L2_CCI_I2C)
+/**
+ * devm_cci_regmap_init_i2c() - Create regmap to use with cci_*() register
+ * access functions
+ *
+ * @client: i2c_client to create the regmap for
+ * @reg_addr_bits: register address width to use (8 or 16)
+ *
+ * Note the memory for the created regmap is devm() managed, tied to the client.
+ *
+ * Return: %0 on success or a negative error code on failure.
+ */
+struct regmap *devm_cci_regmap_init_i2c(struct i2c_client *client,
+ int reg_addr_bits);
+#endif
+
+#endif
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 2/5] media: ov5693: Convert to new CCI register access helpers
2023-06-27 12:51 [PATCH v4 0/5] media: Add MIPI CCI register access helper functions Hans de Goede
2023-06-27 12:51 ` [PATCH v4 1/5] " Hans de Goede
@ 2023-06-27 12:51 ` Hans de Goede
2023-06-27 12:51 ` [PATCH v4 3/5] media: imx290: " Hans de Goede
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2023-06-27 12:51 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart, Rui Miguel Silva, Daniel Scally
Cc: Hans de Goede, Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
Use the new comon CCI register access helpers to replace the private
register access helpers in the ov5693 driver.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note for reviewers all the OV5693_REG_?BIT defines in both the register
address defines as well as in ov5693_global_regs[] were automatically
changed using search replace.
---
Changes in v3:
- Dropped no longer necesary i2c_client member from struct ov5693_device
- Drop unnecessary ret = assignment for cci_update_bits() calls using
the err pointer argument
- Fixed a couple of lines > 80 chars
Changes in v2:
- New patch in v2 of this series
---
drivers/media/i2c/Kconfig | 1 +
drivers/media/i2c/ov5693.c | 585 +++++++++++++++----------------------
2 files changed, 229 insertions(+), 357 deletions(-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9b0bb1d36019..8467f4ce92eb 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -576,6 +576,7 @@ config VIDEO_OV5693
tristate "OmniVision OV5693 sensor support"
depends on I2C && VIDEO_DEV
select V4L2_FWNODE
+ select V4L2_CCI_I2C
help
This is a Video4Linux2 sensor driver for the OmniVision
OV5693 camera.
diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index 7f9212cce239..488ee6d9d301 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -12,7 +12,6 @@
* Jake Day
*/
-#include <asm/unaligned.h>
#include <linux/acpi.h>
#include <linux/clk.h>
#include <linux/delay.h>
@@ -23,36 +22,32 @@
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/types.h>
+
+#include <media/v4l2-cci.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-fwnode.h>
-#define OV5693_REG_8BIT(n) ((1 << 16) | (n))
-#define OV5693_REG_16BIT(n) ((2 << 16) | (n))
-#define OV5693_REG_24BIT(n) ((3 << 16) | (n))
-#define OV5693_REG_SIZE_SHIFT 16
-#define OV5693_REG_ADDR_MASK 0xffff
-
/* System Control */
-#define OV5693_SW_RESET_REG OV5693_REG_8BIT(0x0103)
-#define OV5693_SW_STREAM_REG OV5693_REG_8BIT(0x0100)
+#define OV5693_SW_RESET_REG CCI_REG8(0x0103)
+#define OV5693_SW_STREAM_REG CCI_REG8(0x0100)
#define OV5693_START_STREAMING 0x01
#define OV5693_STOP_STREAMING 0x00
#define OV5693_SW_RESET 0x01
-#define OV5693_REG_CHIP_ID OV5693_REG_16BIT(0x300a)
+#define OV5693_REG_CHIP_ID CCI_REG16(0x300a)
/* Yes, this is right. The datasheet for the OV5693 gives its ID as 0x5690 */
#define OV5693_CHIP_ID 0x5690
/* Exposure */
-#define OV5693_EXPOSURE_CTRL_REG OV5693_REG_24BIT(0x3500)
+#define OV5693_EXPOSURE_CTRL_REG CCI_REG24(0x3500)
#define OV5693_EXPOSURE_CTRL_MASK GENMASK(19, 4)
#define OV5693_INTEGRATION_TIME_MARGIN 8
#define OV5693_EXPOSURE_MIN 1
#define OV5693_EXPOSURE_STEP 1
/* Analogue Gain */
-#define OV5693_GAIN_CTRL_REG OV5693_REG_16BIT(0x350a)
+#define OV5693_GAIN_CTRL_REG CCI_REG16(0x350a)
#define OV5693_GAIN_CTRL_MASK GENMASK(10, 4)
#define OV5693_GAIN_MIN 1
#define OV5693_GAIN_MAX 127
@@ -60,9 +55,9 @@
#define OV5693_GAIN_STEP 1
/* Digital Gain */
-#define OV5693_MWB_RED_GAIN_REG OV5693_REG_16BIT(0x3400)
-#define OV5693_MWB_GREEN_GAIN_REG OV5693_REG_16BIT(0x3402)
-#define OV5693_MWB_BLUE_GAIN_REG OV5693_REG_16BIT(0x3404)
+#define OV5693_MWB_RED_GAIN_REG CCI_REG16(0x3400)
+#define OV5693_MWB_GREEN_GAIN_REG CCI_REG16(0x3402)
+#define OV5693_MWB_BLUE_GAIN_REG CCI_REG16(0x3404)
#define OV5693_MWB_GAIN_MASK GENMASK(11, 0)
#define OV5693_MWB_GAIN_MAX 0x0fff
#define OV5693_DIGITAL_GAIN_MIN 1
@@ -71,36 +66,36 @@
#define OV5693_DIGITAL_GAIN_STEP 1
/* Timing and Format */
-#define OV5693_CROP_START_X_REG OV5693_REG_16BIT(0x3800)
-#define OV5693_CROP_START_Y_REG OV5693_REG_16BIT(0x3802)
-#define OV5693_CROP_END_X_REG OV5693_REG_16BIT(0x3804)
-#define OV5693_CROP_END_Y_REG OV5693_REG_16BIT(0x3806)
-#define OV5693_OUTPUT_SIZE_X_REG OV5693_REG_16BIT(0x3808)
-#define OV5693_OUTPUT_SIZE_Y_REG OV5693_REG_16BIT(0x380a)
+#define OV5693_CROP_START_X_REG CCI_REG16(0x3800)
+#define OV5693_CROP_START_Y_REG CCI_REG16(0x3802)
+#define OV5693_CROP_END_X_REG CCI_REG16(0x3804)
+#define OV5693_CROP_END_Y_REG CCI_REG16(0x3806)
+#define OV5693_OUTPUT_SIZE_X_REG CCI_REG16(0x3808)
+#define OV5693_OUTPUT_SIZE_Y_REG CCI_REG16(0x380a)
-#define OV5693_TIMING_HTS_REG OV5693_REG_16BIT(0x380c)
+#define OV5693_TIMING_HTS_REG CCI_REG16(0x380c)
#define OV5693_FIXED_PPL 2688U
-#define OV5693_TIMING_VTS_REG OV5693_REG_16BIT(0x380e)
+#define OV5693_TIMING_VTS_REG CCI_REG16(0x380e)
#define OV5693_TIMING_MAX_VTS 0xffff
#define OV5693_TIMING_MIN_VTS 0x04
-#define OV5693_OFFSET_START_X_REG OV5693_REG_16BIT(0x3810)
-#define OV5693_OFFSET_START_Y_REG OV5693_REG_16BIT(0x3812)
+#define OV5693_OFFSET_START_X_REG CCI_REG16(0x3810)
+#define OV5693_OFFSET_START_Y_REG CCI_REG16(0x3812)
-#define OV5693_SUB_INC_X_REG OV5693_REG_8BIT(0x3814)
-#define OV5693_SUB_INC_Y_REG OV5693_REG_8BIT(0x3815)
+#define OV5693_SUB_INC_X_REG CCI_REG8(0x3814)
+#define OV5693_SUB_INC_Y_REG CCI_REG8(0x3815)
-#define OV5693_FORMAT1_REG OV5693_REG_8BIT(0x3820)
+#define OV5693_FORMAT1_REG CCI_REG8(0x3820)
#define OV5693_FORMAT1_FLIP_VERT_ISP_EN BIT(6)
#define OV5693_FORMAT1_FLIP_VERT_SENSOR_EN BIT(1)
#define OV5693_FORMAT1_VBIN_EN BIT(0)
-#define OV5693_FORMAT2_REG OV5693_REG_8BIT(0x3821)
+#define OV5693_FORMAT2_REG CCI_REG8(0x3821)
#define OV5693_FORMAT2_HDR_EN BIT(7)
#define OV5693_FORMAT2_FLIP_HORZ_ISP_EN BIT(2)
#define OV5693_FORMAT2_FLIP_HORZ_SENSOR_EN BIT(1)
#define OV5693_FORMAT2_HBIN_EN BIT(0)
-#define OV5693_ISP_CTRL2_REG OV5693_REG_8BIT(0x5002)
+#define OV5693_ISP_CTRL2_REG CCI_REG8(0x5002)
#define OV5693_ISP_SCALE_ENABLE BIT(7)
/* Pixel Array */
@@ -116,7 +111,7 @@
#define OV5693_MIN_CROP_HEIGHT 2
/* Test Pattern */
-#define OV5693_TEST_PATTERN_REG OV5693_REG_8BIT(0x5e00)
+#define OV5693_TEST_PATTERN_REG CCI_REG8(0x5e00)
#define OV5693_TEST_PATTERN_ENABLE BIT(7)
#define OV5693_TEST_PATTERN_ROLLING BIT(6)
#define OV5693_TEST_PATTERN_RANDOM 0x01
@@ -137,19 +132,9 @@ static const char * const ov5693_supply_names[] = {
#define OV5693_NUM_SUPPLIES ARRAY_SIZE(ov5693_supply_names)
-struct ov5693_reg {
- u32 reg;
- u8 val;
-};
-
-struct ov5693_reg_list {
- u32 num_regs;
- const struct ov5693_reg *regs;
-};
-
struct ov5693_device {
- struct i2c_client *client;
struct device *dev;
+ struct regmap *regmap;
/* Protect against concurrent changes to controls */
struct mutex lock;
@@ -189,156 +174,151 @@ struct ov5693_device {
} ctrls;
};
-static const struct ov5693_reg ov5693_global_regs[] = {
- {OV5693_REG_8BIT(0x3016), 0xf0},
- {OV5693_REG_8BIT(0x3017), 0xf0},
- {OV5693_REG_8BIT(0x3018), 0xf0},
- {OV5693_REG_8BIT(0x3022), 0x01},
- {OV5693_REG_8BIT(0x3028), 0x44},
- {OV5693_REG_8BIT(0x3098), 0x02},
- {OV5693_REG_8BIT(0x3099), 0x19},
- {OV5693_REG_8BIT(0x309a), 0x02},
- {OV5693_REG_8BIT(0x309b), 0x01},
- {OV5693_REG_8BIT(0x309c), 0x00},
- {OV5693_REG_8BIT(0x30a0), 0xd2},
- {OV5693_REG_8BIT(0x30a2), 0x01},
- {OV5693_REG_8BIT(0x30b2), 0x00},
- {OV5693_REG_8BIT(0x30b3), 0x83},
- {OV5693_REG_8BIT(0x30b4), 0x03},
- {OV5693_REG_8BIT(0x30b5), 0x04},
- {OV5693_REG_8BIT(0x30b6), 0x01},
- {OV5693_REG_8BIT(0x3080), 0x01},
- {OV5693_REG_8BIT(0x3104), 0x21},
- {OV5693_REG_8BIT(0x3106), 0x00},
- {OV5693_REG_8BIT(0x3406), 0x01},
- {OV5693_REG_8BIT(0x3503), 0x07},
- {OV5693_REG_8BIT(0x350b), 0x40},
- {OV5693_REG_8BIT(0x3601), 0x0a},
- {OV5693_REG_8BIT(0x3602), 0x38},
- {OV5693_REG_8BIT(0x3612), 0x80},
- {OV5693_REG_8BIT(0x3620), 0x54},
- {OV5693_REG_8BIT(0x3621), 0xc7},
- {OV5693_REG_8BIT(0x3622), 0x0f},
- {OV5693_REG_8BIT(0x3625), 0x10},
- {OV5693_REG_8BIT(0x3630), 0x55},
- {OV5693_REG_8BIT(0x3631), 0xf4},
- {OV5693_REG_8BIT(0x3632), 0x00},
- {OV5693_REG_8BIT(0x3633), 0x34},
- {OV5693_REG_8BIT(0x3634), 0x02},
- {OV5693_REG_8BIT(0x364d), 0x0d},
- {OV5693_REG_8BIT(0x364f), 0xdd},
- {OV5693_REG_8BIT(0x3660), 0x04},
- {OV5693_REG_8BIT(0x3662), 0x10},
- {OV5693_REG_8BIT(0x3663), 0xf1},
- {OV5693_REG_8BIT(0x3665), 0x00},
- {OV5693_REG_8BIT(0x3666), 0x20},
- {OV5693_REG_8BIT(0x3667), 0x00},
- {OV5693_REG_8BIT(0x366a), 0x80},
- {OV5693_REG_8BIT(0x3680), 0xe0},
- {OV5693_REG_8BIT(0x3681), 0x00},
- {OV5693_REG_8BIT(0x3700), 0x42},
- {OV5693_REG_8BIT(0x3701), 0x14},
- {OV5693_REG_8BIT(0x3702), 0xa0},
- {OV5693_REG_8BIT(0x3703), 0xd8},
- {OV5693_REG_8BIT(0x3704), 0x78},
- {OV5693_REG_8BIT(0x3705), 0x02},
- {OV5693_REG_8BIT(0x370a), 0x00},
- {OV5693_REG_8BIT(0x370b), 0x20},
- {OV5693_REG_8BIT(0x370c), 0x0c},
- {OV5693_REG_8BIT(0x370d), 0x11},
- {OV5693_REG_8BIT(0x370e), 0x00},
- {OV5693_REG_8BIT(0x370f), 0x40},
- {OV5693_REG_8BIT(0x3710), 0x00},
- {OV5693_REG_8BIT(0x371a), 0x1c},
- {OV5693_REG_8BIT(0x371b), 0x05},
- {OV5693_REG_8BIT(0x371c), 0x01},
- {OV5693_REG_8BIT(0x371e), 0xa1},
- {OV5693_REG_8BIT(0x371f), 0x0c},
- {OV5693_REG_8BIT(0x3721), 0x00},
- {OV5693_REG_8BIT(0x3724), 0x10},
- {OV5693_REG_8BIT(0x3726), 0x00},
- {OV5693_REG_8BIT(0x372a), 0x01},
- {OV5693_REG_8BIT(0x3730), 0x10},
- {OV5693_REG_8BIT(0x3738), 0x22},
- {OV5693_REG_8BIT(0x3739), 0xe5},
- {OV5693_REG_8BIT(0x373a), 0x50},
- {OV5693_REG_8BIT(0x373b), 0x02},
- {OV5693_REG_8BIT(0x373c), 0x41},
- {OV5693_REG_8BIT(0x373f), 0x02},
- {OV5693_REG_8BIT(0x3740), 0x42},
- {OV5693_REG_8BIT(0x3741), 0x02},
- {OV5693_REG_8BIT(0x3742), 0x18},
- {OV5693_REG_8BIT(0x3743), 0x01},
- {OV5693_REG_8BIT(0x3744), 0x02},
- {OV5693_REG_8BIT(0x3747), 0x10},
- {OV5693_REG_8BIT(0x374c), 0x04},
- {OV5693_REG_8BIT(0x3751), 0xf0},
- {OV5693_REG_8BIT(0x3752), 0x00},
- {OV5693_REG_8BIT(0x3753), 0x00},
- {OV5693_REG_8BIT(0x3754), 0xc0},
- {OV5693_REG_8BIT(0x3755), 0x00},
- {OV5693_REG_8BIT(0x3756), 0x1a},
- {OV5693_REG_8BIT(0x3758), 0x00},
- {OV5693_REG_8BIT(0x3759), 0x0f},
- {OV5693_REG_8BIT(0x376b), 0x44},
- {OV5693_REG_8BIT(0x375c), 0x04},
- {OV5693_REG_8BIT(0x3774), 0x10},
- {OV5693_REG_8BIT(0x3776), 0x00},
- {OV5693_REG_8BIT(0x377f), 0x08},
- {OV5693_REG_8BIT(0x3780), 0x22},
- {OV5693_REG_8BIT(0x3781), 0x0c},
- {OV5693_REG_8BIT(0x3784), 0x2c},
- {OV5693_REG_8BIT(0x3785), 0x1e},
- {OV5693_REG_8BIT(0x378f), 0xf5},
- {OV5693_REG_8BIT(0x3791), 0xb0},
- {OV5693_REG_8BIT(0x3795), 0x00},
- {OV5693_REG_8BIT(0x3796), 0x64},
- {OV5693_REG_8BIT(0x3797), 0x11},
- {OV5693_REG_8BIT(0x3798), 0x30},
- {OV5693_REG_8BIT(0x3799), 0x41},
- {OV5693_REG_8BIT(0x379a), 0x07},
- {OV5693_REG_8BIT(0x379b), 0xb0},
- {OV5693_REG_8BIT(0x379c), 0x0c},
- {OV5693_REG_8BIT(0x3a04), 0x06},
- {OV5693_REG_8BIT(0x3a05), 0x14},
- {OV5693_REG_8BIT(0x3e07), 0x20},
- {OV5693_REG_8BIT(0x4000), 0x08},
- {OV5693_REG_8BIT(0x4001), 0x04},
- {OV5693_REG_8BIT(0x4004), 0x08},
- {OV5693_REG_8BIT(0x4006), 0x20},
- {OV5693_REG_8BIT(0x4008), 0x24},
- {OV5693_REG_8BIT(0x4009), 0x10},
- {OV5693_REG_8BIT(0x4058), 0x00},
- {OV5693_REG_8BIT(0x4101), 0xb2},
- {OV5693_REG_8BIT(0x4307), 0x31},
- {OV5693_REG_8BIT(0x4511), 0x05},
- {OV5693_REG_8BIT(0x4512), 0x01},
- {OV5693_REG_8BIT(0x481f), 0x30},
- {OV5693_REG_8BIT(0x4826), 0x2c},
- {OV5693_REG_8BIT(0x4d02), 0xfd},
- {OV5693_REG_8BIT(0x4d03), 0xf5},
- {OV5693_REG_8BIT(0x4d04), 0x0c},
- {OV5693_REG_8BIT(0x4d05), 0xcc},
- {OV5693_REG_8BIT(0x4837), 0x0a},
- {OV5693_REG_8BIT(0x5003), 0x20},
- {OV5693_REG_8BIT(0x5013), 0x00},
- {OV5693_REG_8BIT(0x5842), 0x01},
- {OV5693_REG_8BIT(0x5843), 0x2b},
- {OV5693_REG_8BIT(0x5844), 0x01},
- {OV5693_REG_8BIT(0x5845), 0x92},
- {OV5693_REG_8BIT(0x5846), 0x01},
- {OV5693_REG_8BIT(0x5847), 0x8f},
- {OV5693_REG_8BIT(0x5848), 0x01},
- {OV5693_REG_8BIT(0x5849), 0x0c},
- {OV5693_REG_8BIT(0x5e10), 0x0c},
- {OV5693_REG_8BIT(0x3820), 0x00},
- {OV5693_REG_8BIT(0x3821), 0x1e},
- {OV5693_REG_8BIT(0x5041), 0x14}
-};
-
-static const struct ov5693_reg_list ov5693_global_setting = {
- .num_regs = ARRAY_SIZE(ov5693_global_regs),
- .regs = ov5693_global_regs,
+static const struct cci_reg_sequence ov5693_global_regs[] = {
+ {CCI_REG8(0x3016), 0xf0},
+ {CCI_REG8(0x3017), 0xf0},
+ {CCI_REG8(0x3018), 0xf0},
+ {CCI_REG8(0x3022), 0x01},
+ {CCI_REG8(0x3028), 0x44},
+ {CCI_REG8(0x3098), 0x02},
+ {CCI_REG8(0x3099), 0x19},
+ {CCI_REG8(0x309a), 0x02},
+ {CCI_REG8(0x309b), 0x01},
+ {CCI_REG8(0x309c), 0x00},
+ {CCI_REG8(0x30a0), 0xd2},
+ {CCI_REG8(0x30a2), 0x01},
+ {CCI_REG8(0x30b2), 0x00},
+ {CCI_REG8(0x30b3), 0x83},
+ {CCI_REG8(0x30b4), 0x03},
+ {CCI_REG8(0x30b5), 0x04},
+ {CCI_REG8(0x30b6), 0x01},
+ {CCI_REG8(0x3080), 0x01},
+ {CCI_REG8(0x3104), 0x21},
+ {CCI_REG8(0x3106), 0x00},
+ {CCI_REG8(0x3406), 0x01},
+ {CCI_REG8(0x3503), 0x07},
+ {CCI_REG8(0x350b), 0x40},
+ {CCI_REG8(0x3601), 0x0a},
+ {CCI_REG8(0x3602), 0x38},
+ {CCI_REG8(0x3612), 0x80},
+ {CCI_REG8(0x3620), 0x54},
+ {CCI_REG8(0x3621), 0xc7},
+ {CCI_REG8(0x3622), 0x0f},
+ {CCI_REG8(0x3625), 0x10},
+ {CCI_REG8(0x3630), 0x55},
+ {CCI_REG8(0x3631), 0xf4},
+ {CCI_REG8(0x3632), 0x00},
+ {CCI_REG8(0x3633), 0x34},
+ {CCI_REG8(0x3634), 0x02},
+ {CCI_REG8(0x364d), 0x0d},
+ {CCI_REG8(0x364f), 0xdd},
+ {CCI_REG8(0x3660), 0x04},
+ {CCI_REG8(0x3662), 0x10},
+ {CCI_REG8(0x3663), 0xf1},
+ {CCI_REG8(0x3665), 0x00},
+ {CCI_REG8(0x3666), 0x20},
+ {CCI_REG8(0x3667), 0x00},
+ {CCI_REG8(0x366a), 0x80},
+ {CCI_REG8(0x3680), 0xe0},
+ {CCI_REG8(0x3681), 0x00},
+ {CCI_REG8(0x3700), 0x42},
+ {CCI_REG8(0x3701), 0x14},
+ {CCI_REG8(0x3702), 0xa0},
+ {CCI_REG8(0x3703), 0xd8},
+ {CCI_REG8(0x3704), 0x78},
+ {CCI_REG8(0x3705), 0x02},
+ {CCI_REG8(0x370a), 0x00},
+ {CCI_REG8(0x370b), 0x20},
+ {CCI_REG8(0x370c), 0x0c},
+ {CCI_REG8(0x370d), 0x11},
+ {CCI_REG8(0x370e), 0x00},
+ {CCI_REG8(0x370f), 0x40},
+ {CCI_REG8(0x3710), 0x00},
+ {CCI_REG8(0x371a), 0x1c},
+ {CCI_REG8(0x371b), 0x05},
+ {CCI_REG8(0x371c), 0x01},
+ {CCI_REG8(0x371e), 0xa1},
+ {CCI_REG8(0x371f), 0x0c},
+ {CCI_REG8(0x3721), 0x00},
+ {CCI_REG8(0x3724), 0x10},
+ {CCI_REG8(0x3726), 0x00},
+ {CCI_REG8(0x372a), 0x01},
+ {CCI_REG8(0x3730), 0x10},
+ {CCI_REG8(0x3738), 0x22},
+ {CCI_REG8(0x3739), 0xe5},
+ {CCI_REG8(0x373a), 0x50},
+ {CCI_REG8(0x373b), 0x02},
+ {CCI_REG8(0x373c), 0x41},
+ {CCI_REG8(0x373f), 0x02},
+ {CCI_REG8(0x3740), 0x42},
+ {CCI_REG8(0x3741), 0x02},
+ {CCI_REG8(0x3742), 0x18},
+ {CCI_REG8(0x3743), 0x01},
+ {CCI_REG8(0x3744), 0x02},
+ {CCI_REG8(0x3747), 0x10},
+ {CCI_REG8(0x374c), 0x04},
+ {CCI_REG8(0x3751), 0xf0},
+ {CCI_REG8(0x3752), 0x00},
+ {CCI_REG8(0x3753), 0x00},
+ {CCI_REG8(0x3754), 0xc0},
+ {CCI_REG8(0x3755), 0x00},
+ {CCI_REG8(0x3756), 0x1a},
+ {CCI_REG8(0x3758), 0x00},
+ {CCI_REG8(0x3759), 0x0f},
+ {CCI_REG8(0x376b), 0x44},
+ {CCI_REG8(0x375c), 0x04},
+ {CCI_REG8(0x3774), 0x10},
+ {CCI_REG8(0x3776), 0x00},
+ {CCI_REG8(0x377f), 0x08},
+ {CCI_REG8(0x3780), 0x22},
+ {CCI_REG8(0x3781), 0x0c},
+ {CCI_REG8(0x3784), 0x2c},
+ {CCI_REG8(0x3785), 0x1e},
+ {CCI_REG8(0x378f), 0xf5},
+ {CCI_REG8(0x3791), 0xb0},
+ {CCI_REG8(0x3795), 0x00},
+ {CCI_REG8(0x3796), 0x64},
+ {CCI_REG8(0x3797), 0x11},
+ {CCI_REG8(0x3798), 0x30},
+ {CCI_REG8(0x3799), 0x41},
+ {CCI_REG8(0x379a), 0x07},
+ {CCI_REG8(0x379b), 0xb0},
+ {CCI_REG8(0x379c), 0x0c},
+ {CCI_REG8(0x3a04), 0x06},
+ {CCI_REG8(0x3a05), 0x14},
+ {CCI_REG8(0x3e07), 0x20},
+ {CCI_REG8(0x4000), 0x08},
+ {CCI_REG8(0x4001), 0x04},
+ {CCI_REG8(0x4004), 0x08},
+ {CCI_REG8(0x4006), 0x20},
+ {CCI_REG8(0x4008), 0x24},
+ {CCI_REG8(0x4009), 0x10},
+ {CCI_REG8(0x4058), 0x00},
+ {CCI_REG8(0x4101), 0xb2},
+ {CCI_REG8(0x4307), 0x31},
+ {CCI_REG8(0x4511), 0x05},
+ {CCI_REG8(0x4512), 0x01},
+ {CCI_REG8(0x481f), 0x30},
+ {CCI_REG8(0x4826), 0x2c},
+ {CCI_REG8(0x4d02), 0xfd},
+ {CCI_REG8(0x4d03), 0xf5},
+ {CCI_REG8(0x4d04), 0x0c},
+ {CCI_REG8(0x4d05), 0xcc},
+ {CCI_REG8(0x4837), 0x0a},
+ {CCI_REG8(0x5003), 0x20},
+ {CCI_REG8(0x5013), 0x00},
+ {CCI_REG8(0x5842), 0x01},
+ {CCI_REG8(0x5843), 0x2b},
+ {CCI_REG8(0x5844), 0x01},
+ {CCI_REG8(0x5845), 0x92},
+ {CCI_REG8(0x5846), 0x01},
+ {CCI_REG8(0x5847), 0x8f},
+ {CCI_REG8(0x5848), 0x01},
+ {CCI_REG8(0x5849), 0x0c},
+ {CCI_REG8(0x5e10), 0x0c},
+ {CCI_REG8(0x3820), 0x00},
+ {CCI_REG8(0x3821), 0x1e},
+ {CCI_REG8(0x5041), 0x14}
};
static const struct v4l2_rect ov5693_default_crop = {
@@ -373,115 +353,6 @@ static const u8 ov5693_test_pattern_bits[] = {
OV5693_TEST_PATTERN_ROLLING,
};
-/* I2C I/O Operations */
-
-static int ov5693_read_reg(struct ov5693_device *ov5693, u32 addr, u32 *value)
-{
- struct i2c_client *client = ov5693->client;
- __be16 reg;
- u8 val[4];
- struct i2c_msg msg[] = {
- {
- .addr = client->addr,
- .flags = 0,
- .len = 2,
- .buf = (u8 *)®,
- },
- {
- .addr = client->addr,
- .flags = I2C_M_RD,
- .buf = (u8 *)&val,
- },
- };
- unsigned int len = ((addr >> OV5693_REG_SIZE_SHIFT) & 3);
- unsigned int i;
- int ret;
-
- reg = cpu_to_be16(addr & OV5693_REG_ADDR_MASK);
-
- msg[1].len = len;
-
- ret = i2c_transfer(client->adapter, msg, 2);
- if (ret < 0)
- return dev_err_probe(&client->dev, ret,
- "Failed to read register 0x%04x\n",
- addr & OV5693_REG_ADDR_MASK);
-
- *value = 0;
- for (i = 0; i < len; ++i) {
- *value <<= 8;
- *value |= val[i];
- }
-
- return 0;
-}
-
-static void ov5693_write_reg(struct ov5693_device *ov5693, u32 addr, u32 value,
- int *error)
-{
- struct i2c_client *client = ov5693->client;
- struct {
- __be16 reg;
- u8 val[4];
- } __packed buf;
- struct i2c_msg msg = {
- .addr = client->addr,
- .buf = (u8 *)&buf,
- };
- unsigned int len = ((addr >> OV5693_REG_SIZE_SHIFT) & 3);
- unsigned int i;
- int ret;
-
- if (*error < 0)
- return;
-
- buf.reg = cpu_to_be16(addr & OV5693_REG_ADDR_MASK);
- for (i = 0; i < len; ++i) {
- buf.val[len - i - 1] = value & 0xff;
- value >>= 8;
- }
-
- msg.len = len + 2;
-
- ret = i2c_transfer(client->adapter, &msg, 1);
- if (ret < 0) {
- dev_err(&client->dev, "Failed to write register 0x%04x: %d\n",
- addr & OV5693_REG_ADDR_MASK, ret);
- *error = ret;
- }
-}
-
-static int ov5693_write_reg_array(struct ov5693_device *ov5693,
- const struct ov5693_reg_list *reglist)
-{
- unsigned int i;
- int ret = 0;
-
- for (i = 0; i < reglist->num_regs; i++)
- ov5693_write_reg(ov5693, reglist->regs[i].reg,
- reglist->regs[i].val, &ret);
-
- return ret;
-}
-
-static int ov5693_update_bits(struct ov5693_device *ov5693, u32 address,
- u32 mask, u32 bits)
-{
- u32 value = 0;
- int ret;
-
- ret = ov5693_read_reg(ov5693, address, &value);
- if (ret)
- return ret;
-
- value &= ~mask;
- value |= bits;
-
- ov5693_write_reg(ov5693, address, value, &ret);
-
- return ret;
-}
-
/* V4L2 Controls Functions */
static int ov5693_flip_vert_configure(struct ov5693_device *ov5693,
@@ -491,8 +362,8 @@ static int ov5693_flip_vert_configure(struct ov5693_device *ov5693,
OV5693_FORMAT1_FLIP_VERT_SENSOR_EN;
int ret;
- ret = ov5693_update_bits(ov5693, OV5693_FORMAT1_REG, bits,
- enable ? bits : 0);
+ ret = cci_update_bits(ov5693->regmap, OV5693_FORMAT1_REG, bits,
+ enable ? bits : 0, NULL);
if (ret)
return ret;
@@ -506,8 +377,8 @@ static int ov5693_flip_horz_configure(struct ov5693_device *ov5693,
OV5693_FORMAT2_FLIP_HORZ_SENSOR_EN;
int ret;
- ret = ov5693_update_bits(ov5693, OV5693_FORMAT2_REG, bits,
- enable ? bits : 0);
+ ret = cci_update_bits(ov5693->regmap, OV5693_FORMAT2_REG, bits,
+ enable ? bits : 0, NULL);
if (ret)
return ret;
@@ -516,10 +387,11 @@ static int ov5693_flip_horz_configure(struct ov5693_device *ov5693,
static int ov5693_get_exposure(struct ov5693_device *ov5693, s32 *value)
{
- u32 exposure;
+ u64 exposure;
int ret;
- ret = ov5693_read_reg(ov5693, OV5693_EXPOSURE_CTRL_REG, &exposure);
+ ret = cci_read(ov5693->regmap, OV5693_EXPOSURE_CTRL_REG, &exposure,
+ NULL);
if (ret)
return ret;
@@ -536,17 +408,17 @@ static int ov5693_exposure_configure(struct ov5693_device *ov5693,
exposure = (exposure << 4) & OV5693_EXPOSURE_CTRL_MASK;
- ov5693_write_reg(ov5693, OV5693_EXPOSURE_CTRL_REG, exposure, &ret);
+ cci_write(ov5693->regmap, OV5693_EXPOSURE_CTRL_REG, exposure, &ret);
return ret;
}
static int ov5693_get_gain(struct ov5693_device *ov5693, u32 *gain)
{
- u32 value;
+ u64 value;
int ret;
- ret = ov5693_read_reg(ov5693, OV5693_GAIN_CTRL_REG, &value);
+ ret = cci_read(ov5693->regmap, OV5693_GAIN_CTRL_REG, &value, NULL);
if (ret)
return ret;
@@ -563,9 +435,9 @@ static int ov5693_digital_gain_configure(struct ov5693_device *ov5693,
gain &= OV5693_MWB_GAIN_MASK;
- ov5693_write_reg(ov5693, OV5693_MWB_RED_GAIN_REG, gain, &ret);
- ov5693_write_reg(ov5693, OV5693_MWB_GREEN_GAIN_REG, gain, &ret);
- ov5693_write_reg(ov5693, OV5693_MWB_BLUE_GAIN_REG, gain, &ret);
+ cci_write(ov5693->regmap, OV5693_MWB_RED_GAIN_REG, gain, &ret);
+ cci_write(ov5693->regmap, OV5693_MWB_GREEN_GAIN_REG, gain, &ret);
+ cci_write(ov5693->regmap, OV5693_MWB_BLUE_GAIN_REG, gain, &ret);
return ret;
}
@@ -576,7 +448,7 @@ static int ov5693_analog_gain_configure(struct ov5693_device *ov5693, u32 gain)
gain = (gain << 4) & OV5693_GAIN_CTRL_MASK;
- ov5693_write_reg(ov5693, OV5693_GAIN_CTRL_REG, gain, &ret);
+ cci_write(ov5693->regmap, OV5693_GAIN_CTRL_REG, gain, &ret);
return ret;
}
@@ -586,7 +458,7 @@ static int ov5693_vts_configure(struct ov5693_device *ov5693, u32 vblank)
u16 vts = ov5693->mode.format.height + vblank;
int ret = 0;
- ov5693_write_reg(ov5693, OV5693_TIMING_VTS_REG, vts, &ret);
+ cci_write(ov5693->regmap, OV5693_TIMING_VTS_REG, vts, &ret);
return ret;
}
@@ -595,8 +467,8 @@ static int ov5693_test_pattern_configure(struct ov5693_device *ov5693, u32 idx)
{
int ret = 0;
- ov5693_write_reg(ov5693, OV5693_TEST_PATTERN_REG,
- ov5693_test_pattern_bits[idx], &ret);
+ cci_write(ov5693->regmap, OV5693_TEST_PATTERN_REG,
+ ov5693_test_pattern_bits[idx], &ret);
return ret;
}
@@ -685,59 +557,54 @@ static int ov5693_mode_configure(struct ov5693_device *ov5693)
int ret = 0;
/* Crop Start X */
- ov5693_write_reg(ov5693, OV5693_CROP_START_X_REG, mode->crop.left,
- &ret);
+ cci_write(ov5693->regmap, OV5693_CROP_START_X_REG, mode->crop.left,
+ &ret);
/* Offset X */
- ov5693_write_reg(ov5693, OV5693_OFFSET_START_X_REG, 0, &ret);
+ cci_write(ov5693->regmap, OV5693_OFFSET_START_X_REG, 0, &ret);
/* Output Size X */
- ov5693_write_reg(ov5693, OV5693_OUTPUT_SIZE_X_REG, mode->format.width,
- &ret);
+ cci_write(ov5693->regmap, OV5693_OUTPUT_SIZE_X_REG, mode->format.width,
+ &ret);
/* Crop End X */
- ov5693_write_reg(ov5693, OV5693_CROP_END_X_REG,
- mode->crop.left + mode->crop.width, &ret);
+ cci_write(ov5693->regmap, OV5693_CROP_END_X_REG,
+ mode->crop.left + mode->crop.width, &ret);
/* Horizontal Total Size */
- ov5693_write_reg(ov5693, OV5693_TIMING_HTS_REG, OV5693_FIXED_PPL,
- &ret);
+ cci_write(ov5693->regmap, OV5693_TIMING_HTS_REG, OV5693_FIXED_PPL,
+ &ret);
/* Crop Start Y */
- ov5693_write_reg(ov5693, OV5693_CROP_START_Y_REG, mode->crop.top,
- &ret);
+ cci_write(ov5693->regmap, OV5693_CROP_START_Y_REG, mode->crop.top,
+ &ret);
/* Offset Y */
- ov5693_write_reg(ov5693, OV5693_OFFSET_START_Y_REG, 0, &ret);
+ cci_write(ov5693->regmap, OV5693_OFFSET_START_Y_REG, 0, &ret);
/* Output Size Y */
- ov5693_write_reg(ov5693, OV5693_OUTPUT_SIZE_Y_REG, mode->format.height,
- &ret);
+ cci_write(ov5693->regmap, OV5693_OUTPUT_SIZE_Y_REG, mode->format.height,
+ &ret);
/* Crop End Y */
- ov5693_write_reg(ov5693, OV5693_CROP_END_Y_REG,
- mode->crop.top + mode->crop.height, &ret);
+ cci_write(ov5693->regmap, OV5693_CROP_END_Y_REG,
+ mode->crop.top + mode->crop.height, &ret);
/* Subsample X increase */
- ov5693_write_reg(ov5693, OV5693_SUB_INC_X_REG,
- ((mode->inc_x_odd << 4) & 0xf0) | 0x01, &ret);
+ cci_write(ov5693->regmap, OV5693_SUB_INC_X_REG,
+ ((mode->inc_x_odd << 4) & 0xf0) | 0x01, &ret);
/* Subsample Y increase */
- ov5693_write_reg(ov5693, OV5693_SUB_INC_Y_REG,
- ((mode->inc_y_odd << 4) & 0xf0) | 0x01, &ret);
-
- if (ret)
- return ret;
+ cci_write(ov5693->regmap, OV5693_SUB_INC_Y_REG,
+ ((mode->inc_y_odd << 4) & 0xf0) | 0x01, &ret);
/* Binning */
- ret = ov5693_update_bits(ov5693, OV5693_FORMAT1_REG,
- OV5693_FORMAT1_VBIN_EN,
- mode->binning_y ? OV5693_FORMAT1_VBIN_EN : 0);
- if (ret)
- return ret;
+ cci_update_bits(ov5693->regmap, OV5693_FORMAT1_REG,
+ OV5693_FORMAT1_VBIN_EN,
+ mode->binning_y ? OV5693_FORMAT1_VBIN_EN : 0, &ret);
- ret = ov5693_update_bits(ov5693, OV5693_FORMAT2_REG,
- OV5693_FORMAT2_HBIN_EN,
- mode->binning_x ? OV5693_FORMAT2_HBIN_EN : 0);
+ cci_update_bits(ov5693->regmap, OV5693_FORMAT2_REG,
+ OV5693_FORMAT2_HBIN_EN,
+ mode->binning_x ? OV5693_FORMAT2_HBIN_EN : 0, &ret);
return ret;
}
@@ -746,9 +613,9 @@ static int ov5693_enable_streaming(struct ov5693_device *ov5693, bool enable)
{
int ret = 0;
- ov5693_write_reg(ov5693, OV5693_SW_STREAM_REG,
- enable ? OV5693_START_STREAMING :
- OV5693_STOP_STREAMING, &ret);
+ cci_write(ov5693->regmap, OV5693_SW_STREAM_REG,
+ enable ? OV5693_START_STREAMING : OV5693_STOP_STREAMING,
+ &ret);
return ret;
}
@@ -757,7 +624,7 @@ static int ov5693_sw_reset(struct ov5693_device *ov5693)
{
int ret = 0;
- ov5693_write_reg(ov5693, OV5693_SW_RESET_REG, OV5693_SW_RESET, &ret);
+ cci_write(ov5693->regmap, OV5693_SW_RESET_REG, OV5693_SW_RESET, &ret);
return ret;
}
@@ -771,7 +638,8 @@ static int ov5693_sensor_init(struct ov5693_device *ov5693)
return dev_err_probe(ov5693->dev, ret,
"software reset error\n");
- ret = ov5693_write_reg_array(ov5693, &ov5693_global_setting);
+ ret = cci_multi_reg_write(ov5693->regmap, ov5693_global_regs,
+ ARRAY_SIZE(ov5693_global_regs), NULL);
if (ret)
return dev_err_probe(ov5693->dev, ret,
"global settings error\n");
@@ -871,15 +739,15 @@ static int __maybe_unused ov5693_sensor_resume(struct device *dev)
static int ov5693_detect(struct ov5693_device *ov5693)
{
int ret;
- u32 id;
+ u64 id;
- ret = ov5693_read_reg(ov5693, OV5693_REG_CHIP_ID, &id);
+ ret = cci_read(ov5693->regmap, OV5693_REG_CHIP_ID, &id, NULL);
if (ret)
return ret;
if (id != OV5693_CHIP_ID)
return dev_err_probe(ov5693->dev, -ENODEV,
- "sensor ID mismatch. Found 0x%04x\n", id);
+ "sensor ID mismatch. Got 0x%04llx\n", id);
return 0;
}
@@ -1407,9 +1275,12 @@ static int ov5693_probe(struct i2c_client *client)
if (!ov5693)
return -ENOMEM;
- ov5693->client = client;
ov5693->dev = &client->dev;
+ ov5693->regmap = devm_cci_regmap_init_i2c(client, 16);
+ if (IS_ERR(ov5693->regmap))
+ return PTR_ERR(ov5693->regmap);
+
ret = ov5693_check_hwcfg(ov5693);
if (ret)
return ret;
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 3/5] media: imx290: Convert to new CCI register access helpers
2023-06-27 12:51 [PATCH v4 0/5] media: Add MIPI CCI register access helper functions Hans de Goede
2023-06-27 12:51 ` [PATCH v4 1/5] " Hans de Goede
2023-06-27 12:51 ` [PATCH v4 2/5] media: ov5693: Convert to new CCI register access helpers Hans de Goede
@ 2023-06-27 12:51 ` Hans de Goede
2023-08-15 13:15 ` Laurent Pinchart
2023-06-27 12:51 ` [PATCH v4 4/5] media: atomisp: ov2680: " Hans de Goede
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2023-06-27 12:51 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart, Rui Miguel Silva, Daniel Scally
Cc: Hans de Goede, Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
Use the new comon CCI register access helpers to replace the private
register access helpers in the imx290 driver.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note:
1. This is untested
2. For reviewers: all the IMX290_REG_?BIT defines in both the register
address defines as well as in various reg-sequences were automatically
changed using search replace.
---
Changes in v3:
- Fixed a couple of lines > 80 chars
Changes in v2:
- New patch in v2 of this series
---
drivers/media/i2c/Kconfig | 1 +
drivers/media/i2c/imx290.c | 360 +++++++++++++++----------------------
2 files changed, 150 insertions(+), 211 deletions(-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 8467f4ce92eb..6365c15bc4d4 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -165,6 +165,7 @@ config VIDEO_IMX290
select VIDEO_V4L2_SUBDEV_API
select REGMAP_I2C
select V4L2_FWNODE
+ select V4L2_CCI_I2C
help
This is a Video4Linux2 sensor driver for the Sony
IMX290 camera sensor.
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index b3f832e9d7e1..e78c7b91ae72 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -21,91 +21,86 @@
#include <asm/unaligned.h>
#include <media/media-entity.h>
+#include <media/v4l2-cci.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-event.h>
#include <media/v4l2-fwnode.h>
#include <media/v4l2-subdev.h>
-#define IMX290_REG_SIZE_SHIFT 16
-#define IMX290_REG_ADDR_MASK 0xffff
-#define IMX290_REG_8BIT(n) ((1U << IMX290_REG_SIZE_SHIFT) | (n))
-#define IMX290_REG_16BIT(n) ((2U << IMX290_REG_SIZE_SHIFT) | (n))
-#define IMX290_REG_24BIT(n) ((3U << IMX290_REG_SIZE_SHIFT) | (n))
-
-#define IMX290_STANDBY IMX290_REG_8BIT(0x3000)
-#define IMX290_REGHOLD IMX290_REG_8BIT(0x3001)
-#define IMX290_XMSTA IMX290_REG_8BIT(0x3002)
-#define IMX290_ADBIT IMX290_REG_8BIT(0x3005)
+#define IMX290_STANDBY CCI_REG8(0x3000)
+#define IMX290_REGHOLD CCI_REG8(0x3001)
+#define IMX290_XMSTA CCI_REG8(0x3002)
+#define IMX290_ADBIT CCI_REG8(0x3005)
#define IMX290_ADBIT_10BIT (0 << 0)
#define IMX290_ADBIT_12BIT (1 << 0)
-#define IMX290_CTRL_07 IMX290_REG_8BIT(0x3007)
+#define IMX290_CTRL_07 CCI_REG8(0x3007)
#define IMX290_VREVERSE BIT(0)
#define IMX290_HREVERSE BIT(1)
#define IMX290_WINMODE_1080P (0 << 4)
#define IMX290_WINMODE_720P (1 << 4)
#define IMX290_WINMODE_CROP (4 << 4)
-#define IMX290_FR_FDG_SEL IMX290_REG_8BIT(0x3009)
-#define IMX290_BLKLEVEL IMX290_REG_16BIT(0x300a)
-#define IMX290_GAIN IMX290_REG_8BIT(0x3014)
-#define IMX290_VMAX IMX290_REG_24BIT(0x3018)
+#define IMX290_FR_FDG_SEL CCI_REG8(0x3009)
+#define IMX290_BLKLEVEL CCI_REG16(0x300a)
+#define IMX290_GAIN CCI_REG8(0x3014)
+#define IMX290_VMAX CCI_REG24(0x3018)
#define IMX290_VMAX_MAX 0x3ffff
-#define IMX290_HMAX IMX290_REG_16BIT(0x301c)
+#define IMX290_HMAX CCI_REG16(0x301c)
#define IMX290_HMAX_MAX 0xffff
-#define IMX290_SHS1 IMX290_REG_24BIT(0x3020)
-#define IMX290_WINWV_OB IMX290_REG_8BIT(0x303a)
-#define IMX290_WINPV IMX290_REG_16BIT(0x303c)
-#define IMX290_WINWV IMX290_REG_16BIT(0x303e)
-#define IMX290_WINPH IMX290_REG_16BIT(0x3040)
-#define IMX290_WINWH IMX290_REG_16BIT(0x3042)
-#define IMX290_OUT_CTRL IMX290_REG_8BIT(0x3046)
+#define IMX290_SHS1 CCI_REG24(0x3020)
+#define IMX290_WINWV_OB CCI_REG8(0x303a)
+#define IMX290_WINPV CCI_REG16(0x303c)
+#define IMX290_WINWV CCI_REG16(0x303e)
+#define IMX290_WINPH CCI_REG16(0x3040)
+#define IMX290_WINWH CCI_REG16(0x3042)
+#define IMX290_OUT_CTRL CCI_REG8(0x3046)
#define IMX290_ODBIT_10BIT (0 << 0)
#define IMX290_ODBIT_12BIT (1 << 0)
#define IMX290_OPORTSEL_PARALLEL (0x0 << 4)
#define IMX290_OPORTSEL_LVDS_2CH (0xd << 4)
#define IMX290_OPORTSEL_LVDS_4CH (0xe << 4)
#define IMX290_OPORTSEL_LVDS_8CH (0xf << 4)
-#define IMX290_XSOUTSEL IMX290_REG_8BIT(0x304b)
+#define IMX290_XSOUTSEL CCI_REG8(0x304b)
#define IMX290_XSOUTSEL_XVSOUTSEL_HIGH (0 << 0)
#define IMX290_XSOUTSEL_XVSOUTSEL_VSYNC (2 << 0)
#define IMX290_XSOUTSEL_XHSOUTSEL_HIGH (0 << 2)
#define IMX290_XSOUTSEL_XHSOUTSEL_HSYNC (2 << 2)
-#define IMX290_INCKSEL1 IMX290_REG_8BIT(0x305c)
-#define IMX290_INCKSEL2 IMX290_REG_8BIT(0x305d)
-#define IMX290_INCKSEL3 IMX290_REG_8BIT(0x305e)
-#define IMX290_INCKSEL4 IMX290_REG_8BIT(0x305f)
-#define IMX290_PGCTRL IMX290_REG_8BIT(0x308c)
-#define IMX290_ADBIT1 IMX290_REG_8BIT(0x3129)
+#define IMX290_INCKSEL1 CCI_REG8(0x305c)
+#define IMX290_INCKSEL2 CCI_REG8(0x305d)
+#define IMX290_INCKSEL3 CCI_REG8(0x305e)
+#define IMX290_INCKSEL4 CCI_REG8(0x305f)
+#define IMX290_PGCTRL CCI_REG8(0x308c)
+#define IMX290_ADBIT1 CCI_REG8(0x3129)
#define IMX290_ADBIT1_10BIT 0x1d
#define IMX290_ADBIT1_12BIT 0x00
-#define IMX290_INCKSEL5 IMX290_REG_8BIT(0x315e)
-#define IMX290_INCKSEL6 IMX290_REG_8BIT(0x3164)
-#define IMX290_ADBIT2 IMX290_REG_8BIT(0x317c)
+#define IMX290_INCKSEL5 CCI_REG8(0x315e)
+#define IMX290_INCKSEL6 CCI_REG8(0x3164)
+#define IMX290_ADBIT2 CCI_REG8(0x317c)
#define IMX290_ADBIT2_10BIT 0x12
#define IMX290_ADBIT2_12BIT 0x00
-#define IMX290_CHIP_ID IMX290_REG_16BIT(0x319a)
-#define IMX290_ADBIT3 IMX290_REG_8BIT(0x31ec)
+#define IMX290_CHIP_ID CCI_REG16(0x319a)
+#define IMX290_ADBIT3 CCI_REG8(0x31ec)
#define IMX290_ADBIT3_10BIT 0x37
#define IMX290_ADBIT3_12BIT 0x0e
-#define IMX290_REPETITION IMX290_REG_8BIT(0x3405)
-#define IMX290_PHY_LANE_NUM IMX290_REG_8BIT(0x3407)
-#define IMX290_OPB_SIZE_V IMX290_REG_8BIT(0x3414)
-#define IMX290_Y_OUT_SIZE IMX290_REG_16BIT(0x3418)
-#define IMX290_CSI_DT_FMT IMX290_REG_16BIT(0x3441)
+#define IMX290_REPETITION CCI_REG8(0x3405)
+#define IMX290_PHY_LANE_NUM CCI_REG8(0x3407)
+#define IMX290_OPB_SIZE_V CCI_REG8(0x3414)
+#define IMX290_Y_OUT_SIZE CCI_REG16(0x3418)
+#define IMX290_CSI_DT_FMT CCI_REG16(0x3441)
#define IMX290_CSI_DT_FMT_RAW10 0x0a0a
#define IMX290_CSI_DT_FMT_RAW12 0x0c0c
-#define IMX290_CSI_LANE_MODE IMX290_REG_8BIT(0x3443)
-#define IMX290_EXTCK_FREQ IMX290_REG_16BIT(0x3444)
-#define IMX290_TCLKPOST IMX290_REG_16BIT(0x3446)
-#define IMX290_THSZERO IMX290_REG_16BIT(0x3448)
-#define IMX290_THSPREPARE IMX290_REG_16BIT(0x344a)
-#define IMX290_TCLKTRAIL IMX290_REG_16BIT(0x344c)
-#define IMX290_THSTRAIL IMX290_REG_16BIT(0x344e)
-#define IMX290_TCLKZERO IMX290_REG_16BIT(0x3450)
-#define IMX290_TCLKPREPARE IMX290_REG_16BIT(0x3452)
-#define IMX290_TLPX IMX290_REG_16BIT(0x3454)
-#define IMX290_X_OUT_SIZE IMX290_REG_16BIT(0x3472)
-#define IMX290_INCKSEL7 IMX290_REG_8BIT(0x3480)
+#define IMX290_CSI_LANE_MODE CCI_REG8(0x3443)
+#define IMX290_EXTCK_FREQ CCI_REG16(0x3444)
+#define IMX290_TCLKPOST CCI_REG16(0x3446)
+#define IMX290_THSZERO CCI_REG16(0x3448)
+#define IMX290_THSPREPARE CCI_REG16(0x344a)
+#define IMX290_TCLKTRAIL CCI_REG16(0x344c)
+#define IMX290_THSTRAIL CCI_REG16(0x344e)
+#define IMX290_TCLKZERO CCI_REG16(0x3450)
+#define IMX290_TCLKPREPARE CCI_REG16(0x3452)
+#define IMX290_TLPX CCI_REG16(0x3454)
+#define IMX290_X_OUT_SIZE CCI_REG16(0x3472)
+#define IMX290_INCKSEL7 CCI_REG8(0x3480)
#define IMX290_PGCTRL_REGEN BIT(0)
#define IMX290_PGCTRL_THRU BIT(1)
@@ -181,7 +176,7 @@ enum imx290_model {
struct imx290_model_info {
enum imx290_colour_variant colour_variant;
- const struct imx290_regval *init_regs;
+ const struct cci_reg_sequence *init_regs;
size_t init_regs_num;
const char *name;
};
@@ -192,11 +187,6 @@ enum imx290_clk_freq {
IMX290_NUM_CLK
};
-struct imx290_regval {
- u32 reg;
- u32 val;
-};
-
/*
* Clock configuration for registers INCKSEL1 to INCKSEL6.
*/
@@ -217,7 +207,7 @@ struct imx290_mode {
u8 link_freq_index;
u8 ctrl_07;
- const struct imx290_regval *data;
+ const struct cci_reg_sequence *data;
u32 data_size;
const struct imx290_clk_cfg *clk_cfg;
@@ -271,7 +261,7 @@ static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
* Modes and formats
*/
-static const struct imx290_regval imx290_global_init_settings[] = {
+static const struct cci_reg_sequence imx290_global_init_settings[] = {
{ IMX290_WINWV_OB, 12 },
{ IMX290_WINPH, 0 },
{ IMX290_WINPV, 0 },
@@ -279,56 +269,56 @@ static const struct imx290_regval imx290_global_init_settings[] = {
{ IMX290_WINWV, 1097 },
{ IMX290_XSOUTSEL, IMX290_XSOUTSEL_XVSOUTSEL_VSYNC |
IMX290_XSOUTSEL_XHSOUTSEL_HSYNC },
- { IMX290_REG_8BIT(0x3011), 0x02 },
- { IMX290_REG_8BIT(0x3012), 0x64 },
- { IMX290_REG_8BIT(0x3013), 0x00 },
+ { CCI_REG8(0x3011), 0x02 },
+ { CCI_REG8(0x3012), 0x64 },
+ { CCI_REG8(0x3013), 0x00 },
};
-static const struct imx290_regval imx290_global_init_settings_290[] = {
- { IMX290_REG_8BIT(0x300f), 0x00 },
- { IMX290_REG_8BIT(0x3010), 0x21 },
- { IMX290_REG_8BIT(0x3016), 0x09 },
- { IMX290_REG_8BIT(0x3070), 0x02 },
- { IMX290_REG_8BIT(0x3071), 0x11 },
- { IMX290_REG_8BIT(0x309b), 0x10 },
- { IMX290_REG_8BIT(0x309c), 0x22 },
- { IMX290_REG_8BIT(0x30a2), 0x02 },
- { IMX290_REG_8BIT(0x30a6), 0x20 },
- { IMX290_REG_8BIT(0x30a8), 0x20 },
- { IMX290_REG_8BIT(0x30aa), 0x20 },
- { IMX290_REG_8BIT(0x30ac), 0x20 },
- { IMX290_REG_8BIT(0x30b0), 0x43 },
- { IMX290_REG_8BIT(0x3119), 0x9e },
- { IMX290_REG_8BIT(0x311c), 0x1e },
- { IMX290_REG_8BIT(0x311e), 0x08 },
- { IMX290_REG_8BIT(0x3128), 0x05 },
- { IMX290_REG_8BIT(0x313d), 0x83 },
- { IMX290_REG_8BIT(0x3150), 0x03 },
- { IMX290_REG_8BIT(0x317e), 0x00 },
- { IMX290_REG_8BIT(0x32b8), 0x50 },
- { IMX290_REG_8BIT(0x32b9), 0x10 },
- { IMX290_REG_8BIT(0x32ba), 0x00 },
- { IMX290_REG_8BIT(0x32bb), 0x04 },
- { IMX290_REG_8BIT(0x32c8), 0x50 },
- { IMX290_REG_8BIT(0x32c9), 0x10 },
- { IMX290_REG_8BIT(0x32ca), 0x00 },
- { IMX290_REG_8BIT(0x32cb), 0x04 },
- { IMX290_REG_8BIT(0x332c), 0xd3 },
- { IMX290_REG_8BIT(0x332d), 0x10 },
- { IMX290_REG_8BIT(0x332e), 0x0d },
- { IMX290_REG_8BIT(0x3358), 0x06 },
- { IMX290_REG_8BIT(0x3359), 0xe1 },
- { IMX290_REG_8BIT(0x335a), 0x11 },
- { IMX290_REG_8BIT(0x3360), 0x1e },
- { IMX290_REG_8BIT(0x3361), 0x61 },
- { IMX290_REG_8BIT(0x3362), 0x10 },
- { IMX290_REG_8BIT(0x33b0), 0x50 },
- { IMX290_REG_8BIT(0x33b2), 0x1a },
- { IMX290_REG_8BIT(0x33b3), 0x04 },
+static const struct cci_reg_sequence imx290_global_init_settings_290[] = {
+ { CCI_REG8(0x300f), 0x00 },
+ { CCI_REG8(0x3010), 0x21 },
+ { CCI_REG8(0x3016), 0x09 },
+ { CCI_REG8(0x3070), 0x02 },
+ { CCI_REG8(0x3071), 0x11 },
+ { CCI_REG8(0x309b), 0x10 },
+ { CCI_REG8(0x309c), 0x22 },
+ { CCI_REG8(0x30a2), 0x02 },
+ { CCI_REG8(0x30a6), 0x20 },
+ { CCI_REG8(0x30a8), 0x20 },
+ { CCI_REG8(0x30aa), 0x20 },
+ { CCI_REG8(0x30ac), 0x20 },
+ { CCI_REG8(0x30b0), 0x43 },
+ { CCI_REG8(0x3119), 0x9e },
+ { CCI_REG8(0x311c), 0x1e },
+ { CCI_REG8(0x311e), 0x08 },
+ { CCI_REG8(0x3128), 0x05 },
+ { CCI_REG8(0x313d), 0x83 },
+ { CCI_REG8(0x3150), 0x03 },
+ { CCI_REG8(0x317e), 0x00 },
+ { CCI_REG8(0x32b8), 0x50 },
+ { CCI_REG8(0x32b9), 0x10 },
+ { CCI_REG8(0x32ba), 0x00 },
+ { CCI_REG8(0x32bb), 0x04 },
+ { CCI_REG8(0x32c8), 0x50 },
+ { CCI_REG8(0x32c9), 0x10 },
+ { CCI_REG8(0x32ca), 0x00 },
+ { CCI_REG8(0x32cb), 0x04 },
+ { CCI_REG8(0x332c), 0xd3 },
+ { CCI_REG8(0x332d), 0x10 },
+ { CCI_REG8(0x332e), 0x0d },
+ { CCI_REG8(0x3358), 0x06 },
+ { CCI_REG8(0x3359), 0xe1 },
+ { CCI_REG8(0x335a), 0x11 },
+ { CCI_REG8(0x3360), 0x1e },
+ { CCI_REG8(0x3361), 0x61 },
+ { CCI_REG8(0x3362), 0x10 },
+ { CCI_REG8(0x33b0), 0x50 },
+ { CCI_REG8(0x33b2), 0x1a },
+ { CCI_REG8(0x33b3), 0x04 },
};
#define IMX290_NUM_CLK_REGS 2
-static const struct imx290_regval xclk_regs[][IMX290_NUM_CLK_REGS] = {
+static const struct cci_reg_sequence xclk_regs[][IMX290_NUM_CLK_REGS] = {
[IMX290_CLK_37_125] = {
{ IMX290_EXTCK_FREQ, (37125 * 256) / 1000 },
{ IMX290_INCKSEL7, 0x49 },
@@ -339,13 +329,13 @@ static const struct imx290_regval xclk_regs[][IMX290_NUM_CLK_REGS] = {
},
};
-static const struct imx290_regval imx290_global_init_settings_327[] = {
- { IMX290_REG_8BIT(0x309e), 0x4A },
- { IMX290_REG_8BIT(0x309f), 0x4A },
- { IMX290_REG_8BIT(0x313b), 0x61 },
+static const struct cci_reg_sequence imx290_global_init_settings_327[] = {
+ { CCI_REG8(0x309e), 0x4A },
+ { CCI_REG8(0x309f), 0x4A },
+ { CCI_REG8(0x313b), 0x61 },
};
-static const struct imx290_regval imx290_1080p_settings[] = {
+static const struct cci_reg_sequence imx290_1080p_settings[] = {
/* mode settings */
{ IMX290_WINWV_OB, 12 },
{ IMX290_OPB_SIZE_V, 10 },
@@ -353,7 +343,7 @@ static const struct imx290_regval imx290_1080p_settings[] = {
{ IMX290_Y_OUT_SIZE, 1080 },
};
-static const struct imx290_regval imx290_720p_settings[] = {
+static const struct cci_reg_sequence imx290_720p_settings[] = {
/* mode settings */
{ IMX290_WINWV_OB, 6 },
{ IMX290_OPB_SIZE_V, 4 },
@@ -361,7 +351,7 @@ static const struct imx290_regval imx290_720p_settings[] = {
{ IMX290_Y_OUT_SIZE, 720 },
};
-static const struct imx290_regval imx290_10bit_settings[] = {
+static const struct cci_reg_sequence imx290_10bit_settings[] = {
{ IMX290_ADBIT, IMX290_ADBIT_10BIT },
{ IMX290_OUT_CTRL, IMX290_ODBIT_10BIT },
{ IMX290_ADBIT1, IMX290_ADBIT1_10BIT },
@@ -370,7 +360,7 @@ static const struct imx290_regval imx290_10bit_settings[] = {
{ IMX290_CSI_DT_FMT, IMX290_CSI_DT_FMT_RAW10 },
};
-static const struct imx290_regval imx290_12bit_settings[] = {
+static const struct cci_reg_sequence imx290_12bit_settings[] = {
{ IMX290_ADBIT, IMX290_ADBIT_12BIT },
{ IMX290_OUT_CTRL, IMX290_ODBIT_12BIT },
{ IMX290_ADBIT1, IMX290_ADBIT1_12BIT },
@@ -576,7 +566,7 @@ static inline int imx290_modes_num(const struct imx290 *imx290)
struct imx290_format_info {
u32 code[IMX290_VARIANT_MAX];
u8 bpp;
- const struct imx290_regval *regs;
+ const struct cci_reg_sequence *regs;
unsigned int num_regs;
};
@@ -615,63 +605,15 @@ imx290_format_info(const struct imx290 *imx290, u32 code)
return NULL;
}
-/* -----------------------------------------------------------------------------
- * Register access
- */
-
-static int __always_unused imx290_read(struct imx290 *imx290, u32 addr, u32 *value)
-{
- u8 data[3] = { 0, 0, 0 };
- int ret;
-
- ret = regmap_raw_read(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
- data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
- if (ret < 0) {
- dev_err(imx290->dev, "%u-bit read from 0x%04x failed: %d\n",
- ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
- addr & IMX290_REG_ADDR_MASK, ret);
- return ret;
- }
-
- *value = get_unaligned_le24(data);
- return 0;
-}
-
-static int imx290_write(struct imx290 *imx290, u32 addr, u32 value, int *err)
-{
- u8 data[3];
- int ret;
-
- if (err && *err)
- return *err;
-
- put_unaligned_le24(value, data);
-
- ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
- data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
- if (ret < 0) {
- dev_err(imx290->dev, "%u-bit write to 0x%04x failed: %d\n",
- ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
- addr & IMX290_REG_ADDR_MASK, ret);
- if (err)
- *err = ret;
- }
-
- return ret;
-}
-
static int imx290_set_register_array(struct imx290 *imx290,
- const struct imx290_regval *settings,
+ const struct cci_reg_sequence *settings,
unsigned int num_settings)
{
- unsigned int i;
int ret;
- for (i = 0; i < num_settings; ++i, ++settings) {
- ret = imx290_write(imx290, settings->reg, settings->val, NULL);
- if (ret < 0)
- return ret;
- }
+ ret = cci_multi_reg_write(imx290->regmap, settings, num_settings, NULL);
+ if (ret < 0)
+ return ret;
/* Provide 10ms settle time */
usleep_range(10000, 11000);
@@ -689,12 +631,12 @@ static int imx290_set_clock(struct imx290 *imx290)
ret = imx290_set_register_array(imx290, xclk_regs[clk_idx],
IMX290_NUM_CLK_REGS);
- imx290_write(imx290, IMX290_INCKSEL1, clk_cfg->incksel1, &ret);
- imx290_write(imx290, IMX290_INCKSEL2, clk_cfg->incksel2, &ret);
- imx290_write(imx290, IMX290_INCKSEL3, clk_cfg->incksel3, &ret);
- imx290_write(imx290, IMX290_INCKSEL4, clk_cfg->incksel4, &ret);
- imx290_write(imx290, IMX290_INCKSEL5, clk_cfg->incksel5, &ret);
- imx290_write(imx290, IMX290_INCKSEL6, clk_cfg->incksel6, &ret);
+ cci_write(imx290->regmap, IMX290_INCKSEL1, clk_cfg->incksel1, &ret);
+ cci_write(imx290->regmap, IMX290_INCKSEL2, clk_cfg->incksel2, &ret);
+ cci_write(imx290->regmap, IMX290_INCKSEL3, clk_cfg->incksel3, &ret);
+ cci_write(imx290->regmap, IMX290_INCKSEL4, clk_cfg->incksel4, &ret);
+ cci_write(imx290->regmap, IMX290_INCKSEL5, clk_cfg->incksel5, &ret);
+ cci_write(imx290->regmap, IMX290_INCKSEL6, clk_cfg->incksel6, &ret);
return ret;
}
@@ -703,9 +645,11 @@ static int imx290_set_data_lanes(struct imx290 *imx290)
{
int ret = 0;
- imx290_write(imx290, IMX290_PHY_LANE_NUM, imx290->nlanes - 1, &ret);
- imx290_write(imx290, IMX290_CSI_LANE_MODE, imx290->nlanes - 1, &ret);
- imx290_write(imx290, IMX290_FR_FDG_SEL, 0x01, &ret);
+ cci_write(imx290->regmap, IMX290_PHY_LANE_NUM, imx290->nlanes - 1,
+ &ret);
+ cci_write(imx290->regmap, IMX290_CSI_LANE_MODE, imx290->nlanes - 1,
+ &ret);
+ cci_write(imx290->regmap, IMX290_FR_FDG_SEL, 0x01, &ret);
return ret;
}
@@ -716,8 +660,8 @@ static int imx290_set_black_level(struct imx290 *imx290,
{
unsigned int bpp = imx290_format_info(imx290, format->code)->bpp;
- return imx290_write(imx290, IMX290_BLKLEVEL,
- black_level >> (16 - bpp), err);
+ return cci_write(imx290->regmap, IMX290_BLKLEVEL,
+ black_level >> (16 - bpp), err);
}
static int imx290_set_csi_config(struct imx290 *imx290)
@@ -743,15 +687,16 @@ static int imx290_set_csi_config(struct imx290 *imx290)
return -EINVAL;
}
- imx290_write(imx290, IMX290_REPETITION, csi_cfg->repetition, &ret);
- imx290_write(imx290, IMX290_TCLKPOST, csi_cfg->tclkpost, &ret);
- imx290_write(imx290, IMX290_THSZERO, csi_cfg->thszero, &ret);
- imx290_write(imx290, IMX290_THSPREPARE, csi_cfg->thsprepare, &ret);
- imx290_write(imx290, IMX290_TCLKTRAIL, csi_cfg->tclktrail, &ret);
- imx290_write(imx290, IMX290_THSTRAIL, csi_cfg->thstrail, &ret);
- imx290_write(imx290, IMX290_TCLKZERO, csi_cfg->tclkzero, &ret);
- imx290_write(imx290, IMX290_TCLKPREPARE, csi_cfg->tclkprepare, &ret);
- imx290_write(imx290, IMX290_TLPX, csi_cfg->tlpx, &ret);
+ cci_write(imx290->regmap, IMX290_REPETITION, csi_cfg->repetition, &ret);
+ cci_write(imx290->regmap, IMX290_TCLKPOST, csi_cfg->tclkpost, &ret);
+ cci_write(imx290->regmap, IMX290_THSZERO, csi_cfg->thszero, &ret);
+ cci_write(imx290->regmap, IMX290_THSPREPARE, csi_cfg->thsprepare, &ret);
+ cci_write(imx290->regmap, IMX290_TCLKTRAIL, csi_cfg->tclktrail, &ret);
+ cci_write(imx290->regmap, IMX290_THSTRAIL, csi_cfg->thstrail, &ret);
+ cci_write(imx290->regmap, IMX290_TCLKZERO, csi_cfg->tclkzero, &ret);
+ cci_write(imx290->regmap, IMX290_TCLKPREPARE, csi_cfg->tclkprepare,
+ &ret);
+ cci_write(imx290->regmap, IMX290_TLPX, csi_cfg->tlpx, &ret);
return ret;
}
@@ -817,13 +762,12 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
switch (ctrl->id) {
case V4L2_CID_ANALOGUE_GAIN:
- ret = imx290_write(imx290, IMX290_GAIN, ctrl->val, NULL);
+ ret = cci_write(imx290->regmap, IMX290_GAIN, ctrl->val, NULL);
break;
case V4L2_CID_VBLANK:
- ret = imx290_write(imx290, IMX290_VMAX,
- ctrl->val + imx290->current_mode->height,
- NULL);
+ ret = cci_write(imx290->regmap, IMX290_VMAX,
+ ctrl->val + imx290->current_mode->height, NULL);
/*
* Due to the way that exposure is programmed in this sensor in
* relation to VMAX, we have to reprogramme it whenever VMAX is
@@ -835,20 +779,20 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
fallthrough;
case V4L2_CID_EXPOSURE:
vmax = imx290->vblank->val + imx290->current_mode->height;
- ret = imx290_write(imx290, IMX290_SHS1,
- vmax - ctrl->val - 1, NULL);
+ ret = cci_write(imx290->regmap, IMX290_SHS1,
+ vmax - ctrl->val - 1, NULL);
break;
case V4L2_CID_TEST_PATTERN:
if (ctrl->val) {
imx290_set_black_level(imx290, format, 0, &ret);
usleep_range(10000, 11000);
- imx290_write(imx290, IMX290_PGCTRL,
- (u8)(IMX290_PGCTRL_REGEN |
- IMX290_PGCTRL_THRU |
- IMX290_PGCTRL_MODE(ctrl->val)), &ret);
+ cci_write(imx290->regmap, IMX290_PGCTRL,
+ (u8)(IMX290_PGCTRL_REGEN |
+ IMX290_PGCTRL_THRU |
+ IMX290_PGCTRL_MODE(ctrl->val)), &ret);
} else {
- imx290_write(imx290, IMX290_PGCTRL, 0x00, &ret);
+ cci_write(imx290->regmap, IMX290_PGCTRL, 0x00, &ret);
usleep_range(10000, 11000);
imx290_set_black_level(imx290, format,
IMX290_BLACK_LEVEL_DEFAULT, &ret);
@@ -856,9 +800,8 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
break;
case V4L2_CID_HBLANK:
- ret = imx290_write(imx290, IMX290_HMAX,
- ctrl->val + imx290->current_mode->width,
- NULL);
+ ret = cci_write(imx290->regmap, IMX290_HMAX,
+ ctrl->val + imx290->current_mode->width, NULL);
break;
case V4L2_CID_HFLIP:
@@ -871,7 +814,7 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
reg |= IMX290_HREVERSE;
if (imx290->vflip->val)
reg |= IMX290_VREVERSE;
- ret = imx290_write(imx290, IMX290_CTRL_07, reg, NULL);
+ ret = cci_write(imx290->regmap, IMX290_CTRL_07, reg, NULL);
break;
}
@@ -1074,12 +1017,12 @@ static int imx290_start_streaming(struct imx290 *imx290,
return ret;
}
- imx290_write(imx290, IMX290_STANDBY, 0x00, &ret);
+ cci_write(imx290->regmap, IMX290_STANDBY, 0x00, &ret);
msleep(30);
/* Start streaming */
- return imx290_write(imx290, IMX290_XMSTA, 0x00, &ret);
+ return cci_write(imx290->regmap, IMX290_XMSTA, 0x00, &ret);
}
/* Stop streaming */
@@ -1087,11 +1030,11 @@ static int imx290_stop_streaming(struct imx290 *imx290)
{
int ret = 0;
- imx290_write(imx290, IMX290_STANDBY, 0x01, &ret);
+ cci_write(imx290->regmap, IMX290_STANDBY, 0x01, &ret);
msleep(30);
- return imx290_write(imx290, IMX290_XMSTA, 0x01, &ret);
+ return cci_write(imx290->regmap, IMX290_XMSTA, 0x01, &ret);
}
static int imx290_set_stream(struct v4l2_subdev *sd, int enable)
@@ -1417,11 +1360,6 @@ static const struct dev_pm_ops imx290_pm_ops = {
* Probe & remove
*/
-static const struct regmap_config imx290_regmap_config = {
- .reg_bits = 16,
- .val_bits = 8,
-};
-
static const char * const imx290_supply_name[IMX290_NUM_SUPPLIES] = {
"vdda",
"vddd",
@@ -1588,7 +1526,7 @@ static int imx290_probe(struct i2c_client *client)
return -ENOMEM;
imx290->dev = dev;
- imx290->regmap = devm_regmap_init_i2c(client, &imx290_regmap_config);
+ imx290->regmap = devm_cci_regmap_init_i2c(client, 16);
if (IS_ERR(imx290->regmap)) {
dev_err(dev, "Unable to initialize I2C\n");
return -ENODEV;
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 3/5] media: imx290: Convert to new CCI register access helpers
2023-06-27 12:51 ` [PATCH v4 3/5] media: imx290: " Hans de Goede
@ 2023-08-15 13:15 ` Laurent Pinchart
2023-08-15 13:31 ` Sakari Ailus
2023-08-15 14:15 ` Hans de Goede
0 siblings, 2 replies; 14+ messages in thread
From: Laurent Pinchart @ 2023-08-15 13:15 UTC (permalink / raw)
To: Hans de Goede
Cc: Sakari Ailus, Rui Miguel Silva, Daniel Scally,
Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
Hi Hans,
On Tue, Jun 27, 2023 at 02:51:06PM +0200, Hans de Goede wrote:
> Use the new comon CCI register access helpers to replace the private
> register access helpers in the imx290 driver.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Note:
> 1. This is untested
> 2. For reviewers: all the IMX290_REG_?BIT defines in both the register
> address defines as well as in various reg-sequences were automatically
> changed using search replace.
> ---
> Changes in v3:
> - Fixed a couple of lines > 80 chars
>
> Changes in v2:
> - New patch in v2 of this series
> ---
> drivers/media/i2c/Kconfig | 1 +
> drivers/media/i2c/imx290.c | 360 +++++++++++++++----------------------
> 2 files changed, 150 insertions(+), 211 deletions(-)
[snip]
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index b3f832e9d7e1..e78c7b91ae72 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -21,91 +21,86 @@
[snip]
> @@ -615,63 +605,15 @@ imx290_format_info(const struct imx290 *imx290, u32 code)
> return NULL;
> }
>
> -/* -----------------------------------------------------------------------------
> - * Register access
> - */
> -
> -static int __always_unused imx290_read(struct imx290 *imx290, u32 addr, u32 *value)
> -{
> - u8 data[3] = { 0, 0, 0 };
> - int ret;
> -
> - ret = regmap_raw_read(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
> - if (ret < 0) {
> - dev_err(imx290->dev, "%u-bit read from 0x%04x failed: %d\n",
> - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> - addr & IMX290_REG_ADDR_MASK, ret);
> - return ret;
> - }
> -
> - *value = get_unaligned_le24(data);
> - return 0;
> -}
> -
> -static int imx290_write(struct imx290 *imx290, u32 addr, u32 value, int *err)
> -{
> - u8 data[3];
> - int ret;
> -
> - if (err && *err)
> - return *err;
> -
> - put_unaligned_le24(value, data);
We seem to be having a problem here, as the CCI helpers unconditionally
use big endian for the data :-(
> -
> - ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
> - if (ret < 0) {
> - dev_err(imx290->dev, "%u-bit write to 0x%04x failed: %d\n",
> - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> - addr & IMX290_REG_ADDR_MASK, ret);
> - if (err)
> - *err = ret;
> - }
> -
> - return ret;
> -}
> -
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 3/5] media: imx290: Convert to new CCI register access helpers
2023-08-15 13:15 ` Laurent Pinchart
@ 2023-08-15 13:31 ` Sakari Ailus
2023-08-15 13:43 ` Laurent Pinchart
2023-08-15 13:48 ` Alexander Stein
2023-08-15 14:15 ` Hans de Goede
1 sibling, 2 replies; 14+ messages in thread
From: Sakari Ailus @ 2023-08-15 13:31 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Hans de Goede, Rui Miguel Silva, Daniel Scally,
Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
Hi Laurent,
On Tue, Aug 15, 2023 at 04:15:39PM +0300, Laurent Pinchart wrote:
> Hi Hans,
>
> On Tue, Jun 27, 2023 at 02:51:06PM +0200, Hans de Goede wrote:
> > Use the new comon CCI register access helpers to replace the private
> > register access helpers in the imx290 driver.
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> > Note:
> > 1. This is untested
> > 2. For reviewers: all the IMX290_REG_?BIT defines in both the register
> > address defines as well as in various reg-sequences were automatically
> > changed using search replace.
> > ---
> > Changes in v3:
> > - Fixed a couple of lines > 80 chars
> >
> > Changes in v2:
> > - New patch in v2 of this series
> > ---
> > drivers/media/i2c/Kconfig | 1 +
> > drivers/media/i2c/imx290.c | 360 +++++++++++++++----------------------
> > 2 files changed, 150 insertions(+), 211 deletions(-)
>
> [snip]
>
> > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > index b3f832e9d7e1..e78c7b91ae72 100644
> > --- a/drivers/media/i2c/imx290.c
> > +++ b/drivers/media/i2c/imx290.c
> > @@ -21,91 +21,86 @@
>
> [snip]
>
> > @@ -615,63 +605,15 @@ imx290_format_info(const struct imx290 *imx290, u32 code)
> > return NULL;
> > }
> >
> > -/* -----------------------------------------------------------------------------
> > - * Register access
> > - */
> > -
> > -static int __always_unused imx290_read(struct imx290 *imx290, u32 addr, u32 *value)
> > -{
> > - u8 data[3] = { 0, 0, 0 };
> > - int ret;
> > -
> > - ret = regmap_raw_read(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> > - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
> > - if (ret < 0) {
> > - dev_err(imx290->dev, "%u-bit read from 0x%04x failed: %d\n",
> > - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> > - addr & IMX290_REG_ADDR_MASK, ret);
> > - return ret;
> > - }
> > -
> > - *value = get_unaligned_le24(data);
> > - return 0;
> > -}
> > -
> > -static int imx290_write(struct imx290 *imx290, u32 addr, u32 value, int *err)
> > -{
> > - u8 data[3];
> > - int ret;
> > -
> > - if (err && *err)
> > - return *err;
> > -
> > - put_unaligned_le24(value, data);
>
> We seem to be having a problem here, as the CCI helpers unconditionally
> use big endian for the data :-(
Well spotted. This driver needs to address this.
It's a hardware issue though. It's the only sensor ever I've seen to have
little endian registers.
>
> > -
> > - ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> > - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
> > - if (ret < 0) {
> > - dev_err(imx290->dev, "%u-bit write to 0x%04x failed: %d\n",
> > - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> > - addr & IMX290_REG_ADDR_MASK, ret);
> > - if (err)
> > - *err = ret;
> > - }
> > -
> > - return ret;
> > -}
> > -
>
> [snip]
>
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 3/5] media: imx290: Convert to new CCI register access helpers
2023-08-15 13:31 ` Sakari Ailus
@ 2023-08-15 13:43 ` Laurent Pinchart
2023-08-15 13:48 ` Alexander Stein
1 sibling, 0 replies; 14+ messages in thread
From: Laurent Pinchart @ 2023-08-15 13:43 UTC (permalink / raw)
To: Sakari Ailus
Cc: Hans de Goede, Rui Miguel Silva, Daniel Scally,
Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
On Tue, Aug 15, 2023 at 01:31:55PM +0000, Sakari Ailus wrote:
> On Tue, Aug 15, 2023 at 04:15:39PM +0300, Laurent Pinchart wrote:
> > On Tue, Jun 27, 2023 at 02:51:06PM +0200, Hans de Goede wrote:
> > > Use the new comon CCI register access helpers to replace the private
> > > register access helpers in the imx290 driver.
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > > Note:
> > > 1. This is untested
> > > 2. For reviewers: all the IMX290_REG_?BIT defines in both the register
> > > address defines as well as in various reg-sequences were automatically
> > > changed using search replace.
> > > ---
> > > Changes in v3:
> > > - Fixed a couple of lines > 80 chars
> > >
> > > Changes in v2:
> > > - New patch in v2 of this series
> > > ---
> > > drivers/media/i2c/Kconfig | 1 +
> > > drivers/media/i2c/imx290.c | 360 +++++++++++++++----------------------
> > > 2 files changed, 150 insertions(+), 211 deletions(-)
> >
> > [snip]
> >
> > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > > index b3f832e9d7e1..e78c7b91ae72 100644
> > > --- a/drivers/media/i2c/imx290.c
> > > +++ b/drivers/media/i2c/imx290.c
> > > @@ -21,91 +21,86 @@
> >
> > [snip]
> >
> > > @@ -615,63 +605,15 @@ imx290_format_info(const struct imx290 *imx290, u32 code)
> > > return NULL;
> > > }
> > >
> > > -/* -----------------------------------------------------------------------------
> > > - * Register access
> > > - */
> > > -
> > > -static int __always_unused imx290_read(struct imx290 *imx290, u32 addr, u32 *value)
> > > -{
> > > - u8 data[3] = { 0, 0, 0 };
> > > - int ret;
> > > -
> > > - ret = regmap_raw_read(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> > > - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
> > > - if (ret < 0) {
> > > - dev_err(imx290->dev, "%u-bit read from 0x%04x failed: %d\n",
> > > - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> > > - addr & IMX290_REG_ADDR_MASK, ret);
> > > - return ret;
> > > - }
> > > -
> > > - *value = get_unaligned_le24(data);
> > > - return 0;
> > > -}
> > > -
> > > -static int imx290_write(struct imx290 *imx290, u32 addr, u32 value, int *err)
> > > -{
> > > - u8 data[3];
> > > - int ret;
> > > -
> > > - if (err && *err)
> > > - return *err;
> > > -
> > > - put_unaligned_le24(value, data);
> >
> > We seem to be having a problem here, as the CCI helpers unconditionally
> > use big endian for the data :-(
>
> Well spotted. This driver needs to address this.
>
> It's a hardware issue though. It's the only sensor ever I've seen to have
> little endian registers.
I'm not sure I would call that a hardware issue. It may be a peculiarity
of this sensor, but is it really the only one ?
How would you like to see this addressed ? We could add CCI_REG*_LE
macros, but I'm not sure to like that. Setting a flag at init time would
be nicer, but there's nowhere to store it.
> > > -
> > > - ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> > > - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
> > > - if (ret < 0) {
> > > - dev_err(imx290->dev, "%u-bit write to 0x%04x failed: %d\n",
> > > - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> > > - addr & IMX290_REG_ADDR_MASK, ret);
> > > - if (err)
> > > - *err = ret;
> > > - }
> > > -
> > > - return ret;
> > > -}
> > > -
> >
> > [snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 3/5] media: imx290: Convert to new CCI register access helpers
2023-08-15 13:31 ` Sakari Ailus
2023-08-15 13:43 ` Laurent Pinchart
@ 2023-08-15 13:48 ` Alexander Stein
2023-08-15 13:54 ` Laurent Pinchart
1 sibling, 1 reply; 14+ messages in thread
From: Alexander Stein @ 2023-08-15 13:48 UTC (permalink / raw)
To: Laurent Pinchart, Sakari Ailus
Cc: Hans de Goede, Rui Miguel Silva, Daniel Scally,
Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
Am Dienstag, 15. August 2023, 15:31:55 CEST schrieb Sakari Ailus:
> ********************
> Achtung externe E-Mail: Öffnen Sie Anhänge und Links nur, wenn Sie wissen,
> dass diese aus einer sicheren Quelle stammen und sicher sind. Leiten Sie
> die E-Mail im Zweifelsfall zur Prüfung an den IT-Helpdesk weiter.
> Attention external email: Open attachments and links only if you know that
> they are from a secure source and are safe. In doubt forward the email to
> the IT-Helpdesk to check it. ********************
>
> Hi Laurent,
>
> On Tue, Aug 15, 2023 at 04:15:39PM +0300, Laurent Pinchart wrote:
> > Hi Hans,
> >
> > On Tue, Jun 27, 2023 at 02:51:06PM +0200, Hans de Goede wrote:
> > > Use the new comon CCI register access helpers to replace the private
> > > register access helpers in the imx290 driver.
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > > Note:
> > > 1. This is untested
> > > 2. For reviewers: all the IMX290_REG_?BIT defines in both the register
> > > address defines as well as in various reg-sequences were automatically
> > > changed using search replace.
> > > ---
> > > Changes in v3:
> > > - Fixed a couple of lines > 80 chars
> > >
> > > Changes in v2:
> > > - New patch in v2 of this series
> > > ---
> > >
> > > drivers/media/i2c/Kconfig | 1 +
> > > drivers/media/i2c/imx290.c | 360 +++++++++++++++----------------------
> > > 2 files changed, 150 insertions(+), 211 deletions(-)
> >
> > [snip]
> >
> > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > > index b3f832e9d7e1..e78c7b91ae72 100644
> > > --- a/drivers/media/i2c/imx290.c
> > > +++ b/drivers/media/i2c/imx290.c
> > > @@ -21,91 +21,86 @@
> >
> > [snip]
> >
> > > @@ -615,63 +605,15 @@ imx290_format_info(const struct imx290 *imx290,
> > > u32 code)> >
> > > return NULL;
> > >
> > > }
> > >
> > > -/*
> > > -----------------------------------------------------------------------
> > > ------ - * Register access
> > > - */
> > > -
> > > -static int __always_unused imx290_read(struct imx290 *imx290, u32 addr,
> > > u32 *value) -{
> > > - u8 data[3] = { 0, 0, 0 };
> > > - int ret;
> > > -
> > > - ret = regmap_raw_read(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> > > - data, (addr >> IMX290_REG_SIZE_SHIFT) &
3);
> > > - if (ret < 0) {
> > > - dev_err(imx290->dev, "%u-bit read from 0x%04x failed:
%d\n",
> > > - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> > > - addr & IMX290_REG_ADDR_MASK, ret);
> > > - return ret;
> > > - }
> > > -
> > > - *value = get_unaligned_le24(data);
> > > - return 0;
> > > -}
> > > -
> > > -static int imx290_write(struct imx290 *imx290, u32 addr, u32 value, int
> > > *err) -{
> > > - u8 data[3];
> > > - int ret;
> > > -
> > > - if (err && *err)
> > > - return *err;
> > > -
> > > - put_unaligned_le24(value, data);
> >
> > We seem to be having a problem here, as the CCI helpers unconditionally
> > use big endian for the data :-(
>
> Well spotted. This driver needs to address this.
>
> It's a hardware issue though. It's the only sensor ever I've seen to have
> little endian registers.
It's getting even worse: There are cameras using this sensor when bulk read is
broken (VC MIPI IMX327 C, Laurent knows about this peculiar hardware), so
regmap_config.use_single_read has to be set to true. This is currently not
possible anymore insode this driver.
Best regards,
Alexander
> > > -
> > > - ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> > > - data, (addr >> IMX290_REG_SIZE_SHIFT) &
3);
> > > - if (ret < 0) {
> > > - dev_err(imx290->dev, "%u-bit write to 0x%04x failed:
%d\n",
> > > - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> > > - addr & IMX290_REG_ADDR_MASK, ret);
> > > - if (err)
> > > - *err = ret;
> > > - }
> > > -
> > > - return ret;
> > > -}
> > > -
> >
> > [snip]
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 3/5] media: imx290: Convert to new CCI register access helpers
2023-08-15 13:48 ` Alexander Stein
@ 2023-08-15 13:54 ` Laurent Pinchart
0 siblings, 0 replies; 14+ messages in thread
From: Laurent Pinchart @ 2023-08-15 13:54 UTC (permalink / raw)
To: Alexander Stein
Cc: Sakari Ailus, Hans de Goede, Rui Miguel Silva, Daniel Scally,
Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
On Tue, Aug 15, 2023 at 03:48:53PM +0200, Alexander Stein wrote:
> Am Dienstag, 15. August 2023, 15:31:55 CEST schrieb Sakari Ailus:
> > On Tue, Aug 15, 2023 at 04:15:39PM +0300, Laurent Pinchart wrote:
> > > On Tue, Jun 27, 2023 at 02:51:06PM +0200, Hans de Goede wrote:
> > > > Use the new comon CCI register access helpers to replace the private
> > > > register access helpers in the imx290 driver.
> > > >
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > > ---
> > > > Note:
> > > > 1. This is untested
> > > > 2. For reviewers: all the IMX290_REG_?BIT defines in both the register
> > > > address defines as well as in various reg-sequences were automatically
> > > > changed using search replace.
> > > > ---
> > > > Changes in v3:
> > > > - Fixed a couple of lines > 80 chars
> > > >
> > > > Changes in v2:
> > > > - New patch in v2 of this series
> > > > ---
> > > >
> > > > drivers/media/i2c/Kconfig | 1 +
> > > > drivers/media/i2c/imx290.c | 360 +++++++++++++++----------------------
> > > > 2 files changed, 150 insertions(+), 211 deletions(-)
> > >
> > > [snip]
> > >
> > > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > > > index b3f832e9d7e1..e78c7b91ae72 100644
> > > > --- a/drivers/media/i2c/imx290.c
> > > > +++ b/drivers/media/i2c/imx290.c
> > > > @@ -21,91 +21,86 @@
> > >
> > > [snip]
> > >
> > > > @@ -615,63 +605,15 @@ imx290_format_info(const struct imx290 *imx290, u32 code)
> > > >
> > > > return NULL;
> > > >
> > > > }
> > > >
> > > > -/* -----------------------------------------------------------------------------
> > > > - * Register access
> > > > - */
> > > > -
> > > > -static int __always_unused imx290_read(struct imx290 *imx290, u32 addr, u32 *value)
> > > > -{
> > > > - u8 data[3] = { 0, 0, 0 };
> > > > - int ret;
> > > > -
> > > > - ret = regmap_raw_read(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> > > > - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
> > > > - if (ret < 0) {
> > > > - dev_err(imx290->dev, "%u-bit read from 0x%04x failed: %d\n",
> > > > - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> > > > - addr & IMX290_REG_ADDR_MASK, ret);
> > > > - return ret;
> > > > - }
> > > > -
> > > > - *value = get_unaligned_le24(data);
> > > > - return 0;
> > > > -}
> > > > -
> > > > -static int imx290_write(struct imx290 *imx290, u32 addr, u32 value, int *err)
> > > > -{
> > > > - u8 data[3];
> > > > - int ret;
> > > > -
> > > > - if (err && *err)
> > > > - return *err;
> > > > -
> > > > - put_unaligned_le24(value, data);
> > >
> > > We seem to be having a problem here, as the CCI helpers unconditionally
> > > use big endian for the data :-(
> >
> > Well spotted. This driver needs to address this.
> >
> > It's a hardware issue though. It's the only sensor ever I've seen to have
> > little endian registers.
The IMX296 and IMX297 seem to also have little-endian register values.
> It's getting even worse: There are cameras using this sensor when bulk read is
> broken (VC MIPI IMX327 C, Laurent knows about this peculiar hardware), so
> regmap_config.use_single_read has to be set to true. This is currently not
> possible anymore insode this driver.
I think this could be fixed by passing a flag to the init function. Or,
possibly better, the init function could read a DT property and set the
flag automatically.
> > > > -
> > > > - ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> > > > - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
> > > > - if (ret < 0) {
> > > > - dev_err(imx290->dev, "%u-bit write to 0x%04x failed: %d\n",
> > > > - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> > > > - addr & IMX290_REG_ADDR_MASK, ret);
> > > > - if (err)
> > > > - *err = ret;
> > > > - }
> > > > -
> > > > - return ret;
> > > > -}
> > > > -
> > >
> > > [snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/5] media: imx290: Convert to new CCI register access helpers
2023-08-15 13:15 ` Laurent Pinchart
2023-08-15 13:31 ` Sakari Ailus
@ 2023-08-15 14:15 ` Hans de Goede
2023-08-15 14:21 ` Laurent Pinchart
1 sibling, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2023-08-15 14:15 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sakari Ailus, Rui Miguel Silva, Daniel Scally,
Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
Hi,
On 8/15/23 15:15, Laurent Pinchart wrote:
> Hi Hans,
>
> On Tue, Jun 27, 2023 at 02:51:06PM +0200, Hans de Goede wrote:
>> Use the new comon CCI register access helpers to replace the private
>> register access helpers in the imx290 driver.
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Note:
>> 1. This is untested
>> 2. For reviewers: all the IMX290_REG_?BIT defines in both the register
>> address defines as well as in various reg-sequences were automatically
>> changed using search replace.
>> ---
>> Changes in v3:
>> - Fixed a couple of lines > 80 chars
>>
>> Changes in v2:
>> - New patch in v2 of this series
>> ---
>> drivers/media/i2c/Kconfig | 1 +
>> drivers/media/i2c/imx290.c | 360 +++++++++++++++----------------------
>> 2 files changed, 150 insertions(+), 211 deletions(-)
>
> [snip]
>
>> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
>> index b3f832e9d7e1..e78c7b91ae72 100644
>> --- a/drivers/media/i2c/imx290.c
>> +++ b/drivers/media/i2c/imx290.c
>> @@ -21,91 +21,86 @@
>
> [snip]
>
>> @@ -615,63 +605,15 @@ imx290_format_info(const struct imx290 *imx290, u32 code)
>> return NULL;
>> }
>>
>> -/* -----------------------------------------------------------------------------
>> - * Register access
>> - */
>> -
>> -static int __always_unused imx290_read(struct imx290 *imx290, u32 addr, u32 *value)
>> -{
>> - u8 data[3] = { 0, 0, 0 };
>> - int ret;
>> -
>> - ret = regmap_raw_read(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
>> - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
>> - if (ret < 0) {
>> - dev_err(imx290->dev, "%u-bit read from 0x%04x failed: %d\n",
>> - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
>> - addr & IMX290_REG_ADDR_MASK, ret);
>> - return ret;
>> - }
>> -
>> - *value = get_unaligned_le24(data);
>> - return 0;
>> -}
>> -
>> -static int imx290_write(struct imx290 *imx290, u32 addr, u32 value, int *err)
>> -{
>> - u8 data[3];
>> - int ret;
>> -
>> - if (err && *err)
>> - return *err;
>> -
>> - put_unaligned_le24(value, data);
>
> We seem to be having a problem here, as the CCI helpers unconditionally
> use big endian for the data :-(
That is because that is what the specification says, from the MIPI CSI spec:
"""
6.3.2 The Transmission Byte Order for Multi-byte Register Values
This is a normative section.
The first byte of a CCI message is always the MS byte of a multi-byte register and the last byte is always the LS byte.
"""
So it seems that the IMX sensors are special here and it might be best to just revert the conversion to the CCI helpers?
Alternative would be to make devm_cci_regmap_init_i2c() return a newly allocated struct
which contains both a struct regmap * and a long flags and make the helpers take a pointer to that struct, combined with adding an endianess flag to the flags member.
Regards,
Hans
>
>> -
>> - ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
>> - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
>> - if (ret < 0) {
>> - dev_err(imx290->dev, "%u-bit write to 0x%04x failed: %d\n",
>> - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
>> - addr & IMX290_REG_ADDR_MASK, ret);
>> - if (err)
>> - *err = ret;
>> - }
>> -
>> - return ret;
>> -}
>> -
>
> [snip]
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 3/5] media: imx290: Convert to new CCI register access helpers
2023-08-15 14:15 ` Hans de Goede
@ 2023-08-15 14:21 ` Laurent Pinchart
0 siblings, 0 replies; 14+ messages in thread
From: Laurent Pinchart @ 2023-08-15 14:21 UTC (permalink / raw)
To: Hans de Goede
Cc: Sakari Ailus, Rui Miguel Silva, Daniel Scally,
Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
Hi Hans,
On Tue, Aug 15, 2023 at 04:15:39PM +0200, Hans de Goede wrote:
> On 8/15/23 15:15, Laurent Pinchart wrote:
> > On Tue, Jun 27, 2023 at 02:51:06PM +0200, Hans de Goede wrote:
> >> Use the new comon CCI register access helpers to replace the private
> >> register access helpers in the imx290 driver.
> >>
> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >> Note:
> >> 1. This is untested
> >> 2. For reviewers: all the IMX290_REG_?BIT defines in both the register
> >> address defines as well as in various reg-sequences were automatically
> >> changed using search replace.
> >> ---
> >> Changes in v3:
> >> - Fixed a couple of lines > 80 chars
> >>
> >> Changes in v2:
> >> - New patch in v2 of this series
> >> ---
> >> drivers/media/i2c/Kconfig | 1 +
> >> drivers/media/i2c/imx290.c | 360 +++++++++++++++----------------------
> >> 2 files changed, 150 insertions(+), 211 deletions(-)
> >
> > [snip]
> >
> >> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> >> index b3f832e9d7e1..e78c7b91ae72 100644
> >> --- a/drivers/media/i2c/imx290.c
> >> +++ b/drivers/media/i2c/imx290.c
> >> @@ -21,91 +21,86 @@
> >
> > [snip]
> >
> >> @@ -615,63 +605,15 @@ imx290_format_info(const struct imx290 *imx290, u32 code)
> >> return NULL;
> >> }
> >>
> >> -/* -----------------------------------------------------------------------------
> >> - * Register access
> >> - */
> >> -
> >> -static int __always_unused imx290_read(struct imx290 *imx290, u32 addr, u32 *value)
> >> -{
> >> - u8 data[3] = { 0, 0, 0 };
> >> - int ret;
> >> -
> >> - ret = regmap_raw_read(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> >> - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
> >> - if (ret < 0) {
> >> - dev_err(imx290->dev, "%u-bit read from 0x%04x failed: %d\n",
> >> - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> >> - addr & IMX290_REG_ADDR_MASK, ret);
> >> - return ret;
> >> - }
> >> -
> >> - *value = get_unaligned_le24(data);
> >> - return 0;
> >> -}
> >> -
> >> -static int imx290_write(struct imx290 *imx290, u32 addr, u32 value, int *err)
> >> -{
> >> - u8 data[3];
> >> - int ret;
> >> -
> >> - if (err && *err)
> >> - return *err;
> >> -
> >> - put_unaligned_le24(value, data);
> >
> > We seem to be having a problem here, as the CCI helpers unconditionally
> > use big endian for the data :-(
>
> That is because that is what the specification says, from the MIPI CSI spec:
>
> """
> 6.3.2 The Transmission Byte Order for Multi-byte Register Values
>
> This is a normative section.
>
> The first byte of a CCI message is always the MS byte of a multi-byte
> register and the last byte is always the LS byte.
> """
>
> So it seems that the IMX sensors are special here and it might be best
> to just revert the conversion to the CCI helpers?
I'm fine with reverting for v6.6, as we're close to the release of v6.5.
> Alternative would be to make devm_cci_regmap_init_i2c() return a newly
> allocated struct which contains both a struct regmap * and a long
> flags and make the helpers take a pointer to that struct, combined
> with adding an endianess flag to the flags member.
I think I like the idea, it will probably help adding support for
various device-specific quirks in the future.
> >> -
> >> - ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
> >> - data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
> >> - if (ret < 0) {
> >> - dev_err(imx290->dev, "%u-bit write to 0x%04x failed: %d\n",
> >> - ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
> >> - addr & IMX290_REG_ADDR_MASK, ret);
> >> - if (err)
> >> - *err = ret;
> >> - }
> >> -
> >> - return ret;
> >> -}
> >> -
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 4/5] media: atomisp: ov2680: Convert to new CCI register access helpers
2023-06-27 12:51 [PATCH v4 0/5] media: Add MIPI CCI register access helper functions Hans de Goede
` (2 preceding siblings ...)
2023-06-27 12:51 ` [PATCH v4 3/5] media: imx290: " Hans de Goede
@ 2023-06-27 12:51 ` Hans de Goede
2023-06-27 12:51 ` [PATCH v4 5/5] media: Remove ov_16bit_addr_reg_helpers.h Hans de Goede
2023-07-02 10:51 ` [PATCH v4 0/5] media: Add MIPI CCI register access helper functions Sakari Ailus
5 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2023-06-27 12:51 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart, Rui Miguel Silva, Daniel Scally
Cc: Hans de Goede, Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
Use the new comon CCI register access helpers to replace the private
register access helpers in the ov2680 driver.
While at it also switch to using the same register address defines
as the standard drivers/media/i2c/ov2680.c driver to make merging
the 2 drivers simpler.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note atomisp-ov2680.c is going away. The main atomisp driver now
supports standard v4l2 drivers and a patch series is pending for
the main drivers/media/i2c/ov2680.c to get it into shape to
replace atomisp-ov2680.c .
The only reason to upstream this patch is because much
of the work landing in the main ov2680.c is copy -pasted
from the state of atomisp-ov2680.c *after this patch* ,
so having this in git history before deleting atomisp-ov2680.c
is helpful in case someone ever finds the need to compare
the code.
---
drivers/staging/media/atomisp/i2c/Kconfig | 1 +
.../media/atomisp/i2c/atomisp-ov2680.c | 251 ++++++++----------
drivers/staging/media/atomisp/i2c/ov2680.h | 86 +-----
3 files changed, 121 insertions(+), 217 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/Kconfig b/drivers/staging/media/atomisp/i2c/Kconfig
index 16b6b808d4a7..5aa9d367cebb 100644
--- a/drivers/staging/media/atomisp/i2c/Kconfig
+++ b/drivers/staging/media/atomisp/i2c/Kconfig
@@ -53,6 +53,7 @@ config VIDEO_ATOMISP_OV2680
tristate "Omnivision OV2680 sensor support"
depends on ACPI
depends on I2C && VIDEO_DEV
+ select V4L2_CCI_I2C
help
This is a Video4Linux2 sensor-level driver for the Omnivision
OV2680 raw camera.
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index 4cc2839937af..f933a65ac8d4 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -23,13 +23,50 @@
#include <linux/i2c.h>
#include <linux/module.h>
#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
#include <linux/types.h>
-#include <media/ov_16bit_addr_reg_helpers.h>
#include <media/v4l2-device.h>
#include "ov2680.h"
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL CCI_REG8(0x0100)
+#define OV2680_REG_SOFT_RESET CCI_REG8(0x0103)
+
+#define OV2680_REG_CHIP_ID CCI_REG16(0x300a)
+#define OV2680_REG_SC_CMMN_SUB_ID CCI_REG8(0x302a)
+
+#define OV2680_REG_EXPOSURE_PK CCI_REG24(0x3500)
+#define OV2680_REG_R_MANUAL CCI_REG8(0x3503)
+#define OV2680_REG_GAIN_PK CCI_REG16(0x350a)
+
+#define OV2680_REG_SENSOR_CTRL_0A CCI_REG8(0x370a)
+
+#define OV2680_REG_HORIZONTAL_START CCI_REG16(0x3800)
+#define OV2680_REG_VERTICAL_START CCI_REG16(0x3802)
+#define OV2680_REG_HORIZONTAL_END CCI_REG16(0x3804)
+#define OV2680_REG_VERTICAL_END CCI_REG16(0x3806)
+#define OV2680_REG_HORIZONTAL_OUTPUT_SIZE CCI_REG16(0x3808)
+#define OV2680_REG_VERTICAL_OUTPUT_SIZE CCI_REG16(0x380a)
+#define OV2680_REG_TIMING_HTS CCI_REG16(0x380c)
+#define OV2680_REG_TIMING_VTS CCI_REG16(0x380e)
+#define OV2680_REG_ISP_X_WIN CCI_REG16(0x3810)
+#define OV2680_REG_ISP_Y_WIN CCI_REG16(0x3812)
+#define OV2680_REG_X_INC CCI_REG8(0x3814)
+#define OV2680_REG_Y_INC CCI_REG8(0x3815)
+#define OV2680_REG_FORMAT1 CCI_REG8(0x3820)
+#define OV2680_REG_FORMAT2 CCI_REG8(0x3821)
+
+#define OV2680_REG_ISP_CTRL00 CCI_REG8(0x5080)
+
+#define OV2680_REG_X_WIN CCI_REG16(0x5704)
+#define OV2680_REG_Y_WIN CCI_REG16(0x5706)
+
+#define OV2680_FRAME_RATE 30
+#define OV2680_INTEGRATION_TIME_MARGIN 8
+
static const struct v4l2_rect ov2680_default_crop = {
.left = OV2680_ACTIVE_START_LEFT,
.top = OV2680_ACTIVE_START_TOP,
@@ -37,21 +74,6 @@ static const struct v4l2_rect ov2680_default_crop = {
.height = OV2680_ACTIVE_HEIGHT,
};
-static int ov2680_write_reg_array(struct i2c_client *client,
- const struct ov2680_reg *reglist)
-{
- const struct ov2680_reg *next = reglist;
- int ret;
-
- for (; next->reg != 0; next++) {
- ret = ov_write_reg8(client, next->reg, next->val);
- if (ret)
- return ret;
- }
-
- return 0;
-}
-
static void ov2680_set_bayer_order(struct ov2680_dev *sensor, struct v4l2_mbus_framefmt *fmt)
{
static const int ov2680_hv_flip_bayer_order[] = {
@@ -78,7 +100,8 @@ static int ov2680_set_vflip(struct ov2680_dev *sensor, s32 val)
if (sensor->is_streaming)
return -EBUSY;
- ret = ov_update_reg(sensor->client, OV2680_REG_FORMAT1, BIT(2), val ? BIT(2) : 0);
+ ret = cci_update_bits(sensor->regmap, OV2680_REG_FORMAT1, BIT(2),
+ val ? BIT(2) : 0, NULL);
if (ret < 0)
return ret;
@@ -93,7 +116,8 @@ static int ov2680_set_hflip(struct ov2680_dev *sensor, s32 val)
if (sensor->is_streaming)
return -EBUSY;
- ret = ov_update_reg(sensor->client, OV2680_REG_FORMAT2, BIT(2), val ? BIT(2) : 0);
+ ret = cci_update_bits(sensor->regmap, OV2680_REG_FORMAT2, BIT(2),
+ val ? BIT(2) : 0, NULL);
if (ret < 0)
return ret;
@@ -103,30 +127,29 @@ static int ov2680_set_hflip(struct ov2680_dev *sensor, s32 val)
static int ov2680_exposure_set(struct ov2680_dev *sensor, u32 exp)
{
- return ov_write_reg24(sensor->client, OV2680_REG_EXPOSURE_PK_HIGH, exp << 4);
+ return cci_write(sensor->regmap, OV2680_REG_EXPOSURE_PK, exp << 4,
+ NULL);
}
static int ov2680_gain_set(struct ov2680_dev *sensor, u32 gain)
{
- return ov_write_reg16(sensor->client, OV2680_REG_GAIN_PK, gain);
+ return cci_write(sensor->regmap, OV2680_REG_GAIN_PK, gain, NULL);
}
static int ov2680_test_pattern_set(struct ov2680_dev *sensor, int value)
{
- int ret;
+ int ret = 0;
if (!value)
- return ov_update_reg(sensor->client, OV2680_REG_ISP_CTRL00, BIT(7), 0);
+ return cci_update_bits(sensor->regmap, OV2680_REG_ISP_CTRL00,
+ BIT(7), 0, NULL);
- ret = ov_update_reg(sensor->client, OV2680_REG_ISP_CTRL00, 0x03, value - 1);
- if (ret < 0)
- return ret;
+ cci_update_bits(sensor->regmap, OV2680_REG_ISP_CTRL00, 0x03, value - 1,
+ &ret);
+ cci_update_bits(sensor->regmap, OV2680_REG_ISP_CTRL00, BIT(7), BIT(7),
+ &ret);
- ret = ov_update_reg(sensor->client, OV2680_REG_ISP_CTRL00, BIT(7), BIT(7));
- if (ret < 0)
- return ret;
-
- return 0;
+ return ret;
}
static int ov2680_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -171,17 +194,18 @@ static const struct v4l2_ctrl_ops ov2680_ctrl_ops = {
static int ov2680_init_registers(struct v4l2_subdev *sd)
{
- struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov2680_dev *sensor = to_ov2680_sensor(sd);
int ret;
- ret = ov_write_reg8(client, OV2680_SW_RESET, 0x01);
+ ret = cci_write(sensor->regmap, OV2680_REG_SOFT_RESET, 0x01, NULL);
+ if (ret < 0)
+ return ret;
/* Wait for sensor reset */
usleep_range(1000, 2000);
- ret |= ov2680_write_reg_array(client, ov2680_global_setting);
-
- return ret;
+ return regmap_multi_reg_write(sensor->regmap, ov2680_global_setting,
+ ARRAY_SIZE(ov2680_global_setting));
}
static struct v4l2_mbus_framefmt *
@@ -247,9 +271,8 @@ static void ov2680_calc_mode(struct ov2680_dev *sensor)
static int ov2680_set_mode(struct ov2680_dev *sensor)
{
- struct i2c_client *client = sensor->client;
u8 sensor_ctrl_0a, inc, fmt1, fmt2;
- int ret;
+ int ret = 0;
if (sensor->mode.binning) {
sensor_ctrl_0a = 0x23;
@@ -263,77 +286,36 @@ static int ov2680_set_mode(struct ov2680_dev *sensor)
fmt2 = 0x00;
}
- ret = ov_write_reg8(client, OV2680_REG_SENSOR_CTRL_0A, sensor_ctrl_0a);
- if (ret)
- return ret;
+ cci_write(sensor->regmap, OV2680_REG_SENSOR_CTRL_0A,
+ sensor_ctrl_0a, &ret);
+ cci_write(sensor->regmap, OV2680_REG_HORIZONTAL_START,
+ sensor->mode.h_start, &ret);
+ cci_write(sensor->regmap, OV2680_REG_VERTICAL_START,
+ sensor->mode.v_start, &ret);
+ cci_write(sensor->regmap, OV2680_REG_HORIZONTAL_END,
+ sensor->mode.h_end, &ret);
+ cci_write(sensor->regmap, OV2680_REG_VERTICAL_END,
+ sensor->mode.v_end, &ret);
+ cci_write(sensor->regmap, OV2680_REG_HORIZONTAL_OUTPUT_SIZE,
+ sensor->mode.h_output_size, &ret);
+ cci_write(sensor->regmap, OV2680_REG_VERTICAL_OUTPUT_SIZE,
+ sensor->mode.v_output_size, &ret);
+ cci_write(sensor->regmap, OV2680_REG_TIMING_HTS,
+ sensor->mode.hts, &ret);
+ cci_write(sensor->regmap, OV2680_REG_TIMING_VTS,
+ sensor->mode.vts, &ret);
+ cci_write(sensor->regmap, OV2680_REG_ISP_X_WIN, 0, &ret);
+ cci_write(sensor->regmap, OV2680_REG_ISP_Y_WIN, 0, &ret);
+ cci_write(sensor->regmap, OV2680_REG_X_INC, inc, &ret);
+ cci_write(sensor->regmap, OV2680_REG_Y_INC, inc, &ret);
+ cci_write(sensor->regmap, OV2680_REG_X_WIN,
+ sensor->mode.h_output_size, &ret);
+ cci_write(sensor->regmap, OV2680_REG_Y_WIN,
+ sensor->mode.v_output_size, &ret);
+ cci_write(sensor->regmap, OV2680_REG_FORMAT1, fmt1, &ret);
+ cci_write(sensor->regmap, OV2680_REG_FORMAT2, fmt2, &ret);
- ret = ov_write_reg16(client, OV2680_HORIZONTAL_START_H, sensor->mode.h_start);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_VERTICAL_START_H, sensor->mode.v_start);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_HORIZONTAL_END_H, sensor->mode.h_end);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_VERTICAL_END_H, sensor->mode.v_end);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_HORIZONTAL_OUTPUT_SIZE_H,
- sensor->mode.h_output_size);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_VERTICAL_OUTPUT_SIZE_H,
- sensor->mode.v_output_size);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_HTS, sensor->mode.hts);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_VTS, sensor->mode.vts);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_ISP_X_WIN, 0);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_ISP_Y_WIN, 0);
- if (ret)
- return ret;
-
- ret = ov_write_reg8(client, OV2680_X_INC, inc);
- if (ret)
- return ret;
-
- ret = ov_write_reg8(client, OV2680_Y_INC, inc);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_X_WIN, sensor->mode.h_output_size);
- if (ret)
- return ret;
-
- ret = ov_write_reg16(client, OV2680_Y_WIN, sensor->mode.v_output_size);
- if (ret)
- return ret;
-
- ret = ov_write_reg8(client, OV2680_REG_FORMAT1, fmt1);
- if (ret)
- return ret;
-
- ret = ov_write_reg8(client, OV2680_REG_FORMAT2, fmt2);
- if (ret)
- return ret;
-
- return 0;
+ return ret;
}
static int ov2680_set_fmt(struct v4l2_subdev *sd,
@@ -478,35 +460,26 @@ static int ov2680_init_cfg(struct v4l2_subdev *sd,
return ov2680_set_fmt(sd, sd_state, &fmt);
}
-static int ov2680_detect(struct i2c_client *client)
+static int ov2680_detect(struct ov2680_dev *sensor)
{
- struct i2c_adapter *adapter = client->adapter;
- u32 high = 0, low = 0;
- int ret;
- u16 id;
- u8 revision;
+ u64 chip_id, rev;
+ int ret = 0;
- if (!i2c_check_functionality(adapter, I2C_FUNC_I2C))
- return -ENODEV;
-
- ret = ov_read_reg8(client, OV2680_SC_CMMN_CHIP_ID_H, &high);
- if (ret) {
- dev_err(&client->dev, "sensor_id_high read failed (%d)\n", ret);
- return -ENODEV;
- }
- ret = ov_read_reg8(client, OV2680_SC_CMMN_CHIP_ID_L, &low);
- id = ((((u16)high) << 8) | (u16)low);
-
- if (id != OV2680_ID) {
- dev_err(&client->dev, "sensor ID error 0x%x\n", id);
+ cci_read(sensor->regmap, OV2680_REG_CHIP_ID, &chip_id, &ret);
+ cci_read(sensor->regmap, OV2680_REG_SC_CMMN_SUB_ID, &rev, &ret);
+ if (ret < 0) {
+ dev_err(sensor->dev, "failed to read chip id\n");
return -ENODEV;
}
- ret = ov_read_reg8(client, OV2680_SC_CMMN_SUB_ID, &high);
- revision = (u8)high & 0x0f;
+ if (chip_id != OV2680_CHIP_ID) {
+ dev_err(sensor->dev, "chip id: 0x%04llx does not match expected 0x%04x\n",
+ chip_id, OV2680_CHIP_ID);
+ return -ENODEV;
+ }
- dev_info(&client->dev, "sensor_revision id = 0x%x, rev= %d\n",
- id, revision);
+ dev_info(sensor->dev, "sensor_revision id = 0x%llx, rev= %lld\n",
+ chip_id, rev & 0x0f);
return 0;
}
@@ -538,11 +511,12 @@ static int ov2680_s_stream(struct v4l2_subdev *sd, int enable)
if (ret)
goto error_power_down;
- ret = ov_write_reg8(client, OV2680_SW_STREAM, OV2680_START_STREAMING);
+ ret = cci_write(sensor->regmap, OV2680_REG_STREAM_CTRL, 1,
+ NULL);
if (ret)
goto error_power_down;
} else {
- ov_write_reg8(client, OV2680_SW_STREAM, OV2680_STOP_STREAMING);
+ cci_write(sensor->regmap, OV2680_REG_STREAM_CTRL, 0, NULL);
pm_runtime_put(sensor->sd.dev);
}
@@ -563,6 +537,7 @@ static int ov2680_s_stream(struct v4l2_subdev *sd, int enable)
static int ov2680_s_config(struct v4l2_subdev *sd)
{
+ struct ov2680_dev *sensor = to_ov2680_sensor(sd);
struct i2c_client *client = v4l2_get_subdevdata(sd);
int ret;
@@ -573,7 +548,7 @@ static int ov2680_s_config(struct v4l2_subdev *sd)
}
/* config & detect sensor */
- ret = ov2680_detect(client);
+ ret = ov2680_detect(sensor);
if (ret)
dev_err(&client->dev, "ov2680_detect err s_config.\n");
@@ -586,7 +561,7 @@ static int ov2680_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
interval->interval.numerator = 1;
- interval->interval.denominator = OV2680_FPS;
+ interval->interval.denominator = OV2680_FRAME_RATE;
return 0;
}
@@ -638,7 +613,7 @@ static int ov2680_enum_frame_interval(struct v4l2_subdev *sd,
return -EINVAL;
fie->interval.numerator = 1;
- fie->interval.denominator = OV2680_FPS;
+ fie->interval.denominator = OV2680_FRAME_RATE;
return 0;
}
@@ -738,9 +713,13 @@ static int ov2680_probe(struct i2c_client *client)
if (!sensor)
return -ENOMEM;
+ sensor->regmap = devm_cci_regmap_init_i2c(client, 16);
+ if (IS_ERR(sensor->regmap))
+ return PTR_ERR(sensor->regmap);
+
mutex_init(&sensor->lock);
- sensor->client = client;
+ sensor->dev = &client->dev;
v4l2_i2c_subdev_init(&sensor->sd, client, &ov2680_ops);
/*
diff --git a/drivers/staging/media/atomisp/i2c/ov2680.h b/drivers/staging/media/atomisp/i2c/ov2680.h
index d032af245674..7815522724f7 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.h
+++ b/drivers/staging/media/atomisp/i2c/ov2680.h
@@ -24,6 +24,7 @@
#include <linux/delay.h>
#include <linux/videodev2.h>
#include <linux/spinlock.h>
+#include <media/v4l2-cci.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-device.h>
#include <media/v4l2-ctrls.h>
@@ -44,75 +45,12 @@
/* 1704 * 1294 * 30fps = 66MHz pixel clock */
#define OV2680_PIXELS_PER_LINE 1704
#define OV2680_LINES_PER_FRAME 1294
-#define OV2680_FPS 30
+
#define OV2680_SKIP_FRAMES 3
/* If possible send 16 extra rows / lines to the ISP as padding */
#define OV2680_END_MARGIN 16
-#define OV2680_FOCAL_LENGTH_NUM 334 /*3.34mm*/
-
-#define OV2680_INTEGRATION_TIME_MARGIN 8
-#define OV2680_ID 0x2680
-
-/*
- * OV2680 System control registers
- */
-#define OV2680_SW_SLEEP 0x0100
-#define OV2680_SW_RESET 0x0103
-#define OV2680_SW_STREAM 0x0100
-
-#define OV2680_SC_CMMN_CHIP_ID_H 0x300A
-#define OV2680_SC_CMMN_CHIP_ID_L 0x300B
-#define OV2680_SC_CMMN_SCCB_ID 0x302B /* 0x300C*/
-#define OV2680_SC_CMMN_SUB_ID 0x302A /* process, version*/
-
-#define OV2680_GROUP_ACCESS 0x3208 /*Bit[7:4] Group control, Bit[3:0] Group ID*/
-
-#define OV2680_REG_EXPOSURE_PK_HIGH 0x3500
-#define OV2680_REG_GAIN_PK 0x350a
-
-#define OV2680_REG_SENSOR_CTRL_0A 0x370a
-
-#define OV2680_HORIZONTAL_START_H 0x3800 /* Bit[11:8] */
-#define OV2680_HORIZONTAL_START_L 0x3801 /* Bit[7:0] */
-#define OV2680_VERTICAL_START_H 0x3802 /* Bit[11:8] */
-#define OV2680_VERTICAL_START_L 0x3803 /* Bit[7:0] */
-#define OV2680_HORIZONTAL_END_H 0x3804 /* Bit[11:8] */
-#define OV2680_HORIZONTAL_END_L 0x3805 /* Bit[7:0] */
-#define OV2680_VERTICAL_END_H 0x3806 /* Bit[11:8] */
-#define OV2680_VERTICAL_END_L 0x3807 /* Bit[7:0] */
-#define OV2680_HORIZONTAL_OUTPUT_SIZE_H 0x3808 /* Bit[11:8] */
-#define OV2680_HORIZONTAL_OUTPUT_SIZE_L 0x3809 /* Bit[7:0] */
-#define OV2680_VERTICAL_OUTPUT_SIZE_H 0x380a /* Bit[11:8] */
-#define OV2680_VERTICAL_OUTPUT_SIZE_L 0x380b /* Bit[7:0] */
-#define OV2680_HTS 0x380c
-#define OV2680_VTS 0x380e
-#define OV2680_ISP_X_WIN 0x3810
-#define OV2680_ISP_Y_WIN 0x3812
-#define OV2680_X_INC 0x3814
-#define OV2680_Y_INC 0x3815
-
-#define OV2680_FRAME_OFF_NUM 0x4202
-
-/*Flip/Mirror*/
-#define OV2680_REG_FORMAT1 0x3820
-#define OV2680_REG_FORMAT2 0x3821
-
-#define OV2680_MWB_RED_GAIN_H 0x5004/*0x3400*/
-#define OV2680_MWB_GREEN_GAIN_H 0x5006/*0x3402*/
-#define OV2680_MWB_BLUE_GAIN_H 0x5008/*0x3404*/
-#define OV2680_MWB_GAIN_MAX 0x0fff
-
-#define OV2680_REG_ISP_CTRL00 0x5080
-
-#define OV2680_X_WIN 0x5704
-#define OV2680_Y_WIN 0x5706
-#define OV2680_WIN_CONTROL 0x5708
-
-#define OV2680_START_STREAMING 0x01
-#define OV2680_STOP_STREAMING 0x00
-
/*
* ov2680 device structure.
*/
@@ -121,7 +59,8 @@ struct ov2680_dev {
struct media_pad pad;
/* Protect against concurrent changes to controls */
struct mutex lock;
- struct i2c_client *client;
+ struct device *dev;
+ struct regmap *regmap;
struct gpio_desc *powerdown;
struct fwnode_handle *ep_fwnode;
bool is_streaming;
@@ -150,19 +89,6 @@ struct ov2680_dev {
} ctrls;
};
-/**
- * struct ov2680_reg - MI sensor register format
- * @type: type of the register
- * @reg: 16-bit offset to register
- * @val: 8/16/32-bit register value
- *
- * Define a structure for sensor register initialization values
- */
-struct ov2680_reg {
- u16 reg;
- u32 val; /* @set value for read/mod/write, @mask */
-};
-
#define to_ov2680_sensor(x) container_of(x, struct ov2680_dev, sd)
static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
@@ -173,7 +99,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
return &sensor->sd;
}
-static struct ov2680_reg const ov2680_global_setting[] = {
+static const struct reg_sequence ov2680_global_setting[] = {
/* MIPI PHY, 0x10 -> 0x1c enable bp_c_hs_en_lat and bp_d_hs_en_lat */
{0x3016, 0x1c},
@@ -242,8 +168,6 @@ static struct ov2680_reg const ov2680_global_setting[] = {
/* DPC THRE RATIO 0x04 (4) -> 0x00 (0) */
{0x5792, 0x00},
-
- {}
};
#endif
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 5/5] media: Remove ov_16bit_addr_reg_helpers.h
2023-06-27 12:51 [PATCH v4 0/5] media: Add MIPI CCI register access helper functions Hans de Goede
` (3 preceding siblings ...)
2023-06-27 12:51 ` [PATCH v4 4/5] media: atomisp: ov2680: " Hans de Goede
@ 2023-06-27 12:51 ` Hans de Goede
2023-07-02 10:51 ` [PATCH v4 0/5] media: Add MIPI CCI register access helper functions Sakari Ailus
5 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2023-06-27 12:51 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart, Rui Miguel Silva, Daniel Scally
Cc: Hans de Goede, Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media, Andy Shevchenko
The helpers in this header are not used anywhere anymore,
they have been superseded by the new CCI register access helpers.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
include/media/ov_16bit_addr_reg_helpers.h | 92 -----------------------
1 file changed, 92 deletions(-)
delete mode 100644 include/media/ov_16bit_addr_reg_helpers.h
diff --git a/include/media/ov_16bit_addr_reg_helpers.h b/include/media/ov_16bit_addr_reg_helpers.h
deleted file mode 100644
index 1c60a50bd795..000000000000
--- a/include/media/ov_16bit_addr_reg_helpers.h
+++ /dev/null
@@ -1,92 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * I2C register access helpers for Omnivision OVxxxx image sensors which expect
- * a 16 bit register address in big-endian format and which have 1-3 byte
- * wide registers, in big-endian format (for the higher width registers).
- *
- * Based on the register helpers from drivers/media/i2c/ov2680.c which is:
- * Copyright (C) 2018 Linaro Ltd
- */
-#ifndef __OV_16BIT_ADDR_REG_HELPERS_H
-#define __OV_16BIT_ADDR_REG_HELPERS_H
-
-#include <asm/unaligned.h>
-#include <linux/dev_printk.h>
-#include <linux/i2c.h>
-
-static inline int ov_read_reg(struct i2c_client *client, u16 reg,
- unsigned int len, u32 *val)
-{
- u8 addr_buf[2], data_buf[4] = { };
- struct i2c_msg msgs[2];
- int ret;
-
- if (len > 4)
- return -EINVAL;
-
- put_unaligned_be16(reg, addr_buf);
-
- msgs[0].addr = client->addr;
- msgs[0].flags = 0;
- msgs[0].len = ARRAY_SIZE(addr_buf);
- msgs[0].buf = addr_buf;
-
- 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)) {
- dev_err(&client->dev, "read error: reg=0x%4x: %d\n", reg, ret);
- return -EIO;
- }
-
- *val = get_unaligned_be32(data_buf);
-
- return 0;
-}
-
-#define ov_read_reg8(s, r, v) ov_read_reg(s, r, 1, v)
-#define ov_read_reg16(s, r, v) ov_read_reg(s, r, 2, v)
-#define ov_read_reg24(s, r, v) ov_read_reg(s, r, 3, v)
-
-static inline int ov_write_reg(struct i2c_client *client, u16 reg,
- unsigned int len, u32 val)
-{
- u8 buf[6];
- int ret;
-
- if (len > 4)
- return -EINVAL;
-
- put_unaligned_be16(reg, buf);
- put_unaligned_be32(val << (8 * (4 - len)), buf + 2);
- ret = i2c_master_send(client, buf, len + 2);
- if (ret != len + 2) {
- dev_err(&client->dev, "write error: reg=0x%4x: %d\n", reg, ret);
- return -EIO;
- }
-
- return 0;
-}
-
-#define ov_write_reg8(s, r, v) ov_write_reg(s, r, 1, v)
-#define ov_write_reg16(s, r, v) ov_write_reg(s, r, 2, v)
-#define ov_write_reg24(s, r, v) ov_write_reg(s, r, 3, v)
-
-static inline int ov_update_reg(struct i2c_client *client, u16 reg, u8 mask, u8 val)
-{
- u32 readval;
- int ret;
-
- ret = ov_read_reg8(client, reg, &readval);
- if (ret < 0)
- return ret;
-
- val = (readval & ~mask) | (val & mask);
-
- return ov_write_reg8(client, reg, val);
-}
-
-#endif
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 0/5] media: Add MIPI CCI register access helper functions
2023-06-27 12:51 [PATCH v4 0/5] media: Add MIPI CCI register access helper functions Hans de Goede
` (4 preceding siblings ...)
2023-06-27 12:51 ` [PATCH v4 5/5] media: Remove ov_16bit_addr_reg_helpers.h Hans de Goede
@ 2023-07-02 10:51 ` Sakari Ailus
5 siblings, 0 replies; 14+ messages in thread
From: Sakari Ailus @ 2023-07-02 10:51 UTC (permalink / raw)
To: Hans de Goede
Cc: Sakari Ailus, Laurent Pinchart, Rui Miguel Silva, Daniel Scally,
Mauro Carvalho Chehab, Andy Shevchenko, Kate Hsuan,
Tommaso Merciai, linux-media
Hi Hans, Laurent,
On Tue, Jun 27, 2023 at 02:51:03PM +0200, Hans de Goede wrote:
> Hi Laurent, Sakari, et al.,
>
> Here is v4 of my MIPI CCI register access helper patches.
>
> New in the CCI register access helpers in v4:
> - Add V4L2_CCI_I2C Kconfig and wrap devm_cci_regmap_init_i2c()
> with if IS_ENABLED(CONFIG_V4L2_CCI_I2C)
Laurent: are you fine with this version? I see you've acked all but the
first patch.
I'll pick this to my tree then.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 14+ messages in thread