From patchwork Wed Dec 3 02:37:17 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: addy ke X-Patchwork-Id: 5426441 Return-Path: X-Original-To: patchwork-linux-rockchip@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id AAEBEBEEA8 for ; Wed, 3 Dec 2014 02:38:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A92A72011B for ; Wed, 3 Dec 2014 02:38:56 +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 A16E42010C for ; Wed, 3 Dec 2014 02:38:55 +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 1XvzqB-0003y9-5s; Wed, 03 Dec 2014 02:38:55 +0000 Received: from regular1.263xmail.com ([211.150.99.137]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Xvzpw-0003mG-UR; Wed, 03 Dec 2014 02:38:42 +0000 Received: from addy.ke?rock-chips.com (unknown [192.168.167.190]) by regular1.263xmail.com (Postfix) with SMTP id 6165F6449; Wed, 3 Dec 2014 10:37:57 +0800 (CST) X-263anti-spam: KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 4 Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.263.net (Postfix) with ESMTP id 1A82B29A88; Wed, 3 Dec 2014 10:37:44 +0800 (CST) X-RL-SENDER: addy.ke@rock-chips.com X-FST-TO: wsa@the-dreams.de X-SENDER-IP: 127.0.0.1 X-LOGIN-NAME: addy.ke@rock-chips.com X-UNIQUE-TAG: <36f88272de4824b2d382bd2e1e1d8cc0> X-ATTACHMENT-NUM: 0 X-SENDER: kfx@rock-chips.com X-DNS-TYPE: 0 Received: from localhost.localdomain (unknown [127.0.0.1]) by smtp.263.net (Postfix) whith ESMTP id 25754M1DXJX; Wed, 03 Dec 2014 10:37:55 +0800 (CST) From: Addy Ke To: wsa@the-dreams.de, max.schwarz@online.de, heiko@sntech.de, olof@lixom.net, dianders@chromium.org, robh+dt@kernel.org, pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org Subject: [PATCH v2] i2c: rk3x: fix bug that cause measured high_ns doesn't meet I2C spec Date: Wed, 3 Dec 2014 10:37:17 +0800 Message-Id: <1417574237-4328-1-git-send-email-addy.ke@rock-chips.com> X-Mailer: git-send-email 1.8.3.2 In-Reply-To: <1415261514-4051-1-git-send-email-addy.ke@rock-chips.com> References: <1415261514-4051-1-git-send-email-addy.ke@rock-chips.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20141202_183841_791797_3328A6AF X-CRM114-Status: GOOD ( 14.17 ) X-Spam-Score: -0.0 (/) Cc: huangtao@rock-chips.com, Addy Ke , hl@rock-chips.com, yzq@rock-chips.com, zyw@rock-chips.com, linux-kernel@vger.kernel.org, kever.yang@rock-chips.com, linux-rockchip@lists.infradead.org, xjq@rock-chips.com, linux-i2c@vger.kernel.org, caesar.wang@rock-chips.com, cf@rock-chips.com, hj@rock-chips.com, zhengsq@rock-chips.com, linux-arm-kernel@lists.infradead.org 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: , MIME-Version: 1.0 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 high_ns calculated from the low division of CLKDIV register is the sum of actual measured high_ns and rise_ns. The rise time which related to external pull-up resistor can be up to the maximum rise time in I2C spec. In my test, if external pull-up resistor is 4.7K, rise_ns is about 700ns. So the actual measured high_ns is about 3900ns, which is less than 4000ns(the minimum high_ns in I2C spec). To fix this bug, min_low_ns should include fall time and min_high_ns should include rise time too. This patch merged the patch that Doug submitted to chromium, which can get the rise and fall times for signals from the device tree. This allows us to more accurately calculate timings. see: https://chromium-review.googlesource.com/#/c/232774/ Signed-off-by: Addy Ke Reviewed-by: Doug Anderson --- Changes in v2: - merged the patch that Doug submitted to chromium Documentation/devicetree/bindings/i2c/i2c-rk3x.txt | 10 ++++ drivers/i2c/busses/i2c-rk3x.c | 55 +++++++++++++++------- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt b/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt index dde6c22..faf5997 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt +++ b/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt @@ -21,6 +21,13 @@ Required on RK3066, RK3188 : Optional properties : - clock-frequency : SCL frequency to use (in Hz). If omitted, 100kHz is used. + - rise-ns : Number of nanoseconds the signal takes to rise (t(r) in i2c spec). + If not specified this is assumed to be the max the spec allows + (1000 ns for standard mode, 300 ns for fast mode) which might + cause slightly slower communication. + - fall-ns : Number of nanoseconds the signal takes to fall (t(f) in the i2c0 + spec). If not specified this is assumed to be the max the spec + allows (300 ns) which might cause slightly slower communication. Example: @@ -39,4 +46,7 @@ i2c0: i2c@2002d000 { clock-names = "i2c"; clocks = <&cru PCLK_I2C0>; + + rise-ns = <800>; + fall-ns = <100>; }; diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index 0ee5802..025d4b5 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -102,6 +102,8 @@ struct rk3x_i2c { /* Settings */ unsigned int scl_frequency; + unsigned int rise_ns; + unsigned int fall_ns; /* Synchronization & notification */ spinlock_t lock; @@ -435,6 +437,8 @@ out: * * @clk_rate: I2C input clock rate * @scl_rate: Desired SCL rate + * @rise_ns: How many ns it takes for signals to rise. + * @fall_ns: How many ns it takes for signals to fall. * @div_low: Divider output for low * @div_high: Divider output for high * @@ -443,11 +447,14 @@ out: * 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 rise_ns, unsigned long fall_ns, unsigned long *div_low, unsigned long *div_high) { + unsigned long spec_min_low_ns, spec_min_high_ns; + unsigned long spec_max_data_hold_ns; + unsigned long spec_data_hold_buffer_ns; + 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 clk_rate_khz, scl_rate_khz; @@ -470,28 +477,30 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate, /* * min_low_ns: The minimum number of ns we need to hold low - * to meet i2c spec + * to meet i2c spec, should include fall time. * min_high_ns: The minimum number of ns we need to hold high - * to meet i2c spec + * to meet i2c spec, should include rise time. * max_low_ns: The maximum number of ns we can hold low - * to meet i2c spec + * to meet i2c spec. * * Note: max_low_ns should be (max data hold time * 2 - buffer) * This is because the i2c host on Rockchip holds the data line * for half the low time. */ if (scl_rate <= 100000) { - min_low_ns = 4700; - min_high_ns = 4000; - max_data_hold_ns = 3450; - data_hold_buffer_ns = 50; + spec_min_low_ns = 4700; + spec_min_high_ns = 4000; + spec_max_data_hold_ns = 3450; + spec_data_hold_buffer_ns = 50; } else { - min_low_ns = 1300; - min_high_ns = 600; - max_data_hold_ns = 900; - data_hold_buffer_ns = 50; + spec_min_low_ns = 1300; + spec_min_high_ns = 600; + spec_max_data_hold_ns = 900; + spec_data_hold_buffer_ns = 50; } - max_low_ns = max_data_hold_ns * 2 - data_hold_buffer_ns; + min_low_ns = spec_min_low_ns + fall_ns; + min_high_ns = spec_min_high_ns + rise_ns; + max_low_ns = spec_max_data_hold_ns * 2 - spec_data_hold_buffer_ns; min_total_ns = min_low_ns + min_high_ns; /* Adjust to avoid overflow */ @@ -588,8 +597,8 @@ static void rk3x_i2c_adapt_div(struct rk3x_i2c *i2c, unsigned long clk_rate) u64 t_low_ns, t_high_ns; int ret; - ret = rk3x_i2c_calc_divs(clk_rate, i2c->scl_frequency, &div_low, - &div_high); + ret = rk3x_i2c_calc_divs(clk_rate, i2c->scl_frequency, i2c->rise_ns, + i2c->fall_ns, &div_low, &div_high); WARN_ONCE(ret != 0, "Could not reach SCL freq %u", i2c->scl_frequency); @@ -633,9 +642,9 @@ static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long switch (event) { case PRE_RATE_CHANGE: if (rk3x_i2c_calc_divs(ndata->new_rate, i2c->scl_frequency, - &div_low, &div_high) != 0) { + i2c->rise_ns, i2c->fall_ns, &div_low, + &div_high) != 0) return NOTIFY_STOP; - } /* scale up */ if (ndata->new_rate > ndata->old_rate) @@ -859,6 +868,16 @@ static int rk3x_i2c_probe(struct platform_device *pdev) i2c->scl_frequency = DEFAULT_SCL_RATE; } + /* Read rise and fall ns; if not there use the default max from spec */ + if (of_property_read_u32(pdev->dev.of_node, "rise-ns", &i2c->rise_ns)) { + if (i2c->scl_frequency <= 100000) + i2c->rise_ns = 1000; + else + i2c->rise_ns = 300; + } + if (of_property_read_u32(pdev->dev.of_node, "fall-ns", &i2c->fall_ns)) + i2c->fall_ns = 300; + strlcpy(i2c->adap.name, "rk3x-i2c", sizeof(i2c->adap.name)); i2c->adap.owner = THIS_MODULE; i2c->adap.algo = &rk3x_i2c_algorithm;