On 2018-11-11 15:55, Jonathan Cameron wrote: > On Tue, 6 Nov 2018 16:37:11 +0000 > Peter Rosin <peda@xxxxxxxxxx> wrote: > >> Hi! >> >> Some comments inline... > A few additions from me. *snip* >>> + data = iio_priv(indio_dev); >>> + spi_set_drvdata(spi, indio_dev); >>> + data->spi = spi; >>> + data->cfg = &mcp41010_cfg[devid]; >> >> I'm missing a "of_device_get_match_data(dev)" call somewhere in here, see e.g. the >> max5481.c file in this directory. Yes, that's missing from the mcp4131 driver as >> well, but that's not a valid reason for not doing it here AFAICT... > If you want to use the data elements from the devicetree bindings you'll need > to do that. However, there is an odd path that actually means it will fall back > to getting the right thing as you have it here. > spi_get_device_id calls spi_match_id which compares the sdev->modalias > with the id table names. modalias has been carefully constructed > to be the text after the comma only and as such this works as is. > > Perhaps it's neater though to just use the devicetree access functions. Isn't that part about not looking at the vendor-part of the DT-compatible slightly fragile? Or will modalias/chip-number clashes be detected by something? Cheers, Peter