[PATCH v2 0/2] spi: bcm2835aux: auxiliary spi improvements

STALE3832d

Revision v2 of 2 in this series.

4 messages, 2 authors, 2016-02-15 · open the first message on its own page

[PATCH v2 0/2] spi: bcm2835aux: auxiliary spi improvements

From: Stephan Olbrich <hidden>
Date: 2016-02-14 10:04:27

From: Stephan Olbrich <redacted>

This patch series has some improvements and fixes for the auxiliary spi.

since v1:
- the first two patches "fix bitmask defines" and "disable tx fifo empty irq"
  were picked up by Mark Brown and applied to his spi tree so I don't post
  them again.
- speed bits are now reset before setting the new speed in case the speed
  changes between transfers
- remove CPHA from master->mode_bits
- rename CPHA bit mask to RISING to make it more clear why things are done
  that way

1. set up spi-mode before asserting cs-gpio
 As Martin Sperl suggested this is done in the same way as in spi-bcm2835.c
 acace73df2c1913a526c1b41e4741a4a6704c863

2. fix CPOL/CPHA setting
 From what I've seen in the documentation [1] and seen on the scope this chip
 doesn't support modes with CPHA=1. With this patch spi mode 0 and 2 should
 work correctly whereas mode 1 and 3 are not supported. 
                                                                                                                                                                                                            
Stephan Olbrich (2):
  spi: bcm2835aux: set up spi-mode before asserting cs-gpio
  spi: bcm2835aux: fix CPOL/CPHA setting

 drivers/spi/spi-bcm2835aux.c | 66 +++++++++++++++++++++++++++++---------------
 1 file changed, 44 insertions(+), 22 deletions(-)

-- 
2.5.0

[PATCH v2 1/2] spi: bcm2835aux: set up spi-mode before asserting cs-gpio

From: Stephan Olbrich <hidden>
Date: 2016-02-14 10:04:28

When using reverse polarity for clock (spi-cpol) on a device
the clock line gets altered after chip-select has been asserted
resulting in an additional clock beat, which confuses hardware.

This happens due to the fact, the the hardware was initialized
and reset at the begin and end of each transfer which results
in default state for all lines except chip-select which is
handled by the spi-subsystem as gpio-cs is used.

To avoid this situation this patch moves the setup of polarity
(spi-cpol and spi-cpha) outside of the chip-select into
prepare_message, which is run prior to asserting chip-select.

Signed-off-by: Stephan Olbrich <redacted>
Reviewed-by: Martin Sperl <redacted>
Tested-by: Martin Sperl <redacted>
---
 drivers/spi/spi-bcm2835aux.c | 57 +++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index d2f0067..103335e 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c

@@ -218,9 +218,9 @@ static irqreturn_t bcm2835aux_spi_interrupt(int irq, void *dev_id)
 			BCM2835_AUX_SPI_CNTL1_IDLE);
 	}
 
-	/* and if rx_len is 0 then wake up completion and disable spi */
+	/* and if rx_len is 0 then disable interrupts and wake up completion */
 	if (!bs->rx_len) {
-		bcm2835aux_spi_reset_hw(bs);
+		bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]);
 		complete(&master->xfer_completion);
 	}

@@ -313,9 +313,6 @@ static int bcm2835aux_spi_transfer_one_poll(struct spi_master *master,
 		}
 	}
 
-	/* Transfer complete - reset SPI HW */
-	bcm2835aux_spi_reset_hw(bs);
-
 	/* and return without waiting for completion */
 	return 0;
 }

@@ -336,10 +333,6 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master,
 	 * resulting (potentially) in more interrupts when transferring
 	 * more than 12 bytes
 	 */
-	bs->cntl[0] = BCM2835_AUX_SPI_CNTL0_ENABLE |
-		      BCM2835_AUX_SPI_CNTL0_VAR_WIDTH |
-		      BCM2835_AUX_SPI_CNTL0_MSBF_OUT;
-	bs->cntl[1] = BCM2835_AUX_SPI_CNTL1_MSBF_IN;
 
 	/* set clock */
 	spi_hz = tfr->speed_hz;

@@ -354,17 +347,13 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master,
 	} else { /* the slowest we can go */
 		speed = BCM2835_AUX_SPI_CNTL0_SPEED_MAX;
 	}
+	/* mask out old speed from previous spi_transfer */
+	bs->cntl[0] &= ~(BCM2835_AUX_SPI_CNTL0_SPEED);
+	/* set the new speed */
 	bs->cntl[0] |= speed << BCM2835_AUX_SPI_CNTL0_SPEED_SHIFT;
 
 	spi_used_hz = clk_hz / (2 * (speed + 1));
 
-	/* handle all the modes */
-	if (spi->mode & SPI_CPOL)
-		bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPOL;
-	if (spi->mode & SPI_CPHA)
-		bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPHA_OUT |
-			       BCM2835_AUX_SPI_CNTL0_CPHA_IN;
-
 	/* set transmit buffers and length */
 	bs->tx_buf = tfr->tx_buf;
 	bs->rx_buf = tfr->rx_buf;

@@ -388,6 +377,40 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master,
 	return bcm2835aux_spi_transfer_one_irq(master, spi, tfr);
 }
 
+static int bcm2835aux_spi_prepare_message(struct spi_master *master,
+					  struct spi_message *msg)
+{
+	struct spi_device *spi = msg->spi;
+	struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
+
+	bs->cntl[0] = BCM2835_AUX_SPI_CNTL0_ENABLE |
+		      BCM2835_AUX_SPI_CNTL0_VAR_WIDTH |
+		      BCM2835_AUX_SPI_CNTL0_MSBF_OUT;
+	bs->cntl[1] = BCM2835_AUX_SPI_CNTL1_MSBF_IN;
+
+	/* handle all the modes */
+	if (spi->mode & SPI_CPOL)
+		bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPOL;
+	if (spi->mode & SPI_CPHA)
+		bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPHA_OUT |
+			       BCM2835_AUX_SPI_CNTL0_CPHA_IN;
+
+	bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]);
+	bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL0, bs->cntl[0]);
+
+	return 0;
+}
+
+static int bcm2835aux_spi_unprepare_message(struct spi_master *master,
+					    struct spi_message *msg)
+{
+	struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
+
+	bcm2835aux_spi_reset_hw(bs);
+
+	return 0;
+}
+
 static void bcm2835aux_spi_handle_err(struct spi_master *master,
 				      struct spi_message *msg)
 {

@@ -416,6 +439,8 @@ static int bcm2835aux_spi_probe(struct platform_device *pdev)
 	master->num_chipselect = -1;
 	master->transfer_one = bcm2835aux_spi_transfer_one;
 	master->handle_err = bcm2835aux_spi_handle_err;
+	master->prepare_message = bcm2835aux_spi_prepare_message;
+	master->unprepare_message = bcm2835aux_spi_unprepare_message;
 	master->dev.of_node = pdev->dev.of_node;
 
 	bs = spi_master_get_devdata(master);

-- 
2.5.0

[PATCH v2 2/2] spi: bcm2835aux: fix CPOL/CPHA setting

From: Stephan Olbrich <hidden>
Date: 2016-02-14 10:04:29

The auxiliary spi supports only CPHA=0 modes as the first bit is
always output to the pin before the first clock cycle. In CPHA=1
modes the first clock edge outputs the second bit hence the slave
can never read the first bit.

Also the CPHA registers switch between clocking data in/out on
rising/falling edge hence depend on the CPOL setting.

Signed-off-by: Stephan Olbrich <redacted>
---
 drivers/spi/spi-bcm2835aux.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
index 103335e..7428091 100644
--- a/drivers/spi/spi-bcm2835aux.c
+++ b/drivers/spi/spi-bcm2835aux.c

@@ -64,9 +64,9 @@
 #define BCM2835_AUX_SPI_CNTL0_VAR_WIDTH	0x00004000
 #define BCM2835_AUX_SPI_CNTL0_DOUTHOLD	0x00003000
 #define BCM2835_AUX_SPI_CNTL0_ENABLE	0x00000800
-#define BCM2835_AUX_SPI_CNTL0_CPHA_IN	0x00000400
+#define BCM2835_AUX_SPI_CNTL0_IN_RISING	0x00000400
 #define BCM2835_AUX_SPI_CNTL0_CLEARFIFO	0x00000200
-#define BCM2835_AUX_SPI_CNTL0_CPHA_OUT	0x00000100
+#define BCM2835_AUX_SPI_CNTL0_OUT_RISING	0x00000100
 #define BCM2835_AUX_SPI_CNTL0_CPOL	0x00000080
 #define BCM2835_AUX_SPI_CNTL0_MSBF_OUT	0x00000040
 #define BCM2835_AUX_SPI_CNTL0_SHIFTLEN	0x0000003F

@@ -92,9 +92,6 @@
 #define BCM2835_AUX_SPI_POLLING_LIMIT_US	30
 #define BCM2835_AUX_SPI_POLLING_JIFFIES		2
 
-#define BCM2835_AUX_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | SPI_CS_HIGH \
-				  | SPI_NO_CS)
-
 struct bcm2835aux_spi {
 	void __iomem *regs;
 	struct clk *clk;

@@ -389,12 +386,12 @@ static int bcm2835aux_spi_prepare_message(struct spi_master *master,
 	bs->cntl[1] = BCM2835_AUX_SPI_CNTL1_MSBF_IN;
 
 	/* handle all the modes */
-	if (spi->mode & SPI_CPOL)
+	if (spi->mode & SPI_CPOL) {
 		bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPOL;
-	if (spi->mode & SPI_CPHA)
-		bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPHA_OUT |
-			       BCM2835_AUX_SPI_CNTL0_CPHA_IN;
-
+		bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_OUT_RISING;
+	} else {
+		bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_IN_RISING;
+	}
 	bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]);
 	bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL0, bs->cntl[0]);

@@ -434,7 +431,7 @@ static int bcm2835aux_spi_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, master);
-	master->mode_bits = BCM2835_AUX_SPI_MODE_BITS;
+	master->mode_bits = (SPI_CPOL | SPI_CS_HIGH | SPI_NO_CS);
 	master->bits_per_word_mask = SPI_BPW_MASK(8);
 	master->num_chipselect = -1;
 	master->transfer_one = bcm2835aux_spi_transfer_one;

-- 
2.5.0

[PATCH v2 0/2] spi: bcm2835aux: auxiliary spi improvements

From: Eric Anholt <hidden>
Date: 2016-02-15 19:25:18

Stephan Olbrich [off-list ref] writes:

From: Stephan Olbrich <redacted>

This patch series has some improvements and fixes for the auxiliary spi.

since v1:
- the first two patches "fix bitmask defines" and "disable tx fifo empty irq"
  were picked up by Mark Brown and applied to his spi tree so I don't post
  them again.
- speed bits are now reset before setting the new speed in case the speed
  changes between transfers
- remove CPHA from master->mode_bits
- rename CPHA bit mask to RISING to make it more clear why things are done
  that way

1. set up spi-mode before asserting cs-gpio
 As Martin Sperl suggested this is done in the same way as in spi-bcm2835.c
 acace73df2c1913a526c1b41e4741a4a6704c863

2. fix CPOL/CPHA setting
 From what I've seen in the documentation [1] and seen on the scope this chip
 doesn't support modes with CPHA=1. With this patch spi mode 0 and 2 should
 work correctly whereas mode 1 and 3 are not supported.

These are both:

Reviewed-by: Eric Anholt <redacted>

Thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160215/4831e8e9/attachment.sig>

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help