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

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

 



Hi Marek,

On Mon, Jan 9, 2017 at 6:30 PM, Marek Vasut <marek.vasut@xxxxxxxxx> wrote:
>> On Mon, Jan 9, 2017 at 3:16 PM, Marek Vasut <marek.vasut@xxxxxxxxx> wrote:
>>> On 01/09/2017 01:13 PM, Geert Uytterhoeven wrote:
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
>>>>> @@ -0,0 +1,52 @@
>>>>> +* Renesas RCar GyroADC device driver
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible:  Should be "renesas,r8a7791-gyroadc" for regular GyroADC or
>>>>> +               "renesas,r8a7792-gyroadc" for a GyroADC without interrupt
>>>>> +               block found in R8A7792.
>>>>
>>>> I would have kept "renesas,rcar-gyroadc", too.
>>>
>>> Do we have some sort of standard practice here ? Ie. NXP SoCs use the
>>> oldest compatible SoC in the DT compat string (so the driver can bind
>>> to that) and another DT compat string with that particular SoC name
>>> encoded in it (so it's possible to discern it in the future in the
>>> driver, if there is some problem).
>>
>> On Renesas SoCs, we usually define family- and SoC-specific compatible
>> values, and match on the family-specific values if possible.
>>
>>>> Upon closer look, GyroADC in r8a7792 aka R-Car V2H has builtin interrupt
>>>> functionality, (SPI IRQ 18), while other variants use the interrupt
>>>> functionality of the Speed-pulse I/F (SPI IRQ 236), which is not present
>>>> on V2H.
>>>>
>>>> Hence I think we can distinguish between the two variants by looking at
>>>> the presence of an "interrupts" property, if we make that mandatory on
>>>> V2H. If/when the need arises later, non-V2H variants will need a phandle
>>>> to the Speed-pulse I/F to use its interrupt.
>>>
>>> Well, the interrupt is pretty much useless (it generates 100ms pulses),
>>> all we want to do it set the register to 0x0 to make sure the block
>>> doesn't generate any. Do we want to add interrupts property for just
>>> this purpose ?
>>
>> The alternative is to let the driver match against all of
>> "renesas,r8a77<n>-gyroadc" for n=78,79,90..96 (and more to come).
>
> I think this would be slightly better, since I can precisely support
> only the SoC which I have and which I can test. Then again, there's
> already "renesas,rcar-gen2-jpu" for example, which looks appealing to
> use, but I think it's not a good idea as there might be some bug found

The JPU bindings also mandate specifying e.g. "renesas,jpu-r8a7791".

> in the hardware later on and the DT wouldn't allow us to discern that
> block if it only has a "renesas,rcar-gyroadc" in it.
>
> I think using
> compatible = "renesas,r8a77<n>-gyroadc", "renesas,rcar-gyroadc";
> would work.

Yes, that's exactly what I meant.

> The driver could match on "renesas,rcar-gyroadc" and if a bug is ever
> found, the driver can match on "renesas,r8a77<n>-gyroadc"
> instead and handle the bug.

The driver still needs to know if it's running on V2H or not, because of
the interrupt registers.

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