Review on spi driver

[Arthur LAMBERT]

Hi,

Not sure that it is the good place to do that. I am quite new in kernel dev and
it will be great to have some review/advise on a implementation in kernel.

My need : Read periodically data from a sensor with spi. I know that data is available
thanks to a GPIO. Data are available every 2 ms.

First step for me was to :

Declare a irq handler on the ready signal GPIO thanks to gpio_to_irq and request_irq
function. First I try to read from spi in my irq handler.I finally understand that I
cannot make call which implies sleep in irq handler like spi_sync for example.
I finally use wake_up_interruptible call in my irq handler to make the spi work
in another piece of code.

static irqreturn_t irq_handler(int irq, void *id)
{
	flag = 1;
	wake_up_interruptible(&wq);
	return IRQ_HANDLED;
}

On my first try I just use a kernel thread which read data from spi and then which send
my data to userspace thanks to a netlink socket.

   while(!kthread_should_stop())
   {
	wait_event_interruptible(wq, flag != 0 || kthread_should_stop());
	if (kthread_should_stop()) do_exit (0);
	flag = 0;

	// read data from spi
	(...)
	// send data in netlink socket
	skb = nlmsg_new(NLMSG_ALIGN(msg_size + 1), GFP_ATOMIC);
	nlh = nlmsg_put(skb, 0, 1, NLMSG_DONE, msg_size + 1, 0);
	memcpy(nlmsg_data(nlh), spi->rx_buf, msg_size);
	ret = nlmsg_multicast(nl_sk, skb, 0, MYGRP, GFP_ATOMIC);
	schedule(); // not sure that I need this line of code.
    }

Problem I was not sure that netlink socket was the best solution to my problem. Also
I was not able to use the same socket during my data acquisition. I must call nlmsg_new
on each new data sample. Other strange stuff, If I call nsmsg_free at the end of the
process I have big stability issue during my test. Moreover, if I am to slow to send
data from kernel space to user space, I will miss event from my irq handler and miss
sample from my sensor.

My second try was to use a character device to send data from kernel space to
userpsace. I was already using the character device to drive my kernel driver.
Open/Release to init/clean sensor. And ioctl to send command to the sensor like start/stop
acquisition. So my plan was just to add a read callback on the character device.

static const struct file_operations fops = {
       .owner = THIS_MODULE,
       .unlocked_ioctl = ads_ioctl,
       .open = ads_open,
       .release = ads_release,
       .read = ads_read,
       .unlocked_ioctl = ads_ioctl,
};

static int ads_read(struct file *filp, char __user *data, size_t len, loff_t *ppos)
{
	struct ads_dev *dev = filp->private_data;

	wait_event_interruptible(wq, flag != 0);
	flag = 0;
	// read data from spi
	(...)
	copy_to_user(data, spi->rx_buf, DATA_SIZE);
	return DATA_SIZE;
};

This code seems to be easy compare to the netlink solution. Even in the user space code
side. Just need to use a read instead of using socket and libnl. But the problem is
always there. If the read operation over the spi and the copy_to_user take more than 2 ms, I
will lost sample from my sensor.

My last plan was to use mixed the two previous solutions and use a kthread + the character
device. The irq wake up the thread. I read data over spi. I put the data in a queue.
The read callback from character device will gather data from the queue. I have always
a synchrone call which can take more than 2 ms. But At least I can send the data to user space
in a synchrone way.

But I was not able to find a proper way to use a queue with semaphore. So it was quite basic.
A static queue + offset + wake_up_interruptible/wait_event_interruptible to synchronize
kernel thread and my read callback.

static uint8_t queue[50 * DATA_SIZE];
static int read_offset = 0;
static int write_offset = 0;

I guess that my problems are quite common in kernel driver which want to gather some data over spi.
Is there a good kernel driver that I can take as good practice/example to implement my solution ?
Feel free to say that my code is stupid !

Thanks,
Arthur.