Skip to contentSkip to conversation

spi: atmel-spi fix cs_change behaviour

PATCH In review Manuel Stahl 3 participants 3 messages Mar 16, 2010 – May 28, 2010 2 messages not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 spi: atmel-spi fix cs_change behaviour

There is a problem in the cs_change behaviour in atmel-spi.

In include/linux/spi/spi.h it says:

Chipselect stays active during the entire message
(unless modified by spi_transfer.cs_change != 0).

The last parameter in atmel_spi_msg_done is named stay, so it must be called with inverted cs_change value.

drivers/spi/atmel_spi.c +1 -1
@@ -501,7 +501,7 @@ atmel_spi_interrupt(int irq, void *dev_id)
 			if (atmel_spi_xfer_is_last(msg, xfer)) {
 				/* report completed message */
 				atmel_spi_msg_done(master, as, msg, 0,
-						xfer->cs_change);
+						!xfer->cs_change);
 			} else {
 				if (xfer->cs_change) {
 					cs_deactivate(as, msg->spi);

------------------------------------------------------------------------------
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-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

A message that is not in the archive (sent off-list or not yet received): 4B9FA253.5030109-GeUHRtUQU7nSyEMIgutvibNAH6kLmebB@public.gmane.org

#2 lore raw
Quoted 9 lines · Manuel Stahl

There is a problem in the cs_change behaviour in atmel-spi.

In include/linux/spi/spi.h it says:

Chipselect stays active during the entire message
(unless modified by spi_transfer.cs_change != 0).

The last parameter in atmel_spi_msg_done is named stay, so it must be called with inverted cs_change value.

Any users of this driver have an ack/nack for this patch? Haavard perhaps?

g.

Quoted 25 lines
drivers/spi/atmel_spi.c +1 -2
@@ -501,7 +501,7 @@ atmel_spi_interrupt(int irq, void *dev_id)
                        if (atmel_spi_xfer_is_last(msg, xfer)) {
                                /* report completed message */
                                atmel_spi_msg_done(master, as, msg, 0,
-                                               xfer->cs_change);
+                                               !xfer->cs_change);
                        } else {
                                if (xfer->cs_change) {
                                        cs_deactivate(as, msg->spi);
 
------------------------------------------------------------------------------

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-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Signature · 4 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

------------------------------------------------------------------------------

A message that is not in the archive (sent off-list or not yet received): AANLkTilADPBrg9J3n5d3r0HYjTdiaSUbKAH4Iz9Imjb8-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#4 lore raw
Quoted 6 lines · Grant Likely
Quoted 4 lines · Manuel Stahl

There is a problem in the cs_change behaviour in atmel-spi.

In include/linux/spi/spi.h it says:

Quoted 7 lines
Quoted 5 lines

Chipselect stays active during the entire message
(unless modified by spi_transfer.cs_change != 0).

The last parameter in atmel_spi_msg_done is named stay, so it must be called with inverted cs_change value.

Any users of this driver have an ack/nack for this patch? Haavard perhaps?

No, the current driver is doing it correctly. I've NACK'ed several patches like this in the past.

Quoted 6 lines

drivers/spi/atmel_spi.c atmel_spi_interrupt(int irq, void *dev_id) · line 504

Quoted 6 lines
drivers/spi/atmel_spi.c +1 -1
@@ -501,7 +501,7 @@ atmel_spi_interrupt(int irq, void *dev_id)
                        if (atmel_spi_xfer_is_last(msg, xfer)) {
                                /* report completed message */
                                atmel_spi_msg_done(master, as, msg, 0,
-                                               xfer->cs_change);
+                                               !xfer->cs_change);

The normal behaviour is:
* After all transfers except the last: CS stays active
* After the last transfer: CS goes inactive

The cs_change flag is used to invert this behaviour. So for the last transfer, cs_change indicates that CS should stay active after the transfer (though the driver may not actually be able to do so, e.g. if the message is followed by a message to a different device.)

Haavard

------------------------------------------------------------------------------