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@xxxxxxxxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> 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
