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@xxxxxxxxxxxxxxx>
> ---
> 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);
--
Sakari Ailus
