On Wed, Nov 22, 2023 at 08:45:56PM +0100, Francisco Iglesias wrote:
> On Sun, Nov 19, 2023 at 11:51:02PM +0100, Philippe Mathieu-Daudé wrote:
> > Per
> > https://docs.xilinx.com/r/en-US/ug1085-zynq-ultrascale-trm/Message-Format
> >
> > Message Format
> >
> > The same message format is used for RXFIFO, TXFIFO, and TXHPB.
> > Each message includes four words (16 bytes). Software must read
> > and write all four words regardless of the actual number of data
> > bytes and valid fields in the message.
> >
> > There is no mention in this reference manual about what the
> > hardware does when not all four words are read. To fix the
> > reported underflow behavior, I choose to fill the 4 frame data
> > registers when the first register (ID) is accessed, which is how
> > I expect hardware would do.
> >
> > Reported-by: Qiang Liu <cyruscy...@gmail.com>
> > Fixes: 98e5d7a2b7 ("hw/net/can: Introduce Xilinx ZynqMP CAN controller")
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1427
> > Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
>
> Reviewed-by: Francisco Iglesias <francisco.igles...@amd.com>
Reviewed-by: Vikram Garhwal <vikram.garh...@amd.com>
>
>
> > ---
> > hw/net/can/xlnx-zynqmp-can.c | 17 +++++++++--------
> > 1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
> > index 58938b574e..c63fb4a83c 100644
> > --- a/hw/net/can/xlnx-zynqmp-can.c
> > +++ b/hw/net/can/xlnx-zynqmp-can.c
> > @@ -777,14 +777,18 @@ static void update_rx_fifo(XlnxZynqMPCANState *s,
> > const qemu_can_frame *frame)
> > }
> > }
> >
> > -static uint64_t can_rxfifo_pre_read(RegisterInfo *reg, uint64_t val)
> > +static uint64_t can_rxfifo_post_read_id(RegisterInfo *reg, uint64_t val)
> > {
> > XlnxZynqMPCANState *s = XLNX_ZYNQMP_CAN(reg->opaque);
> > + unsigned used = fifo32_num_used(&s->rx_fifo);
> >
> > - if (!fifo32_is_empty(&s->rx_fifo)) {
> > - val = fifo32_pop(&s->rx_fifo);
> > - } else {
> > + if (used < CAN_FRAME_SIZE) {
> > ARRAY_FIELD_DP32(s->regs, INTERRUPT_STATUS_REGISTER, RXUFLW, 1);
> > + } else {
> > + val = s->regs[R_RXFIFO_ID] = fifo32_pop(&s->rx_fifo);
> > + s->regs[R_RXFIFO_DLC] = fifo32_pop(&s->rx_fifo);
> > + s->regs[R_RXFIFO_DATA1] = fifo32_pop(&s->rx_fifo);
> > + s->regs[R_RXFIFO_DATA2] = fifo32_pop(&s->rx_fifo);
> > }
> >
> > can_update_irq(s);
> > @@ -945,14 +949,11 @@ static const RegisterAccessInfo can_regs_info[] = {
> > .post_write = can_tx_post_write,
> > },{ .name = "RXFIFO_ID", .addr = A_RXFIFO_ID,
> > .ro = 0xffffffff,
> > - .post_read = can_rxfifo_pre_read,
> > + .post_read = can_rxfifo_post_read_id,
> > },{ .name = "RXFIFO_DLC", .addr = A_RXFIFO_DLC,
> > .rsvd = 0xfff0000,
> > - .post_read = can_rxfifo_pre_read,
> > },{ .name = "RXFIFO_DATA1", .addr = A_RXFIFO_DATA1,
> > - .post_read = can_rxfifo_pre_read,
> > },{ .name = "RXFIFO_DATA2", .addr = A_RXFIFO_DATA2,
> > - .post_read = can_rxfifo_pre_read,
> > },{ .name = "AFR", .addr = A_AFR,
> > .rsvd = 0xfffffff0,
> > .post_write = can_filter_enable_post_write,
> > --
> > 2.41.0
> >
