Re: [PATCH v3 0/8] dp8393x: fixes and improvements

[Finn Thain]

On Sat, 10 Jul 2021, Philippe Mathieu-Daudé wrote:

> 
> The last 2 patches are included for Mark, but I don't plan to merge
> 
> them without Finn's Ack, and apparently they require more work.
> 


I tested the patch series both with and without the last 2 patches. Both 
builds worked fine with my NetBSD/arc, Linux/mipsel and Linux/m68k guests.

Tested-by: Finn Thain <fthain@linux-m68k.org>

I have no objection to patch 8/8 ("dp8393x: don't force 32-bit register 
access"). I asked Mark to explain why it was a bug fix (since it didn't 
change QEMU behaviour in my tests) but when I looked into it I found that 
he is quite right, the patch does fix a theoretical bug.

My only objection to patch 7/8 ("dp8393x: Rewrite dp8393x_get() / 
dp8393x_put()") was that it could be churn.

If I'm right that the big_endian flag should go away, commit b1600ff195 
("hw/mips/jazz: specify correct endian for dp8393x device") has already 
taken mainline in the wrong direction and amounts to churn.

I have the same reservations about patch 6/8 ("dp8393x: Store CRC using 
device configured endianess"). Perhaps that should be NOTFORMERGE too 
(even though it too a theoretical bug fix).

Is there a good way to avoid using big_endian for storing the CRC and the 
other DMA operations?

BTW, if you see "sn0: receive buffers exhausted" occasionally logged by 
the NetBSD 9.2 kernel, accompanied by packet loss, it's not a regression 
in QEMU. I first observed it last year when stress testing dp8393x with 
NetBSD 5.1. I believe this to be an old NetBSD sn driver bug because Linux 
is unaffected.