Re: [PATCH] imon_raw: respect DMA coherency

[Sean Young <sean@mess.org>]

On Tue, May 17, 2022 at 10:28:35AM +0200, Oliver Neukum wrote:
> sorry for being a bit obnoxious about this, but there is a slight issue.

Pointing out potential issues is the opposite of obnoxious :)

> This is the old code:
> 
> static void imon_ir_data(struct imon *imon)
> {
>        struct ir_raw_event rawir = {};
>        u64 data = be64_to_cpu(imon->ir_buf);
>        u8 packet_no = data & 0xff;
>        int offset = 40;
>        int bit;
> 
>        if (packet_no == 0xff)
>                return;
> 
>        dev_dbg(imon->dev, "data: %*ph", 8, &imon->ir_buf);

That should be imon->ir_buf (not &imon->ir_buf) after your changes.

> The dev_dbg() logs the data as it is in the buffer. If you use
> be64_to_cpup() instead of be64_to_cpu() you reverse
> the buffer on a little endian CPU and hence the debug
> output will be changed.

I'm confused. be64_to_cpup() does not do an in-place byte swap on little
endian. 

I've just tested and the debug message still works fine (barring the issue
above).

> The actual driver code is unaffected, because the
> buffer is never used again, so this is not a big deal.
> 
> The error is mine by changing the type of imon->ir_buf
> But the fix is not quite the best.

Your change is good.


Sean