Hi Laurent,

Thank you for the review.

-----Original Message-----
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 13 March 2020 21:24
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>; Shawn Guo
[off-list ref]; Sascha Hauer [off-list ref];
Pengutronix Kernel Team [off-list ref]; Rob Herring
[off-list ref]; Mark Rutland [off-list ref]; Sakari
Ailus [off-list ref]; NXP Linux Team [off-list ref];
Magnus Damm [off-list ref]; Ezequiel Garcia
[off-list ref]; Geert Uytterhoeven [off-list ref];
devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-renesas-
soc@vger.kernel.org; Fabio Estevam [off-list ref]; linux-
media@vger.kernel.org; linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 3/4] media: i2c: ov5645: Set maximum leverage of
external clock frequency to 24480000

Hi Prabhakar,

Thank you for the patch.

On Fri, Mar 13, 2020 at 09:12:33PM +0000, Lad Prabhakar wrote:

quoted

While testing on Renesas RZ/G2E platform, noticed the clock frequency
to be 24242424 as a result the probe failed. However increasing the
maximum leverage of external clock frequency to 24480000 fixes this
issue. Since this difference is small enough and is insignificant set
the same in the driver.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-

lad.rj@bp.renesas.com>

quoted

---
 drivers/media/i2c/ov5645.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
index 4fbabf3..b49359b 100644
--- a/drivers/media/i2c/ov5645.c
+++ b/drivers/media/i2c/ov5645.c

@@ -1107,8 +1107,10 @@ static int ov5645_probe(struct i2c_client *client)
 }

 xclk_freq = clk_get_rate(ov5645->xclk);
-/* external clock must be 24MHz, allow 1% tolerance */
-if (xclk_freq < 23760000 || xclk_freq > 24240000) {
+/* external clock must be 24MHz, allow a minimum 1% and a

maximum of 2%

quoted

+ * tolerance

So where do these numbers come from ? I understand that 2% is what you
need to make your clock fit in the range, but why -1%/+2% instead of -
2%/+2% ? And why not 2.5 or 3% ? The sensor datasheet documents the
range of supported xvclk frequencies to be 6MHz to 54MHz. I understand
that PLL parameters depend on the clock frequency, but could they be
calculated instead of hardcoded, to avoid requiring an exact 24MHz input
frequency ?

To be honest I don't have the datasheet for ov5645, the flyer says 6-54Mhz but the
logs/comment says 24Mhz.

Cheers,
--Prabhakar

quoted

+ */
+if (xclk_freq < 23760000 || xclk_freq > 24480000) {
 dev_err(dev, "external clock frequency %u is not

supported\n",

quoted

 xclk_freq);
 return -EINVAL;

--
Regards,

Laurent Pinchart

Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel