Hi Marti,

Sorry for a late reply. I would like to add a few points to the
discussion. At first I'll quote your email as you wrote it a while ago.

On 16.12.2017 03:00, Marti Bolivar wrote:
> I've been reading through the ADC API, its users, and its test cases,
> and I'm confused about the semantics of struct adc_seq_entry, in
> particular the field named "buffer". The API docs are vague and the
> users contradict each other; something seems wrong to me.
>
> (I volunteer to try to improve the documentation if we can clear
> things up; I'm working on an ADC driver and would like to make sure
> I'm doing the right thing.)
>
> The main header says "buffer" is a byte array:
>
> https://github.com/zephyrproject-rtos/zephyr/blob/master/include/adc.h#L39
>
> But it doesn't say anything about the contents. That's strange,
> especially since common ADC IPs can do 12+ bit conversions. (I at
> least expected a u16*, and something written about the left- or
> right-alignment of sample data, e.g. how a 12 bit sample is stored in
> a 16 bit word.)
>
> That same header only has this to say about the returned values from
> an adc_read() call:
>
> * The sample data can be retrieved from the memory buffers in
> * the sequence table structure.
>
> So I looked at the API users to find out more, but the results were
> even more confusing.
>
> This "simple" test wants to interpret the results as though the buffer
> field points at an array of u32 (note the _print_sample_in_hex
> implementation and the "delta" printk in adc_test):
>
> https://github.com/zephyrproject-rtos/zephyr/blob/master/tests/drivers/adc/adc_simple/src/main.c#L36
>
> It also says buffer should be void*, which isn't true:
>
> https://github.com/zephyrproject-rtos/zephyr/blob/master/tests/drivers/adc/adc_simple/src/main.c#L78
>
> This "api" test thinks buffer is u16*:
>
> https://github.com/zephyrproject-rtos/zephyr/blob/master/tests/drivers/adc/adc_api/src/test_adc.c#L53
>
> The ADC-based grove temperature driver treats Zephyr samples as if
> they were 12 bit right aligned values in a u16 array, and has a
> comment saying that's the common convention:
>
> https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/grove/temperature_sensor.c#L46
>
> Can anyone clarify what the correct interpretation of this field is?
>

The alignment of the data within the buffer is unfortunately not the
only problem with this API. Also the meaning of the 'buffer_length'
field is pretty mysterious. I will not go into the details, suffice to
say that every single adc driver (we have a few of them already)
understands and implements it differently.

The fact that the implementation of ADC API among existing drivers is
incompatible is actually the smaller of the issues. The bigger problem
is that this API does not support typical use cases in a straightforward
way and neither takes into account capabilities of common hardware. IMHO
improving the documentation will not help much here. We should better
deprecate this API and come up with a new design.

Regards,
Piotr

_______________________________________________
Zephyr-devel mailing list
Zephyr-devel@lists.zephyrproject.org
https://lists.zephyrproject.org/mailman/listinfo/zephyr-devel