This series fixes several problems I ran into while trying to simulate
the AMD/Xilinx Versal CANFD controller in the xlnx-versal-virt machine
using Xilinx's v6.6_LTS_2024.1 kernel. With all of these patches
applied, everything works correctly alongside actual CAN devices.

- IRQs were accidentally not being delivered due to having a level other
  than 1. The IRQ count in /proc/interrupts in Linux was stuck at 0.
- Incoming CAN FD frames were being treated as non-FD.
- The CAN IDs were garbled in both RX and TX directions.
- The ESI and BRS flags were not being handled.
- The byte ordering was wrong in the data in both directions.
- Incoming CAN FD frames with DLC = 1-7 weren't handled correctly.
- The FIFO read_index and store_index wrapping logic was incorrect.

I don't have any actual Versal hardware to compare behavior against, but
with these changes, it plays nicely with SocketCAN on the host system.

Changes in v2:
- Added handling of ESI and BRS flags, ensured frame->flags is initialized
- Switched to use common can_dlc2len() and can_len2dlc() functions
- Added fix for FIFO wrapping problems I observed during stress testing

Doug Brown (7):
  hw/net/can/xlnx-versal-canfd: Fix interrupt level
  hw/net/can/xlnx-versal-canfd: Fix CAN FD flag check
  hw/net/can/xlnx-versal-canfd: Translate CAN ID registers
  hw/net/can/xlnx-versal-canfd: Handle flags correctly
  hw/net/can/xlnx-versal-canfd: Fix byte ordering
  hw/net/can/xlnx-versal-canfd: Simplify DLC conversions
  hw/net/can/xlnx-versal-canfd: Fix FIFO issues

 hw/net/can/xlnx-versal-canfd.c | 173 ++++++++++++++-------------------
 1 file changed, 72 insertions(+), 101 deletions(-)

-- 
2.34.1

On Tue, 27 Aug 2024 at 04:51, Doug Brown <doug@schmorgal.com> wrote:
>
> This series fixes several problems I ran into while trying to simulate
> the AMD/Xilinx Versal CANFD controller in the xlnx-versal-virt machine
> using Xilinx's v6.6_LTS_2024.1 kernel. With all of these patches
> applied, everything works correctly alongside actual CAN devices.
>
> - IRQs were accidentally not being delivered due to having a level other
>   than 1. The IRQ count in /proc/interrupts in Linux was stuck at 0.
> - Incoming CAN FD frames were being treated as non-FD.
> - The CAN IDs were garbled in both RX and TX directions.
> - The ESI and BRS flags were not being handled.
> - The byte ordering was wrong in the data in both directions.
> - Incoming CAN FD frames with DLC = 1-7 weren't handled correctly.
> - The FIFO read_index and store_index wrapping logic was incorrect.
>
> I don't have any actual Versal hardware to compare behavior against, but
> with these changes, it plays nicely with SocketCAN on the host system.
>
> Changes in v2:
> - Added handling of ESI and BRS flags, ensured frame->flags is initialized
> - Switched to use common can_dlc2len() and can_len2dlc() functions
> - Added fix for FIFO wrapping problems I observed during stress testing

I've applied this series to target-arm.next; thanks!

-- PMM

On Tue, 27 Aug 2024 at 04:51, Doug Brown <doug@schmorgal.com> wrote:
>
> This series fixes several problems I ran into while trying to simulate
> the AMD/Xilinx Versal CANFD controller in the xlnx-versal-virt machine
> using Xilinx's v6.6_LTS_2024.1 kernel. With all of these patches
> applied, everything works correctly alongside actual CAN devices.

Couple of Qs for the Xilinx/AMD folks:
 (1) do you intend to review patch 7 of this series? That's
     the one unreviewed one and it could use a look from
     somebody familiar with how the versal canfd h/w works.
     If you don't I propose to apply this series as-is next week.
 (2) it sounds like Vikram Garhwal's AMD email is bouncing --
     do you want to nominate somebody else to take his place
     as co-maintainer/reviewer of CAN bus stuff?
     If not, we can just drop his lines from MAINTAINERS.

> - IRQs were accidentally not being delivered due to having a level other
>   than 1. The IRQ count in /proc/interrupts in Linux was stuck at 0.
> - Incoming CAN FD frames were being treated as non-FD.
> - The CAN IDs were garbled in both RX and TX directions.
> - The ESI and BRS flags were not being handled.
> - The byte ordering was wrong in the data in both directions.
> - Incoming CAN FD frames with DLC = 1-7 weren't handled correctly.
> - The FIFO read_index and store_index wrapping logic was incorrect.
>
> I don't have any actual Versal hardware to compare behavior against, but
> with these changes, it plays nicely with SocketCAN on the host system.
>
> Changes in v2:
> - Added handling of ESI and BRS flags, ensured frame->flags is initialized
> - Switched to use common can_dlc2len() and can_len2dlc() functions
> - Added fix for FIFO wrapping problems I observed during stress testing
>
> Doug Brown (7):
>   hw/net/can/xlnx-versal-canfd: Fix interrupt level
>   hw/net/can/xlnx-versal-canfd: Fix CAN FD flag check
>   hw/net/can/xlnx-versal-canfd: Translate CAN ID registers
>   hw/net/can/xlnx-versal-canfd: Handle flags correctly
>   hw/net/can/xlnx-versal-canfd: Fix byte ordering
>   hw/net/can/xlnx-versal-canfd: Simplify DLC conversions
>   hw/net/can/xlnx-versal-canfd: Fix FIFO issues
>
>  hw/net/can/xlnx-versal-canfd.c | 173 ++++++++++++++-------------------
>  1 file changed, 72 insertions(+), 101 deletions(-)

thanks
-- PMM

Hi Peter,

On Fri, Sep 06, 2024 at 03:36:10PM +0100, Peter Maydell wrote:
> On Tue, 27 Aug 2024 at 04:51, Doug Brown <doug@schmorgal.com> wrote:
> >
> > This series fixes several problems I ran into while trying to simulate
> > the AMD/Xilinx Versal CANFD controller in the xlnx-versal-virt machine
> > using Xilinx's v6.6_LTS_2024.1 kernel. With all of these patches
> > applied, everything works correctly alongside actual CAN devices.
> 
> Couple of Qs for the Xilinx/AMD folks:
>  (1) do you intend to review patch 7 of this series? That's
>      the one unreviewed one and it could use a look from
>      somebody familiar with how the versal canfd h/w works.
>      If you don't I propose to apply this series as-is next week.

I've started reviewing it (but got interrupted), I'll comeback later today.

>  (2) it sounds like Vikram Garhwal's AMD email is bouncing --
>      do you want to nominate somebody else to take his place
>      as co-maintainer/reviewer of CAN bus stuff?
>      If not, we can just drop his lines from MAINTAINERS.

I can take Vikram's place, I'll post a patch with the update.

Best regards,
Francisco

> 
> > - IRQs were accidentally not being delivered due to having a level other
> >   than 1. The IRQ count in /proc/interrupts in Linux was stuck at 0.
> > - Incoming CAN FD frames were being treated as non-FD.
> > - The CAN IDs were garbled in both RX and TX directions.
> > - The ESI and BRS flags were not being handled.
> > - The byte ordering was wrong in the data in both directions.
> > - Incoming CAN FD frames with DLC = 1-7 weren't handled correctly.
> > - The FIFO read_index and store_index wrapping logic was incorrect.
> >
> > I don't have any actual Versal hardware to compare behavior against, but
> > with these changes, it plays nicely with SocketCAN on the host system.
> >
> > Changes in v2:
> > - Added handling of ESI and BRS flags, ensured frame->flags is initialized
> > - Switched to use common can_dlc2len() and can_len2dlc() functions
> > - Added fix for FIFO wrapping problems I observed during stress testing
> >
> > Doug Brown (7):
> >   hw/net/can/xlnx-versal-canfd: Fix interrupt level
> >   hw/net/can/xlnx-versal-canfd: Fix CAN FD flag check
> >   hw/net/can/xlnx-versal-canfd: Translate CAN ID registers
> >   hw/net/can/xlnx-versal-canfd: Handle flags correctly
> >   hw/net/can/xlnx-versal-canfd: Fix byte ordering
> >   hw/net/can/xlnx-versal-canfd: Simplify DLC conversions
> >   hw/net/can/xlnx-versal-canfd: Fix FIFO issues
> >
> >  hw/net/can/xlnx-versal-canfd.c | 173 ++++++++++++++-------------------
> >  1 file changed, 72 insertions(+), 101 deletions(-)
> 
> thanks
> -- PMM