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

Anton Kochkov posted 1 patch 1 year, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220817141754.2105981-1-anton.kochkov@proton.me
Maintainers: Pavel Pisa <pisa@cmp.felk.cvut.cz>, Vikram Garhwal <fnu.vikram@xilinx.com>, Francisco Iglesias <francisco.iglesias@xilinx.com>, Jason Wang <jasowang@redhat.com>
hw/net/can/xlnx-zynqmp-can.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
[PATCH v2] hw/net/can: fix Xilinx ZynqMP CAN RX FIFO logic
Posted by Anton Kochkov 1 year, 8 months ago
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(-)

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);
--
2.37.2
Re: [PATCH v2] hw/net/can: fix Xilinx ZynqMP CAN RX FIFO logic
Posted by Peter Maydell 1 year, 8 months ago
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
Re: [PATCH v2] hw/net/can: fix Xilinx ZynqMP CAN RX FIFO logic
Posted by Peter Maydell 1 year, 7 months ago
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
Re: [PATCH v2] hw/net/can: fix Xilinx ZynqMP CAN RX FIFO logic
Posted by Peter Maydell 1 year, 8 months ago
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