Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756105Ab2EBTGS (ORCPT ); Wed, 2 May 2012 15:06:18 -0400 Received: from mail-lpp01m010-f46.google.com ([209.85.215.46]:42556 "EHLO mail-lpp01m010-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755587Ab2EBTGQ (ORCPT ); Wed, 2 May 2012 15:06:16 -0400 MIME-Version: 1.0 In-Reply-To: <20120502181435.GF16981@andi> References: <1335967293-7728-1-git-send-email-peter.huewe@infineon.com> <1335967416-7564-1-git-send-email-peter.huewe@infineon.com> <20120502151734.GD16981@andi> <20120502181435.GF16981@andi> Date: Wed, 2 May 2012 12:06:14 -0700 X-Google-Sender-Auth: QrtsdSvpzLLTL_yWHOuoV3hNcJ0 Message-ID: Subject: Re: [PATCH] CHROMIUM: tpm: tpm_tis_i2c: Lock the I2C adapter for a sequence of requests. From: Bryan Freed To: Bryan Freed , Peter Huewe , Rajiv Andrade , tpmdd@selhorst.net, tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Olof Johansson , Luigi Semenzato Content-Type: text/plain; charset=ISO-8859-1 X-System-Of-Record: true Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2529 Lines: 62 Hi Andi, On Wed, May 2, 2012 at 11:14 AM, Andi Shyti wrote: > Hi Bryan, > >> > try to have a look to the i2c_smbus* function, you could avoid >> > lot of code > On Wed, May 02, 2012 at 10:25:09AM -0700, Bryan Freed wrote: >> (Sorry for resending...) >> >> Andi, it is not clear what i2c_smbus_* functions in particular will >> improve upon this change. >> >> All i2c_smbus_* functions go through i2c_smbus_xfer() which at some >> point will i2c_lock_adapter() for each request. >> This is true for adapters that support SMBUS (where the lock occurs >> directly in i2c_smbus_xfer()) or just I2C (where the lock occurs in >> i2c_transfer() called through i2c_smbus_xfer_emulated()). >> >> The goal of this change is for the tpm_tis_i2c driver to be able to >> lock an adapter over the duration of several I2C requests. >> Presumably, that is why i2c_lock_adapter() is exported. > > the i2c_smbus_* functions will not improve anything to the > driver, it will increase the readability and it will allow you to > do the same stuff with less code. I think I see what you are saying. We could (in the unmodified version of this driver) replace all the iic_tpm_read() calls of one byte (which sends an address byte before reading a data byte) with an i2c_smbus_read_byte_command() call which does the same thing. Switching to the SMBUS calls in this driver will still work on adapters that only support I2C because of i2c_smbus_xfer_emulated(). Right? > All the i2c communication implementation you wrote here, is > already written in the i2c-core.c file. Right. Unfortunately, we cannot use any of the i2c_smbus_* functions in this driver. The device will fail because the adapter lock is not held long enough to prevent I2C traffic going to other devices on the same bus. That other traffic to other devices confuses the i2c core in this device. Our only driver solution is to lock the adapter for a longer duration. Yes, the code we have here is copied from the i2c-core.c file. In fact, we comment this with, "Copy i2c-core:i2c_transfer() as close as possible without the adapter locks and algorithm check". And that really is the problem with using the i2c-core.c calls. This driver needs to lock the adapter for an extended duration. > Andi bryan. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/