diff mbox series

[v3,7/7] media: i2c: ov9282: Add regmap support

Message ID 20220722131947.2456988-8-alexander.stein@ew.tq-group.com (mailing list archive)
State New, archived
Headers show
Series OV9281 support | expand

Commit Message

Alexander Stein July 22, 2022, 1:19 p.m. UTC 
Register values apparently are stored in MSB format, hence the manual
swap when reading/writing.
regmap's endianness swapping is not applied when reading with val_bits = 8.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/media/i2c/ov9282.c | 59 +++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

Comments

Sakari Ailus July 22, 2022, 1:46 p.m. UTC | #1 
Hi Alexander,

Thanks for the update.

On Fri, Jul 22, 2022 at 03:19:47PM +0200, Alexander Stein wrote:
> Register values apparently are stored in MSB format, hence the manual
> swap when reading/writing.
> regmap's endianness swapping is not applied when reading with val_bits = 8.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
>  drivers/media/i2c/ov9282.c | 59 +++++++++++++++++++-------------------
>  1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index dbc0a4cd060f..d8eb71245aae 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -10,6 +10,7 @@
>  #include <linux/delay.h>
>  #include <linux/i2c.h>
>  #include <linux/module.h>
> +#include <linux/regmap.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
>  
> @@ -113,6 +114,7 @@ struct ov9282_mode {
>  /**
>   * struct ov9282 - ov9282 sensor device structure
>   * @dev: Pointer to generic device
> + * @regmap: sensor's regmap
>   * @sd: V4L2 sub-device
>   * @pad: Media pad. Only one pad supported
>   * @reset_gpio: Sensor reset gpio
> @@ -131,6 +133,7 @@ struct ov9282_mode {
>   */
>  struct ov9282 {
>  	struct device *dev;
> +	struct regmap *regmap;
>  	struct v4l2_subdev sd;
>  	struct media_pad pad;
>  	struct gpio_desc *reset_gpio;
> @@ -151,6 +154,11 @@ struct ov9282 {
>  	bool streaming;
>  };
>  
> +static const struct regmap_config ov9282_regmap_config = {
> +	.reg_bits = 16,
> +	.val_bits = 8,
> +};
> +
>  static const s64 link_freq[] = {
>  	OV9282_LINK_FREQ,
>  };
> @@ -297,35 +305,20 @@ static inline struct ov9282 *to_ov9282(struct v4l2_subdev *subdev)
>   */
>  static int ov9282_read_reg(struct ov9282 *ov9282, u16 reg, u32 len, u32 *val)
>  {
> -	struct i2c_client *client = v4l2_get_subdevdata(&ov9282->sd);
> -	struct i2c_msg msgs[2] = {0};
> -	u8 addr_buf[2] = {0};
> -	u8 data_buf[4] = {0};
> +	u8 data[4] = {0, 0, 0, 0};

You'll just need to initialise one of the array entries. I.e. this should
be { 0 }.

>  	int ret;
>  
>  	if (WARN_ON(len > 4))
>  		return -EINVAL;
>  
> -	put_unaligned_be16(reg, addr_buf);
> -
> -	/* Write register address */
> -	msgs[0].addr = client->addr;
> -	msgs[0].flags = 0;
> -	msgs[0].len = ARRAY_SIZE(addr_buf);
> -	msgs[0].buf = addr_buf;
> -
> -	/* Read data from register */
> -	msgs[1].addr = client->addr;
> -	msgs[1].flags = I2C_M_RD;
> -	msgs[1].len = len;
> -	msgs[1].buf = &data_buf[4 - len];
> -
> -	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> -	if (ret != ARRAY_SIZE(msgs))
> -		return -EIO;
> -
> -	*val = get_unaligned_be32(data_buf);
> +	ret = regmap_raw_read(ov9282->regmap, reg, &data[4 - len], len);
> +	if (ret < 0) {
> +		dev_err(ov9282->dev, "read from 0x%04x failed: %d\n",
> +			reg, ret);
> +		return ret;
> +	}
>  
> +	*val = get_unaligned_be32(data);
>  	return 0;
>  }
>  
> @@ -340,16 +333,19 @@ static int ov9282_read_reg(struct ov9282 *ov9282, u16 reg, u32 len, u32 *val)
>   */
>  static int ov9282_write_reg(struct ov9282 *ov9282, u16 reg, u32 len, u32 val)
>  {
> -	struct i2c_client *client = v4l2_get_subdevdata(&ov9282->sd);
> -	u8 buf[6] = {0};
> +	u8 data[4] = {0, 0, 0, 0};

Ditto.

> +	int ret;
>  
>  	if (WARN_ON(len > 4))
>  		return -EINVAL;
>  
> -	put_unaligned_be16(reg, buf);
> -	put_unaligned_be32(val << (8 * (4 - len)), buf + 2);
> -	if (i2c_master_send(client, buf, len + 2) != len + 2)
> -		return -EIO;
> +	put_unaligned_be32(val << (8 * (4 - len)), data);
> +
> +	ret = regmap_raw_write(ov9282->regmap, reg, data, len);
> +	if (ret < 0) {
> +		dev_err(ov9282->dev, "write to 0x%04x failed: %d\n",
> +			reg, ret);

Fits on one line.

> +	}
>  
>  	return 0;
>  }
> @@ -1044,6 +1040,11 @@ static int ov9282_probe(struct i2c_client *client)
>  		return -ENOMEM;
>  
>  	ov9282->dev = &client->dev;
> +	ov9282->regmap = devm_regmap_init_i2c(client, &ov9282_regmap_config);
> +	if (IS_ERR(ov9282->regmap)) {
> +		dev_err(ov9282->dev, "Unable to initialize I2C\n");
> +		return -ENODEV;
> +	}
>  
>  	/* Initialize subdev */
>  	v4l2_i2c_subdev_init(&ov9282->sd, client, &ov9282_subdev_ops);
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index dbc0a4cd060f..d8eb71245aae 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -10,6 +10,7 @@ 
 #include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 
@@ -113,6 +114,7 @@  struct ov9282_mode {
 /**
  * struct ov9282 - ov9282 sensor device structure
  * @dev: Pointer to generic device
+ * @regmap: sensor's regmap
  * @sd: V4L2 sub-device
  * @pad: Media pad. Only one pad supported
  * @reset_gpio: Sensor reset gpio
@@ -131,6 +133,7 @@  struct ov9282_mode {
  */
 struct ov9282 {
 	struct device *dev;
+	struct regmap *regmap;
 	struct v4l2_subdev sd;
 	struct media_pad pad;
 	struct gpio_desc *reset_gpio;
@@ -151,6 +154,11 @@  struct ov9282 {
 	bool streaming;
 };
 
+static const struct regmap_config ov9282_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+};
+
 static const s64 link_freq[] = {
 	OV9282_LINK_FREQ,
 };
@@ -297,35 +305,20 @@  static inline struct ov9282 *to_ov9282(struct v4l2_subdev *subdev)
  */
 static int ov9282_read_reg(struct ov9282 *ov9282, u16 reg, u32 len, u32 *val)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(&ov9282->sd);
-	struct i2c_msg msgs[2] = {0};
-	u8 addr_buf[2] = {0};
-	u8 data_buf[4] = {0};
+	u8 data[4] = {0, 0, 0, 0};
 	int ret;
 
 	if (WARN_ON(len > 4))
 		return -EINVAL;
 
-	put_unaligned_be16(reg, addr_buf);
-
-	/* Write register address */
-	msgs[0].addr = client->addr;
-	msgs[0].flags = 0;
-	msgs[0].len = ARRAY_SIZE(addr_buf);
-	msgs[0].buf = addr_buf;
-
-	/* Read data from register */
-	msgs[1].addr = client->addr;
-	msgs[1].flags = I2C_M_RD;
-	msgs[1].len = len;
-	msgs[1].buf = &data_buf[4 - len];
-
-	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
-	if (ret != ARRAY_SIZE(msgs))
-		return -EIO;
-
-	*val = get_unaligned_be32(data_buf);
+	ret = regmap_raw_read(ov9282->regmap, reg, &data[4 - len], len);
+	if (ret < 0) {
+		dev_err(ov9282->dev, "read from 0x%04x failed: %d\n",
+			reg, ret);
+		return ret;
+	}
 
+	*val = get_unaligned_be32(data);
 	return 0;
 }
 
@@ -340,16 +333,19 @@  static int ov9282_read_reg(struct ov9282 *ov9282, u16 reg, u32 len, u32 *val)
  */
 static int ov9282_write_reg(struct ov9282 *ov9282, u16 reg, u32 len, u32 val)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(&ov9282->sd);
-	u8 buf[6] = {0};
+	u8 data[4] = {0, 0, 0, 0};
+	int ret;
 
 	if (WARN_ON(len > 4))
 		return -EINVAL;
 
-	put_unaligned_be16(reg, buf);
-	put_unaligned_be32(val << (8 * (4 - len)), buf + 2);
-	if (i2c_master_send(client, buf, len + 2) != len + 2)
-		return -EIO;
+	put_unaligned_be32(val << (8 * (4 - len)), data);
+
+	ret = regmap_raw_write(ov9282->regmap, reg, data, len);
+	if (ret < 0) {
+		dev_err(ov9282->dev, "write to 0x%04x failed: %d\n",
+			reg, ret);
+	}
 
 	return 0;
 }
@@ -1044,6 +1040,11 @@  static int ov9282_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	ov9282->dev = &client->dev;
+	ov9282->regmap = devm_regmap_init_i2c(client, &ov9282_regmap_config);
+	if (IS_ERR(ov9282->regmap)) {
+		dev_err(ov9282->dev, "Unable to initialize I2C\n");
+		return -ENODEV;
+	}
 
 	/* Initialize subdev */
 	v4l2_i2c_subdev_init(&ov9282->sd, client, &ov9282_subdev_ops);