Re: [PATCH v2] hw/net/can: Add mcp25625 model

[Pavel Pisa]

Hello Ben,

On Thursday 16 of March 2023 13:41:13 Ben Dooks wrote:
> From: Ben Dooks <ben.dooks@sifive.com>
>
> Add support for Microchip MCP25625 SPI based CAN controller which is
> very similar to the MCP2515 (and covered by the same Linux driver).
>
> This can be added to any machine with SPI support in the machine
> model file.
>
> Example for using this when configured into a machine:
>
>       -object can-bus,id=canbus0 \
>       -object can-host-socketcan,id=canhost0,if=vcan0,canbus=canbus0 \
>       -global driver=mcp25625,property=canbus,value=canbus0
>
> There is tracing support with --trace "*mcp25*"
>
> Signed-off-by: Ben Dooks <ben.dooks@sifive.com>
> Co-developed-by: Nazar Kazakov <nazar.kazakov@codethink.co.uk>
> Signed-off-by: Nazar Kazakov <nazar.kazakov@codethink.co.uk>
> Co-developed-by: Lawrence Hunter <lawrence.hunter@codethink.co.uk>
> Signed-off-by: Lawrence Hunter <lawrence.hunter@codethink.co.uk>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>

Tested-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Acked-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>

I have used your test branch

https://gitlab.com/CodethinkLabs/qemu/-/commits/mcp25625_test

I confirm that it contains unmodified mcp25625 model patch
applied directly to QEMU master and then additional patches
for actual integration with qemu/hw/riscv/sifive_u.c

I have used stripped down initramfs system build from
debootstraped Debian Bookworm - GLIBC, busybox, ip route,
can-utils and Linux kernel have been used. Kernel version

  Linux (none) 6.1.0-7-riscv64 #1 SMP Debian 6.1.20-1 (2023-03-19)
  riscv64 GNU/Linux  

QEMU invocation command

  qemu-system-riscv64 -m 1G -M sifive_u -smp 2 \
      -initrd ramdisk.cpio \
      -kernel vmlinux-6.1.0-7-riscv64 \
      -nographic \
      -object can-bus,id=canbus0 \
      -object can-host-socketcan,id=canhost0,if=can0,canbus=canbus0 \
      -global driver=mcp25625,property=canbus,value=canbus0

The virtual SocketCAN interface has been used on the host
side and mutual CAN dump/CAN gen has been used and worked
in the both directons.

I support inclusion of your patch in QEMU mainline
to allow keep it in sync with the rest of the projects.

But I would be happy if some integration is included as well.
Do you plan to submit your integration to qemu/hw/riscv/sifive_u.c
as well? If so, is there some plan to make mapping to SPI
configurable? May be some other developer could help with
suggestion what are the plans in SPI peripherals mapping
in QEMU.

The mapping of IRQ over SIFIVE_U_MISC_IRQ in your followup
testing patches seems to me quite ad-hock solution.
It should go through some GPIO pin on a real board
probably.

I would like to see at least one more system and
architecture to provide option to map and test
the mcp25625 model. Raspberry Pi or some other
ARM would be nice.

Anyway, I would suggest to think about integration
of CAN/CAN FD controller to SiFive SoC directly
on the CPU bus because access over SPI causes
latencies as I have already pointed to Martin Prudek's
thesis prepared for Honeywell when we tried to enhance
their choice of Raspberry Pi Matlab Simulink target
used with SPI based Microchip CAN controller

  
https://dspace.cvut.cz/bitstream/handle/10467/68605/F3-DP-2017-Prudek-Martin-Dp_2017_prudek_martin.pdf

See Figure 6.27. Cumulative latency histogram of CAN messages
on page 43...

We have lot more data and tools now from our CAN latency
tester project based on our CTU CAN FD IP core.
There is some documentation in Matej Vasilevski's
thesis

  
https://dspace.cvut.cz/bitstream/handle/10467/101450/F3-DP-2022-Vasilevski-Matej-vasilmat.pdf

and the followup project with continuous latency
testing on actual Linux kernel mainline and preemp-rt
will be submitted by this June.

Best wishes,

                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
    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



On Tuesday 21 of March 2023 11:49:15 Ben Dooks wrote:
> On 17/03/2023 14:11, Pavel Pisa wrote:
> > Hello Ben,
> >
> > thanks for update.
> >
> > On Thursday 16 of March 2023 13:41:13 Ben Dooks wrote:
> >> From: Ben Dooks <ben.dooks@sifive.com>
> >>
> >> Add support for Microchip MCP25625 SPI based CAN controller which is
> >> very similar to the MCP2515 (and covered by the same Linux driver).
> >>
> >> This can be added to any machine with SPI support in the machine
> >> model file.
> >>
> >> Example for using this when configured into a machine:
> >>
> >>    -object can-bus,id=canbus0 \
> >>    -object can-host-socketcan,id=canhost0,if=vcan0,canbus=canbus0 \
> >>    -global driver=mcp25625,property=canbus,value=canbus0
> >>
> >> There is tracing support with --trace "*mcp25*"
> >
> > Code looks good, I have patched actual QEMU sources and build
> > it successfully with your change.
> >
> > I have not seen any warning.
> >
> > I would like to test the mcp25625 CAN functionality.
> >
> > I would prefer against some target which is already available
> > in QEMU and Linux kernel mainlines, so if somebody can suggest
> > some ARM which can connect SPI/SSI device it would be great.
> >
> > I have setup /srv/nfs/debian-riscv64 chroot and used
> > it to prepare minimal 3 MB ramdisk.cpio with busybox
> > and full GLIBC and ip package.
> >
> > I can run it with Debian provided RISC-V kernel
> > under QEMU compiled with your mcp25625 chip emulation
> >
> > qemu-system-riscv64 -m 1G -M sifive_u -smp 2 \
> >        -initrd ramdisk.cpio \
> >        -kernel vmlinux-6.1.0-6-riscv64 \
> >        -nographic \
> >        -object can-bus,id=canbus0 \
> >        -object can-host-socketcan,id=canhost0,if=can0,canbus=canbus0 \
> >        -global driver=mcp25625,property=canbus,value=canbus0
> >
> > I can see
> >
> > /sys/bus/platform/devices/10040000.spi
> > /sys/bus/platform/devices/10050000.spi
> >
> > I can run
> >
> >    modprobe spi-sifive.ko
> >
> > [   41.524160] sifive_spi 10040000.spi: mapped; irq=21, cs=1
> > [   41.529305] sifive_spi 10050000.spi: mapped; irq=22, cs=1
> >
> >    modprobe mcp251x.ko
> >
> > I can imagine to build device tree overlay and setup it from within
> > kernel if the device is already mapped
> >
> >    cd /sys/kernel/config/device-tree/overlays
> >    [ -d  sifive_u-mcp25625 ] && rmdir sifive_u-mcp25625
> >    mkdir sifive_u-mcp25625
> >    cd sifive_u-mcp25625
> >    cat sifive_u-mcp25625.dtbo >dtbo
> >    echo 1 >status
> >
> > which is what we do with CTU CAN FD ip on Zynq system
> > to run PL/FPGA update.
> >
> > But from QEMU info qtree, I see that device is not mapped in QEMU...
> > Which is logic...
> >
> > So please, can you send instruction how to proceed forward.
> >
> > Do you have DTB prepared for testing or something similar?
> >
> > In a longer term perspective, it would be ideal to provide
> > some update for documentation, how to use mcp25625 emulation
> >
> >    https://www.qemu.org/docs/master/system/devices/can.html
> >
> > By the way, if the Raspberry Pi emulation does not provide
> > right SPI emulation as you have noticed, what about BeagleBoneBlack?
>
> At the moment it seems that the as a whole qemu doesn't have a good
> way of adding a generic spi device to a bus.
>
> > Does it support SPI? It could be good target to test that mcp25625
> > chip emulation is portable..
>
> I've pushed our test branch out to:
> https://gitlab.com/CodethinkLabs/qemu/-/commits/mcp25625_test
>
> That adds an spi channel to the sifive_u machine and puts the
> right dtb entry in there.