[RFC PATCH] hw/net/can: clean-up unnecessary includes

Alex Bennée posted 1 patch 4 months ago
hw/net/can/can_kvaser_pci.c   | 4 ----
hw/net/can/can_mioe3680_pci.c | 4 ----
hw/net/can/can_pcm3680_pci.c  | 4 ----
hw/net/can/can_sja1000.c      | 2 +-
hw/net/can/ctucan_core.c      | 3 ++-
hw/net/can/ctucan_pci.c       | 4 ----
6 files changed, 3 insertions(+), 18 deletions(-)
[RFC PATCH] hw/net/can: clean-up unnecessary includes
Posted by Alex Bennée 4 months ago
The event_notifier, thread and socket includes look like copy and
paste of standard headers. None of the canbus devices use chardev
although some relied on chardev to bring in bitops and byte swapping
headers. In this case include them directly.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/net/can/can_kvaser_pci.c   | 4 ----
 hw/net/can/can_mioe3680_pci.c | 4 ----
 hw/net/can/can_pcm3680_pci.c  | 4 ----
 hw/net/can/can_sja1000.c      | 2 +-
 hw/net/can/ctucan_core.c      | 3 ++-
 hw/net/can/ctucan_pci.c       | 4 ----
 6 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/hw/net/can/can_kvaser_pci.c b/hw/net/can/can_kvaser_pci.c
index 38434d3a04..9e363d532f 100644
--- a/hw/net/can/can_kvaser_pci.c
+++ b/hw/net/can/can_kvaser_pci.c
@@ -30,12 +30,8 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/event_notifier.h"
 #include "qemu/module.h"
-#include "qemu/thread.h"
-#include "qemu/sockets.h"
 #include "qapi/error.h"
-#include "chardev/char.h"
 #include "hw/irq.h"
 #include "hw/pci/pci_device.h"
 #include "hw/qdev-properties.h"
diff --git a/hw/net/can/can_mioe3680_pci.c b/hw/net/can/can_mioe3680_pci.c
index 21659b7afb..580f099e00 100644
--- a/hw/net/can/can_mioe3680_pci.c
+++ b/hw/net/can/can_mioe3680_pci.c
@@ -26,12 +26,8 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/event_notifier.h"
 #include "qemu/module.h"
-#include "qemu/thread.h"
-#include "qemu/sockets.h"
 #include "qapi/error.h"
-#include "chardev/char.h"
 #include "hw/irq.h"
 #include "hw/pci/pci_device.h"
 #include "hw/qdev-properties.h"
diff --git a/hw/net/can/can_pcm3680_pci.c b/hw/net/can/can_pcm3680_pci.c
index af21dc6855..3195b79954 100644
--- a/hw/net/can/can_pcm3680_pci.c
+++ b/hw/net/can/can_pcm3680_pci.c
@@ -26,12 +26,8 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/event_notifier.h"
 #include "qemu/module.h"
-#include "qemu/thread.h"
-#include "qemu/sockets.h"
 #include "qapi/error.h"
-#include "chardev/char.h"
 #include "hw/irq.h"
 #include "hw/pci/pci_device.h"
 #include "hw/qdev-properties.h"
diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
index 6694d7bfd8..5b6ba9df6c 100644
--- a/hw/net/can/can_sja1000.c
+++ b/hw/net/can/can_sja1000.c
@@ -27,7 +27,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
-#include "chardev/char.h"
+#include "qemu/bitops.h"
 #include "hw/irq.h"
 #include "migration/vmstate.h"
 #include "net/can_emu.h"
diff --git a/hw/net/can/ctucan_core.c b/hw/net/can/ctucan_core.c
index 812b83e93e..4402d4cb1f 100644
--- a/hw/net/can/ctucan_core.c
+++ b/hw/net/can/ctucan_core.c
@@ -28,7 +28,8 @@
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
-#include "chardev/char.h"
+#include "qemu/bswap.h"
+#include "qemu/bitops.h"
 #include "hw/irq.h"
 #include "migration/vmstate.h"
 #include "net/can_emu.h"
diff --git a/hw/net/can/ctucan_pci.c b/hw/net/can/ctucan_pci.c
index 65f1f82303..a8c77b9194 100644
--- a/hw/net/can/ctucan_pci.c
+++ b/hw/net/can/ctucan_pci.c
@@ -27,12 +27,8 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/event_notifier.h"
 #include "qemu/module.h"
-#include "qemu/thread.h"
-#include "qemu/sockets.h"
 #include "qapi/error.h"
-#include "chardev/char.h"
 #include "hw/irq.h"
 #include "hw/pci/pci_device.h"
 #include "hw/qdev-properties.h"
-- 
2.39.5


Re: [RFC PATCH] hw/net/can: clean-up unnecessary includes
Posted by Pierrick Bouvier 4 months ago
On 12/9/24 02:06, Alex Bennée wrote:
> The event_notifier, thread and socket includes look like copy and
> paste of standard headers. None of the canbus devices use chardev
> although some relied on chardev to bring in bitops and byte swapping
> headers. In this case include them directly.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   hw/net/can/can_kvaser_pci.c   | 4 ----
>   hw/net/can/can_mioe3680_pci.c | 4 ----
>   hw/net/can/can_pcm3680_pci.c  | 4 ----
>   hw/net/can/can_sja1000.c      | 2 +-
>   hw/net/can/ctucan_core.c      | 3 ++-
>   hw/net/can/ctucan_pci.c       | 4 ----
>   6 files changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/net/can/can_kvaser_pci.c b/hw/net/can/can_kvaser_pci.c
> index 38434d3a04..9e363d532f 100644
> --- a/hw/net/can/can_kvaser_pci.c
> +++ b/hw/net/can/can_kvaser_pci.c
> @@ -30,12 +30,8 @@
>    */
>   
>   #include "qemu/osdep.h"
> -#include "qemu/event_notifier.h"
>   #include "qemu/module.h"
> -#include "qemu/thread.h"
> -#include "qemu/sockets.h"
>   #include "qapi/error.h"
> -#include "chardev/char.h"
>   #include "hw/irq.h"
>   #include "hw/pci/pci_device.h"
>   #include "hw/qdev-properties.h"
> diff --git a/hw/net/can/can_mioe3680_pci.c b/hw/net/can/can_mioe3680_pci.c
> index 21659b7afb..580f099e00 100644
> --- a/hw/net/can/can_mioe3680_pci.c
> +++ b/hw/net/can/can_mioe3680_pci.c
> @@ -26,12 +26,8 @@
>    */
>   
>   #include "qemu/osdep.h"
> -#include "qemu/event_notifier.h"
>   #include "qemu/module.h"
> -#include "qemu/thread.h"
> -#include "qemu/sockets.h"
>   #include "qapi/error.h"
> -#include "chardev/char.h"
>   #include "hw/irq.h"
>   #include "hw/pci/pci_device.h"
>   #include "hw/qdev-properties.h"
> diff --git a/hw/net/can/can_pcm3680_pci.c b/hw/net/can/can_pcm3680_pci.c
> index af21dc6855..3195b79954 100644
> --- a/hw/net/can/can_pcm3680_pci.c
> +++ b/hw/net/can/can_pcm3680_pci.c
> @@ -26,12 +26,8 @@
>    */
>   
>   #include "qemu/osdep.h"
> -#include "qemu/event_notifier.h"
>   #include "qemu/module.h"
> -#include "qemu/thread.h"
> -#include "qemu/sockets.h"
>   #include "qapi/error.h"
> -#include "chardev/char.h"
>   #include "hw/irq.h"
>   #include "hw/pci/pci_device.h"
>   #include "hw/qdev-properties.h"
> diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> index 6694d7bfd8..5b6ba9df6c 100644
> --- a/hw/net/can/can_sja1000.c
> +++ b/hw/net/can/can_sja1000.c
> @@ -27,7 +27,7 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/log.h"
> -#include "chardev/char.h"
> +#include "qemu/bitops.h"
>   #include "hw/irq.h"
>   #include "migration/vmstate.h"
>   #include "net/can_emu.h"
> diff --git a/hw/net/can/ctucan_core.c b/hw/net/can/ctucan_core.c
> index 812b83e93e..4402d4cb1f 100644
> --- a/hw/net/can/ctucan_core.c
> +++ b/hw/net/can/ctucan_core.c
> @@ -28,7 +28,8 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/log.h"
> -#include "chardev/char.h"
> +#include "qemu/bswap.h"
> +#include "qemu/bitops.h"
>   #include "hw/irq.h"
>   #include "migration/vmstate.h"
>   #include "net/can_emu.h"
> diff --git a/hw/net/can/ctucan_pci.c b/hw/net/can/ctucan_pci.c
> index 65f1f82303..a8c77b9194 100644
> --- a/hw/net/can/ctucan_pci.c
> +++ b/hw/net/can/ctucan_pci.c
> @@ -27,12 +27,8 @@
>    */
>   
>   #include "qemu/osdep.h"
> -#include "qemu/event_notifier.h"
>   #include "qemu/module.h"
> -#include "qemu/thread.h"
> -#include "qemu/sockets.h"
>   #include "qapi/error.h"
> -#include "chardev/char.h"
>   #include "hw/irq.h"
>   #include "hw/pci/pci_device.h"
>   #include "hw/qdev-properties.h"

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [RFC PATCH] hw/net/can: clean-up unnecessary includes
Posted by Pavel Pisa 4 months ago
Hello Alex,

On Monday 09 of December 2024 11:06:35 Alex Bennée wrote:
> The event_notifier, thread and socket includes look like copy and
> paste of standard headers. None of the canbus devices use chardev
> although some relied on chardev to bring in bitops and byte swapping
> headers. In this case include them directly.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

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

Tested on Debian/GNU/Linux for SJA1000 and CTU CAN FD 

QEMU=/home/pi/repo/qemu/qemu-build/qemu-system-x86_64

$QEMU -enable-kvm -kernel $KERNEL \
      -m 512M \
      -initrd ramdisk.cpio \
      -virtfs local,path=shareddir,security_model=none,mount_tag=shareddir \
      -vga cirrus \
      -append "console=ttyS0 \
      -object can-bus,id=canbus0-bus \
      -object can-host-socketcan,if=can0,canbus=canbus0-bus,id=canbus0-socketcan \
      -device kvaser_pci,canbus=canbus0-bus \
      -device ctucan_pci,canbus0=canbus0-bus,canbus1=canbus0-bus \
      -nographic

By the way, I would like to discuse how to update CTU CAN FD a SJA1000
IRQ handling to allow mapping on FPGA target platform buses from command
line.

I have some working prototype

  https://github.com/ppisa/qemu/commits/net-can-ctucanfd-platform/

but I have some questions how to implement interrupts processing
(or logic function) correct and acceptable way. I wills tart new
thread and hope somebody responses and teaches me what is the
proper solution.

Best wishes,

                Pavel
-- 
                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
    social:     https://social.kernel.org/ppisa
    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
Re: [RFC PATCH] hw/net/can: clean-up unnecessary includes
Posted by Philippe Mathieu-Daudé 4 months ago
On 9/12/24 12:23, Pavel Pisa wrote:
> Hello Alex,
> 
> On Monday 09 of December 2024 11:06:35 Alex Bennée wrote:
>> The event_notifier, thread and socket includes look like copy and
>> paste of standard headers. None of the canbus devices use chardev
>> although some relied on chardev to bring in bitops and byte swapping
>> headers. In this case include them directly.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> Acked-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
> 
> Tested on Debian/GNU/Linux for SJA1000 and CTU CAN FD
> 
> QEMU=/home/pi/repo/qemu/qemu-build/qemu-system-x86_64
> 
> $QEMU -enable-kvm -kernel $KERNEL \
>        -m 512M \
>        -initrd ramdisk.cpio \
>        -virtfs local,path=shareddir,security_model=none,mount_tag=shareddir \
>        -vga cirrus \
>        -append "console=ttyS0 \
>        -object can-bus,id=canbus0-bus \
>        -object can-host-socketcan,if=can0,canbus=canbus0-bus,id=canbus0-socketcan \
>        -device kvaser_pci,canbus=canbus0-bus \
>        -device ctucan_pci,canbus0=canbus0-bus,canbus1=canbus0-bus \
>        -nographic
> 
> By the way, I would like to discuse how to update CTU CAN FD a SJA1000
> IRQ handling to allow mapping on FPGA target platform buses from command
> line.

+ Gustavo

> 
> I have some working prototype
> 
>    https://github.com/ppisa/qemu/commits/net-can-ctucanfd-platform/
> 
> but I have some questions how to implement interrupts processing
> (or logic function) correct and acceptable way. I wills tart new
> thread and hope somebody responses and teaches me what is the
> proper solution.
> 
> Best wishes,
> 
>                  Pavel