Re: [PATCH V2] iio: adc: Add Renesas GyroADC driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marek,

On Tue, Jan 10, 2017 at 7:35 PM, Marek Vasut <marek.vasut@xxxxxxxxx> wrote:
>> 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.
>
> Which driver, the gyroadc driver ? That's only an ADC driver, it

The gyroadc driver.

> shouldn't be aware of any temperature/pressure sampling or any such
> stuff IMO. What you suggest would result in overbloating the gyroadc
> driver with support for all sorts of ADCs connected to it's channels
> and providing all sorts of information, which I think would violate
> the functional separation in the driver model.

If the need ever arises, it can be turned into a "gyroadc" framework,
and the actual sensor drivers can be enhanced to support the gyroadc
driver, in addition to regular SPI master drivers?

Something for the far future, of course ;-)
For now we can limit it to the 3 types of devices officially supported by
the GyroADC hardware.

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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux