On 18/01/15 02:19, Marek Vasut wrote:
> On Saturday, January 17, 2015 at 01:22:08 AM, Kristina Martšenko wrote:
> 
> Hi!

Hi!

> Good stuff, thank you! Just minor nitpicks below.
>
> [...]
> 
>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
>> b/drivers/staging/iio/adc/mxs-lradc.c index e0e91836eec1..fc65cd311be9
>> 100644
>> --- a/drivers/staging/iio/adc/mxs-lradc.c
>> +++ b/drivers/staging/iio/adc/mxs-lradc.c
>> @@ -214,11 +214,14 @@ struct mxs_lradc {
>>      unsigned long           is_divided;
>>
>>      /*
>> -     * Touchscreen LRADC channels receives a private slot in the CTRL4
>> -     * register, the slot #7. Therefore only 7 slots instead of 8 in the
>> -     * CTRL4 register can be mapped to LRADC channels when using the
>> -     * touchscreen.
>> -     *
>> +     * When the touchscreen is enabled, we give it two private virtual
>> +     * channels: #6 and #7. This means that only 6 virtual channels (instead
>> +     * of 8) will be available for buffered capture.
>> +     */
>> +#define TS_VCH1             7
>> +#define TS_VCH2             6
> 
> Please use a bit more explicit name for the macro, it's really not clear what
> the macro represents from it's name. Something like TOUCHSCREEN_VCHANNEL1 
> might
> work better for example. What do you think ?

Sure, I tried to keep it short and similar to the TS_CH_YP (etc) macros,
but clearer is better, so I'll change it to TOUCHSCREEN_VCHANNEL1 in v2.

>> +
>> +    /*
>>       * Furthermore, certain LRADC channels are shared between touchscreen
>>       * and/or touch-buttons and generic LRADC block. Therefore when using
>>       * either of these, these channels are not available for the regular
> [...]
> 
> I also have a general question/idea here, it's explicitly not something I'd 
> like
> to force upon you to implement. I see we have some kind of a hardware, which 
> can
> sample up-to N inputs in parallel . Each input is muxed between M possible 
> sources . Is such a thing common in the ADC/DAC world ? Would it be worth to
> implement generic helper to handle this kind of a N:M mapping ? What do you 
> all
> think please ?

Hmm, I don't know how other drivers do this, or if there's anything in
common. The IIO people will probably have a better idea, so I'll wait
for them to respond.

Kristina
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to