On Sun, 11 Nov 2018 16:49:28 +0000 Peter Rosin <peda@xxxxxxxxxx> wrote: > 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? Yes :) Well actually in this case not so bad. It will use a match on the of_device_id to find which driver to probe if there is one. The modalias part is only being abused to then figure out which device we have within the driver. I definitely prefer the explicit route. Just wanted to point out it 'worked' :) > > Cheers, > Peter
