On Wed, Jul 4, 2018 at 1:46 PM, Nikolaus Voss <nikolaus.voss@xxxxxxxxxxxxxxxxxxxxx> wrote: [snip] >>> >>> Ok, in my opinion it is an elegant way of not bloating the driver when no >>> explicit handling (e.g. reading DT properties) is needed. Just adding an >>> of_device_id doesn't change any driver functionality then but only maps >>> to >>> an already existing i2c_table_id. >>> >> >> I disagree, in the case of OF for example a compatible string is >> composed of both a vendor a device, the complete tuple is what should >> be matched. >> >> But with the fallback, only the device portion would be used so both >> <foo,bar> and <baz,bar> will match against the i2c device with id >> "bar". It may or may not be correct but the vendor portion is very >> important to disambiguate. > > > Disambiguation via DT implies there is already a name collision in i2c > modalias name space, so adding an of_id would work around this collision for There wouldn't be a name collision between OF modaliases in that case since the vendor would be different (of:N*T*Cfoo,bar and of:N*T*Cbaz,bar). So it wouldn't be a problem for DT-only drivers. > DT enumeration. I2c_board_info driver binding would still be broken. The > right fix would be to remove the name collision. > Yes, for legacy drivers using board files it would be an issue. But that's unrelated to what I'm saying. What I don't think is correct is to ignore the vendor prefix since that's part of the compatible string semantics. In general, I think that there should be consistency between the firmware interface used to register the device, how the module alias uevent is reported to auto-load a driver and how the driver is matched with the device. So drivers supporting DT should have a n OF table (and export it with MODULE_DEVICE_TABLE), drivers supporting ACPI should have a ACPI table and so on. But this discussion isn't really related to your patch. I think is correct but just said that (b) wasn't a justification to leave the I2C table, points (a) and (c) are though. I won't really be convinced that the fallback is the correct thing to do or even a good idea. [snip] >>> >>> It could have been a null pointer and device driver binding (and >>> auto-loading) done just via driver.name. >>> >> >> I'm not sure I understood this comment. > > > What I was trying to say is that if the i2c_device_id table information was > not needed (i.d. only one single id), even the old probe() could be used > without defining an i2c_device_id table, the i2c_device_id* argument to > probe() being a nullptr. > I see, yes that would work too. I didn't authored the .probe_new callback so I don't know if other options were discussed. I also can't remember if the I2C core attempted to probe even without an I2C device ID table, I thought it didn't but I can be wrong. > Niko Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
