From patchwork Thu Nov 20 09:26:50 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Schwarz X-Patchwork-Id: 5346781 Return-Path: X-Original-To: patchwork-linux-rockchip@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 6AC879F387 for ; Thu, 20 Nov 2014 09:27:48 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 06452201BC for ; Thu, 20 Nov 2014 09:27:47 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8022F20125 for ; Thu, 20 Nov 2014 09:27:45 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1XrO1h-00033V-4y; Thu, 20 Nov 2014 09:27:45 +0000 Received: from mout.kundenserver.de ([212.227.17.24]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1XrO1e-0002Pn-MN for linux-rockchip@lists.infradead.org; Thu, 20 Nov 2014 09:27:44 +0000 Received: from typ.localnet (xdsl-78-35-53-128.netcologne.de [78.35.53.128]) by mrelayeu.kundenserver.de (node=mreue101) with ESMTP (Nemesis) id 0M1nxq-1Y7Gvw3Ayo-00tiEZ; Thu, 20 Nov 2014 10:26:51 +0100 From: Max Schwarz To: Wolfram Sang Subject: [PATCH v7] i2c: rk3x: handle dynamic clock rate changes correctly Date: Thu, 20 Nov 2014 10:26:50 +0100 Message-ID: <1827790.Sq6eyYqr17@typ> User-Agent: KMail/4.14.2 (Linux/3.15.7-031507-generic; KDE/4.14.2; x86_64; ; ) MIME-Version: 1.0 X-Provags-ID: V02:K0:bnAPS5OGVtQWytwQ231PiXD9fKpT30Zu7QvoHXYqU4U ydqJJVdSvpPFAHIabnV7/ItMmOYyRYjazuK4Q7DodfwW4YoYet L50naSqYq4sT3F3I018gd92YabVkjPkOOJKNdwRlyCJcFl/kGU izF2/BGwxc9eGaQhb0gPCb7Za8t/pP4tb/qo678NFnrTnNmZyt fvEpbSOVmLuQSejG5NKCCHLhL4WtzFG+pBJvjHQWA3xfdB7Qk9 J68KmD/9HBcPd+MtDg9k/lBOIOEIgsEdTQ6x3k9OEwnpnBdcQW btpwwxe8uBtB9KnJzkpOVycSs0Drqxxo/2eIzJOUPEzH84TKg= = X-UI-Out-Filterresults: notjunk:1; X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20141120_012743_112154_C35A8574 X-CRM114-Status: GOOD ( 23.43 ) X-Spam-Score: -0.0 (/) Cc: Huang Tao , addy ke , Heiko =?ISO-8859-1?Q?St=FCbner?= , Doug Anderson , linux-rockchip@lists.infradead.org, Linux I2C X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The i2c input clock can change dynamically, e.g. on the RK3066 where pclk_i2c0 and pclk_i2c1 are connected to the armclk, which changes rate on cpu frequency scaling. Until now, we incorrectly called clk_get_rate() while holding the i2c->lock in rk3x_i2c_xfer() to adapt to clock rate changes. Thanks to Huang Tao for reporting this issue. Do it properly now using the clk notifier framework. The callback logic was taken from i2c-cadence.c. Also rename all misleading "i2c_rate" variables to "clk_rate", as they describe the *input* clk rate. Signed-off-by: Max Schwarz Tested-by: Max Schwarz on RK3188 Tested-by: Doug Anderson on RK3288 Reviewed-by: Doug Anderson --- Changes since v6: - "best effort" logic in rk3x_i2c_calc_divs() (sugg. by Doug Anderson) - fix range check in rk3x_i2c_calc_divs() (sugg. by Doug Anderson) Changes since v5: - document rk3x_i2c_calc_divs (Doug Anderson) - rename all i2c_rate to clk_rate (Doug Anderson) - remove obsolete braces in rk3x_i2c_clk_notifier_cb (Doug Anderson) Changes since v4: - fixed a bug in rk3x_i2c_clk_notifier_cb() which would feed the input clock frequency as the desired SCL frequency into rk3x_i2c_set_scl_frequency(i2c, scl_rate). Rename & change to rk3x_i2c_adapt_div(i2c, clk_rate) to make things clear. - trimmed comment in rk3x_i2c_adapt_div() (sugg. by Doug Anderson) Changes since v3: - drop leftover write-only clk_freq variable (sugg. by Doug Anderson) Changes since v2: - allow rate changes which result in lower than desired SCL frequencies (sugg. by Doug Anderson) - simplified divider range checks (Doug Anderson) - removed duplicate clk_enable()/disable() (Doug Anderson) - added missing unregister in rk3x_i2c_remove() (Doug Anderson) Changes since v1: - make sure the i2c input clock is active during prescaler register write by explicitly enabling/disabling it in rk3x_i2c_set_scl_frequency(). Bug found by Addy Ke. Here are waveforms from rate change tests: Rate change from default rate to half rate: http://x-quadraht.de/fast_to_slow.png From half rate to default rate: http://x-quadraht.de/slow_to_fast.png The third and fourth line show the begin and end of the clk_set_rate() call, respectively. You can see nicely that both rate transitions result in a few slower SCL cycles, but never in faster ones. Cheers, Max drivers/i2c/busses/i2c-rk3x.c | 153 ++++++++++++++++++++++++++++++++---------- 1 file changed, 118 insertions(+), 35 deletions(-) diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index e276ffb..0ee5802 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -98,6 +98,7 @@ struct rk3x_i2c { /* Hardware resources */ void __iomem *regs; struct clk *clk; + struct notifier_block clk_rate_nb; /* Settings */ unsigned int scl_frequency; @@ -429,15 +430,27 @@ out: return IRQ_HANDLED; } -static int rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate, - unsigned long *div_low, unsigned long *div_high) +/** + * Calculate divider values for desired SCL frequency + * + * @clk_rate: I2C input clock rate + * @scl_rate: Desired SCL rate + * @div_low: Divider output for low + * @div_high: Divider output for high + * + * Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that case + * a best-effort divider value is returned in divs. If the target rate is + * too high, we silently use the highest possible rate. + */ +static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate, + unsigned long *div_low, unsigned long *div_high) { unsigned long min_low_ns, min_high_ns; unsigned long max_data_hold_ns; unsigned long data_hold_buffer_ns; unsigned long max_low_ns, min_total_ns; - unsigned long i2c_rate_khz, scl_rate_khz; + unsigned long clk_rate_khz, scl_rate_khz; unsigned long min_low_div, min_high_div; unsigned long max_low_div; @@ -445,6 +458,8 @@ static int rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate, unsigned long min_div_for_hold, min_total_div; unsigned long extra_div, extra_low_div, ideal_low_div; + int ret = 0; + /* Only support standard-mode and fast-mode */ if (WARN_ON(scl_rate > 400000)) scl_rate = 400000; @@ -480,25 +495,25 @@ static int rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate, min_total_ns = min_low_ns + min_high_ns; /* Adjust to avoid overflow */ - i2c_rate_khz = DIV_ROUND_UP(i2c_rate, 1000); + clk_rate_khz = DIV_ROUND_UP(clk_rate, 1000); scl_rate_khz = scl_rate / 1000; /* * We need the total div to be >= this number * so we don't clock too fast. */ - min_total_div = DIV_ROUND_UP(i2c_rate_khz, scl_rate_khz * 8); + min_total_div = DIV_ROUND_UP(clk_rate_khz, scl_rate_khz * 8); /* These are the min dividers needed for min hold times. */ - min_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, 8 * 1000000); - min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 1000000); + min_low_div = DIV_ROUND_UP(clk_rate_khz * min_low_ns, 8 * 1000000); + min_high_div = DIV_ROUND_UP(clk_rate_khz * min_high_ns, 8 * 1000000); min_div_for_hold = (min_low_div + min_high_div); /* * This is the maximum divider so we don't go over the max. * We don't round up here (we round down) since this is a max. */ - max_low_div = i2c_rate_khz * max_low_ns / (8 * 1000000); + max_low_div = clk_rate_khz * max_low_ns / (8 * 1000000); if (min_low_div > max_low_div) { WARN_ONCE(true, @@ -526,7 +541,7 @@ static int rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate, * biasing slightly towards having a higher div * for low (spend more time low). */ - ideal_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, + ideal_low_div = DIV_ROUND_UP(clk_rate_khz * min_low_ns, scl_rate_khz * 8 * min_total_ns); /* Don't allow it to go over the max */ @@ -547,40 +562,99 @@ static int rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate, } /* - * Adjust to the fact that the hardware has an implicit "+1". - * NOTE: Above calculations always produce div_low > 0 and div_high > 0. - */ + * Adjust to the fact that the hardware has an implicit "+1". + * NOTE: Above calculations always produce div_low > 0 and div_high > 0. + */ *div_low = *div_low - 1; *div_high = *div_high - 1; - if (*div_low >= 0xffff || *div_high >= 0xffff) - return -EINVAL; - else - return 0; + /* Maximum divider supported by hw is 0xffff */ + if (*div_low > 0xffff) { + *div_low = 0xffff; + ret = -EINVAL; + } + + if (*div_high > 0xffff) { + *div_high = 0xffff; + ret = -EINVAL; + } + + return ret; } -static int rk3x_i2c_set_scl_rate(struct rk3x_i2c *i2c, unsigned long scl_rate) +static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate) { - unsigned long i2c_rate = clk_get_rate(i2c->clk); unsigned long div_low, div_high; u64 t_low_ns, t_high_ns; - int ret = 0; + int ret; - ret = rk3x_i2c_calc_divs(i2c_rate, scl_rate, &div_low, &div_high); - if (ret < 0) - return ret; + ret = rk3x_i2c_calc_divs(clk_rate, i2c->scl_frequency, &div_low, + &div_high); + WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->scl_frequency); + + clk_enable(i2c->clk); i2c_writel(i2c, (div_high << 16) | (div_low & 0xffff), REG_CLKDIV); + clk_disable(i2c->clk); - t_low_ns = div_u64(((u64)div_low + 1) * 8 * 1000000000, i2c_rate); - t_high_ns = div_u64(((u64)div_high + 1) * 8 * 1000000000, i2c_rate); + t_low_ns = div_u64(((u64)div_low + 1) * 8 * 1000000000, clk_rate); + t_high_ns = div_u64(((u64)div_high + 1) * 8 * 1000000000, clk_rate); dev_dbg(i2c->dev, - "CLK %lukhz, Req %luns, Act low %lluns high %lluns\n", - i2c_rate / 1000, - 1000000000 / scl_rate, + "CLK %lukhz, Req %uns, Act low %lluns high %lluns\n", + clk_rate / 1000, + 1000000000 / i2c->scl_frequency, t_low_ns, t_high_ns); +} - return ret; +/** + * rk3x_i2c_clk_notifier_cb - Clock rate change callback + * @nb: Pointer to notifier block + * @event: Notification reason + * @data: Pointer to notification data object + * + * The callback checks whether a valid bus frequency can be generated after the + * change. If so, the change is acknowledged, otherwise the change is aborted. + * New dividers are written to the HW in the pre- or post change notification + * depending on the scaling direction. + * + * Code adapted from i2c-cadence.c. + * + * Return: NOTIFY_STOP if the rate change should be aborted, NOTIFY_OK + * to acknowedge the change, NOTIFY_DONE if the notification is + * considered irrelevant. + */ +static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long + event, void *data) +{ + struct clk_notifier_data *ndata = data; + struct rk3x_i2c *i2c = container_of(nb, struct rk3x_i2c, clk_rate_nb); + unsigned long div_low, div_high; + + switch (event) { + case PRE_RATE_CHANGE: + if (rk3x_i2c_calc_divs(ndata->new_rate, i2c->scl_frequency, + &div_low, &div_high) != 0) { + return NOTIFY_STOP; + } + + /* scale up */ + if (ndata->new_rate > ndata->old_rate) + rk3x_i2c_adapt_div(i2c, ndata->new_rate); + + return NOTIFY_OK; + case POST_RATE_CHANGE: + /* scale down */ + if (ndata->new_rate < ndata->old_rate) + rk3x_i2c_adapt_div(i2c, ndata->new_rate); + return NOTIFY_OK; + case ABORT_RATE_CHANGE: + /* scale up */ + if (ndata->new_rate > ndata->old_rate) + rk3x_i2c_adapt_div(i2c, ndata->old_rate); + return NOTIFY_OK; + default: + return NOTIFY_DONE; + } } /** @@ -677,11 +751,6 @@ static int rk3x_i2c_xfer(struct i2c_adapter *adap, clk_enable(i2c->clk); - /* The clock rate might have changed, so setup the divider again */ - ret = rk3x_i2c_set_scl_rate(i2c, i2c->scl_frequency); - if (ret < 0) - goto exit; - i2c->is_last_msg = false; /* @@ -728,7 +797,6 @@ static int rk3x_i2c_xfer(struct i2c_adapter *adap, } } -exit: clk_disable(i2c->clk); spin_unlock_irqrestore(&i2c->lock, flags); @@ -768,6 +836,7 @@ static int rk3x_i2c_probe(struct platform_device *pdev) int bus_nr; u32 value; int irq; + unsigned long clk_rate; i2c = devm_kzalloc(&pdev->dev, sizeof(struct rk3x_i2c), GFP_KERNEL); if (!i2c) @@ -868,16 +937,28 @@ static int rk3x_i2c_probe(struct platform_device *pdev) return ret; } + i2c->clk_rate_nb.notifier_call = rk3x_i2c_clk_notifier_cb; + ret = clk_notifier_register(i2c->clk, &i2c->clk_rate_nb); + if (ret != 0) { + dev_err(&pdev->dev, "Unable to register clock notifier\n"); + goto err_clk; + } + + clk_rate = clk_get_rate(i2c->clk); + rk3x_i2c_adapt_div(i2c, clk_rate); + ret = i2c_add_adapter(&i2c->adap); if (ret < 0) { dev_err(&pdev->dev, "Could not register adapter\n"); - goto err_clk; + goto err_clk_notifier; } dev_info(&pdev->dev, "Initialized RK3xxx I2C bus at %p\n", i2c->regs); return 0; +err_clk_notifier: + clk_notifier_unregister(i2c->clk, &i2c->clk_rate_nb); err_clk: clk_unprepare(i2c->clk); return ret; @@ -888,6 +969,8 @@ static int rk3x_i2c_remove(struct platform_device *pdev) struct rk3x_i2c *i2c = platform_get_drvdata(pdev); i2c_del_adapter(&i2c->adap); + + clk_notifier_unregister(i2c->clk, &i2c->clk_rate_nb); clk_unprepare(i2c->clk); return 0;