daniel-santos / mcp2210-linux

MCP2210 driver for linux
http://danielthesantos.blogspot.com/search/label/mcp2210
51 stars 31 forks source link

Do commands really have to be queued/delayed? #34

Closed Psalm27-4 closed 5 years ago

Psalm27-4 commented 6 years ago

I will be using this driver to talk to a HOLT chip which receives ARINC data (roll, pitch, latitude, longitude) at high speed. There is more than one device on the SPI, but I will chip select the HOLT chip. I need to be able to read from the HOLT at high speed, using the GPIO ports on the SPI as interrupts to tell me when ARINC data is available (HOLT chip sets RX flag, which is wired to the GPIO on the SPI).

I will then poll the HOLT chip and grab all data available until empty. Data will be moving at 5ms intervals, which is why I need to use this kernel driver in kernel space instead of user space. When ARINC data is read from the HOLT through the SPI, I will copy it to a /dev device to be read from a user space app.

My question is, how can I avoid my read commands being delayed or queued? Can I set a delay of 0?

daniel-santos commented 6 years ago

Hello.

The queueing is necessary because of the multitude of command types types, their priorities, scheduling and such. But it is also necessary because some operations must occur in an atomic context and others cannot and I use the same command queue for "commands" that are just things that need to be scheduled in another context and not necessarily a command to the hardware. In fact, setting up an SPI master without the subsystem's built-in queuing mechanism is now depreciated -- I continue to use it because I already queue my messages and it would be wasteful to have two queues.

In general, we want to do as much as possible in a non-atomic context to keep the kernel's latency low. The real-time kernel project spent a lot of time on this. None the less, much of the queueing in the driver is superficial. If you look at queue_msg() in mcp2210-spi.c you'll see that as soon as it queues the command it runs the command queue, so if there was nothing else pending then the message is immediately sent.

HOWEVER the mcp2210 is a slow device. I'm actually glad you opened this question because I have some updates to the README.md that I haven't yet pushed that really need to be. While my driver is great, this device is NOT a high speed device. This was my first Linux driver and I naively took Microchip at their word on the performance of the MCP2210 -- a mistake I won't repeat.

This chip is great for applications that require less than 12kBss (kilobytes), but useless elsewhere. The purpose I originally wrote it for needed only about 4kB/s so I was fine, but the hardware problems weren't exposed until somebody else needed for something high speed. The largest latencies occur in-between each byte in an SPI message, but there are also ugly delays from CS assert to first byte, from last byte to CS deassert and from the time the chip gets the USB command until the time it starts to transmit. I have those numbers somewhere if you need them as I had to scope them all a few months ago.

Also note that I'm moving to gitlab soon, I just need to figure out how to put up a redirect on github.

Psalm27-4 commented 6 years ago

Hi Daniel,

Thank you for your feedback. ARINC labels (such as roll, pitch, and heading) will be received on the HOLT chip at a rate of up to up to 100 messages / second. I will have the RX flags of the HOLT chip signaling data is ready hooked up to the GPIO ports on the SPI and treated as interrupts. When the interrupt is triggered I will read from the driver to the HOLT chip all labels available. I have a user level application that will read from the driver on 10ms intervals. Labels are 8 bytes each, and according to the spec, the SPI can do 64 byte transfers. So I think I can read 8 data words per read from the SPI.

Unfortunately I have no say in what hardware was used for this project I'm working on.

Is there any way to get rid of the asserts? I don't think the queueing of commands is going to be a big deal, as once everything is set up I'm just reading over and over. I'm still learning how this driver works. Where is the entry point to send a command to a device attached to the SPI?

Psalm27-4 commented 6 years ago

Also, should I modify the Makefile to not treat warnings as errors? I get this when I compile. My platform is Ubuntu Linux 14.04 GCC version(Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4.

Psalm27-4 commented 5 years ago

Hi Daniel, I have successfully been talking to my HOLT chip through the driver. I use mcp2210_add_ctl_cmd() to SET_CHIP_CONFIG, SET_SPI_CONFIG, and issue a spi transfer. This isn't the correct way to use this though. I will never have a userland app sending SPI commands or control commands to the driver, rather the driver will copy data it reads to a character device that the userland app interacts with (/dev/data).

One thing I have noted is that to read a register off of the HOLT, I have to do three reads. The first URB response is transfer completed, the second response is transfer started back, and finally the third is transfer completed and I get the value of the register. Does it seem right to you that it should take 3 read commands? This MCP chip seems to do a queuing thing.

I have tried unsuccessfully using the userland utility in /user, because I don't know how to create the /dev/spidev0.0 that it is looking for. How can I create that?

Could you please get back to me on this. I really appreciate any help you can give. Could please give me some pointers on how to create the spi transfer command from within the driver or how they are created via the IOCTL interaction?

daniel-santos commented 5 years ago

Hello Psalm27-4, The easiest way to use this driver is to write your configuration to the user-area of the EEPROM on the chip using the Creek encoding. I do not yet have a mechanism to use device tree. The only other way to properly configure it is from user space. My implementation is admittedly a bit hacky since I have you modify user/settings.h (copy it from user/settings-example.h if it doesn't already exist) and modify that. But after doing so, you simply build the userland tool and then the executable contains the configuration from settings.h. This is all explained in the README.md!

Do that and then you implement your driver as an ordinary "SPI protocol driver" -- google that in quotes and you'll find some documentation and probably some tutorials and/or examples. This is the correct way to do SPI in Linux. In the settings.h, the modalias field of struct mcp2210_board_config my_board_config specifies the name of your SPI protocol driver. Set this for the correct CS pin and also set the other settings and it should work automatically, as long as your driver is loaded (via modprobe or udev rule) prior to probing the mcp2210.

I noticed that I don't have interrupts documented in the README.md, but they are somewhat documented in the settings-example.h. You can either use GP6 in it's dedicated mode for edge interrupts or you can use any other pin in GPIO mode for a level interrupt (use GP6 unless your interrupt can only be de-asserted via software).

The only other kink is that the userspace program uses a lib and there's no install yet, so you have to set LD_LIBRARY_PATH, e.g.:

cd mcp2210/user
make
LD_LIBRARY_PATH=. ./mcp2210-util --help

So that part is messy. There are a lot of things I need to clean up in this project. In a nutshell, you'll do something like:

LD_LIBRARY_PATH=. ./mcp2210-util encode > config.dat
LD_LIBRARY_PATH=. ./mcp2210-util -i config.dat eeprom write addr=0 
LD_LIBRARY_PATH=. ./mcp2210-util test config

EDIT Crap, I forgot about setting the MCP2210's own configuration. You should do this before writing the user-EEPROM area.

LD_LIBRARY_PATH=. ./mcp2210-util set config 31

You only need to do this once and the chip stores in it's private non-volatile memory. Be sure to run "test config" as it will verify that there are no indescrepencies between the MCP2210's configuration and the settings in the Creek configuration.

daniel-santos commented 5 years ago

I think the userland utility only needs /dev/spidev<n>.<n> when you are trying to send an spi command. There are other programs out there that do this as well. If you want to do userland SPI, look at settings-example.h and set .modalias = "spidev". For other uses, you don't need an spidev device, but you do need a /dev/usb2spi_bridge<n> device -- you get this as soon as the chip is probed.

I see a typo in the help text! The help text for the "spi" command says to use "-d", but it's "-D"!

I forgot to address the queing. Yes, both my driver and the MCP2210 are behaving as they MUST behave. In the real world, you don't receive an URB, do some work that takes a while and then respond. The USB standard gives you less than one millisecond to respond. This is the CORRECT way to do it.

Psalm27-4 commented 5 years ago

HI Daniel,

Thank you again for your feedback. It is very helpful and I've gotten a much better grasp on this. I guess what I'm trying to get clarification on is how can I send SPI commands from within the driver? I won't have a userland utility talking to this driver over SPI.

All that I have to do is read a register value from a spi peripheral over and over again from within this driver, on a timer, and copy it to user space somehow. Can I fopen("/dev/usb2spi_bridge") to send a spi command that way?

So for SPI communication, and to make use of the code in mcp2210-spi.c, I need to write a separate SPI protocol driver? I guess that is where I get confused as I know that your driver can talk over SPI, and this is a spi protocol driver with all the commands internal to send and receive messages.

I have never been able to get the eeeprom reading and config to work from the userland utility, because I get this error: Jul 31 10:28:59 u-build kernel: [ 280.357211] usb 2-2: eeprom_read_complete: creek magic not detected: 0xffffffff != 0x0df01dc0

So I issue some manual add_ctl_cmd() in the probe() function, passing in filled out cmd bodies for setting chip config and spi_config parameters, which works.

Currently I was adding several add_ctl_cmd calls to the end of the mcp2210_probe() function, after the initial process_commands call. For instance, to read a register from the HOLT chip over SPI, I was doing this:

struct mcp2210_cmd *holt_rd;
[...]
holt_rd = mcp2210_alloc_ctl_cmd(dev, MCP2210_CMD_SPI_TRANSFER, 0, NULL, 0, false, GFP_KERNEL);
holt_rd->req.head.req.spi.size = 2;
holt_rd->req.body.raw[0] = 0x84;
mcp2210_add_cmd((void *)holt_rd, true);

I have to do that 3 times to actually get the value from register 0x84:

First response the first time I get

Jul 25 12:02:01 u-build kernel: [  753.750099] URB in: 00000000: 42 00 02 10 00 00 0f 00 ff 01 ef 01 00 00 00 00  B...............
Jul 25 12:02:01 u-build kernel: [  753.750101] URB in: 00000010: 00 00 02 00 00 28 ca b4 a9 40 0c c6 95 65 06 01  .....(...@...e..
Jul 25 12:02:01 u-build kernel: [  753.750102] URB in: 00000020: 2a 61 18 46 11 09 83 71 88 0a 8a 16 42 80 1e 00  *a.F...q....B...
Jul 25 12:02:01 u-build kernel: [  753.750112] URB in: 00000030: 84 18 ec 5a 35 20 c6 9f 20 40 2c 16 60 92 30 42  ...Z5 .. @,.`.0B

Second read I get

Jul 25 12:02:01 u-build kernel: [  753.753419] URB in: 00000000: 42 00 00 20 00 00 0f 00 ff 01 ef 01 00 00 00 00  B.. ............
Jul 25 12:02:01 u-build kernel: [  753.753421] URB in: 00000010: 00 00 02 00 00 28 ca b4 a9 40 0c c6 95 65 06 01  .....(...@...e..
Jul 25 12:02:01 u-build kernel: [  753.753422] URB in: 00000020: 2a 61 18 46 11 09 83 71 88 0a 8a 16 42 80 1e 00  *a.F...q....B...
Jul 25 12:02:01 u-build kernel: [  753.753423] URB in: 00000030: 84 18 ec 5a 35 20 c6 9f 20 40 2c 16 60 92 30 42  ...Z5 .. @,.`.0B

Finally on the third read, I get the value of 0xCC stored in register 0x84

Jul 25 12:02:01 u-build kernel: [  753.757852] URB in: 00000000: 42 00 02 10 00 cc 0f 00 ff 01 ef 01 00 00 00 00  B...............
Jul 25 12:02:01 u-build kernel: [  753.757854] URB in: 00000010: 00 00 02 00 00 28 ca b4 a9 40 0c c6 95 65 06 01  .....(...@...e..
Jul 25 12:02:01 u-build kernel: [  753.757855] URB in: 00000020: 2a 61 18 46 11 09 83 71 88 0a 8a 16 42 80 1e 00  *a.F...q....B...
Jul 25 12:02:01 u-build kernel: [  753.757856] URB in: 00000030: 84 18 ec 5a 35 20 c6 9f 20 40 2c 16 60 92 30 42  ...Z5 .. @,.`.0B

I believe if SPI communication is correctly being used, (I.E using the code in mcp2210-spi.c) I won't have to send three read commands to get the value of the register. Is that correct?

Is it possible to add the functionality of a SPI protocol driver to your driver? Or can that be done in your driver, but eliminating the 3 commands it takes to read?

What my objective basically is, I only need to read one register from this spi peripheral over and over again on a timer and copy it to user space. It is very simple.

I'm looking at the alloc_cmd_type() function, but I'm not sure how to fill out the cmd_spi_msg struct. If this driver isn't a spi protocol driver, there must be a prewritten one in the linux distro already. I don't think any secondary driver would need to be written to talk to this chip...

Can I just fopen("/dev/usb2spi_bridge") from a userland program to do spi transfers that way? That would be my second alternative if using spi commands from within the driver wouldn't work.

I really do appreciate your input on this.

daniel-santos commented 5 years ago

I guess what I'm trying to get clarification on is how can I send SPI commands from within the driver?

YOU DON'T! You don't modify the driver, you write a goddam spi protocol driver like I said. Did you even google "spi protocol driver" like I said? I'm not going to help you with this idiocy. If you used the goddam userland utility to program the EEPROM you wouldn't get "creek magic not detected".

Don't waste my time asking for help and then ignore what I tell you.

Psalm27-4 commented 5 years ago

Hi Daniel, You are very rude, and very disrespectful. I understand you are offering this help for free, and I appreciate that. But just closing the question is rude, and calling me an idiot is too. Is this how you treat your colleagues at work? I doubt it. You wouldn't get anywhere in life by that.

I did google that, and it appears unnecessary when this driver can do it. Could you please explain why a separate SPI protocol driver has to be written in addition to this driver?

daniel-santos commented 5 years ago

Wow! I didn't call you an idiot, I called what you are doing idiocy because it is. This is free and open source software so you can do whatever you want with it, but if you think you're smart enough to flaunt the designs of the entire linux kernel development community then I don't have time for you. Now please stop posting here.