On 28, April 2018 17:11, Jonathan Cameron wrote:
> On Mon, 23 Apr 2018 00:02:51 +0200
> David Veenstra <davidjulianveenstra@xxxxxxxxx> wrote:
>
>> Add missing kernel docs to the ad2s1200 driver state.
>>
>> Signed-off-by: David Veenstra <davidjulianveenstra@xxxxxxxxx>
> Hi David,
>
> Comment inline.
>
>> ---
>> Changes in v3:
>> - Added more explanation to mutex lock.
>>
>> drivers/staging/iio/resolver/ad2s1200.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 357fe3c382b3..17d65cb437fd 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -33,6 +33,15 @@
>> /* clock period in nano second */
>> #define AD2S1200_TSCLK (1000000000 / AD2S1200_HZ)
>>
>> +/**
>> + * struct ad2s1200_state - driver instance specific data.
>> + * @lock: protects both the GPIO pins and the rx buffer, to prevent
>> + * concurrent spi transactions.
> It isn't actually preventing concurrent spi transactions - that
> is done by the spi core itself. What it is preventing is
> concurrent 'actions' with the device - these involve
> a mixture of gpio changes and spi transactions.
>
> That would take some considerable explaining and frankly the code
> does the job just fine once people know to look.
>
> I'd just leave it as "protects both the GPIO pins and the rx buffer."
> Or feel free to see if you can come up with a brief description of
> exactly what we need to be 'atomic'.
Agreed that the code is clear. I'll revert the description.
Best regards,
David Veenstra
>
> Thanks,
>
> Jonathan
>
>> + * @sdev: spi device.
>> + * @sample: GPIO pin SAMPLE.
>> + * @rdvel: GPIO pin RDVEL.
>> + * @rx: buffer for spi transfers.
>> + */
>> struct ad2s1200_state {
>> struct mutex lock;
>> struct spi_device *sdev;
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
