[GSoC] CAN SJA1000

[Pavel Pisa]

Hello Jin Yang and all others,

congratulation to achieving the main goal.

I have read your instructions and build sucesfull
the QEMU on AMD64 Debian Wheezy.
I have run the compiled QEMU with 64-bit 3.2.x kernel
used on a host and guest side of the QEMU.

I have included board integration into LinCAN driver

http://sourceforge.net/p/ortcan/lincan/ci/master/tree/lincan/src/pcisja1000mm.c

and run successfully LinCAN test tools on the guest side
against your SJA1000 HW emulation. I have connected host
side to the virtual SocketCAN bus and used OrtCAN qCANalyzer
on the host side

  http://ortcan.sourceforge.net/qcanalyzer/

I have not found any problems during this test.
But tat is quit simple test and it would worth to
try some more demanding testing. But as you know
we are not fast (unfortuantelly) in many cases so
it would take some time. Anyway it worth to run
our CAN Benchmarks to torture code and clean possible
bugs

  http://rtime.felk.cvut.cz/can/

By the way, we have included actual MPC5200 RTEMS CAN
driver into our benchmarks this summer and results are
quite in favor for RTEMS but that is partially caused
by dummy implementation of CAN gateway user on RTEMS
for testing. But that worths separate e-mail as we
finish the work.

As for actual Jin Yang's QEMU CAN code, I think that
it is valuable achievement for GSoC project and if
stability is confirmed by testing, it is good base
for RTEMS CAN infrastructure development.

On the other hand, for long term sustainability
it is required to invest into inclusion of code
to the QEMU mainline. This requires to port code
to the actual QEMU git version. I do not know
how big amount it means because QEMU is fast
mowing target. It may be above GSoC project contract
size. But it really worth to be done.

As for the code review, I think that there is too
much CAN specific code push into generic qemu-char.c
file

  qemu-jin-yang/qemu-char.c

The registration in actual QEMU git version seems to be more
modular

    register_char_driver_qapi("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
                              qemu_chr_parse_parallel);

The all SocketCAN specific code has to be moved to other
file and there must be mechanism to disable build of that
code

qemu-jin-yang/qemu-char.c
+#include <linux/can.h>
+#include <linux/can/raw.h>

The reason is that SocketCAN is Linux specific and that part
of the code included in the common QEMU infrastructure
would made it non-portable.

As for the actual can controller emulation, it is/or can be
made fully portable to all host systems and for all target
configurations which support PCI

  qemu-jin-yang/hw/can-pci.c

It would worth to separate SJA1000 part of the code from
PCI mapping to allows use SJA1000 emulator even on non-PCI
platforms. It would worth even to allows to use SJA1000
emulator with different PCI cards mapping. The most of real
CAN interface cards have more PCI regions/BARs and actual CAN
chips are mapped to region 1 or 2. Region 0 is usually used
to control PCI bridge part of the card. Use of generic PCI
bridge chips usually means that some interrupt configuration
is required in this bridge control area.
But these two are not high priority for now.

As for actual SJA1000 chip emulation, I would prefer personally
if the register bit masks are not hardcoded by numbers but
if some better readable constants are used. I.e.

http://sourceforge.net/p/ortcan/lincan/ci/master/tree/lincan/include/sja1000p.h

But again that can be done later.

Best wishes,

             Pavel Pisa


On Sunday 15 of September 2013 10:16:39 jinyang.sia at gmail.com wrote:
> Hi,
>
> I just update the http://wiki.rtems.org/wiki/index.php/Qemu_simulations 
> remove some unecessary data and add some new information. Some of them is
> not complete. However I don't know how to upload pictures to the site?
>
> Thanks,
> Jin Yang
>
>
>
>
> jinyang.sia at gmail.com