Re: [PATCH 03/19] media: i2c: imx290: Specify HMAX values in decimal

Alexander Stein
2022-07-21T09:18:43Z

Hi Laurent,

thanks for working on this, I've also some patches on top for imx327 support.

Am Donnerstag, 21. Juli 2022, 10:35:24 CEST schrieb Laurent Pinchart:
> The HMAX value specifies the total line length in pixels. It's thus more
> readable in decimal than hexadecimal. Fix it.

I understand the motivation, pixels are more natural in decimal numbers, but
e.g. what is 4400 pixels on this sensor? It is only mentioned sparsely and
3300d is not mentioned at all, 0ce4h is instead.
I also like to have the hexadecimal numbers here as you can find them directly
in the datasheet. To me this seems more sensible for cross-checking. Just my
opinion.

Best regards,
Alexander

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/i2c/imx290.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index 64bd43813dbf..f60a4135dc9c 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -308,7 +308,7 @@ static const struct imx290_mode imx290_modes_2lanes[] =
> { {
> .width = 1920,
> .height = 1080,
> - .hmax = 0x1130,
> + .hmax = 4400,
> .link_freq_index = FREQ_INDEX_1080P,
> .data = imx290_1080p_settings,
> .data_size = ARRAY_SIZE(imx290_1080p_settings),
> @@ -316,7 +316,7 @@ static const struct imx290_mode imx290_modes_2lanes[] =
> { {
> .width = 1280,
> .height = 720,
> - .hmax = 0x19c8,
> + .hmax = 6600,
> .link_freq_index = FREQ_INDEX_720P,
> .data = imx290_720p_settings,
> .data_size = ARRAY_SIZE(imx290_720p_settings),
> @@ -327,7 +327,7 @@ static const struct imx290_mode imx290_modes_4lanes[] =
> { {
> .width = 1920,
> .height = 1080,
> - .hmax = 0x0898,
> + .hmax = 2200,
> .link_freq_index = FREQ_INDEX_1080P,
> .data = imx290_1080p_settings,
> .data_size = ARRAY_SIZE(imx290_1080p_settings),
> @@ -335,7 +335,7 @@ static const struct imx290_mode imx290_modes_4lanes[] =
> { {
> .width = 1280,
> .height = 720,
> - .hmax = 0x0ce4,
> + .hmax = 3300,
> .link_freq_index = FREQ_INDEX_720P,
> .data = imx290_720p_settings,
> .data_size = ARRAY_SIZE(imx290_720p_settings),