Hi Marek, On Mon, Jan 9, 2017 at 10:55 PM, Marek Vasut <marek.vasut@xxxxxxxxx> wrote: > On 01/09/2017 10:02 PM, Lars-Peter Clausen wrote: >> On 01/09/2017 09:56 PM, Marek Vasut wrote: >>> On 01/09/2017 09:07 PM, Lars-Peter Clausen wrote: >>>> On 01/09/2017 02:47 PM, Marek Vasut wrote: >>>>> On 01/09/2017 02:45 PM, Lars-Peter Clausen wrote: >>>>>> On 01/09/2017 01:03 AM, Marek Vasut wrote: >>>>>> [...] >>>>>>> +- renesas,gyroadc-mode: GyroADC mode of operation, must be either of: >>>>>>> + 1 - MB88101A mode, 12bit sampling, 4 channels >>>>>>> + 2 - ADCS7476 mode, 15bit sampling, 8 channels >>>>>>> + 3 - MAX1162 mode, 16bit sampling, 8 channels >>>>>> >>>>>> So is this a ADC, or is this just a specialized SPI controller that >>>>>> interfaces to an external ADC? >>>>> >>>>> It's a special SPI controller, except one cannot access the SPI bus >>>>> directly. It sends out clock and reads in the data from the ADC . >>>> >>>> OK, thanks for the clarification. The commit message does not mention this >>>> at the moment and makes it sound like this is a built-in ADC. >>>> >>>> Also the renesas,gyroadc-mode property is more of a driver configuration >>>> setting rather than a description of the hardware, at least it is not very >>>> DT-ish. >>>> >>>> I'd go with something like >>>> >>>> >>>> &adc { >>>> compatible = "renesas,r8a7791-gyroadc"; >>>> ... >>>> >>>> adc@0 { >>>> reg = <0>; >>>> compatible = "maxim,max1162"; >>> >>> But then the max1162 ADC driver* will try to bind to this, right ? >>> And since the MAX1162 is an SPI device, it will fail to work as the >>> ADC driver does not provide any sort of SPI interface. Or is that >>> actually OK ? >> >> Only if you call of_register_spi_devices(). >> >> We for example have devices that can either work in SPI or I2C mode and both >> use the same compatible string. Depending on whether the device is a subnode >> of a SPI or I2S controller it is registered as a SPI or I2C device. >> >> This situation is not that different. > > So in this case, the gyroadc driver would instead iterate over the > subnodes, check the compatible string and configure the channel mode > according to the subnode compatible string? That's correct. > But then, the gyroadc only has one configuration register which applies > to all channels, which selects how many bits to read from the connected > SPI-ADC device(s) (sample width). This is not per-channel configurable. > The driver would then have to check if all the compatible properties in > the ADC subnodes select the same ADC or at least ADC which has the same Correct again. > sample width. This looks like a duplication to me. Thoughts ? While the protocols have to be the same for each channel, you can connect different devices. If you have e.g. a thermometer or barimeter that speaks the same protocol as a max1162 (i.e. it responds with 2 zero bytes and 2 bytes of data), you can mix and match that with max1162 devices, once you have added support for the thermometer or barimeter to your driver. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html