| Message ID | 20240827034927.66659-1-doug@schmorgal.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | hw/net/can/xlnx-versal-canfd: Miscellaneous fixes | expand |
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
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