[spi-devel-general] [PATCH -mm 1/1] max3100: Fixes for the MAX31x0 console
This patch fixes problems reported about the MAX31x0 console. RTS is
asserted only if flow control is explicitly enabled on kernel command
line. Chars inputed during console output are not lost.
Signed-off-by: Christian Pellegrin
---
drivers/serial/max3100.c | 48 +
1 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/drivers/serial/max3100.c b/drivers/serial/max3100.c
index 6644222..c7def2f 100644
--- a/drivers/serial/max3100.c
+++ b/drivers/serial/max3100.c
@@ -147,6 +147,8 @@ struct max3100_port {
struct work_struct console_work;
/* char tx timeout */
int console_tout;
+ /* lock on receiving chars */
+ struct mutex rx_lock;
#endif
};
@@ -192,6 +194,19 @@ static void max3100_calc_parity(struct max3100_port *s,
u16 *c)
*c |= max3100_do_parity(s, *c) << 8;
}
+static void max3100_flip(struct max3100_port *s)
+{
+ if (s->port.state->port.tty != NULL) {
+#ifdef CONFIG_SERIAL_MAX3100_CONSOLE
+ mutex_lock(&s->rx_lock);
+#endif
+ tty_flip_buffer_push(s->port.state->port.tty);
+#ifdef CONFIG_SERIAL_MAX3100_CONSOLE
+ mutex_unlock(&s->rx_lock);
+#endif
+ }
+}
+
static void max3100_resume_work(struct work_struct *w)
{
struct max3100_port *s = container_of(w, struct max3100_port,
@@ -242,6 +257,10 @@ static int max3100_handlerx(struct max3100_port *s, u16 rx)
unsigned int ch, flg, status = 0;
int ret = 0, cts;
+#ifdef CONFIG_SERIAL_MAX3100_CONSOLE
+ mutex_lock(&s->rx_lock);
+#endif
+
if (rx & MAX3100_R && s->rx_enabled) {
dev_dbg(&s->spi->dev, "%s\n", __func__);
ch = rx & (s->parity & MAX3100_7BIT ? 0x7f : 0xff);
@@ -274,6 +293,10 @@ static int max3100_handlerx(struct max3100_port *s, u16 rx)
uart_handle_cts_change(&s->port, cts ? TIOCM_CTS : 0);
}
+#ifdef CONFIG_SERIAL_MAX3100_CONSOLE
+ mutex_unlock(&s->rx_lock);
+#endif
+
return ret;
}
@@ -329,8 +352,8 @@ static irqreturn_t max3100_ist(int irq, void *dev_id)
}
}
- if (rxchars > 16 && s->port.state->port.tty != NULL) {
- tty_flip_buffer_push(s->port.state->port.tty);
+ if (rxchars > 16) {
+ max3100_flip(s);
rxchars = 0;
}
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
@@ -341,8 +364,8 @@ static irqreturn_t max3100_ist(int irq, void *dev_id)
(!uart_circ_empty(xmit) &&
!uart_tx_stopped(&s->port;
- if (rxchars > 0 && s->port.state->port.tty != NULL)
- tty_flip_buffer_push(s->port.state->port.tty);
+ if (rxchars > 0)
+ max3100_flip(s);
return IRQ_HANDLED;
}
@@ -699,6 +722,7 @@ static void max3100_console_work(struct work_struct *w)
console_work);
unsigned long start;
u16 tx, rx;
+ int rxchars = 0;
while (s->console_head != s->console_tail &&
(s->console_flags & MAX3100_SUSPENDING) == 0) {
@@ -710,10 +734,19 @@ static void max3100_console_work(struct work_struct *w)
!time_after(jiffies, start + s->console_tout));
tx = s->console_buf[s->console_tail];
max3100_calc_parity(s, &tx);
- tx |= MAX3100_WD | MAX3100_RTS;
+ tx |= MAX3100_WD | (s->rts ? MAX3100_RTS : 0);
max3100_sr(s, tx, &rx);
+ if (s->port.state->port.tty != NULL) {
+ rxchars += max3100_handlerx(s, rx);
+ if (rxchars > 16) {
+ max3100_flip(s);
+ rxchars = 0;
+ }
+ }
s->console_tail = (s->console_tail + 1) % CONSOLE_BUF_SIZE;
}
+ if (rxchars)
+ max3100_flip(s);
}
static void max3100_console_putchar(struct uart_port *port, int ch)
@@ -809,6 +842,10 @@ static int max3100_console_setup(struct console *co, char
*options)
if (parity == 'o')
s->parity |= MAX3100_PARITY_ODD;
s->console_tout = 1 + (20 * HZ) / baud; /* jiffies to send 20 bits */
+ if (flow != 'n')
+ s->rts = 1;
+ else
+ s->rts = 0;
tx |= MAX3100_WC;
max3100_sr(s, tx, &rx);
@@ -912,6 +949,7 @@ static int __devinit max3100_probe(struct spi_device *spi)
i, retval);
#ifdef CONFIG_SERIAL_MAX3100_CONSOLE
+ mutex_init(&max3100s[i]->rx_l
[spi-devel-general] [PATCH -mm 0/1] max3100: Fixes for the MAX31x0 console
This patch fixes the problems reported by Feng Tang in the console code. After this I intend to do a complete rework to apply the ideas outlined by Grant Likely to avoid the use of delayed work and, if I find a way, the batching of TX requests proposed by Feng Tang and Alan Cox (I guess I will have at least to add a new function to the SPI core to know the real bit/rate the system is using to make this work). The relevant patches from the -mm kernel were tested on a 2.6.34-rc2 kernel on a S3C ARM machine. This patch ris based on them. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v1 3/4] max3100: adds console support for MAX3100
On Thu, Apr 8, 2010 at 11:31 AM, christian pellegrin wrote: > On Mon, Mar 29, 2010 at 4:48 AM, Feng Tang wrote: > >>> + tx |= MAX3100_WD | MAX3100_RTS; >> >> Does this imply to have to work with HW flow control? on my platform >> I have to remove the RTS bit to make it work. >> > > Finally I had time to check this. If you compare the 8250 (or similar) > data-sheet with the MAX31x0 one you see that the handling of the > RTX/CTS bits is exactly the same (8250 about RTS bit for example: > "When any of these bits are cleared, the associated output is forced > high." and MAX3110: "Request-to-Send Bit. Controls the state of the > RTS output. This bit is reset on power-up (RTS > bit = 0 sets the RTS pin = logic high)."). If you look at the 8250.c > driver (grep for UART_MCR_RTS) you notice that the bit is set on > device open (together with DTR of course). So I think the driver is > doing the right thing here. > anyway I'll set it to on only in case the flow control is enabled. So even if, for some reason, you have to keep it to off on your platform, it won't make troubles. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v1 3/4] max3100: adds console support for MAX3100
On Mon, Mar 29, 2010 at 4:48 AM, Feng Tang wrote: >> + tx |= MAX3100_WD | MAX3100_RTS; > > Does this imply to have to work with HW flow control? on my platform > I have to remove the RTS bit to make it work. > Finally I had time to check this. If you compare the 8250 (or similar) data-sheet with the MAX31x0 one you see that the handling of the RTX/CTS bits is exactly the same (8250 about RTS bit for example: "When any of these bits are cleared, the associated output is forced high." and MAX3110: "Request-to-Send Bit. Controls the state of the RTS output. This bit is reset on power-up (RTS bit = 0 sets the RTS pin = logic high)."). If you look at the 8250.c driver (grep for UART_MCR_RTS) you notice that the bit is set on device open (together with DTR of course). So I think the driver is doing the right thing here. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v1 3/4] max3100: adds console support for MAX3100
Hi, first of all I'm sorry for the late response, just now I found time to work on this. On Wed, Mar 31, 2010 at 8:04 AM, Grant Likely wrote: > Is checking the T bit really necessary if Linux instead tracks the > timing between bytes internally? ie. If the driver uses an hrtimer > to schedule the submission of SPI transfers such that the time between > SPI submissions is always greater than the time required for a serial > character to be transmitted? I think you have to check it anyway. For example the SPI bus may be shared with another device so we don't know when our char will be sent (it might be delayed for more than the duration of a char being sent on the serial line if the initial execution of the SPI command is delayed). But using a hrtimer will be for sure more fair than polling T bit as far as resource usage is concerned. I was always hesitant about using hrtimers: I really don't know if all platforms support them with the needed granularity (at 115200 a char takes around 100us) and the aren't many users of them in the drivers directory (quite all of them are in the staging one). But it's definitely a good idea if hrtimers do work. I'll make some tests. > > You may be able to set this up into a free-running state machine. > Submit the SPI transfers asynchronously, and use a callback to be > notified when the transfer is completed. In most cases, the transfer > will be completed every time, but if it is not, then the state machine > can just wait another time period before submitting. By doing it this > way, all the work can be handled at the atomic context without any > workqueues or threaded IRQ handlers. > yes a completely async design could improve performance (the greatest culprit for low performance (not mentioning slow SPI master drivers) is the latency in the delayed work being started). When I first wrote this driver I wanted to keep it simple so I was a bit frightened by a state-machine like design, but it can be done for sure. My concern here is: everything is the kernel is moving to doing as much as possible in a delayed work mechanics (see the introduction of threaded interrupts (which could became the default) or the "coming soon" death of IRQF_DISABLED). Is doing a big part of the work (of course I would use spi_async directly in the interrupt handler and handle the incoming/outgoing chars in spi_async callback which is usually called in an interrupt context) in an interrupt context "antihistorical", isn't it? BTW: both of the design changes you mentioned seem sensible to me for better performance of the driver. But they don't do any form of batching and won't help if the underlying SPI master driver uses some form of delayed work itself. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v1 3/4] max3100: adds console support for MAX3100
On Tue, Mar 30, 2010 at 10:46 AM, Alan Cox wrote: > > There I would partly disagree. Fixing the spi driver clearly makes sense > but the serial driver should be batching better. Is there a reason the > driver couldn't adjust the size based upon the tty speed when getting a > termios request ? Just a small note for those who don't know max31x0. There are just 2 instructions to tx/rx data: simple data rx (in which we read one data from the fifo (the data is valid if R status bit is set), control lines (CTS) and status bits) and a combined tx/rx (like before but we also send one byte (if the T status bit the char will be accepted) and set control lines (RTS)). Every of these instructions is 2 bytes long. It's a full SPI message (so you have to assert CS before and de-assert it afterwards to make it work). TX buffer is just 1 byte deep, RX one is 8 bytes. Unfortunately we cannot batch TX if SPI speed (in chars going in / time unit) is higher than the baud rate of serial communication. We need to check that T bis is asserted before sending a char, otherwise it is lost. For low baud rates doing this means monopolizing the bus. And there is a subtle problem if we reduce SPI speed to the UART baud rate: we can ask only for a maximum speed on the SPI. In case this is lower the one we asked (maybe because of SPI clock divisors) and we do a TX batch big enough we could not be emptying the RX FIFO fast enough and so we risk to loose chars in a full duplex operation (*). We could do batch RX operation but perhaps we are just transferring bytes for nothing because we don't know in advance if the R bit is set. And we cannot interleave TX and RX like current drivers do because we are forced to use the simple rx operation and not the combined rx/tx. There's another point about batching: single instructions are complete SPI messages. Many SPI master drivers I worked with (S3C24xx, AT91 because of a bug in the hardware) manage CS as a normal gpio. Another problem here is that many devices require a configurable delay on CS change (see delay_usecs field of spi_transfer) which is not handle by hardware and so the drivers must somehow break a SPI messages in single transfers and do something at the end of every one of these. So even we batch 10 tx/rx instructions I really don't think many SPI master controllers will be able to do a DMA of 20 bytes. Anyway I agree that if we find a way of doing batching it's a good thing because better controllers can benefit. > >> 2) even worse is that we can do flow control decision only on such boundary. > > For serial flow control it doesn't matter, its implicitly asynchronous > and if you only turn the fifo on at high speed you response time will be > reasonably bounded. > if we somehow manage to batch a TX of n bytes and after the first one CTS changes we are going to send n-1 chars anyway. OK, maybe our peer de-asserted CTS when still having n-1 byes of buffer free and somehow higher layer protocols will recover. So I agree this could be not so worrisome. But we should keep n small enough. >> 3) this is not reliable: think of what happens if the actual SPI >> transfer speed we get will be slower that the one we requested. We >> won't be emptying the RX buffer fastly enough even if could. > > Consoles are not usually balanced for I/O. I grant you probably don't > want to be using full fifo sized blocks but I'm not sure I understand why > there is a problem below that ? > My concern is expressed above (*). Perhaps it's me missing some point. To make it clear: I'm more than happy to test a driver that takes another approach and switch to it if it's better (but it has to have support of multiple instances and a fair usage of SPI bandwidth at least). But I really don't see a reliable way of adding batching of tx/rx. And I think it will be better to provide a patch to the current driver than to rewrite it completely. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v1 3/4] max3100: adds console support for MAX3100
On Tue, Mar 30, 2010 at 9:19 AM, Feng Tang wrote: > No words in my last email suggested to use dynamic SPI clock speed, > I admited that method is brutal and not mature when my driver was > reviewed. In early version of our 3110 driver, we don't use dynamic > clock speed, but the maxim clock supports by 3110, while we still use > 8 words buffer read for RX. > > Let me cut my question clear: why not use the 8 words RX HW fifo? For > bulk RX in 230400 case, is 50us enough to handle controller IRQ + > spi subsystem overhead + tty receive overhead for one char per spi > transfer model? > For TX it's impossible. Before sending a char we have to check if T bit says that the TX buffer is empty. Otherwise if SPI bus speed > UART baud rate we will loose TX chars for sure. For RX maybe it might be worth to always do 8 byte transfers and the look at R bit of transfer i to see if we have a valid char in (i+1) instead of doing single transfers. But here again we are fixing the insane latency per start of transfer of the underlying SPI master controller. And let me say that in the actual driver the 8 byte RX fifo *is* used: the loop in max3100_ist drains RX buffer at a speed greater than the chars coming (also at 115200). The 8-bytes RX buffer fills as a consequence of delays in interrupt thread being awaken. The tests I did with zmodem show that in the direction max3100 <- PC has very good efficiency compared to max3100 -> PC. I guess the reason is that the tx buffer is just 1 byte deep and we don't have any other way to handle the T bit other than the do loop in max3100_ist. Maybe I will try to do some instrumentation with systemtap to show this (it's always nice to find some uses for systemtap). Anyway I'm sure that it's always better to not loose chars being received than delaying transmitted one for a bit. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v1 3/4] max3100: adds console support for MAX3100
On Tue, Mar 30, 2010 at 4:14 AM, Feng Tang wrote: > On Mon, 29 Mar 2010 20:55:51 +0800 > DW controller driver don't need max3110 driver to use big spi transfer, > the early version of our max3110 is also one word per spi transfer mode, > and the 2 drivers work fine, though the rx performance is not good (copy > & paste a bulk message to console). What I'm saying is that the reason of bad performance is that the underlying SPI driver has bad performance. A SPI master driver should do as little work as possible. For small transfers (2 bytes == 16 bits) at 5MHz (== 3.2 us) it's much better to spin in spi_transfer waiting for SPI done than scheduling anything or setting up a DMA transfer. Especially if you do *2* task schedule (spi_transfer queues a workqueue that schedules a tasklet) be prepared for *big* latency. I hope other people can comment on this if I'm saying something wrong. > > When the HW works at 230400 bps, when we copy bulk data to the console, > max3110 will probably receive about 20K characters, so the time for > handling each char is 50us. If you use one char per spi transfer, it > will have to execute 20K spi_transfer in one second, and each need be > done in 50us, in this 50us you'll have to deal with controller IRQ > handler + spi system overhead + tty receiving overhead. Is it a little > over-killing to use one char per spi transfer? while the HW does > have a 8 words RX FIFO this is too hackish, max3100 wasn't conceived this way. Let me point some problems of such an approach: 1) latency on rx chars becomes very high because we can process incoming transfers only after a full 8 byte (or whatever the spi transfer dimension is). For a 9600 this is too much. 2) even worse is that we can do flow control decision only on such boundary. 3) this is not reliable: think of what happens if the actual SPI transfer speed we get will be slower that the one we requested. We won't be emptying the RX buffer fastly enough even if could. 4) for low speeds we are going to monopolize the SPI bus for ages. So I'm rather convinced that the SPI transfer has to happen as soon as possible at a SPI device driver level without any guess on how the data will be clocked out. I suggest you to improve the dw_spi driver for better performance. > [[ ..00 iiuu eessoo 44rr11((eeggffnn--77 > ggccvvrriinn11((bbnnuu1144bbnnuu > [[ ..00 BBOO--8800 1100--8800 uuaall)) huh, here I'm seeing another kind of problem: the same char is repeated two times (for example BBOO is written instead of BIOS). I'm quite sure (but I will triple check shortly) the the console driver doesn't do this (it's quite an easy loop in max3100_console_work, I will check wit a printascii to another serial port what is sent). So I guess the problem can be in the SPI master driver: is it correctly handling the CS changes? Isn't it having problems with DMA for example (I always found DMA for small data transfer (such 2 bytes in this case) problematic (for example many platforms just do it on a 4 byte boundary))? Have you tested it with other devices? -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v1 3/4] max3100: adds console support for MAX3100
On Mon, Mar 29, 2010 at 9:06 AM, Feng Tang wrote: > Our platform is Intel Moorestown platform, and use a spi controller core > from Designware (drivers/spi/dw_*.c). I know the problem may probably be > caused by my setting, but the dw_spi driver works fine with our own > 3110 driver. I had a look at the dw_spi driver. The spi_transfer path queues some work to a worqueue that itself schedules a tasklet. I don't think this is good for latency, I won't bet that such an architecture could deliver good performance. Now I see why you needed to do only big fat SPI transfers. Anyway this doesn't justify the 2 seconds delay between chars coming in and going out through the max31x0 you are seeing. I will try to analyze what's going on. BTW is only input slow or output is sluggish too? The initial messages from the console are coming out fast? If you disable the MAX3110 for console but you use just as a normal terminal (set console=/dev/null in the kernel command line and add getty 115200 /dev/ttyMAX0 in iniitab) is the interaction with the system fine? Thanks for helping sorting out this. >> >> yes but I think it's quite difficult to solve this problem in every >> case. Console output is massively used only on boot when the user is >> not supposed to type a lot. > > It's difficult but not impossible, actually our driver checks every word > read back and handle it if it contains a valid data > Of course it is possible, I just wanted to keep the max3100 a small clean driver. Unfortunately console and serial drivers are two different beings in the Linux kernel, but the max3100 implements the tx-rx in one indivisible instruction (that is slow compared to registers IO and has to be called in an preemptible contex for added fun). To implement what you are saying we need: 1) the console code has to check if the serial port associated to the same physical max3100 is up (console driver start their life much before serial ones). 2) if yes send data to the tty associated to the serial driver. Locking is needed here. I will implement this ASAP. >> Have you configured your SPI controller as LSB first somehow, haven't >> you? BTW my platform is a quite usual ARM9 S3C2440 which is little >> endian. >> > yeah, you hit the point that our spi controller is LSB naturally (not > configured to), here may need a check for whether to do a swap > ok, I think the dw_spi driver has to be fixed. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v1 3/4] max3100: adds console support for MAX3100
On Mon, Mar 29, 2010 at 4:48 AM, Feng Tang wrote:
> Hi,
Hi,
>
> I modified the code a little and run it on our HW platform, it really show
> some sigh of life: it can boots to console (the print format is not so good),
> I can input command and it execute correctly, but very slow, I type 3
> characters and it takes about 2 seconds to echo back on screen and start the
> execution, and after about 1 minute, the console hang there and input stopped
> to work.
never seen such a behavior. Which platform are you using? Which SPI
driver? Do you have a low level printk (printascii) that puts output
somewhere else so I can send you a patch with some debugging output?
Can you log in some other way (like via network) and see if the CPU
load is at 100% for some reason?
>> This patch adds console support for the MAX3100 UART
>> (console=ttyMAX0,11500). The SPI subsystem and an
>
> 115200?
>
ack
> Does this imply to have to work with HW flow control? on my platform
> I have to remove the RTS bit to make it work.
>
no, I put RTS on because it looks like a good default. I can make it
configurable. I just noticed on the data sheet that RTS is actually
inverted so a more sensible default would be to put it off. For
testing you should have flow control set to none on the machine you
are using as a terminal emulator.
>> + max3100_sr(s, tx, &rx);
>
> It doesn't handle received characters here? If the console is printing out
> a bulk of message while user input some command, the command may be ignored.
> Myself have met the same problem in our driver.
>
yes but I think it's quite difficult to solve this problem in every
case. Console output is massively used only on boot when the user is
not supposed to type a lot.
>> + if (next != s->console_tail) {
>> + s->console_buf[next] = ch;
>> + s->console_head = next;
>> + }
>
> Also I saw max3100_sr() uses cpu_to_be16() and be16_to_cpu(), is it really
> necessary, our platform is little-endian(x86), and I have to disable them
> to make the code work. Is your test platform big-endian?
>
Have you configured your SPI controller as LSB first somehow, haven't
you? BTW my platform is a quite usual ARM9 S3C2440 which is little
endian.
--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH 3/3] max3100: adds console support for MAX3100
On Mon, Mar 22, 2010 at 2:31 AM, Feng Tang wrote: > > Hi Christian, Hi, > > You'd better not call max3110_sr (which calls spi_sync) here, a console's > putchar() func may be called in some atomic context. > ack, I didn't realize that. Unfortunately this makes reliable use of MAX3100 as a console near impossible. Some SPI master driver use workqueues (see the bit banging ones). I don't see any way to yield CPU to them while somehow wait the output has finished (a busy loop won't work: perhaps the thread calling the console has a higher priority than the one doing actual SPI output so the system will deadlock). The only way out I see is to provide a buffer large enough for all practical purposes. Will try it in the next version of the patch. Thanks. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH 3/3] max3100: adds console support for MAX3100
On Fri, Mar 19, 2010 at 8:31 PM, Grant Likely wrote:
>> +config SERIAL_MAX3100_CONSOLE_N
>> + int "Number of MAX3100 chips to wait before registering console"
>> + depends on SERIAL_MAX3100_CONSOLE=y
>> + default "1"
>> + help
>> + Unfortunately due to the async nature of Linux boot we must
>> + know in advance when to register the console. If we do it
>> + too early not all the MAX3100 chips are known to the system
>> + yet. And we just cannot know how many MAX3100 will be in the
>> + system so it's impossible to wait for the last one. If you
>> + just want to have the console on the first MAX3100 chip
>> + probed (ttyMAX0) it's safe to leave this to 1.
>
> This seems wrong and fragile. It certainly isn't multiplatform safe
> to have it as a CONFIG setting because every board will have a
> different number of devices that it needs to wait for. I don't know
> the console code very well though, but it seems to me that there
> should be a way to register the console regardless and then be able to
> hold off output until the actual backing device shows up.
>
> Anybody know the right thing to do here?
I'll try to dig in the console code, any suggestion from others if
more than welcome.
>> + else {
>> + u16 tx, rx;
>
> inconsistent braces. If you use {} on the else side, then please use
> {} on the if side too.
ack, strange checkpatch.pl didin't catch these.
>> + IRQF_TRIGGER_FALLING, "max3100", s) < 0) {
>
> try to avoid unrelated whitespace changes, it adds noise when reviewing.
>
ack
>> + parity |= MAX3100_PARITY_ON;
>
> s->parity?
>
huh, well spotted. ack
>> + } else {
>> + tx &= ~MAX3100_PE;
>> + parity &= ~MAX3100_PARITY_ON;
>> + }
>> +
>> + if (parity == 'o')
>> + parity |= MAX3100_PARITY_ODD;
>> + else
>> + parity &= ~MAX3100_PARITY_ODD;
>
> ditto?
ack
>
>
> Also, these blocks are really verbose; maybe do this:
ack, I'll reorganize this chunk of code.
>
>> +
>> + msleep(50);
>
> Why the sleep? Shouldn't the console driver be able to handle the SPI
> device not being ready yet?
it's for the max3100 to settle. I'll double check the needed time and
add a comment.
>> + max3100_sr(max3100s[i], tx, &rx);
>> + }
>
> Same comment on braces as for above.
>
ack
Thanks again for the review.
--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH 2/3] max3100: moved to threaded interrupt
Hi, On Fri, Mar 19, 2010 at 6:58 PM, Grant Likely wrote: > > On a quick parse, it seems mostly okay. I haven't dug into the driver > execution flow though. A few minor comments below. > thank you very much for the review. I'm waiting a bit for other people to comment and then prepare another patch. >> + resume_work); > > container_of is used a lot. Maybe consider a follow-on patch to > create a to_max3100_port() static inline. > ack >> +static irqreturn_t max3100_ist(int irq, void *dev_id) > > 'ist'? > interrupt service thread (compared to isr, interrupt service routine, for the code executed in hard interrupt context). I know it's windozece-speak and so *very lame*. Any suggestion for a better name is welcome! -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH 3/3] max3100: adds console support for MAX3100
This patch adds console support for the MAX3100 UART
(console=ttyMAX0,11500). The SPI subsystem and an
suitable SPI master driver have to be compiled in the kernel.
Signed-off-by: Christian Pellegrin
---
drivers/serial/Kconfig | 20 +
drivers/serial/max3100.c | 184 +
2 files changed, 187 insertions(+), 17 deletions(-)
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 9ff47db..bc72e84 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -547,6 +547,26 @@ config SERIAL_MAX3100
help
MAX3100 chip support
+config SERIAL_MAX3100_CONSOLE
+ bool "Support console on MAX3100"
+ depends on SERIAL_MAX3100=y
+ select SERIAL_CORE_CONSOLE
+ help
+ Console on MAX3100
+
+config SERIAL_MAX3100_CONSOLE_N
+ int "Number of MAX3100 chips to wait before registering console"
+ depends on SERIAL_MAX3100_CONSOLE=y
+ default "1"
+ help
+ Unfortunately due to the async nature of Linux boot we must
+ know in advance when to register the console. If we do it
+ too early not all the MAX3100 chips are known to the system
+ yet. And we just cannot know how many MAX3100 will be in the
+ system so it's impossible to wait for the last one. If you
+ just want to have the console on the first MAX3100 chip
+ probed (ttyMAX0) it's safe to leave this to 1.
+
config SERIAL_DZ
bool "DECstation DZ serial driver"
depends on MACH_DECSTATION && 32BIT
diff --git a/drivers/serial/max3100.c b/drivers/serial/max3100.c
index 0c972c6..f6d250d 100644
--- a/drivers/serial/max3100.c
+++ b/drivers/serial/max3100.c
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
@@ -131,6 +132,10 @@ struct max3100_port {
int poll_time;
/* and its timer */
struct timer_list timer;
+
+ int console_flags;
+ /* is this port a console? */
+#define MAX3100_IS_CONSOLE (1 << 0)
};
static struct max3100_port *max3100s[MAX_MAX3100]; /* the chips */
@@ -175,7 +180,8 @@ static void max3100_resume_work(struct work_struct *w)
struct max3100_port *s = container_of(w, struct max3100_port,
resume_work);
- raise_threaded_irq(s->irq);
+ if (s->irq)
+ raise_threaded_irq(s->irq);
}
static void max3100_timeout(unsigned long data)
@@ -552,14 +558,16 @@ static void max3100_shutdown(struct uart_port *port)
if (s->irq)
free_irq(s->irq, s);
- /* set shutdown mode to save power */
- if (s->max3100_hw_suspend)
- s->max3100_hw_suspend(1);
- else {
- u16 tx, rx;
+ if (!(s->console_flags & MAX3100_IS_CONSOLE)) {
+ /* set shutdown mode to save power */
+ if (s->max3100_hw_suspend)
+ s->max3100_hw_suspend(1);
+ else {
+ u16 tx, rx;
- tx = MAX3100_WC | MAX3100_SHDN;
- max3100_sr(s, tx, &rx);
+ tx = MAX3100_WC | MAX3100_SHDN;
+ max3100_sr(s, tx, &rx);
+ }
}
}
@@ -578,10 +586,8 @@ static int max3100_startup(struct uart_port *port)
s->parity = 0;
s->rts = 0;
- INIT_WORK(&s->resume_work, max3100_resume_work);
-
if (request_threaded_irq(s->irq, NULL, max3100_ist,
- IRQF_TRIGGER_FALLING, "max3100", s) < 0) {
+IRQF_TRIGGER_FALLING, "max3100", s) < 0) {
dev_err(&s->spi->dev, "cannot allocate irq %d\n", s->irq);
s->irq = 0;
return -EBUSY;
@@ -699,6 +705,136 @@ static struct uart_ops max3100_ops = {
.verify_port= max3100_verify_port,
};
+#ifdef CONFIG_SERIAL_MAX3100_CONSOLE
+
+static void max3100_console_putchar(struct uart_port *port, int ch)
+{
+ struct max3100_port *s = container_of(port, struct max3100_port, port);
+ unsigned long tout = 10 * HZ / 1000; /* 10ms */
+ unsigned long start;
+ u16 tx, rx;
+
+ if (tout == 0)
+ tout = 1;
+ start = jiffies;
+ do {
+ tx = MAX3100_RC;
+ max3100_sr(s, tx, &rx);
+ } while ((rx & MAX3100_T) == 0 &&
+!time_after(jiffies, start + tout));
+ tx = ch;
+ max3100_calc_parity(s, &tx);
+ tx |= MAX3100_WD | MAX3100_RTS;
+ max3100_sr(s, tx, &rx);
+}
+
+static void max3100_console_write(struct console *co,
+ const char *str, unsigned int count)
+{
+ struct max3100_port *s = max3100s[co->index];;
+
+ uart_console_write(&am
[spi-devel-general] [PATCH 2/3] max3100: moved to threaded interrupt
The driver was reworked to use threaded interrupts instead of workqueus.
This is a major boost in performance because the former are scheduled as
SCHED_FIFO processes. As a side effect suspend/resume code was greatly
simplified.
Signed-off-by: Christian Pellegrin
---
drivers/serial/max3100.c | 102 ++---
1 files changed, 32 insertions(+), 70 deletions(-)
diff --git a/drivers/serial/max3100.c b/drivers/serial/max3100.c
index 3c30c56..0c972c6 100644
--- a/drivers/serial/max3100.c
+++ b/drivers/serial/max3100.c
@@ -45,7 +45,9 @@
#include
#include
#include
-#include
+#include
+#include
+#include
#include
@@ -113,18 +115,15 @@ struct max3100_port {
int irq;/* irq assigned to the max3100 */
+ /* the workqueue is needed because we cannot schedule
+ a threaded interrupt during PM
+*/
+ struct work_struct resume_work;
+
int minor; /* minor number */
int crystal;/* 1 if 3.6864Mhz crystal 0 for 1.8432 */
int loopback; /* 1 if we are in loopback mode */
- /* for handling irqs: need workqueue since we do spi_sync */
- struct workqueue_struct *workqueue;
- struct work_struct work;
- /* set to 1 to make the workhandler exit as soon as possible */
- int force_end_work;
- /* need to know we are suspending to avoid deadlock on workqueue */
- int suspending;
-
/* hook for suspending MAX3100 via dedicated pin */
void (*max3100_hw_suspend) (int suspend);
@@ -171,13 +170,12 @@ static void max3100_calc_parity(struct max3100_port *s,
u16 *c)
*c |= max3100_do_parity(s, *c) << 8;
}
-static void max3100_work(struct work_struct *w);
-
-static void max3100_dowork(struct max3100_port *s)
+static void max3100_resume_work(struct work_struct *w)
{
- if (!s->force_end_work && !work_pending(&s->work) &&
- !freezing(current) && !s->suspending)
- queue_work(s->workqueue, &s->work);
+ struct max3100_port *s = container_of(w, struct max3100_port,
+ resume_work);
+
+ raise_threaded_irq(s->irq);
}
static void max3100_timeout(unsigned long data)
@@ -185,7 +183,7 @@ static void max3100_timeout(unsigned long data)
struct max3100_port *s = (struct max3100_port *)data;
if (s->port.state) {
- max3100_dowork(s);
+ raise_threaded_irq(s->irq);
mod_timer(&s->timer, jiffies + s->poll_time);
}
}
@@ -255,9 +253,9 @@ static int max3100_handlerx(struct max3100_port *s, u16 rx)
return ret;
}
-static void max3100_work(struct work_struct *w)
+static irqreturn_t max3100_ist(int irq, void *dev_id)
{
- struct max3100_port *s = container_of(w, struct max3100_port, work);
+ struct max3100_port *s = dev_id;
int rxchars;
u16 tx, rx;
int conf, cconf, rts, crts;
@@ -314,23 +312,14 @@ static void max3100_work(struct work_struct *w)
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
uart_write_wakeup(&s->port);
- } while (!s->force_end_work &&
-!freezing(current) &&
+ } while (!kthread_should_stop() &&
((rx & MAX3100_R) ||
(!uart_circ_empty(xmit) &&
!uart_tx_stopped(&s->port;
if (rxchars > 0 && s->port.state->port.tty != NULL)
tty_flip_buffer_push(s->port.state->port.tty);
-}
-
-static irqreturn_t max3100_irq(int irqno, void *dev_id)
-{
- struct max3100_port *s = dev_id;
-
- dev_dbg(&s->spi->dev, "%s\n", __func__);
- max3100_dowork(s);
return IRQ_HANDLED;
}
@@ -353,7 +342,7 @@ static void max3100_start_tx(struct uart_port *port)
dev_dbg(&s->spi->dev, "%s\n", __func__);
- max3100_dowork(s);
+ raise_threaded_irq(s->irq);
}
static void max3100_stop_rx(struct uart_port *port)
@@ -369,7 +358,7 @@ static void max3100_stop_rx(struct uart_port *port)
s->conf &= ~MAX3100_RM;
s->conf_commit = 1;
spin_unlock(&s->conf_lock);
- max3100_dowork(s);
+ raise_threaded_irq(s->irq);
}
static unsigned int max3100_tx_empty(struct uart_port *port)
@@ -381,7 +370,7 @@ static unsigned int max3100_tx_empty(struct uart_port *port)
dev_dbg(&s->spi->dev, "%s\n", __func__);
/* may not be truly up-to-date */
- max3100_dowork(s);
+ raise_threaded_irq(s->irq);
return s->tx_empty;
}
@@ -394,7 +383,7 @@ static unsigned int max3100_get_mctrl(struct uart_port
*port)
dev_dbg(&s->spi->dev, "%s\n&
[spi-devel-general] [PATCH 1/3] max3100: added raise_threaded_irq
raise_threaded_irq schedules the execution of an interrupt thread
Signed-off-by: Christian Pellegrin
---
include/linux/interrupt.h |3 +++
kernel/irq/manage.c | 27 +++
2 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 75f3f00..14c0c13 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -144,6 +144,9 @@ request_threaded_irq(unsigned int irq, irq_handler_t
handler,
static inline void exit_irq_thread(void) { }
#endif
+extern int raise_threaded_irq(unsigned int irq);
+
+
extern void free_irq(unsigned int, void *);
struct device;
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index eb6078c..a7d21e0 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1088,3 +1088,30 @@ int request_threaded_irq(unsigned int irq, irq_handler_t
handler,
return retval;
}
EXPORT_SYMBOL(request_threaded_irq);
+
+/**
+ * raise_threaded_irq - triggers a threded interrupt
+ * @irq: Interrupt line to trigger
+ */
+int raise_threaded_irq(unsigned int irq)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+ struct irqaction *action;
+
+ if (!desc)
+ return -ENOENT;
+ action = desc->action;
+ if (!action)
+ return -ENOENT;
+ if (unlikely(!action->thread_fn))
+ return -EINVAL;
+ if (likely(!test_bit(IRQTF_DIED,
+&action->thread_flags))) {
+ set_bit(IRQTF_RUNTHREAD, &action->thread_flags);
+ wake_up_process(action->thread);
+ } else {
+ return -ECHILD;
+ }
+ return 0;
+}
+EXPORT_SYMBOL(raise_threaded_irq);
--
1.5.6.5
--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH 0/3] max3100: improvements
Hi all, first of all I'm sending this patch to all the people that were in CC to the original Feng's email. Let me know if this bothers you. This bunch of patches should solve the problems noted by Feng to the max3100 driver. It was tested on an S3C2440 system which has a rather bad SPI master controller. The efficiency is very good on zmodem receive (near 100% at 115200) and not so bad on zmodem send (around 95% at 115200). I guess the reason is the TX buffer of the MAX3100 being just one byte deep. All the tests were done with two MAX3100 running on the same SPI bus: the first one as a console and under test, the second one happily receiving a 4800 bit/s NMEA stream from a GPS (with a process checking we don't get wrong sentences). I also did the simple "cut&paste a screenful of charters" test and it worked! The console was stressed as much I could (things like putting the S3C to suspend during a zmodem transfer and checking it restarts after resume). -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v6] serial: spi: add spi-uart driver for Maxim 3110
, 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 :-/ -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] spi-devel-general Digest, Vol 46, Issue 10
Hi,
On Thu, Mar 4, 2010 at 8:06 PM,
wrote:
>> + if (use_irq) {
>> + ret = request_irq(max->irq, serial_m3110_irq,
>> + IRQ_TYPE_EDGE_FALLING, "max3110", max);
>
> According to the manufacturer's datasheet, it looks like MAX3110'irq is
> level interrupt. Refer Figure 6 of the datasheet.
>
It should work anyway if you are sure you clear the the condition that
generated the interrupt (so the interrupt line will have another
transition for sure). It's quite common when using deferred work
(worqueues for example) otherwise you have to turn irq off to avoid an
interrupt storm. Usually turning interrupts on and off is quite tricky
because they are counted.
>> + pmax = max;
>
> If this driver supports only one instance of devices, how about
> declaring a global struct uart_m3100 instead of kmallc()?
>
huh didn't noticed that it supports only one instance. I guess this is
very bad (my test machine for example has 2 MAX3100s, one for a GPS
receiver and one for a slow RS485 link).
--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH v6] serial: spi: add spi-uart driver for Maxim 3110
On Thu, Mar 4, 2010 at 8:22 AM,
wrote:
>
> Hi All,
>
Hi, first of all let me apologize that I haven't see your previous
versions of the driver. I completely missed them. I am the author of
the max3100 driver. As far as I can see the max3110 is just a max3100
with the TTL to EIA level converter. So, first of all, may I ask why
you did start a new driver from scratch? I'm quite happy with the
current one, the only problem I see is that it uses worqueues which
are scheduled like normal processes and so under load their latency is
*big*. I wanted to move to threaded interrupts (otherwise you have to
use some ugly tricks to give workqueue threads real-time scheduling
class) but was waiting for my beta testers^H^H^H^H^H^H^H^H^H^H^H^H^H
customers to move to modern kernel that support them. Anyway the patch
is trivial and I can provide it if there is some interest. A quick
glance at your patch shows you are using the same sched_other threads
so you will face the same problems. I saw you added console capability
which isn't present in current driver, but it's easy to add (I didn't
implement it initially because I don't see a good idea using the
max3100 for initial bring-up of an embedded system because it relies
on the SPI subsystem, worqueues, scheduler and such complicated
things). Anyway keep in mind that the MAX3100 is rather low-end UART
so don't push it too much.
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.
> + * 1. From Max3110 spec, the Rx FIFO has 8 words, while the Tx FIFO only has
> + * 1 word. If SPI master controller doesn't support sclk frequency change,
> + * then the char need be sent out one by one with some delay
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.
> + *
> + * 2. Currently only RX availabe interrrupt is used
yeah, this is one of the reasons why MAX31x0 sucks: when you change
configuration of interrupts the RX buffer is cleared. So you cannot
activate/deactivate TX empty interrupt as needed.
> +static int use_irq = 1;
> +module_param(use_irq, int, 0444);
> +MODULE_PARM_DESC(use_irq, "Whether using Max3110's IRQ capability");
I don't think it's a good idea using an UART in polling mode, it just
kills the system. Just pretend this is not possible otherwise some
hardware guys will pester us to do this just to save an electric wire.
> + memset(&x, 0, sizeof x);
> + x.len = len;
> + x.tx_buf = txbuf;
> + x.rx_buf = rxbuf;
why don't initialize this like:
struct spi_transfer x = {
.tx_buf = txbuf,
.rx_buf = rxbuf,
.len = len,
};
> + buf = kmalloc(8, GFP_KERNEL | GFP_DMA);
you do kmalloc and kfree in routines that are called in quite tight
loops: I guess it's an overkill for performance.
> +static unsigned int serial_m3110_tx_empty(struct uart_port *port)
> +{
> + return 1;
> +}
> +
> +static void serial_m3110_stop_tx(struct uart_port *port)
> +{
> + return;
> +}
> +
> +static void serial_m3110_stop_rx(struct uart_port *port)
> +{
> + return;
> +}
are you sure it's sane to just ignore these from higher level?
> + u8 recv_buf[512], *pbuf;
is this sane to allocate 512 byte on the stack?
> + wait_event_interruptible(*wq,
> + max->flags || kthread_should_stop());
> + test_and_set_bit(0, &max->mthread_up);
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.
> +/* Don't handle hw handshaking */
> +static unsigned int serial_m3110_get_mctrl(struct uart_port *port)
> +{
> + return TIOCM_DSR | TIOCM_CAR | TIOCM_DSR;
> +}
> +
> +static void serial_m3110_set_mctrl(struct uart_port *port, unsigned int
> mctrl)
> +{
> +}
> +
this is quite bad because the MAX3100 is rather slow.
Bye!
--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH v3 net-next-2.6] can: Driver for the Microchip MCP251x SPI CAN controllers
Signed-off-by: Christian Pellegrin
---
drivers/net/can/Kconfig |6 +
drivers/net/can/Makefile |1 +
drivers/net/can/mcp251x.c| 1164 ++
include/linux/can/platform/mcp251x.h | 36 +
4 files changed, 1207 insertions(+), 0 deletions(-)
create mode 100644 drivers/net/can/mcp251x.c
create mode 100644 include/linux/can/platform/mcp251x.h
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 26d77cc..b819cc2 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -102,6 +102,12 @@ config CAN_TI_HECC
Driver for TI HECC (High End CAN Controller) module found on many
TI devices. The device specifications are available from www.ti.com
+config CAN_MCP251X
+ tristate "Microchip MCP251x SPI CAN controllers"
+ depends on CAN_DEV && SPI
+ ---help---
+ Driver for the Microchip MCP251x SPI CAN controllers.
+
config CAN_DEBUG_DEVICES
bool "CAN devices debugging messages"
depends on CAN
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 31f4ab5..1489181 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -12,5 +12,6 @@ obj-y += usb/
obj-$(CONFIG_CAN_SJA1000) += sja1000/
obj-$(CONFIG_CAN_AT91) += at91_can.o
obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
+obj-$(CONFIG_CAN_MCP251X) += mcp251x.o
ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
new file mode 100644
index 000..8f48f4b
--- /dev/null
+++ b/drivers/net/can/mcp251x.c
@@ -0,0 +1,1164 @@
+/*
+ * CAN bus driver for Microchip 251x CAN Controller with SPI Interface
+ *
+ * MCP2510 support and bug fixes by Christian Pellegrin
+ *
+ *
+ * Copyright 2009 Christian Pellegrin EVOL S.r.l.
+ *
+ * Copyright 2007 Raymarine UK, Ltd. All Rights Reserved.
+ * Written under contract by:
+ * Chris Elston, Katalix Systems, Ltd.
+ *
+ * Based on Microchip MCP251x CAN controller driver written by
+ * David Vrabel, Copyright 2006 Arcom Control Systems Ltd.
+ *
+ * Based on CAN bus driver for the CCAN controller written by
+ * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
+ * - Simon Kallweit, intefo AG
+ * Copyright 2007
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the version 2 of the GNU General Public License
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ *
+ *
+ * Your platform definition file should specify something like:
+ *
+ * static struct mcp251x_platform_data mcp251x_info = {
+ * .oscillator_frequency = 800,
+ * .board_specific_setup = &mcp251x_setup,
+ * .model = CAN_MCP251X_MCP2510,
+ * .power_enable = mcp251x_power_enable,
+ * .transceiver_enable = NULL,
+ * };
+ *
+ * static struct spi_board_info spi_board_info[] = {
+ * {
+ * .modalias = "mcp251x",
+ * .platform_data = &mcp251x_info,
+ * .irq = IRQ_EINT13,
+ * .max_speed_hz = 2*1000*1000,
+ * .chip_select = 2,
+ * },
+ * };
+ *
+ * Please see mcp251x.h for a description of the fields in
+ * struct mcp251x_platform_data.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* SPI interface instruction set */
+#define INSTRUCTION_WRITE 0x02
+#define INSTRUCTION_READ 0x03
+#define INSTRUCTION_BIT_MODIFY 0x05
+#define INSTRUCTION_LOAD_TXB(n)(0x40 + 2 * (n))
+#define INSTRUCTION_READ_RXB(n)(((n) == 0) ? 0x90 : 0x94)
+#define INSTRUCTION_RESET 0xC0
+
+/* MPC251x registers */
+#define CANSTAT 0x0e
+#define CANCTRL 0x0f
+# define CANCTRL_REQOP_MASK 0xe0
+# define CANCTRL_REQOP_CONF 0x80
+# define CANCTRL_REQOP_LISTEN_ONLY 0x60
+# define CANCTRL_REQOP_LOOPBACK0x40
+# define CANCTRL_REQOP_SLEEP 0x20
+# define CANCTRL_REQOP_NORMAL 0x00
+# define CANCTRL_OSM 0x08
+# define CANCTRL_ABAT 0x10
+#define TEC 0x1c
+#define REC 0x1d
+#define CNF1 0x2a
+# define CNF1_SJW_SHIFT 6
+#define CNF2 0x29
+# define CNF2_BTLMODE0x80
+# define CNF2_SAM 0x40
+# define CNF2_PS1_SHIFT 3
+#define CNF
Re: [spi-devel-general] [PATCH] can: Driver for the Microchip MCP251x SPI CAN controllers
Hi,
On Mon, Nov 2, 2009 at 8:49 PM, Wolfgang Grandegger wrote:
> I assume this is v2 of the patch.
>
Yes I put the version in the subject. Will reply with v3 shortly
>> + buf[TXBDLC_OFF] = (rtr << DLC_RTR_SHIFT) | frame->can_dlc;
>
> Two spaces before "=".
>
ack. This could be a good idea for a checkpatch.pl rule. Unfortunately
I don't know much about perlre. I used some emacs regexp so I hope
this is the last time you find double whitespaces or tabs around
assignment in this patch
>> + }
>
> Here the transceiver should be switched off!?
>
ack, all the error paths now seem checked now.
>> + mcp251x_write_bits(spi, CANINTF, intf, 0x00);
>
> Assigning variables within if or while expressions is not allowed. I
> wonder why checkpatch did not spot it.
>
ack. Out of curiosity I checked checkpatch.pl, unfortunately it looks
like it looks only for ifs:
if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) {
ERROR("do not use assignment in if condition\n" . $herecurr);
}
anyway I looked at this loop and tested: there is no need for it.
>> + net->flags |= IFF_ECHO;
>
> Remove spaces, please.
>
ack, sorry
>> + if (ret >= 0) {
>
> if (!ret) ?
>
ack
>> + dev_info(&spi->dev, "probed\n");
>> + return ret;
>> + }
>
> Shouldn't the power be switched off?
>
ack on the error patch
>
> We are close...
>
thank you for the patience
--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
--
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH] can: Driver for the Microchip MCP251x SPI CAN controllers
Signed-off-by: Christian Pellegrin
---
drivers/net/can/Kconfig |6 +
drivers/net/can/Makefile |1 +
drivers/net/can/mcp251x.c| 1164 ++
include/linux/can/platform/mcp251x.h | 36 +
4 files changed, 1207 insertions(+), 0 deletions(-)
create mode 100644 drivers/net/can/mcp251x.c
create mode 100644 include/linux/can/platform/mcp251x.h
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 26d77cc..b819cc2 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -102,6 +102,12 @@ config CAN_TI_HECC
Driver for TI HECC (High End CAN Controller) module found on many
TI devices. The device specifications are available from www.ti.com
+config CAN_MCP251X
+ tristate "Microchip MCP251x SPI CAN controllers"
+ depends on CAN_DEV && SPI
+ ---help---
+ Driver for the Microchip MCP251x SPI CAN controllers.
+
config CAN_DEBUG_DEVICES
bool "CAN devices debugging messages"
depends on CAN
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 31f4ab5..1489181 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -12,5 +12,6 @@ obj-y += usb/
obj-$(CONFIG_CAN_SJA1000) += sja1000/
obj-$(CONFIG_CAN_AT91) += at91_can.o
obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
+obj-$(CONFIG_CAN_MCP251X) += mcp251x.o
ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
new file mode 100644
index 000..9471bb7
--- /dev/null
+++ b/drivers/net/can/mcp251x.c
@@ -0,0 +1,1164 @@
+/*
+ * CAN bus driver for Microchip 251x CAN Controller with SPI Interface
+ *
+ * MCP2510 support and bug fixes by Christian Pellegrin
+ *
+ *
+ * Copyright 2009 Christian Pellegrin EVOL S.r.l.
+ *
+ * Copyright 2007 Raymarine UK, Ltd. All Rights Reserved.
+ * Written under contract by:
+ * Chris Elston, Katalix Systems, Ltd.
+ *
+ * Based on Microchip MCP251x CAN controller driver written by
+ * David Vrabel, Copyright 2006 Arcom Control Systems Ltd.
+ *
+ * Based on CAN bus driver for the CCAN controller written by
+ * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
+ * - Simon Kallweit, intefo AG
+ * Copyright 2007
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the version 2 of the GNU General Public License
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ *
+ *
+ * Your platform definition file should specify something like:
+ *
+ * static struct mcp251x_platform_data mcp251x_info = {
+ * .oscillator_frequency = 800,
+ * .board_specific_setup = &mcp251x_setup,
+ * .model = CAN_MCP251X_MCP2510,
+ * .power_enable = mcp251x_power_enable,
+ * .transceiver_enable = NULL,
+ * };
+ *
+ * static struct spi_board_info spi_board_info[] = {
+ * {
+ * .modalias = "mcp251x",
+ * .platform_data = &mcp251x_info,
+ * .irq = IRQ_EINT13,
+ * .max_speed_hz = 2*1000*1000,
+ * .chip_select = 2,
+ * },
+ * };
+ *
+ * Please see mcp251x.h for a description of the fields in
+ * struct mcp251x_platform_data.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* SPI interface instruction set */
+#define INSTRUCTION_WRITE 0x02
+#define INSTRUCTION_READ 0x03
+#define INSTRUCTION_BIT_MODIFY 0x05
+#define INSTRUCTION_LOAD_TXB(n)(0x40 + 2 * (n))
+#define INSTRUCTION_READ_RXB(n)(((n) == 0) ? 0x90 : 0x94)
+#define INSTRUCTION_RESET 0xC0
+
+/* MPC251x registers */
+#define CANSTAT 0x0e
+#define CANCTRL 0x0f
+# define CANCTRL_REQOP_MASK 0xe0
+# define CANCTRL_REQOP_CONF 0x80
+# define CANCTRL_REQOP_LISTEN_ONLY 0x60
+# define CANCTRL_REQOP_LOOPBACK0x40
+# define CANCTRL_REQOP_SLEEP 0x20
+# define CANCTRL_REQOP_NORMAL 0x00
+# define CANCTRL_OSM 0x08
+# define CANCTRL_ABAT 0x10
+#define TEC 0x1c
+#define REC 0x1d
+#define CNF1 0x2a
+# define CNF1_SJW_SHIFT 6
+#define CNF2 0x29
+# define CNF2_BTLMODE0x80
+# define CNF2_SAM 0x40
+# define CNF2_PS1_SHIFT 3
Re: [spi-devel-general] [PATCH v2 net-next-2.6] can: Driver for the Microchip MCP251x SPI CAN controllers
On Mon, Nov 2, 2009 at 4:25 PM, Wolfram Sang wrote: > > We'd need your Signed-off, please. > Sorry, I'm an idiot, forgot -s to git format-pach! -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH v2 net-next-2.6] can: Driver for the Microchip MCP251x SPI CAN controllers
---
drivers/net/can/Kconfig |6 +
drivers/net/can/Makefile |1 +
drivers/net/can/mcp251x.c| 1164 ++
include/linux/can/platform/mcp251x.h | 36 +
4 files changed, 1207 insertions(+), 0 deletions(-)
create mode 100644 drivers/net/can/mcp251x.c
create mode 100644 include/linux/can/platform/mcp251x.h
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 26d77cc..b819cc2 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -102,6 +102,12 @@ config CAN_TI_HECC
Driver for TI HECC (High End CAN Controller) module found on many
TI devices. The device specifications are available from www.ti.com
+config CAN_MCP251X
+ tristate "Microchip MCP251x SPI CAN controllers"
+ depends on CAN_DEV && SPI
+ ---help---
+ Driver for the Microchip MCP251x SPI CAN controllers.
+
config CAN_DEBUG_DEVICES
bool "CAN devices debugging messages"
depends on CAN
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 31f4ab5..1489181 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -12,5 +12,6 @@ obj-y += usb/
obj-$(CONFIG_CAN_SJA1000) += sja1000/
obj-$(CONFIG_CAN_AT91) += at91_can.o
obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
+obj-$(CONFIG_CAN_MCP251X) += mcp251x.o
ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
new file mode 100644
index 000..9471bb7
--- /dev/null
+++ b/drivers/net/can/mcp251x.c
@@ -0,0 +1,1164 @@
+/*
+ * CAN bus driver for Microchip 251x CAN Controller with SPI Interface
+ *
+ * MCP2510 support and bug fixes by Christian Pellegrin
+ *
+ *
+ * Copyright 2009 Christian Pellegrin EVOL S.r.l.
+ *
+ * Copyright 2007 Raymarine UK, Ltd. All Rights Reserved.
+ * Written under contract by:
+ * Chris Elston, Katalix Systems, Ltd.
+ *
+ * Based on Microchip MCP251x CAN controller driver written by
+ * David Vrabel, Copyright 2006 Arcom Control Systems Ltd.
+ *
+ * Based on CAN bus driver for the CCAN controller written by
+ * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
+ * - Simon Kallweit, intefo AG
+ * Copyright 2007
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the version 2 of the GNU General Public License
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ *
+ *
+ * Your platform definition file should specify something like:
+ *
+ * static struct mcp251x_platform_data mcp251x_info = {
+ * .oscillator_frequency = 800,
+ * .board_specific_setup = &mcp251x_setup,
+ * .model = CAN_MCP251X_MCP2510,
+ * .power_enable = mcp251x_power_enable,
+ * .transceiver_enable = NULL,
+ * };
+ *
+ * static struct spi_board_info spi_board_info[] = {
+ * {
+ * .modalias = "mcp251x",
+ * .platform_data = &mcp251x_info,
+ * .irq = IRQ_EINT13,
+ * .max_speed_hz = 2*1000*1000,
+ * .chip_select = 2,
+ * },
+ * };
+ *
+ * Please see mcp251x.h for a description of the fields in
+ * struct mcp251x_platform_data.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* SPI interface instruction set */
+#define INSTRUCTION_WRITE 0x02
+#define INSTRUCTION_READ 0x03
+#define INSTRUCTION_BIT_MODIFY 0x05
+#define INSTRUCTION_LOAD_TXB(n)(0x40 + 2 * (n))
+#define INSTRUCTION_READ_RXB(n)(((n) == 0) ? 0x90 : 0x94)
+#define INSTRUCTION_RESET 0xC0
+
+/* MPC251x registers */
+#define CANSTAT 0x0e
+#define CANCTRL 0x0f
+# define CANCTRL_REQOP_MASK 0xe0
+# define CANCTRL_REQOP_CONF 0x80
+# define CANCTRL_REQOP_LISTEN_ONLY 0x60
+# define CANCTRL_REQOP_LOOPBACK0x40
+# define CANCTRL_REQOP_SLEEP 0x20
+# define CANCTRL_REQOP_NORMAL 0x00
+# define CANCTRL_OSM 0x08
+# define CANCTRL_ABAT 0x10
+#define TEC 0x1c
+#define REC 0x1d
+#define CNF1 0x2a
+# define CNF1_SJW_SHIFT 6
+#define CNF2 0x29
+# define CNF2_BTLMODE0x80
+# define CNF2_SAM 0x40
+# define CNF2_PS1_SHIFT 3
+#define CNF3 0x28
+# defi
Re: [spi-devel-general] [PATCH net-next-2.6] Driver for the Microchip MCP251x SPI CAN controllers
On Sun, Nov 1, 2009 at 5:40 PM, Paul Thomas wrote: > Yes, I will be testing ready to work on this hopefully on Monday or That will be great: I only have a mcp2510 right now. Thanks in advance, I'll put you in CC with v2. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH net-next-2.6] Driver for the Microchip MCP251x SPI CAN controllers
;> + priv->can.do_set_bittiming = mcp251x_do_set_bittiming;
>
> Don't align expressions. Use just *one* space before and after "=".
>
ack
>> + mcp251x_enable_dma = 0;
>
> Please use {} here as well.
>
ack
>> + priv->spi_tx_buf, priv->spi_tx_dma);
>
> Please use {} here as well.
>
ack
>> + priv->after_suspend = AFTER_SUSPEND_DOWN;
>
> Please use {} here as well.
>
ack
>
> Please use {} here as well and check for similar cases. I might not have
> spotted all.
>
ack, I searched for all else and checked
>> + .resume = mcp251x_can_resume,
>
> Use just *one* space before and after "=".
>
ack
Thanks for the review!
--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
--
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH net-next-2.6] Driver for the Microchip MCP251x SPI CAN controllers
Signed-off-by: Christian Pellegrin
---
drivers/net/can/Kconfig |6 +
drivers/net/can/Makefile |1 +
drivers/net/can/mcp251x.c| 1182 ++
include/linux/can/platform/mcp251x.h | 34 +
4 files changed, 1223 insertions(+), 0 deletions(-)
create mode 100644 drivers/net/can/mcp251x.c
create mode 100644 include/linux/can/platform/mcp251x.h
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 26d77cc..e987526 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -102,6 +102,12 @@ config CAN_TI_HECC
Driver for TI HECC (High End CAN Controller) module found on many
TI devices. The device specifications are available from www.ti.com
+config CAN_MCP251X
+ tristate "Microchip MCP251x SPI CAN controllers"
+ depends on CAN && CAN_DEV && SPI
+ ---help---
+ Driver for the Microchip MCP251x SPI CAN controllers.
+
config CAN_DEBUG_DEVICES
bool "CAN devices debugging messages"
depends on CAN
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 31f4ab5..1489181 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -12,5 +12,6 @@ obj-y += usb/
obj-$(CONFIG_CAN_SJA1000) += sja1000/
obj-$(CONFIG_CAN_AT91) += at91_can.o
obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
+obj-$(CONFIG_CAN_MCP251X) += mcp251x.o
ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
new file mode 100644
index 000..f444cac
--- /dev/null
+++ b/drivers/net/can/mcp251x.c
@@ -0,0 +1,1182 @@
+/*
+ * CAN bus driver for Microchip 251x CAN Controller with SPI Interface
+ *
+ * MCP2510 support and bug fixes by Christian Pellegrin
+ *
+ *
+ * Copyright 2007 Raymarine UK, Ltd. All Rights Reserved.
+ * Written under contract by:
+ * Chris Elston, Katalix Systems, Ltd.
+ *
+ * Based on Microchip MCP251x CAN controller driver written by
+ * David Vrabel, Copyright 2006 Arcom Control Systems Ltd.
+ *
+ * Based on CAN bus driver for the CCAN controller written by
+ * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
+ * - Simon Kallweit, intefo AG
+ * Copyright 2007
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the version 2 of the GNU General Public License
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ *
+ *
+ * Your platform definition file should specify something like:
+ *
+ * static struct mcp251x_platform_data mcp251x_info = {
+ * .oscillator_frequency = 800,
+ * .board_specific_setup = &mcp251x_setup,
+ * .model = CAN_MCP251X_MCP2510,
+ * .power_enable = mcp251x_power_enable,
+ * .transceiver_enable = NULL,
+ * };
+ *
+ * static struct spi_board_info spi_board_info[] = {
+ * {
+ * .modalias = "mcp251x",
+ * .platform_data = &mcp251x_info,
+ * .irq = IRQ_EINT13,
+ * .max_speed_hz = 2*1000*1000,
+ * .chip_select = 2,
+ * },
+ * };
+ *
+ * Please see mcp251x.h for a description of the fields in
+ * struct mcp251x_platform_data.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* SPI interface instruction set */
+#define INSTRUCTION_WRITE 0x02
+#define INSTRUCTION_READ 0x03
+#define INSTRUCTION_BIT_MODIFY 0x05
+#define INSTRUCTION_LOAD_TXB(n)(0x40 + 2 * (n))
+#define INSTRUCTION_READ_RXB(n)(((n) == 0) ? 0x90 : 0x94)
+#define INSTRUCTION_RESET 0xC0
+
+/* MPC251x registers */
+#define CANSTAT 0x0e
+#define CANCTRL 0x0f
+# define CANCTRL_REQOP_MASK 0xe0
+# define CANCTRL_REQOP_CONF 0x80
+# define CANCTRL_REQOP_LISTEN_ONLY 0x60
+# define CANCTRL_REQOP_LOOPBACK0x40
+# define CANCTRL_REQOP_SLEEP 0x20
+# define CANCTRL_REQOP_NORMAL 0x00
+# define CANCTRL_OSM 0x08
+# define CANCTRL_ABAT 0x10
+#define TEC 0x1c
+#define REC 0x1d
+#define CNF1 0x2a
+# define CNF1_SJW_SHIFT 6
+#define CNF2 0x29
+# define CNF2_BTLMODE0x80
+# define CNF2_SAM 0x40
+# define CNF2_PS1_SHIFT 3
+#define CNF3 0x28
Re: [spi-devel-general] [PATCH RESEND] MAX3100 SPI UART driver
On Fri, Jan 9, 2009 at 11:21 PM, Andrew Morton wrote: > > afaik this patch is still stuck, awaiting a nod for this: > >> --- a/include/linux/serial_core.h >> +++ b/include/linux/serial_core.h >> @@ -155,6 +155,9 @@ >> >> #define PORT_SC26XX 82 >> >> +/* MAX3100 */ >> +#define PORT_MAX310083 >> + > Hello, I'm sorry. I asked two times following all the instructions in devices.txt (or at least trying to do so) without any response. I hope I didn't do something wrong. :-/ Thanks, -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- Check out the new SourceForge.net Marketplace. It is the best place to buy or sell services for just about anything Open Source. http://p.sf.net/sfu/Xq1LFB ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH RESEND] MAX3100 SPI UART driver
This patch adds support for the MAX3100 SPI UART. I am still
waiting for a minor number from [EMAIL PROTECTED] so that
may change.
Generated on 20081002 against v2.6.27-rc7
Signed-off-by: Christian Pellegrin <[EMAIL PROTECTED]>
---
drivers/serial/Kconfig |6 +-
drivers/serial/Makefile|1 +
drivers/serial/max3100.c | 924
include/linux/serial_core.h|3 +
include/linux/serial_max3100.h | 53 +++
5 files changed, 986 insertions(+), 1 deletions(-)
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 77cb342..de3193d 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -509,7 +509,11 @@ config SERIAL_S3C2440
help
Serial port support for the Samsung S3C2440 and S3C2442 SoC
-
+config SERIAL_MAX3100
+tristate "MAX3100 support"
+depends on SPI
+help
+ MAX3100 chip support
config SERIAL_DZ
bool "DECstation DZ serial driver"
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 7e7383e..21c3daf 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_SERIAL_S3C2400) += s3c2400.o
obj-$(CONFIG_SERIAL_S3C2410) += s3c2410.o
obj-$(CONFIG_SERIAL_S3C2412) += s3c2412.o
obj-$(CONFIG_SERIAL_S3C2440) += s3c2440.o
+obj-$(CONFIG_SERIAL_MAX3100) += max3100.o
obj-$(CONFIG_SERIAL_SUNCORE) += suncore.o
obj-$(CONFIG_SERIAL_SUNHV) += sunhv.o
obj-$(CONFIG_SERIAL_SUNZILOG) += sunzilog.o
diff --git a/drivers/serial/max3100.c b/drivers/serial/max3100.c
new file mode 100644
index 000..7a269a6
--- /dev/null
+++ b/drivers/serial/max3100.c
@@ -0,0 +1,924 @@
+/*
+ *
+ * Copyright (C) 2008 Christian Pellegrin <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ *
+ * Notes: the MAX3100 doesn't provide an interrupt on CTS so we have
+ * to use polling for flow control. TX empty IRQ is unusable, since
+ * writing conf clears FIFO buffer and we cannot have this interrupt
+ * always asking us for attention.
+ *
+ * Example platform data:
+
+ static struct plat_max3100 max3100_plat_data = {
+ .loopback = 0,
+ .crystal = 0,
+ .poll_time = 100,
+ };
+
+ static struct spi_board_info spi_board_info[] = {
+ {
+ .modalias = "max3100",
+ .platform_data= &max3100_plat_data,
+ .irq = IRQ_EINT12,
+ .max_speed_hz = 5*1000*1000,
+ .chip_select = 0,
+ },
+ };
+
+ * The initial minor number is 192 in the low-density serial port:
+ * mknod /dev/ttyMAX0 c 204 192
+ */
+
+#define MAX3100_MAJOR 204
+#define MAX3100_MINOR 192
+/* 4 MAX3100s should be enough for everyone */
+#define MAX_MAX3100 4
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#define MAX3100_C(1<<14)
+#define MAX3100_D(0<<14)
+#define MAX3100_W(1<<15)
+#define MAX3100_RX (0<<15)
+
+#define MAX3100_WC (MAX3100_W | MAX3100_C)
+#define MAX3100_RC (MAX3100_RX | MAX3100_C)
+#define MAX3100_WD (MAX3100_W | MAX3100_D)
+#define MAX3100_RD (MAX3100_RX | MAX3100_D)
+#define MAX3100_CMD (3 << 14)
+
+#define MAX3100_T(1<<14)
+#define MAX3100_R(1<<15)
+
+#define MAX3100_FEN (1<<13)
+#define MAX3100_SHDN (1<<12)
+#define MAX3100_TM (1<<11)
+#define MAX3100_RM (1<<10)
+#define MAX3100_PM (1<<9)
+#define MAX3100_RAM (1<<8)
+#define MAX3100_IR (1<<7)
+#define MAX3100_ST (1<<6)
+#define MAX3100_PE (1<<5)
+#define MAX3100_L(1<<4)
+#define MAX3100_BAUD (0xf)
+
+#define MAX3100_TE (1<<10)
+#define MAX3100_RAFE (1<<10)
+#define MAX3100_RTS (1<<9)
+#define MAX3100_CTS (1<<9)
+#define MAX3100_PT (1<<8)
+#define MAX3100_DATA (0xff)
+
+#define MAX3100_RT (MAX3100_R | MAX3100_T)
+#define MAX3100_RTC (MAX3100_RT | MAX3100_CTS | MAX3100_RAFE)
+
+/* the following simulate a status reg for ignore_status_mask */
+#define MAX3100_STATUS_PE 1
+#define MAX3100_STATUS_FE 2
+#define MAX3100_STATUS_OE 4
+
+struct max3100_port {
+ struct uart_port port;
+ struct spi_device *spi;
+
+ int cts;/* last CTS received for flow ctrl */
+ int tx_empty; /* last TX empty bit */
+
+ spinlock_t conf_lock; /* shared data */
+ int conf_commit;/* need to make changes */
+ int conf; /* configuration for the MAX31000
+* (bits 0-7, bits 8-11 are irqs) */
+ int rts_commit; /* need to change rts */
+ int rts;/* rts status */
+ int baud; /* current baud rate */
+
+ int parity; /* keeps track if we
[spi-devel-general] [PATCH RESEND] max3100 driver
This patch adds support for the MAX3100 SPI UART.
Generated on 20080920 against v2.6.27-rc6
Signed-off-by: Christian Pellegrin <[EMAIL PROTECTED]>
---
Documentation/devices.txt |3 +
drivers/serial/Kconfig |6 +-
drivers/serial/Makefile|1 +
drivers/serial/max3100.c | 953
include/linux/serial_core.h|3 +
include/linux/serial_max3100.h | 31 ++
6 files changed, 996 insertions(+), 1 deletions(-)
diff --git a/Documentation/devices.txt b/Documentation/devices.txt
index 05c8064..c228887 100644
--- a/Documentation/devices.txt
+++ b/Documentation/devices.txt
@@ -2807,6 +2807,9 @@ Your cooperation is appreciated.
...
190 = /dev/ttyUL3 Xilinx uartlite - port 3
191 = /dev/xvc0Xen virtual console - port 0
+192 = /dev/ttyMAX0 first MAX3100 UART
+ ...
+195 = /dev/ttyMAX3 fourth MAX3100 UART
205 char Low-density serial ports (alternate device)
0 = /dev/culu0Callout device for ttyLU0
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 77cb342..de3193d 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -509,7 +509,11 @@ config SERIAL_S3C2440
help
Serial port support for the Samsung S3C2440 and S3C2442 SoC
-
+config SERIAL_MAX3100
+tristate "MAX3100 support"
+depends on SPI
+help
+ MAX3100 chip support
config SERIAL_DZ
bool "DECstation DZ serial driver"
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 7e7383e..21c3daf 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_SERIAL_S3C2400) += s3c2400.o
obj-$(CONFIG_SERIAL_S3C2410) += s3c2410.o
obj-$(CONFIG_SERIAL_S3C2412) += s3c2412.o
obj-$(CONFIG_SERIAL_S3C2440) += s3c2440.o
+obj-$(CONFIG_SERIAL_MAX3100) += max3100.o
obj-$(CONFIG_SERIAL_SUNCORE) += suncore.o
obj-$(CONFIG_SERIAL_SUNHV) += sunhv.o
obj-$(CONFIG_SERIAL_SUNZILOG) += sunzilog.o
diff --git a/drivers/serial/max3100.c b/drivers/serial/max3100.c
new file mode 100644
index 000..12889b3
--- /dev/null
+++ b/drivers/serial/max3100.c
@@ -0,0 +1,953 @@
+/*
+ *
+ * Copyright (C) 2008 Christian Pellegrin <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ *
+ * Notes: the MAX3100 doesn't provide an interrupt on CTS so we have
+ * to use polling for flow control. TX empty IRQ is unusable, since
+ * writing conf clears FIFO buffer and we cannot have this interrupt
+ * always asking us for attention.
+ *
+ * Example platform data:
+
+static struct plat_max3100 max3100_plat_data = {
+ .loopback = 0,
+ .crystal = 0,
+ .only_edge_irq = 0,
+ .poll_time = 100,
+};
+
+static struct spi_board_info spi_board_info[] = {
+ {
+ .modalias = "max3100",
+ .platform_data = &max3100_plat_data,
+ .irq= IRQ_EINT12,
+ .max_speed_hz = 5*1000*1000,
+ .chip_select= 0,
+ },
+};
+
+ * The initial minor number is 192 in the low-density serial port:
+ * mknod /dev/ttyMAX0 c 204 192
+ */
+
+#define MAX3100_MAJOR 204
+#define MAX3100_MINOR 192
+/* 4 MAX3100s should be enough for everyone */
+#define MAX_MAX3100 4
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#define MAX3100_C(1<<14)
+#define MAX3100_D(0<<14)
+#define MAX3100_W(1<<15)
+#define MAX3100_RX (0<<15)
+
+#define MAX3100_WC (MAX3100_W | MAX3100_C)
+#define MAX3100_RC (MAX3100_RX | MAX3100_C)
+#define MAX3100_WD (MAX3100_W | MAX3100_D)
+#define MAX3100_RD (MAX3100_RX | MAX3100_D)
+#define MAX3100_CMD (3 << 14)
+
+#define MAX3100_T(1<<14)
+#define MAX3100_R(1<<15)
+
+#define MAX3100_FEN (1<<13)
+#define MAX3100_SHDN (1<<12)
+#define MAX3100_TM (1<<11)
+#define MAX3100_RM (1<<10)
+#define MAX3100_PM (1<<9)
+#define MAX3100_RAM (1<<8)
+#define MAX3100_IR (1<<7)
+#define MAX3100_ST (1<<6)
+#define MAX3100_PE (1<<5)
+#define MAX3100_L(1<<4)
+#define MAX3100_BAUD (0xf)
+
+#define MAX3100_TE (1<<10)
+#define MAX3100_RAFE (1<<10)
+#define MAX3100_RTS (1<<9)
+#define MAX3100_CTS (1<<9)
+#define MAX3100_PT (1<<8)
+#define MAX3100_DATA (0xff)
+
+#define MAX3100_RT (MAX3100_R | MAX3100_T)
+#define MAX3100_RTC (MAX3100_RT | MAX3100_CTS | MAX3100_RAFE)
+
+/* the following simulate a status reg for ignore_statu
