From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 077C5C0018C for ; Mon, 7 Dec 2020 06:00:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CA9C8229EF for ; Mon, 7 Dec 2020 06:00:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725881AbgLGGAp (ORCPT ); Mon, 7 Dec 2020 01:00:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45934 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725773AbgLGGAp (ORCPT ); Mon, 7 Dec 2020 01:00:45 -0500 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 480D9C0613D3 for ; Sun, 6 Dec 2020 22:00:05 -0800 (PST) Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1km9Yx-0003Dh-4W; Mon, 07 Dec 2020 06:59:55 +0100 Received: from sha by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1km9Yu-00024l-PM; Mon, 07 Dec 2020 06:59:52 +0100 Date: Mon, 7 Dec 2020 06:59:52 +0100 From: Sascha Hauer To: michael.srba@seznam.cz Cc: Dave Stevenson , Mauro Carvalho Chehab , Rob Herring , Shawn Guo , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Geert Uytterhoeven , Magnus Damm , linux-media@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-renesas-soc@vger.kernel.org Subject: Re: [PATCH v2 1/3] media: i2c: imx219: add support for specifying clock-frequencies Message-ID: <20201207055952.GB14307@pengutronix.de> References: <20201206172720.9406-1-michael.srba@seznam.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201206172720.9406-1-michael.srba@seznam.cz> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 06:33:53 up 4 days, 18:00, 39 users, load average: 0.07, 0.11, 0.14 User-Agent: Mutt/1.10.1 (2018-07-13) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: sha@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: devicetree@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Hi Michael, On Sun, Dec 06, 2020 at 06:27:18PM +0100, michael.srba@seznam.cz wrote: > From: Michael Srba > > This patch adds 1% tolerance on input clock, similar to other camera sensor > drivers. It also allows for specifying the actual clock in the device tree, > instead of relying on it being already set to the right frequency (which is > often not the case). > > Signed-off-by: Michael Srba > > --- > > changes since v1: default to exactly 24MHz when `clock-frequency` is not present > > --- > drivers/media/i2c/imx219.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index f64c0ef7a897..b6500e2ab19e 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -1443,13 +1443,28 @@ static int imx219_probe(struct i2c_client *client) > return PTR_ERR(imx219->xclk); > } > > - imx219->xclk_freq = clk_get_rate(imx219->xclk); > - if (imx219->xclk_freq != IMX219_XCLK_FREQ) { > + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", &imx219->xclk_freq); > + if (ret) { > + dev_warn(dev, "could not get xclk frequency\n"); > + > + /* default to 24MHz */ > + imx219->xclk_freq = 24000000; > + } > + > + /* this driver currently expects 24MHz; allow 1% tolerance */ > + if (imx219->xclk_freq < 23760000 || imx219->xclk_freq > 24240000) { > dev_err(dev, "xclk frequency not supported: %d Hz\n", > imx219->xclk_freq); > return -EINVAL; > } > > + ret = clk_set_rate(imx219->xclk, imx219->xclk_freq); > + if (ret) { > + dev_err(dev, "could not set xclk frequency\n"); > + return ret; > + } clk_set_rate() returns successfully when the rate change has succeeded. It tells you nothing about the actual rate that has been set. The rate could be very different from what you want to get, depending on what the hardware is able to archieve. There's clk_round_rate() that tells you which rate you'll get when you call clk_set_rate() with that value. You would have to call clk_round_rate() first and see if you are happy with the result, afterwards set the rate. From that view it doesn't make much sense to check the device tree if a number between 23760000 and 24240000 is specified there, the clk api will do rounding anyway. Also there's the assigned-clocks device tree binding, see Documentation/devicetree/bindings/clock/clock-bindings.txt. This allows you to set the desired clock rate directly in the device tree. All that's left to do in the driver is to replace the check for the exact rate with a check which allows a certain tolerance. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 97CC2C4361B for ; Mon, 7 Dec 2020 06:01:19 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 59944229C5 for ; Mon, 7 Dec 2020 06:01:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 59944229C5 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=KtQG/0KgaRyq5E/xAQoOQS45PQsWgQJhHP1Gj5lTfzs=; b=iHF3Qz4es4rT9osi5ptA3gw8o 15EH62mcg9Fi4c35ITGjMAh4STEP11jkTGwg5mvaYTtnGOHgWTR2hLTWhhgeb4JoLp6nVqf1nJzEE M1IpEE4nZTBarZT+SUlUDSSN/cUn4B7abBpG7VzIZA4WjfwagRR+3NV/ehFZpuVT1NFQEkMhZqfis j5usPB0krF9i3Xj3tGWQDSBO92i7aPPzp+CUIqPdascKQ7hXdhcLnnd7FXzye6o5clgIATD7mAUPB 8MMEWZG29gWXueMvwwh8JZyfzHidBgh6CgUQJtrmlj0P4VOm2/gRGp/VnRtFsrCZ1ZtkqYTfB7LlO uhoElmA4g==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1km9Z7-0006EQ-Lm; Mon, 07 Dec 2020 06:00:05 +0000 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1km9Z5-0006D6-DB for linux-arm-kernel@lists.infradead.org; Mon, 07 Dec 2020 06:00:04 +0000 Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1km9Yx-0003Dh-4W; Mon, 07 Dec 2020 06:59:55 +0100 Received: from sha by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1km9Yu-00024l-PM; Mon, 07 Dec 2020 06:59:52 +0100 Date: Mon, 7 Dec 2020 06:59:52 +0100 From: Sascha Hauer To: michael.srba@seznam.cz Subject: Re: [PATCH v2 1/3] media: i2c: imx219: add support for specifying clock-frequencies Message-ID: <20201207055952.GB14307@pengutronix.de> References: <20201206172720.9406-1-michael.srba@seznam.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201206172720.9406-1-michael.srba@seznam.cz> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 06:33:53 up 4 days, 18:00, 39 users, load average: 0.07, 0.11, 0.14 User-Agent: Mutt/1.10.1 (2018-07-13) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: sha@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201207_010003_471986_6A5DD28D X-CRM114-Status: GOOD ( 30.36 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, Geert Uytterhoeven , Dave Stevenson , Shawn Guo , Magnus Damm , linux-renesas-soc@vger.kernel.org, Rob Herring , NXP Linux Team , Pengutronix Kernel Team , Mauro Carvalho Chehab , Fabio Estevam , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Michael, On Sun, Dec 06, 2020 at 06:27:18PM +0100, michael.srba@seznam.cz wrote: > From: Michael Srba > > This patch adds 1% tolerance on input clock, similar to other camera sensor > drivers. It also allows for specifying the actual clock in the device tree, > instead of relying on it being already set to the right frequency (which is > often not the case). > > Signed-off-by: Michael Srba > > --- > > changes since v1: default to exactly 24MHz when `clock-frequency` is not present > > --- > drivers/media/i2c/imx219.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index f64c0ef7a897..b6500e2ab19e 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -1443,13 +1443,28 @@ static int imx219_probe(struct i2c_client *client) > return PTR_ERR(imx219->xclk); > } > > - imx219->xclk_freq = clk_get_rate(imx219->xclk); > - if (imx219->xclk_freq != IMX219_XCLK_FREQ) { > + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", &imx219->xclk_freq); > + if (ret) { > + dev_warn(dev, "could not get xclk frequency\n"); > + > + /* default to 24MHz */ > + imx219->xclk_freq = 24000000; > + } > + > + /* this driver currently expects 24MHz; allow 1% tolerance */ > + if (imx219->xclk_freq < 23760000 || imx219->xclk_freq > 24240000) { > dev_err(dev, "xclk frequency not supported: %d Hz\n", > imx219->xclk_freq); > return -EINVAL; > } > > + ret = clk_set_rate(imx219->xclk, imx219->xclk_freq); > + if (ret) { > + dev_err(dev, "could not set xclk frequency\n"); > + return ret; > + } clk_set_rate() returns successfully when the rate change has succeeded. It tells you nothing about the actual rate that has been set. The rate could be very different from what you want to get, depending on what the hardware is able to archieve. There's clk_round_rate() that tells you which rate you'll get when you call clk_set_rate() with that value. You would have to call clk_round_rate() first and see if you are happy with the result, afterwards set the rate. From that view it doesn't make much sense to check the device tree if a number between 23760000 and 24240000 is specified there, the clk api will do rounding anyway. Also there's the assigned-clocks device tree binding, see Documentation/devicetree/bindings/clock/clock-bindings.txt. This allows you to set the desired clock rate directly in the device tree. All that's left to do in the driver is to replace the check for the exact rate with a check which allows a certain tolerance. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel