Re: [PATCH V2 2/2] spi/bcm63xx: work around inability to keep CS up
On Tue, 5 Feb 2013 16:00:04 +0100, Jonas Gorski wrote: > On Tue, 05 Feb 2013 14:35:30 + > Grant Likely wrote: > > > On Sun, 3 Feb 2013 15:15:13 +0100, Jonas Gorski wrote: > > > This SPI controller does not support keeping CS asserted after sending > > > a transfer. > > > Since messages expected on this SPI controller are rather short, we can > > > work around it for normal use cases by sending all transfers at once in > > > a big full duplex stream. > > > > > > This means that we cannot change the speed between transfers if they > > > require CS to be kept asserted, but these would have been rejected > > > before anyway because of the inability of keeping CS asserted. > > > > > > Signed-off-by: Jonas Gorski > > > > Are you checking the state of transfer->cs_change when merging > > transfers? If cs_change is set, then the transfers cannot be merged. > > Yes, I do; I "flush" on each cs_change and after the last transfer: Okay, applied. Thanks. g. -- Free Next-Gen Firewall Hardware Offer Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more. http://p.sf.net/sfu/sophos-d2d-feb ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH V2 2/2] spi/bcm63xx: work around inability to keep CS up
On Tue, 05 Feb 2013 14:35:30 +
Grant Likely wrote:
> On Sun, 3 Feb 2013 15:15:13 +0100, Jonas Gorski wrote:
> > This SPI controller does not support keeping CS asserted after sending
> > a transfer.
> > Since messages expected on this SPI controller are rather short, we can
> > work around it for normal use cases by sending all transfers at once in
> > a big full duplex stream.
> >
> > This means that we cannot change the speed between transfers if they
> > require CS to be kept asserted, but these would have been rejected
> > before anyway because of the inability of keeping CS asserted.
> >
> > Signed-off-by: Jonas Gorski
>
> Are you checking the state of transfer->cs_change when merging
> transfers? If cs_change is set, then the transfers cannot be merged.
Yes, I do; I "flush" on each cs_change and after the last transfer:
> > + if (t->cs_change ||
> > + list_is_last(&t->transfer_list, &m->transfers)) {
> > + /* configure adapter for a new transfer */
> > + bcm63xx_spi_setup_transfer(spi, first);
> >
> > - /* send the data */
> > - status = bcm63xx_txrx_bufs(spi, t);
> > - if (status)
> > - goto exit;
> > + /* send the data */
> > + status = bcm63xx_txrx_bufs(spi, first, n_transfers);
> > + if (status)
> > + goto exit;
> > +
> > + m->actual_length += total_len;
> >
> > - m->actual_length += t->len;
> > + first = NULL;
> > + n_transfers = 0;
> > + total_len = 0;
> > + can_use_prepend = false;
> > + }
> > }
> > exit:
> > m->status = status;
> > --
> > 1.7.10.4
> >
>
> --
> Grant Likely, B.Sc, P.Eng.
> Secret Lab Technologies, Ltd.
P.S: Sorry Grant for receiving this double - I can't email.
--
Jonas Gorski
--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH V2 2/2] spi/bcm63xx: work around inability to keep CS up
On Sun, 3 Feb 2013 15:15:13 +0100, Jonas Gorski wrote:
> This SPI controller does not support keeping CS asserted after sending
> a transfer.
> Since messages expected on this SPI controller are rather short, we can
> work around it for normal use cases by sending all transfers at once in
> a big full duplex stream.
>
> This means that we cannot change the speed between transfers if they
> require CS to be kept asserted, but these would have been rejected
> before anyway because of the inability of keeping CS asserted.
>
> Signed-off-by: Jonas Gorski
Are you checking the state of transfer->cs_change when merging
transfers? If cs_change is set, then the transfers cannot be merged.
g.
> ---
> V1 -> V2:
> * split out rejection logic into separate patch
> * fixed return type of bcm63xx_txrx_bufs()
> * slightly reworked bcm63xx_txrx_bufs, obsoleting one local variable
>
> drivers/spi/spi-bcm63xx.c | 134
> +++--
> 1 file changed, 106 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c
> index 27667c1..9578af7 100644
> --- a/drivers/spi/spi-bcm63xx.c
> +++ b/drivers/spi/spi-bcm63xx.c
> @@ -37,6 +37,8 @@
>
> #define PFX KBUILD_MODNAME
>
> +#define BCM63XX_SPI_MAX_PREPEND 15
> +
> struct bcm63xx_spi {
> struct completion done;
>
> @@ -169,13 +171,17 @@ static int bcm63xx_spi_setup(struct spi_device *spi)
> return 0;
> }
>
> -static int bcm63xx_txrx_bufs(struct spi_device *spi, struct spi_transfer *t)
> +static int bcm63xx_txrx_bufs(struct spi_device *spi, struct spi_transfer
> *first,
> + unsigned int num_transfers)
> {
> struct bcm63xx_spi *bs = spi_master_get_devdata(spi->master);
> u16 msg_ctl;
> u16 cmd;
> u8 rx_tail;
> - unsigned int timeout = 0;
> + unsigned int i, timeout = 0, prepend_len = 0, len = 0;
> + struct spi_transfer *t = first;
> + bool do_rx = false;
> + bool do_tx = false;
>
> /* Disable the CMD_DONE interrupt */
> bcm_spi_writeb(bs, 0, SPI_INT_MASK);
> @@ -183,19 +189,45 @@ static int bcm63xx_txrx_bufs(struct spi_device *spi,
> struct spi_transfer *t)
> dev_dbg(&spi->dev, "txrx: tx %p, rx %p, len %d\n",
> t->tx_buf, t->rx_buf, t->len);
>
> - if (t->tx_buf)
> - memcpy_toio(bs->tx_io, t->tx_buf, t->len);
> + if (num_transfers > 1 && t->tx_buf && t->len <= BCM63XX_SPI_MAX_PREPEND)
> + prepend_len = t->len;
> +
> + /* prepare the buffer */
> + for (i = 0; i < num_transfers; i++) {
> + if (t->tx_buf) {
> + do_tx = true;
> + memcpy_toio(bs->tx_io + len, t->tx_buf, t->len);
> +
> + /* don't prepend more than one tx */
> + if (t != first)
> + prepend_len = 0;
> + }
> +
> + if (t->rx_buf) {
> + do_rx = true;
> + /* prepend is half-duplex write only */
> + if (t == first)
> + prepend_len = 0;
> + }
> +
> + len += t->len;
> +
> + t = list_entry(t->transfer_list.next, struct spi_transfer,
> +transfer_list);
> + }
> +
> + len -= prepend_len;
>
> init_completion(&bs->done);
>
> /* Fill in the Message control register */
> - msg_ctl = (t->len << SPI_BYTE_CNT_SHIFT);
> + msg_ctl = (len << SPI_BYTE_CNT_SHIFT);
>
> - if (t->rx_buf && t->tx_buf)
> + if (do_rx && do_tx && prepend_len == 0)
> msg_ctl |= (SPI_FD_RW << bs->msg_type_shift);
> - else if (t->rx_buf)
> + else if (do_rx)
> msg_ctl |= (SPI_HD_R << bs->msg_type_shift);
> - else if (t->tx_buf)
> + else if (do_tx)
> msg_ctl |= (SPI_HD_W << bs->msg_type_shift);
>
> switch (bs->msg_ctl_width) {
> @@ -209,7 +241,7 @@ static int bcm63xx_txrx_bufs(struct spi_device *spi,
> struct spi_transfer *t)
>
> /* Issue the transfer */
> cmd = SPI_CMD_START_IMMEDIATE;
> - cmd |= (0 << SPI_CMD_PREPEND_BYTE_CNT_SHIFT);
> + cmd |= (prepend_len << SPI_CMD_PREPEND_BYTE_CNT_SHIFT);
> cmd |= (spi->chip_select << SPI_CMD_DEVICE_ID_SHIFT);
> bcm_spi_writew(bs, cmd, SPI_CMD);
>
> @@ -223,9 +255,25 @@ static int bcm63xx_txrx_bufs(struct spi_device *spi,
> struct spi_transfer *t)
> /* read out all data */
> rx_tail = bcm_spi_readb(bs, SPI_RX_TAIL);
>
> + if (do_rx && rx_tail != len)
> + return -EIO;
> +
> + if (!rx_tail)
> + return 0;
> +
> + len = 0;
> + t = first;
> /* Read out all the data */
> - if (rx_tail)
> - memcpy_fromio(t->rx_ptr, bs->rx_io, rx_tail);
> + for (i = 0; i < num_transfers; i++) {
> + if (t->rx_buf)
> + memcpy_fromio(t-
[PATCH V2 2/2] spi/bcm63xx: work around inability to keep CS up
This SPI controller does not support keeping CS asserted after sending
a transfer.
Since messages expected on this SPI controller are rather short, we can
work around it for normal use cases by sending all transfers at once in
a big full duplex stream.
This means that we cannot change the speed between transfers if they
require CS to be kept asserted, but these would have been rejected
before anyway because of the inability of keeping CS asserted.
Signed-off-by: Jonas Gorski
---
V1 -> V2:
* split out rejection logic into separate patch
* fixed return type of bcm63xx_txrx_bufs()
* slightly reworked bcm63xx_txrx_bufs, obsoleting one local variable
drivers/spi/spi-bcm63xx.c | 134 +++--
1 file changed, 106 insertions(+), 28 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c
index 27667c1..9578af7 100644
--- a/drivers/spi/spi-bcm63xx.c
+++ b/drivers/spi/spi-bcm63xx.c
@@ -37,6 +37,8 @@
#define PFXKBUILD_MODNAME
+#define BCM63XX_SPI_MAX_PREPEND15
+
struct bcm63xx_spi {
struct completion done;
@@ -169,13 +171,17 @@ static int bcm63xx_spi_setup(struct spi_device *spi)
return 0;
}
-static int bcm63xx_txrx_bufs(struct spi_device *spi, struct spi_transfer *t)
+static int bcm63xx_txrx_bufs(struct spi_device *spi, struct spi_transfer
*first,
+ unsigned int num_transfers)
{
struct bcm63xx_spi *bs = spi_master_get_devdata(spi->master);
u16 msg_ctl;
u16 cmd;
u8 rx_tail;
- unsigned int timeout = 0;
+ unsigned int i, timeout = 0, prepend_len = 0, len = 0;
+ struct spi_transfer *t = first;
+ bool do_rx = false;
+ bool do_tx = false;
/* Disable the CMD_DONE interrupt */
bcm_spi_writeb(bs, 0, SPI_INT_MASK);
@@ -183,19 +189,45 @@ static int bcm63xx_txrx_bufs(struct spi_device *spi,
struct spi_transfer *t)
dev_dbg(&spi->dev, "txrx: tx %p, rx %p, len %d\n",
t->tx_buf, t->rx_buf, t->len);
- if (t->tx_buf)
- memcpy_toio(bs->tx_io, t->tx_buf, t->len);
+ if (num_transfers > 1 && t->tx_buf && t->len <= BCM63XX_SPI_MAX_PREPEND)
+ prepend_len = t->len;
+
+ /* prepare the buffer */
+ for (i = 0; i < num_transfers; i++) {
+ if (t->tx_buf) {
+ do_tx = true;
+ memcpy_toio(bs->tx_io + len, t->tx_buf, t->len);
+
+ /* don't prepend more than one tx */
+ if (t != first)
+ prepend_len = 0;
+ }
+
+ if (t->rx_buf) {
+ do_rx = true;
+ /* prepend is half-duplex write only */
+ if (t == first)
+ prepend_len = 0;
+ }
+
+ len += t->len;
+
+ t = list_entry(t->transfer_list.next, struct spi_transfer,
+ transfer_list);
+ }
+
+ len -= prepend_len;
init_completion(&bs->done);
/* Fill in the Message control register */
- msg_ctl = (t->len << SPI_BYTE_CNT_SHIFT);
+ msg_ctl = (len << SPI_BYTE_CNT_SHIFT);
- if (t->rx_buf && t->tx_buf)
+ if (do_rx && do_tx && prepend_len == 0)
msg_ctl |= (SPI_FD_RW << bs->msg_type_shift);
- else if (t->rx_buf)
+ else if (do_rx)
msg_ctl |= (SPI_HD_R << bs->msg_type_shift);
- else if (t->tx_buf)
+ else if (do_tx)
msg_ctl |= (SPI_HD_W << bs->msg_type_shift);
switch (bs->msg_ctl_width) {
@@ -209,7 +241,7 @@ static int bcm63xx_txrx_bufs(struct spi_device *spi, struct
spi_transfer *t)
/* Issue the transfer */
cmd = SPI_CMD_START_IMMEDIATE;
- cmd |= (0 << SPI_CMD_PREPEND_BYTE_CNT_SHIFT);
+ cmd |= (prepend_len << SPI_CMD_PREPEND_BYTE_CNT_SHIFT);
cmd |= (spi->chip_select << SPI_CMD_DEVICE_ID_SHIFT);
bcm_spi_writew(bs, cmd, SPI_CMD);
@@ -223,9 +255,25 @@ static int bcm63xx_txrx_bufs(struct spi_device *spi,
struct spi_transfer *t)
/* read out all data */
rx_tail = bcm_spi_readb(bs, SPI_RX_TAIL);
+ if (do_rx && rx_tail != len)
+ return -EIO;
+
+ if (!rx_tail)
+ return 0;
+
+ len = 0;
+ t = first;
/* Read out all the data */
- if (rx_tail)
- memcpy_fromio(t->rx_ptr, bs->rx_io, rx_tail);
+ for (i = 0; i < num_transfers; i++) {
+ if (t->rx_buf)
+ memcpy_fromio(t->rx_buf, bs->rx_io + len, t->len);
+
+ if (t != first || prepend_len == 0)
+ len += t->len;
+
+ t = list_entry(t->transfer_list.next, struct spi_transfer,
+ transfer_list);
+ }
return 0;
}
@@ -252,46 +300,76 @@ static int
