On Thu, Feb 5, 2009 at 12:38 PM, Jean Delvare <khali@xxxxxxxxxxxx> wrote:
> Hi Roel,
>
> Adding back the linux-i2c list in the loop...
>
> On Wed, 4 Feb 2009 16:54:20 +0100, roel kluin wrote:
>> Jean Delvare <khali@xxxxxxxxxxxx>:
>> > Depends on the hardware implementation. If I2C_PCF_LAB can be set while
>> > I2C_PCF_PIN is set then indeed this is wrong. But I suspect the
>> > hardware is such that I2C_PCF_LAB can't be set without I2C_PCF_PIN
>> > being clear. You'd have to check the PCF8584 datasheet to make sure.
>>
>> I took a look at the datasheet, and I think this is not so:
>>
>> From Fig 1: PIN and LAB are control status registers (s1)
>>
>> From section 6.8.1.1, PIN (Pending Interrupt Not):
>> "When the PIN bit is written with a logic 1, all status bits are reset
>> to logic 0.
>> [...] PIN is the only bit in S1 which may be both read and written to."
>>
>> Later in 6.8.2.1: PIN bit:
>> Each time a serial data transmission is initiated [...], the PIN bit
>> will be set to
>> logic 1 automatically (inactive).
>> [...]
>> After transmission or reception of one byte on the I2C-bus, the PIN bit will be
>> automatically reset to logic 0 (active) indicating a complete byte
>> transmission/reception.
>>
>> and in 6.8.2.6 LAB:
>> 'Lost Arbitration' Bit. This bit is set when, in multi-master operation,
>> arbitration is lost to another master on the I2C-bus.
>>
>> Note: setting I2C_PCF_PIN clears bits, including I2C_PCF_LAB, but
>> unsetting does not.
>>
>> now consider the function
>>
>> static int wait_for_pin(struct i2c_algo_pcf_data *adap, int *status)
>> {
>>
>> int timeout = DEF_TIMEOUT;
>>
>> *status = get_pcf(adap, 1);
>>
>> while (timeout-- && (*status & I2C_PCF_PIN)) {
>> adap->waitforpin(adap->data);
>> *status = get_pcf(adap, 1);
>> }
>> if (*status & I2C_PCF_LAB) {
>> handle_lab(adap, status);
>> return -EINTR;
>> }
>>
>> if (timeout <= 0)
>> return -1;
>> else
>> return 0;
>> }
>>
>> We wait until I2C_PCF_PIN is set (transmission/reception was succesful),
>> but we want to ensure that arbitration is not lost to another master on
>> the I2C-bus.
>>
>> > I you don't have a PCF8584-based adapter to test, and aren't going to
>> > read the datasheet to see how the device is supposed to behave, and
>> > have no proof that the current code is buggy, than please don't touch
>> > it.
>>
>> I do not have a PCF8584-based adapter, but according to the datasheet
>> it does seem wrong. I do think the timeout-based return should go first.
>
> I fail to see how you come to this conclusion based on the datasheet
> elements you listed above. Anyway, I suspect that lost arbitration and
> timeout can't happen at the same time so it doesn't really matter what
> test is done first.
>
> If you really want to change the code, please discuss it with Eric
> Brower and Dan Smolik (Cc'd). They worked on arbitration loss in
> i2c-algo-pcf back in July 2008, so presumably they have systems where
> they can test this specific condition.
>
> --
> Jean Delvare
>
I do have access to such a system (for some unknown duration of time)
and I am patient enough to wait for LAB conditions, so if you'd like
to specify a test case, Roel, I'm happy to give it a whirl.
--
E
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
