[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c4c14051-2ba2-4d80-a22d-4deb3709f727@linaro.org>
Date: Fri, 31 Oct 2025 12:32:03 +0100
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: jic23@...nel.org, dlechner@...libre.com, nuno.sa@...log.com,
andy@...nel.org, robh@...nel.org, conor+dt@...nel.org, krzk+dt@...nel.org,
linux-iio@...r.kernel.org, s32@....com, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, chester62515@...il.com, mbrugger@...e.com,
ghennadi.procopciuc@....nxp.com
Subject: Re: [PATCH v5 2/2] iio: adc: Add the NXP SAR ADC support for the
s32g2/3 platforms
On 10/30/25 10:28, Andy Shevchenko wrote:
> On Thu, Oct 30, 2025 at 09:27:21AM +0100, Daniel Lezcano wrote:
>> On 10/18/25 22:12, Andy Shevchenko wrote:
>>> On Fri, Oct 17, 2025 at 06:42:38PM +0200, Daniel Lezcano wrote:
[ ... ]
>>>> +#define NXP_SAR_ADC_IIO_BUFF_SZ (NXP_SAR_ADC_NR_CHANNELS + (sizeof(u64) / sizeof(u16)))
>>>
>>> Hmm... Don't we have some macros so we can avoid this kind of hard coding?
>>
>> I don't find such a macro, do you have a pointer ?
>
> If I got the use case correctly, I was thinking of IIO_DECLARE_BUFFER_WITH_TS().
Oh right ! Actually, IIO_DECLARE_BUFFER_WITH_TS() is used but the macro
above was not removed :)
[ ... ]
>>>> + dma_samples = (u32 *)dma_buf->buf;
>>>
>>> Is it aligned properly for this type of casting?
>>
>> TBH, I don't know the answer :/
>>
>> How can I check that ?
>
> Is buf defined as a pointer to u32 / int or bigger? or is it just byte buffer?
> If the latter, how does the address of it being formed? Does it come from a heap
> (memory allocator)? If yes, we are fine, as this is usually the case for all
> (k)malloc'ed memory.
buf is a byte buffer allocated with dmam_alloc_coherent(..., GFP_KERNEL)
> ...
>
>>>> + dmaengine_tx_status(info->dma_chan, info->cookie, &state);
>>>
>>> No return value check?
>>
>> The return value is not necessary here because the caller of the callback
>> will check with dma_submit_error() in case of error which covers the
>> DMA_ERROR case and the other cases are not useful because the residue is
>> taken into account right after.
>
> In some cases it might return DMA_PAUSE (and actually this is the correct way
> to get residue, one needs to pause the channel to read it, otherwise it will
> give outdated / incorrect information).
But if the residue is checked in the callback routine without checking
DMA_PAUSED, the result is the same no ?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Powered by blists - more mailing lists