diff mbox series

[v2] hw/net/can: fix Xilinx ZynqMP CAN RX FIFO logic

Message ID 20220817141754.2105981-1-anton.kochkov@proton.me
State New
Headers show
Series [v2] hw/net/can: fix Xilinx ZynqMP CAN RX FIFO logic | expand

Commit Message

Anton Kochkov Aug. 17, 2022, 2:22 p.m. UTC 
For consistency, function "update_rx_fifo()" should use
the RX FIFO register names, not the TX FIFO ones even if
they refer to the same memory region.

Signed-off-by: Anton Kochkov <anton.kochkov@proton.me>
Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1123
---
 hw/net/can/xlnx-zynqmp-can.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

--
2.37.2

Comments

Peter Maydell Aug. 17, 2022, 2:33 p.m. UTC | #1 
On Wed, 17 Aug 2022 at 15:24, Anton Kochkov <anton.kochkov@proton.me> wrote:
>
> For consistency, function "update_rx_fifo()" should use
> the RX FIFO register names, not the TX FIFO ones even if

"register field names"

> they refer to the same memory region.

"same bit positions in the register".

(No need to spin a v3 just for that; if there's no other
issues with the patch I'll fix it up when I take it into
target-arm.next.)

thanks
-- PMM
Peter Maydell Aug. 24, 2022, 7:25 p.m. UTC | #2 
On Wed, 17 Aug 2022 at 15:33, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 17 Aug 2022 at 15:24, Anton Kochkov <anton.kochkov@proton.me> wrote:
> >
> > For consistency, function "update_rx_fifo()" should use
> > the RX FIFO register names, not the TX FIFO ones even if
>
> "register field names"
>
> > they refer to the same memory region.
>
> "same bit positions in the register".
>
> (No need to spin a v3 just for that; if there's no other
> issues with the patch I'll fix it up when I take it into
> target-arm.next.)

Richard, since you're accumulating target-arm patches currently,
could you pick this one up (with the commit message fixups), please?

thanks
-- PMM
Peter Maydell Sept. 20, 2022, 10:19 a.m. UTC | #3 
On Wed, 17 Aug 2022 at 15:33, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 17 Aug 2022 at 15:24, Anton Kochkov <anton.kochkov@proton.me> wrote:
> >
> > For consistency, function "update_rx_fifo()" should use
> > the RX FIFO register names, not the TX FIFO ones even if
>
> "register field names"
>
> > they refer to the same memory region.
>
> "same bit positions in the register".
>
> (No need to spin a v3 just for that; if there's no other
> issues with the patch I'll fix it up when I take it into
> target-arm.next.)



Applied to target-arm.next, thanks.

-- PMM
diff mbox series

Patch

diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
index 82ac48cee2..e93e6c5e19 100644
--- a/hw/net/can/xlnx-zynqmp-can.c
+++ b/hw/net/can/xlnx-zynqmp-can.c
@@ -696,30 +696,30 @@  static void update_rx_fifo(XlnxZynqMPCANState *s, const qemu_can_frame *frame)
                                                timestamp));

             /* First 32 bit of the data. */
-            fifo32_push(&s->rx_fifo, deposit32(0, R_TXFIFO_DATA1_DB3_SHIFT,
-                                               R_TXFIFO_DATA1_DB3_LENGTH,
+            fifo32_push(&s->rx_fifo, deposit32(0, R_RXFIFO_DATA1_DB3_SHIFT,
+                                               R_RXFIFO_DATA1_DB3_LENGTH,
                                                frame->data[0]) |
-                                     deposit32(0, R_TXFIFO_DATA1_DB2_SHIFT,
-                                               R_TXFIFO_DATA1_DB2_LENGTH,
+                                     deposit32(0, R_RXFIFO_DATA1_DB2_SHIFT,
+                                               R_RXFIFO_DATA1_DB2_LENGTH,
                                                frame->data[1]) |
-                                     deposit32(0, R_TXFIFO_DATA1_DB1_SHIFT,
-                                               R_TXFIFO_DATA1_DB1_LENGTH,
+                                     deposit32(0, R_RXFIFO_DATA1_DB1_SHIFT,
+                                               R_RXFIFO_DATA1_DB1_LENGTH,
                                                frame->data[2]) |
-                                     deposit32(0, R_TXFIFO_DATA1_DB0_SHIFT,
-                                               R_TXFIFO_DATA1_DB0_LENGTH,
+                                     deposit32(0, R_RXFIFO_DATA1_DB0_SHIFT,
+                                               R_RXFIFO_DATA1_DB0_LENGTH,
                                                frame->data[3]));
             /* Last 32 bit of the data. */
-            fifo32_push(&s->rx_fifo, deposit32(0, R_TXFIFO_DATA2_DB7_SHIFT,
-                                               R_TXFIFO_DATA2_DB7_LENGTH,
+            fifo32_push(&s->rx_fifo, deposit32(0, R_RXFIFO_DATA2_DB7_SHIFT,
+                                               R_RXFIFO_DATA2_DB7_LENGTH,
                                                frame->data[4]) |
-                                     deposit32(0, R_TXFIFO_DATA2_DB6_SHIFT,
-                                               R_TXFIFO_DATA2_DB6_LENGTH,
+                                     deposit32(0, R_RXFIFO_DATA2_DB6_SHIFT,
+                                               R_RXFIFO_DATA2_DB6_LENGTH,
                                                frame->data[5]) |
-                                     deposit32(0, R_TXFIFO_DATA2_DB5_SHIFT,
-                                               R_TXFIFO_DATA2_DB5_LENGTH,
+                                     deposit32(0, R_RXFIFO_DATA2_DB5_SHIFT,
+                                               R_RXFIFO_DATA2_DB5_LENGTH,
                                                frame->data[6]) |
-                                     deposit32(0, R_TXFIFO_DATA2_DB4_SHIFT,
-                                               R_TXFIFO_DATA2_DB4_LENGTH,
+                                     deposit32(0, R_RXFIFO_DATA2_DB4_SHIFT,
+                                               R_RXFIFO_DATA2_DB4_LENGTH,
                                                frame->data[7]));

             ARRAY_FIELD_DP32(s->regs, INTERRUPT_STATUS_REGISTER, RXOK, 1);