Re: [PATCH next 1/4] iio: sca3000: cache SPI device ID in probe

[Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>]

Hi Jonathan,


> Hi Harshit,
> 
> A few comments inline but they are all about extra things to do rather
> than really being comments on this patch which is fine as a starting point.
> 
....
>>   static int sca3000_probe(struct spi_device *spi)
>>   {
>> -	int ret;
>> +	const struct spi_device_id *id = spi_get_device_id(spi);
> 
> This is probably ok as an intermediate step, but ultimately we
> shouldn't be using the spi_device_id at all.
> 
> id->driver_data should be a pointer to a named (not in array)
> sca3000_chip_info structure and so should the data in the of_match_id
> table etc.
> 
> Then we use spi_get_device_match_data() which prefers the of_device_id
> table entry (which would needed adding).
> 
> 
...
>> -	st->info = &sca3000_spi_chip_info_tbl[spi_get_device_id(spi)
>> -					      ->driver_data];
>> +	st->info = &sca3000_spi_chip_info_tbl[id->driver_data];
>>   
>> -	indio_dev->name = spi_get_device_id(spi)->name;
>> +	indio_dev->name = id->name;
> 
> This also needs an update to make it get the name from within the
> chip_info structure.  Whilst this works today it can be fragile
> if we have dt compatibles that don't match exactly with the spi_device_id
> table entries.
> 

Thanks a lot for the review, will try to make this change on top of 
these refactors.

Regards,
Harshit