Am Mittwoch, den 17.03.2010, 08:39 +0100 schrieb christian pellegrin: > On Wed, Mar 17, 2010 at 3:35 AM, Feng Tang <feng.tang@xxxxxxxxx> wrote: > > Hi Christian, > > Hi, > > > your driver is posted on public. And the reasons I still posted driver here are: > > 1. It provides a console > > I'll provide a patch to the mainline driver for the console. Let me > just finish my daily job duties ;-) > > > 2. It use buffer read/write to meet our performance goal, max3110 doesn't have > > good FIFO, but spi controller have and we can leverage that for performance > > I think this line of thinking is wrong. SPI generic layer is well .... > generic. For example the platform I use is the S3C24xx which has a SPI > shift register depth of just one byte. And using DMA is not an option > because the setup of a DMA transfer takes tens of microseconds. > Additionally the SPI driver in mainline is based on bit-banging which > uses workqueues and so is limited by the fact that it fights with > other sched_other processes. Unfortunately an alternative driver which > was posted never got mainlined and we, who need it, have to maintain > it ourself. But this is another story, the bottom line is that we have > to cope with the most basic SPI support we can meet. > > This said I tested the throughput of the driver by doing a zmodem > transfer and, as far as I remember, the results were pretty good. > Please note that the transmission is already buffered in higher layer. > Don't be fooled by the fact that tx_empty is correctly implemented in > the actual driver: you *have* to implement it otherwise some things > like termios call tcdrain() will be broken. For example applications > that do RS485 rx/tx switching in user-space will go nuts without it. > As far receiving is concerned ..... Hi, tx_empty isn't that easy to implement if there is no way to get the tx-empty status from the uart, like with the MAX3100. There are several serial drivers that simply return TIOCSER_TEMT. I know it's really bad for applications like RS485... For this reason, and due to the lack of large of tx/rx fifos in the MAX3100, I'm using Atmel AVRs where you can easily implement larger fifos and reliable HW flow control in the firmware. Besides, the cost is just a fraction of the MAX3100. -Erwin > > > the datasheet say it supports 230400~300 baud rate, then driver guy need make > > it work as it claims, like copying 500 bytes string somewhere and paste it on > > this console in 230400/600 rate. > > .... this is exactly the problem I was facing: loosing chars on RX. I > tracked the problem to a too long latency in awakening of the > workqueue (the RX FIFO of the MAX3100 overflows). I will post shortly > a patch that moves this to threaded interrupts which solved the > problem on the S3C platform I mentioned above working at 115200. A > quick and dirty fix for workqueues is posted here: > http://www.evolware.org/chri/paciugo/index.html. Anyway it has to do > with scheduling of processes so many more factors are concerned, like > HZ and preemption being enabled. > > I will post some tests together with the patch, if you have some > benchmarks you like to be run don't hesitate to send them. Thanks in > advance! > > >> I'm more than happy to work with you to have just one perfect (or > >> almost perfect) driver instead of two of them half-done. See some > >> comments inline. > > Can't agree more, all I want is just a upstream driver which meets the basic > > requirement. > > > > I think upstream drivers have to meet the requirement of the average > user not be tailored to specific user-cases (see the Android code > inclusion debate). For example changing SPI speed to meet some rate of > data flow is wrong. Nothing will assure you that you get the rate you > asked: it all depends of the master SPI clock and the granularity > resulting by its division. So I still think the best way is poll the T > bit by using the SPI bus as little as possible. > > >> I don't think this is a good idea to change speed. Just check the T > >> bit to see when the TX buffer is empty and send at the maximum > >> available speed. So the usage of the SPI bus is better. > > Yes, checking T bit is a good idea for single word transfer model, but it > > doesn't help when using buffer read/write > > I really don't understand why, can you elaborate on this? > > > I didn't use kmalloc/kfree in my earlier submission, but other developers > > mentioned the buffer allocated here need be DMA safe, as 3110 may work > > with many kinds of SPI controller, some of them only supports DMA mode. > > > > why you don't preallocate buffer for SPI transfers that are DMA-safe? > For example the mcp251x driver > (http://lxr.free-electrons.com/source/drivers/net/can/mcp251x.c?v=2.6.34-rc1) > does this. > > >> I guess you are using mthread_up to avoid awakening the main thread > >> too many times. But this makes sense? Anyway because the awakening and > >> the setting of the bit is not atomic it won't work anyway. > > Why this can't work? the test/set_bit are atomic operations. This bit > > check is not exclusive, it just try to save some waking up, though waking a > > already running thread doesn't cost too much as mentioned by Andrew > > First of all I think that the cost of re-awakening an already running > thread is lower than the logic of testing bits. My note was just > nit-picking: there is a window between when the task was woken-up and > where you set the bit, so it doesn't guarantee you that you aren't > awakening the thread more than once anyway. > > Bye, > > PS.: sorry to have missed you previous emails where some points where > already discussed. Please CC me next time, unfortunately spi-devel > mailing list carries quite a bit of spam and so I have to search its > post in the trash folder. I realized this is the reason why I didn't > see your previous posts :-/ > -- Dipl.-Ing. Erwin Authried Softwareentwicklung und Systemdesign -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
