Review on spi driver

Andrey Skvortsov andrej.skvortzov at
Sat Nov 19 14:45:24 EST 2016

On 16-10-07 18:28, Arthur LAMBERT wrote:
> 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 !

Hi Arthur,

from my point of view your device driver can use IIO (Industrial IO)
subsystem. There are a lot of supported ADCs there.
If your ADC is not supported yet, you have great opportunity to submit
new driver to the mainline kernel.

Best regards,
Andrey Skvortsov

More information about the Kernelnewbies mailing list