myriadrf / xtrx_linux_pcie_drv

XTRX PCI driver for linux
Apache License 2.0
16 stars 22 forks source link

Timer API changes at some point #4

Closed smunaut closed 5 years ago

smunaut commented 6 years ago

This allows to build with the newer timer API which has been in the kernel for a while. And so this re-enabled the disabled uart_timer code.

Patch both in attachement and inline.

xtrx-fix-timer-api.txt

diff --git a/xtrx.c b/xtrx.c
index 00ad862..3a41a7b 100644
--- a/xtrx.c
+++ b/xtrx.c
@@ -410,10 +410,10 @@ txq_empty:
 // 20ms timer
 #define XTRX_UART_DELAY_TIME ((HZ/100)+1)

-static void xtrx_uart_timer(unsigned long data)
+static void xtrx_uart_timer(struct timer_list *tl)
 {
+   struct xtrx_dev* dev = from_timer(dev, tl, uart_timer);
    unsigned tx_fifo_used;
-   struct xtrx_dev* dev = (struct xtrx_dev*)data;

    spin_lock(&dev->port_gps.lock);
    if (dev->gps_ctrl_state) {
@@ -511,20 +511,17 @@ static int xtrx_uart_init(struct xtrx_dev* dev)

    dev->sim_ctrl_state = 0;

-   //init_timer(&dev->uart_timer);
-   dev->uart_timer.data = (unsigned long )dev;
-   dev->uart_timer.function = xtrx_uart_timer;
+   timer_setup(&dev->uart_timer, xtrx_uart_timer, 0);
    dev->uart_timer.expires = jiffies + XTRX_UART_DELAY_TIME;
-   //add_timer(&dev->uart_timer);
+   add_timer(&dev->uart_timer);
    return 0;
 }

 static void xtrx_uart_deinit(struct xtrx_dev* dev)
 {
-   //del_timer(&dev->uart_timer);
+   del_timer(&dev->uart_timer);
    uart_remove_one_port(&xtrx_uart_driver, &dev->port_gps);
    uart_remove_one_port(&xtrx_uart_driver, &dev->port_sim);
-
 }

 // DMA functions
chemeris commented 6 years ago

Hi, Sylvain, I assume you've tested the code to make sure it works?

brchiu commented 6 years ago

hi, @smunaut, since the change of struct timer_list in linux/timer.h started from 4.15 [1], I would like to suggest :

  1. adding `#include <linux/version.h>" at head of the file.
  2. use #if (LINUX_VERSION_CODE >= KERNEL_VERSION(4, 15, 0)) to mitigate the transition between old and new kernels.
    #if (LINUX_VERSION_CODE >= KERNEL_VERSION(4, 15, 0))
    // code for kernel after and include 14.15
    #else
    // code for kernel before 4.15
    #endif

    [1]: The data field inside struct timer_list was removed from kernel 14.5-rc1 : https://elixir.bootlin.com/linux/v4.15-rc1/source/include/linux/timer.h

smunaut commented 6 years ago

@chemeris Well, to be honest I'm not entirely sure what that timer is for :/ But what I know is that I captured samples with the driver as I submitted and I can also use minicom to talk to the GPS.

@brchiu it doesn't matter when the field was removed is irrelevant, it's when the new API was added that is. And that was added in 4.14. And I hate plastering #ifdef all over ...

sergforce commented 5 years ago

The problem is resolved by removing the timer entirely