From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756321AbcDGNha (ORCPT ); Thu, 7 Apr 2016 09:37:30 -0400 Received: from nopam.alitech.com ([202.3.176.31]:14828 "EHLO nopam.alitech.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753511AbcDGNh2 (ORCPT ); Thu, 7 Apr 2016 09:37:28 -0400 split_mail: 1 Subject: Re: [PATCH] i2c: designware: do not disable adapter after transfer To: Lucas De Marchi , linux-i2c@vger.kernel.org References: <1459478866-3896-1-git-send-email-lucas.de.marchi@gmail.com> Cc: Lucas De Marchi , linux-kernel@vger.kernel.org, Wolfram Sang , Jarkko Nikula , Mika Westerberg From: Christian Ruppert Message-ID: <57066284.30403@alitech.com> Date: Thu, 7 Apr 2016 15:37:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1459478866-3896-1-git-send-email-lucas.de.marchi@gmail.com> X-MIMETrack: =?Big5?B?SXRlbWl6ZSBieSBTTVRQIFNlcnZlciBvbiBUV0FMSU5TMi9BTElfVFBFL0FMaSg=?= =?Big5?B?UmVsZWFzZSA4LjAuMkZQNnxKdWx5IDE1LCAyMDEwKSBhdCAyMDE2LzA0LzA3IKRV?= =?Big5?B?pMggMDk6Mzg6MTM=?=, =?Big5?B?U2VyaWFsaXplIGJ5IFJvdXRlciBvbiBUV0FMSU5TMi9BTElfVFBFL0FMaSg=?= =?Big5?B?UmVsZWFzZSA4LjAuMkZQNnxKdWx5IDE1LCAyMDEwKSBhdCAyMDE2LzA0LzA3IKRV?= =?Big5?B?pMggMDk6Mzg6MjY=?=, =?Big5?B?U2VyaWFsaXplIGNvbXBsZXRlIGF0IDIwMTYvMDQvMDcgpFWkyCAwOTozODoyNg==?= X-TNEFEvaluated: 1 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=windows-1252 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dear Lucas, Sorry for the late reply but I had to put our test environment back together to check this patch. I'll keep it around for a while in case you have further iterations to test. On 2016-04-01 04:47, Lucas De Marchi wrote: > From: Lucas De Marchi > > Disabling the adapter after each transfer is pretty bad for sensors and > other devices doing small transfers at a high rate. It slows down the > transfer rate a lot since each of them have to wait the adapter to be > enabled again. > > It was done in order to avoid the adapter to generate interrupts when > it's not being used. Instead of doing that here we just disable the > interrupt generation in the controller. With a small program test to > read/write registers in a sensor the speed doubled. Example below with > write sequences of 16 bytes: > > Before: > i2c-transfer-time -w -a 0x40 -x 6 -n 20000 -- 0 0 0xd0 0x07 0 0 0xd0 0x07 0 0 0xd0 0x07 0 0 0xd0 0x07 > num_transfers=20000 > transfer_time_avg=1032.728500us > > After: > i2c-transfer-time -w -a 0x40 -x 6 -n 20000 -- 0 0 0xd0 0x07 0 0 0xd0 0x07 0 0 0xd0 0x07 0 0 0xd0 0x07 > num_transfers=20000 > transfer_time_avg=470.256050us > > During the init we check the status register for no activity and TX > buffer being empty since otherwise we can't change IC_TAR dynamically. > > Signed-off-by: Lucas De Marchi > --- > > Mika / Christian, > > This is a second approach to a patch sent last year: > https://patchwork.ozlabs.org/patch/481952/ > > I hope that now it's in a better form. This has been tested on a Baytrail soc > (MinnowBoard Turbot) and is working. Would be nice to know if it also works on > Christian's machine since it was the one giving problems. Christian, could you > give this a try? It has been tested on top of 4.4.6 (+ some backports) and > 4.6.0-rc1. On Linux-4.6.0-rc2 the behaviour is still the same: The kernel locks up in an irq loop at dwi2c driver probe time. If I don't apply the patch everything works fine and the system boots and talks normally on the i2c bus. One solution might be to add a device-tree (and acpi) flag to tell the driver that it does not need to expect any nastily behaved devices on the bus. If this flag is given, the driver could use the faster disable-interrupt strategy. Without the flag we need to fall back to the conservative disable-i2c-controller strategy. I think such a flag should be OK because I2C buses are generally quite static and the list of devices should be known at system integration time. In the rare cases where this is not true, users could still go with the conservative approach. I know that the "cleaner" method would be to rather mark nasty devices, either in device tree or in the driver, but I guess the required changes in the I2C framework might be overkill for this dwi2c specific problem. Greetings, Christian