Hi Andy, On Wed, Feb 22, 2023 at 8:21 PM Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > On Wed, Feb 22, 2023 at 07:46:25PM +0100, Krzysztof Kozlowski wrote: > > On 22/02/2023 18:20, Andy Shevchenko wrote: > > >>> Which effectively breaks i.e. user-space instantiation for other display > > >>> types which now do work due to i2c_of_match_device(). > > >>> (so my suggestion above is not sufficient). > > >>> > > >>> Are you proposing extending and searching the I2C ID table to work around > > >>> that? > > >> > > >> See (1) above. This is the downside I have noticed after sending this series. > > >> So, the I²C ID table match has to be restored, but the above mentioned issues > > >> with existing table are not gone, hence they need to be addressed in the next > > >> version. > > > > > > I see now what you mean. So, we have even more issues in this driver: > > > - I²C table is not in sync with all devices supported > > > > Does anything actually rely on i2c_device_id table? ACPI would match > > either via ACPI or OF tables. All modern ARM systems (e.g. imx6) are > > DT-based. Maybe just drop the I2C ID table? > > For I²C it's still possible to enumerate the device via sysfs, which is ABI. Yes, and AFAIK, that worked fine. E.g. echo adafruit,3130 0x70 > /sys/class/i2c/i2c-adapter/.../new_device Cfr. https://lore.kernel.org/all/20211019144520.3613926-3-geert@xxxxxxxxxxxxxx/ Note that that example actually includes the manufacturer. I didn't check whether the I2C core takes that part into account when matching, or just strips it. > > > - the OF ID table seems has something really badly formed for adafruit > > > (just a number after a comma) > > > > Maybe it is a model number? It was documented: > > Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml > > Yes, it's not a problem for ACPI/DT platforms, the problem is for the above > way of enumeration, so if we have more than 1 manufacturer that uses plain > numbers for the model, I²C framework may not distinguish which driver to use. > > I.o.w. the part after comma in the compatible strings of the I²C devices must > be unique globally to make that enumeration disambiguous. Which is not unique to this driver? I bet you can find other compatible values that become non-unique after stripping the manufacturer. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds