lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPybu_0o+csbkyS7bbMUjB+VSUwj2DK_STy=wubCT_frH0DzgA@mail.gmail.com>
Date: Wed, 30 Oct 2024 12:25:25 +0100
From: Ricardo Ribalda Delgado <ribalda@...nel.org>
To: git@...tzsch.eu
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, Sakari Ailus <sakari.ailus@...ux.intel.com>, 
	~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org, 
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Dave Stevenson <dave.stevenson@...pberrypi.com>
Subject: Re: [PATCH v2 01/13] media: i2c: imx214: Fix link frequency

Hi Andre

On Mon, Oct 21, 2024 at 12:14 AM André Apitzsch via B4 Relay
<devnull+git.apitzsch.eu@...nel.org> wrote:
>
> From: André Apitzsch <git@...tzsch.eu>
>
> The driver defines IMX214_DEFAULT_LINK_FREQ 480000000, and then
> IMX214_DEFAULT_PIXEL_RATE ((IMX214_DEFAULT_LINK_FREQ * 8LL) / 10),
> which works out as 384MPix/s. (The 8 is 4 lanes and DDR.)
>
> Parsing the PLL registers with the defined 24MHz input. We're in single
> PLL mode, so MIPI frequency is directly linked to pixel rate.  VTCK ends
> up being 1200MHz, and VTPXCK and OPPXCK both are 120MHz.  Section 5.3
> "Frame rate calculation formula" says "Pixel rate
> [pixels/s] = VTPXCK [MHz] * 4", so 120 * 4 = 480MPix/s, which basically
> agrees with my number above.
>
> 3.1.4. MIPI global timing setting says "Output bitrate = OPPXCK * reg
> 0x113[7:0]", so 120MHz * 10, or 1200Mbit/s. That would be a link
> frequency of 600MHz due to DDR.
> That also matches to 480MPix/s * 10bpp / 4 lanes / 2 for DDR.
>
I think your calculations are correct and the value should be 600M...
but if we land this change, there will be boards that will stop
working until they update their dts.
Not sure if we allow that.

Can we move this change to the last one of the series and add something like:

diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
index 2aca3d88a0a7..8b4ded4cb5ce 100644
--- a/drivers/media/i2c/imx214.c
+++ b/drivers/media/i2c/imx214.c
@@ -1281,13 +1281,9 @@ static int imx214_parse_fwnode(struct device
*dev, struct imx214 *imx214)
                if (bus_cfg.link_frequencies[i] == IMX214_DEFAULT_LINK_FREQ)
                        break;

-       if (i == bus_cfg.nr_of_link_frequencies) {
-               dev_err_probe(dev, -EINVAL,
-                             "link-frequencies %d not supported,
Please review your DT\n",
+       if (i == bus_cfg.nr_of_link_frequencies)
+               dev_warn(dev, "link-frequencies %d not supported,
Please review your DT. Continuing anyway\n",
                              IMX214_DEFAULT_LINK_FREQ);
-               ret = -EINVAL;
-               goto done;
-       }



> Signed-off-by: André Apitzsch <git@...tzsch.eu>
> ---
>  drivers/media/i2c/imx214.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> index 4962cfe7c83d62425aeccb46a400fa93146f14ea..5d411452d342fdb177619cd1c9fd9650d31089bb 100644
> --- a/drivers/media/i2c/imx214.c
> +++ b/drivers/media/i2c/imx214.c
> @@ -24,7 +24,7 @@
>  #define IMX214_MODE_STREAMING          0x01
>
>  #define IMX214_DEFAULT_CLK_FREQ        24000000
> -#define IMX214_DEFAULT_LINK_FREQ 480000000
> +#define IMX214_DEFAULT_LINK_FREQ 600000000
>  #define IMX214_DEFAULT_PIXEL_RATE ((IMX214_DEFAULT_LINK_FREQ * 8LL) / 10)
>  #define IMX214_FPS 30
>  #define IMX214_MBUS_CODE MEDIA_BUS_FMT_SRGGB10_1X10
>
> --
> 2.47.0
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ