Re: [Qemu-devel] [PATCH 0/5 v2] CAN bus support for QEMU (SJA1000 PCI so far)

[Philippe Mathieu-Daudé]

On 11/01/2017 11:10 PM, Pavel Pisa wrote:
>>> Some page about the project
>>>
>>>   https://gitlab.fel.cvut.cz/canbus/qemu-canbus/wikis/home
>>>
>>> FEE CTU GitLab repository with can-pci branch for 2.3, 2.4, 2.7, 2.8 and
>>> 2.10 version if QEMU is available there
>>>
>>>   https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci
>>
>> This GitLab seems to be restricted to your university :(
> 
> Thanks fort the report, I have moved work from GitHub to local GitLab
> for future continuous integration support.
> 
> Repository has been public from beginning, but I forgot
> to make wiki public. It should be OK now.

Indeed it is :) I was writing another mail asking for details about how
to test your series.

More useful than the wiki would be to have part of this info in
docs/can.txt (or docs/socketcan.txt).

> 
> Unfortunately, my attempt to follow suggestion to ensure
> that even conditionalized debug code is build always
> leads to patchew.org problem report because now exposed
> static function is not used in non Linux builds
> 
> /tmp/qemu-test/src/hw/can/can_core.c:153:13: error: 'can_display_msg' defined 
> but not used [-Werror=unused-function]
>  static void can_display_msg(struct qemu_can_frame *msg)
>              ^~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors

If you have Docker installed you can run:

$ make docker-image-debian-win32-cross
[...]

Then:

$ make docker-run TEST="test-quick" IMAGE="debian-win32-cross"
HEAD is now at b154b28678... WIP on canbus: 00a6a3d87f CAN bus MIOe-3680
PCI (dual SJA1000 channel) emulation added.
  COPY    RUNNER
    RUN test-quick in qemu:debian-win32-cross
[...]
C compiler        i686-w64-mingw32.shared-gcc
Host C compiler   cc
...
mingw32 support   yes
[...]
  CC      hw/bt/hci.o
  CC      hw/bt/hid.o
  CC      hw/bt/hci-csr.o
  CC      hw/can/can_core.o
/tmp/qemu-test/src/hw/can/can_core.c:153:13: error: 'can_display_msg'
defined but not used [-Werror=unused-function]
 static void can_display_msg(struct qemu_can_frame *msg)
             ^
cc1: all warnings being treated as errors
make: *** [hw/can/can_core.o] Error 1
/tmp/qemu-test/src/rules.mak:66: recipe for target 'hw/can/can_core.o'
failed
tests/docker/Makefile.include:126: recipe for target 'docker-run' failed
make: *** [docker-run] Error 1

> 
> It is used for connection to the host CAN bus interface
> and that is implemented only for SocketCAN on Linux.
> 
> So patches v3 series is necessary anyway.
> 
> I would be happy if I receive some review remarks
> for this reorganized series to lower iteration count.

I'm not sure my critical review made you happy ;)

You'll get review but since we are entering SoftFreeze I doubt your
series will enter in this release. But surely other will review.

Can you think of some test to add? The easiest test is to just start
QEMU with your device (i.e. tests/ne2000-test.c)
Ideally you'd use it like ping pong'ing frame with device in loopback
mode (see tests/e1000e-test.c).

Regards,

Phil.