i2c probing

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;
       }

Hi Matthieu,

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.

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