On Tue, 16 Aug 2022 14:16:13 +0300 Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > On 16/08/2022 13:28, George Mois wrote: > > ADXL312 and ADXL314 are small, thin, low power, 3-axis accelerometers > > with high resolution (13-bit) measurement up to +/-12 g and +/- 200 g > > respectively. > > > > Thank you for your patch. There is something to discuss/improve. > > > > > static const struct spi_device_id adxl313_spi_id[] = { > > - { "adxl313" }, > > + { "adxl312", ADXL312 }, > > + { "adxl313", ADXL313 }, > > + { "adxl314", ADXL314 }, > > { } > > }; > > > > MODULE_DEVICE_TABLE(spi, adxl313_spi_id); > > > > static const struct of_device_id adxl313_of_match[] = { > > + { .compatible = "adi,adxl312" }, > > { .compatible = "adi,adxl313" }, > > + { .compatible = "adi,adxl314" }, > > You miss here driver data. I don't remember which driver matching takes > precedence (especially in various cases like DT platforms with device > instantiated by MFD), but for consistency I think both device id tables > should have same driver data. You can set it up to try device_get_match_data() first then fallback to the adxl313_spi_id[] table but there isn't a nice 'standard' way to do it. If that isn't done, then IIRC the match is against the compatible with the vendor ID dropped and the table used is the spi_device_id one. Which is just annoyingly complex and relies on the strings matching. In the ideal world the spi_device_id table would go away but there are still a few users (greybus - I think + remaining board files). So for now something like a = device_get_match_data(dev); if (!a) a = &adxl31x_spi_regmap_config[id->data]; Provides a good way of ensuring the id tables don't need to remain in sync. Jonathan > > > Best regards, > Krzysztof