Re: [PATCH v2 7/7] hw/net/can/xlnx-versal-canfd: Fix FIFO issues

[Pavel Pisa]

On Tuesday 27 of August 2024 05:49:27 Doug Brown wrote:
> The read index should not be changed when storing a new message into the
> RX or TX FIFO. Changing it at this point will cause the reader to get
> out of sync. The wrapping of the read index is already handled by the
> pre-write functions for the FIFO status registers anyway.
>
> Additionally, the calculation for wrapping the store index was off by
> one, which caused new messages to be written to the wrong location in
> the FIFO. This caused incorrect messages to be delivered.
>
> Signed-off-by: Doug Brown <doug@schmorgal.com>

Generally, I agree that index should wrap up for cyclic FIFO
implementation and change looks logical to me but I do not
see and studied all consequences related to emulated HW.

If that is confirmed or corrected by somebody from AMD/XilinX,
it would be better. I can find more time to do deeper analysis
if no other looks into the whole code.

Best wishes,

                Pavel
--
                Pavel Pisa
    phone:      +420 603531357
    e-mail:     pisa@cmp.felk.cvut.cz
    Department of Control Engineering FEE CVUT
    Karlovo namesti 13, 121 35, Prague 2
    university: http://control.fel.cvut.cz/
    personal:   http://cmp.felk.cvut.cz/~pisa
    social:     https://social.kernel.org/ppisa
    projects:   https://www.openhub.net/accounts/ppisa
    CAN related:http://canbus.pages.fel.cvut.cz/
    RISC-V education: https://comparch.edu.cvut.cz/
    Open Technologies Research Education and Exchange Services
    https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home