i2c probing

i2c probing

[matthieu castet]

Hi,

Do you know why there is 2 methods of probing i2c [1] and [2], with different quirks for eeprom.
Why can't they be merged together ?


Thanks

Matthieu

PS : please keep me in CC


[1]
i2c_detect_address

    /* Make sure the address is valid */
    if (addr < 0x03 || addr > 0x77) {
        dev_warn(&adapter->dev, "Invalid probe address 0x%02x\n",
             addr);
        return -EINVAL;
    }

    /* Skip if already in use */
    if (i2c_check_addr(adapter, addr))
        return 0;

    /* Make sure there is something at this address */
    if (i2c_smbus_xfer(adapter, addr, 0, 0, 0, I2C_SMBUS_QUICK, NULL) < 0)
        return 0;

    /* Prevent 24RF08 corruption */
    if ((addr & ~0x0f) == 0x50)
        i2c_smbus_xfer(adapter, addr, 0, 0, 0, I2C_SMBUS_QUICK, NULL);

[2]
i2c_new_probed_device
        if (addr_list[i] < 0x03 || addr_list[i] > 0x77) {
            dev_warn(&adap->dev, "Invalid 7-bit address "
                 "0x%02x\n", addr_list[i]);
            continue;
        }

        /* Check address availability */
        if (i2c_check_addr(adap, addr_list[i])) {
            dev_dbg(&adap->dev, "Address 0x%02x already in "
                "use, not probing\n", addr_list[i]);
            continue;
        }

        /* Test address responsiveness
           The default probe method is a quick write, but it is known
           to corrupt the 24RF08 EEPROMs due to a state machine bug,
           and could also irreversibly write-protect some EEPROMs, so
           for address ranges 0x30-0x37 and 0x50-0x5f, we use a byte
           read instead. Also, some bus drivers don't implement
           quick write, so we fallback to a byte read it that case
           too. */
        if ((addr_list[i] & ~0x07) == 0x30
         || (addr_list[i] & ~0x0f) == 0x50
         || !i2c_check_functionality(adap, I2C_FUNC_SMBUS_QUICK)) {
            union i2c_smbus_data data;

            if (i2c_smbus_xfer(adap, addr_list[i], 0,
                       I2C_SMBUS_READ, 0,
                       I2C_SMBUS_BYTE, &data) >= 0)
                break;
        } else {
            if (i2c_smbus_xfer(adap, addr_list[i], 0,
                       I2C_SMBUS_WRITE, 0,
                       I2C_SMBUS_QUICK, NULL) >= 0)
                break;
        }

Re: i2c probing

[Jean Delvare]

Hi Matthieu,

On Sun, 21 Mar 2010 15:22:00 +0100, matthieu castet wrote:
> Do you know why there is 2 methods of probing i2c [1] and [2], with different quirks for eeprom.

Historical reasons, mainly. i2c_detect() is an old thing, it is used by
device drivers which want to automatically probe for devices they
support (and provide a reliable detection function to this purpose.)
Mainly hardware monitoring drivers do this. i2c_new_probed_device() is
more recent, it is meant for bus drivers which want to probe for
device presence at a selected address. There is no detection function
involved in this case.

i2c_detect() is using the device presence detection method that has
been used since 1999 or so. For i2c_new_probed_device(), we learned
from our past mistakes and opted for a safer approach. But in practice,
both should work equally fine.

> Why can't they be merged together ?

I don't really remember, and in all honesty, I did not realize the
legacy probing method was still present until I read your post. I guess
that I didn't dare changing the legacy probing code originally. Now
that we have good results with the new one, I agree that it would be
nice to merge both (that is, get rid of the old one and use the new one
everywhere.)

Would you be interested in working on this? You can register yourself
on the wiki [1] if you want, and add an action item. Or just send a
patch right away if you think it is simple enough.

Note that I have been working lately on letting callers of
i2c_new_probed_device() pass a custom probe function. These changes
will certainly collide with the ones you are suggesting. So if you want
to work on this, please let me know so that we can coordinate our
efforts.

[1] https://i2c.wiki.kernel.org/index.php/Main_Page

Thanks,
-- 
Jean Delvare