| Message ID | 20180807121655.22346-1-sr@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | [U-Boot,1/4,v2] spi: spi-mem: Use 2 SPI messages instead of a single full-duplex one | expand |
On Tue, 7 Aug 2018 14:16:52 +0200 Stefan Roese <sr@denx.de> wrote: > Some SPI controller do not support full-duplex SPI transfers. This patch > changes the SPI transfer into 2 separate transfers - or 1, if no data is > to transmitted. > > With this change, no buffers need to be allocated anymore. We use the > TX and RX buffers that are passed to spi_mem_exec_op() directly. > > Signed-off-by: Stefan Roese <sr@denx.de> > Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> > Cc: Miquel Raynal <miquel.raynal@bootlin.com> > Cc: Boris Brezillon <boris.brezillon@bootlin.com> > Cc: Jagan Teki <jagan@openedev.com> Looks good overall, just a few comments (that you might chose to ignore if you disagree). Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > v2: > - Replaces patch "spi: spi-mem: Add optional half-duplex SPI transfer mode" > from first patchset version > - No compile-time option but the default to use 2 separate SPI messages > to transfer the command and data > > drivers/spi/spi-mem.c | 74 +++++++++++++++++++++++-------------------- > 1 file changed, 39 insertions(+), 35 deletions(-) > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c > index 07ce799170..84e33aa979 100644 > --- a/drivers/spi/spi-mem.c > +++ b/drivers/spi/spi-mem.c > @@ -15,6 +15,8 @@ > #include <spi-mem.h> > #endif > > +#define OP_BUFFER_SIZE_MAX 16 /* Max size for cmd + addr + dummy */ > + > #ifndef __UBOOT__ > /** > * spi_controller_dma_map_mem_op_data() - DMA-map the buffer attached to a > @@ -200,8 +202,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) > bool tx_data = op->data.nbytes && (op->data.dir == SPI_MEM_DATA_OUT); > struct udevice *bus = slave->dev->parent; > struct dm_spi_ops *ops = spi_get_ops(bus); > - unsigned int xfer_len, pos = 0; > - u8 *tx_buf, *rx_buf = NULL; > + unsigned int pos = 0; > + const u8 *tx_buf; > + u8 *rx_buf; > + u8 op_buf[OP_BUFFER_SIZE_MAX]; u8 op_buf[OP_BUFFER_SIZE_MAX] = { }; and you can get rid of the memset(0) in the code. > + int op_len; > + u32 flag; > int ret; > int i; > > @@ -330,67 +336,65 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) > return -ENOTSUPP; > } > > - xfer_len = sizeof(op->cmd.opcode) + op->addr.nbytes + > - op->dummy.nbytes + op->data.nbytes; > + tx_buf = op->data.buf.out; > + rx_buf = op->data.buf.in; I think you can get rid of rx/tx_data and just keep rx/tx_buf. Initialize them to NULL at declaration time and then do: if (op->data.nbytes) { if (op->data->dir == SPI_MEM_DATA_IN) rx_buf = op->data.buf.in; else tx_buf = op->data.buf.out; } > > - /* > - * Allocate a buffer to transmit the CMD, ADDR cycles with kmalloc() so > - * we're guaranteed that this buffer is DMA-able, as required by the > - * SPI layer. > - */ > - tx_buf = kzalloc(xfer_len, GFP_KERNEL); > - if (!tx_buf) > - return -ENOMEM; > - > - if (rx_data) { > - rx_buf = kzalloc(xfer_len, GFP_KERNEL); > - if (!rx_buf) > - return -ENOMEM; > + op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes; > + if (op_len > OP_BUFFER_SIZE_MAX) { > + printf("Length for cmd+addr+dummy too big (%d)\n", op_len); > + return -EIO; > } While I agree we shouldn't exceed the 16 bytes for cmd, addr and dummy cycles, I'm not a big fan of those hardcoded limitations that needs to be adjusted every time we realize the initial choice was too restrictive. Do you have a good reason for avoiding kzalloc() here (perfs, dynamic allocation not available in some cases?)? > + memset(op_buf, 0x00, op_len); > > ret = spi_claim_bus(slave); > if (ret < 0) > return ret; > > - tx_buf[pos++] = op->cmd.opcode; > + op_buf[pos++] = op->cmd.opcode; > > if (op->addr.nbytes) { > for (i = 0; i < op->addr.nbytes; i++) > - tx_buf[pos + i] = op->addr.val >> > - (8 * (op->addr.nbytes - i - 1)); > + op_buf[pos + i] = op->addr.val >> > + (8 * (op->addr.nbytes - i - 1)); > > pos += op->addr.nbytes; > } > > - if (op->dummy.nbytes) { > - memset(tx_buf + pos, 0xff, op->dummy.nbytes); > - pos += op->dummy.nbytes; > + if (op->dummy.nbytes) > + memset(op_buf + pos, 0xff, op->dummy.nbytes); > + > + /* 1st transfer: opcode + address + dummy cycles */ > + flag = SPI_XFER_BEGIN; > + /* Make sure to set END bit if no tx or rx data messages follow */ > + if (!tx_data && !rx_data) > + flag |= SPI_XFER_END; I'd add a blank line here. > + ret = spi_xfer(slave, op_len * 8, op_buf, NULL, flag); You should check ret here. > + > + if (tx_data) { > + /* 2nd transfer a: tx data path */ > + ret = spi_xfer(slave, op->data.nbytes * 8, tx_buf, NULL, > + SPI_XFER_END); and here > } > > - if (tx_data) > - memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes); > + if (rx_data) { > + /* 2nd transfer b: rx data path */ > + ret = spi_xfer(slave, op->data.nbytes * 8, NULL, rx_buf, > + SPI_XFER_END); and here > + } If you've initialized rx/tx_buf as suggested above, you can do: if (tx_buf || rx_buf) { ret = spi_xfer(slave, op->data.nbytes * 8, tx_buf, rx_buf, SPI_XFER_END); if (ret) return ret; } > > - ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf, > - SPI_XFER_BEGIN | SPI_XFER_END); > spi_release_bus(slave); > > for (i = 0; i < pos; i++) > - debug("%02x ", tx_buf[i]); > + debug("%02x ", op_buf[i]); > debug("| [%dB %s] ", > tx_data || rx_data ? op->data.nbytes : 0, > tx_data || rx_data ? (tx_data ? "out" : "in") : "-"); > - for (; i < xfer_len; i++) > + for (i = 0; i < op->data.nbytes; i++) > debug("%02x ", tx_data ? tx_buf[i] : rx_buf[i]); > debug("[ret %d]\n", ret); > > if (ret < 0) > return ret; > - > - if (rx_data) > - memcpy(op->data.buf.in, rx_buf + pos, op->data.nbytes); > - > - kfree(tx_buf); > - kfree(rx_buf); > #endif /* __UBOOT__ */ > > return 0;
Hi Stefan, Jagan, Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 7 Aug 2018 15:28:02 +0200: > On Tue, 7 Aug 2018 14:16:52 +0200 > Stefan Roese <sr@denx.de> wrote: > > > Some SPI controller do not support full-duplex SPI transfers. This patch > > changes the SPI transfer into 2 separate transfers - or 1, if no data is > > to transmitted. > > > > With this change, no buffers need to be allocated anymore. We use the > > TX and RX buffers that are passed to spi_mem_exec_op() directly. > > > > Signed-off-by: Stefan Roese <sr@denx.de> > > Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> > > Cc: Miquel Raynal <miquel.raynal@bootlin.com> > > Cc: Boris Brezillon <boris.brezillon@bootlin.com> > > Cc: Jagan Teki <jagan@openedev.com> > > Looks good overall, just a few comments (that you might chose to ignore > if you disagree). > > Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com> > Sorry for being a bit late on the discussion, but while I do agree with the change, I'm not sure about its implementation : I think SPI controllers are supposed to be abstracted by the SPI layer. Addressing the controller's limitations in the SPI-mem layer would not be appropriate. Would it be possible to adapt spi_xfer() to handle such case? Thanks, Miquèl
Hi Stefan, Minor typo if you decide to send a v3. Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 7 Aug 2018 15:28:02 +0200: > On Tue, 7 Aug 2018 14:16:52 +0200 > Stefan Roese <sr@denx.de> wrote: > > > Some SPI controller do not support full-duplex SPI transfers. This patch > > changes the SPI transfer into 2 separate transfers - or 1, if no data is > > to transmitted. ^ be ? > > > > With this change, no buffers need to be allocated anymore. We use the > > TX and RX buffers that are passed to spi_mem_exec_op() directly. > > > > Signed-off-by: Stefan Roese <sr@denx.de> > > Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> > > Cc: Miquel Raynal <miquel.raynal@bootlin.com> > > Cc: Boris Brezillon <boris.brezillon@bootlin.com> > > Cc: Jagan Teki <jagan@openedev.com> > Thanks, Miquèl
Hi Miquel, On 08.08.2018 10:56, Miquel Raynal wrote: > Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 7 Aug 2018 > 15:28:02 +0200: > >> On Tue, 7 Aug 2018 14:16:52 +0200 >> Stefan Roese <sr@denx.de> wrote: >> >>> Some SPI controller do not support full-duplex SPI transfers. This patch >>> changes the SPI transfer into 2 separate transfers - or 1, if no data is >>> to transmitted. >>> >>> With this change, no buffers need to be allocated anymore. We use the >>> TX and RX buffers that are passed to spi_mem_exec_op() directly. >>> >>> Signed-off-by: Stefan Roese <sr@denx.de> >>> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> >>> Cc: Miquel Raynal <miquel.raynal@bootlin.com> >>> Cc: Boris Brezillon <boris.brezillon@bootlin.com> >>> Cc: Jagan Teki <jagan@openedev.com> >> >> Looks good overall, just a few comments (that you might chose to ignore >> if you disagree). >> >> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com> >> > > Sorry for being a bit late on the discussion, but while I do agree with > the change, I'm not sure about its implementation : I think SPI > controllers are supposed to be abstracted by the SPI layer. > Addressing the controller's limitations in the SPI-mem layer would > not be appropriate. > > Would it be possible to adapt spi_xfer() to handle such case? No, I don't think so. Its impossible to guess in the SPI driver layer, which parts of the message are TX data and which are RX data. So this message can't be split up into a half-duplex one here. This can only be done in the driver which constructs the SPI messages. Or did I miss something here? Thanks, Stefan
Hi Miquel, On 08.08.2018 11:58, Miquel Raynal wrote: > Hi Stefan, > > Minor typo if you decide to send a v3. > > Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 7 Aug 2018 > 15:28:02 +0200: > >> On Tue, 7 Aug 2018 14:16:52 +0200 >> Stefan Roese <sr@denx.de> wrote: >> >>> Some SPI controller do not support full-duplex SPI transfers. This patch >>> changes the SPI transfer into 2 separate transfers - or 1, if no data is >>> to transmitted. > > ^ be ? Thanks. I'll be working an v3 today. Thanks, Stefan
Hi Stefan, Stefan Roese <sr@denx.de> wrote on Thu, 9 Aug 2018 07:24:14 +0200: > Hi Miquel, > > On 08.08.2018 10:56, Miquel Raynal wrote: > > Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 7 Aug 2018 > > 15:28:02 +0200: > > >> On Tue, 7 Aug 2018 14:16:52 +0200 > >> Stefan Roese <sr@denx.de> wrote: > >> > >>> Some SPI controller do not support full-duplex SPI transfers. This patch > >>> changes the SPI transfer into 2 separate transfers - or 1, if no data is > >>> to transmitted. > >>> > >>> With this change, no buffers need to be allocated anymore. We use the > >>> TX and RX buffers that are passed to spi_mem_exec_op() directly. > >>> > >>> Signed-off-by: Stefan Roese <sr@denx.de> > >>> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> > >>> Cc: Miquel Raynal <miquel.raynal@bootlin.com> > >>> Cc: Boris Brezillon <boris.brezillon@bootlin.com> > >>> Cc: Jagan Teki <jagan@openedev.com> > >> > >> Looks good overall, just a few comments (that you might chose to ignore > >> if you disagree). > >> > >> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com> > >> > > > Sorry for being a bit late on the discussion, but while I do agree with > > the change, I'm not sure about its implementation : I think SPI > > controllers are supposed to be abstracted by the SPI layer. > > Addressing the controller's limitations in the SPI-mem layer would > > not be appropriate. > > > Would it be possible to adapt spi_xfer() to handle such case? > > No, I don't think so. Its impossible to guess in the SPI driver layer, > which parts of the message are TX data and which are RX data. So this > message can't be split up into a half-duplex one here. This can only > be done in the driver which constructs the SPI messages. > > Or did I miss something here? Actually I'm fine with it as SPI-mem only uses half-duplex. It's not necessary to limit its use to SPI controllers (drivers) supporting full-duplex. Stefan, shall I fold your changes in my series and resend? Or do I resend only my original patches and Jagan/Tom will apply yours on top of it? Thanks, Miquèl
Hi Miquel, On 09.08.2018 10:13, Miquel Raynal wrote: > Stefan Roese <sr@denx.de> wrote on Thu, 9 Aug 2018 07:24:14 +0200: > >> Hi Miquel, >> >> On 08.08.2018 10:56, Miquel Raynal wrote: >>> Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 7 Aug 2018 >>> 15:28:02 +0200: >>>>> On Tue, 7 Aug 2018 14:16:52 +0200 >>>> Stefan Roese <sr@denx.de> wrote: >>>> >>>>> Some SPI controller do not support full-duplex SPI transfers. This patch >>>>> changes the SPI transfer into 2 separate transfers - or 1, if no data is >>>>> to transmitted. >>>>> >>>>> With this change, no buffers need to be allocated anymore. We use the >>>>> TX and RX buffers that are passed to spi_mem_exec_op() directly. >>>>> >>>>> Signed-off-by: Stefan Roese <sr@denx.de> >>>>> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> >>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com> >>>>> Cc: Boris Brezillon <boris.brezillon@bootlin.com> >>>>> Cc: Jagan Teki <jagan@openedev.com> >>>> >>>> Looks good overall, just a few comments (that you might chose to ignore >>>> if you disagree). >>>> >>>> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com> >>>> >>>> Sorry for being a bit late on the discussion, but while I do agree with >>> the change, I'm not sure about its implementation : I think SPI >>> controllers are supposed to be abstracted by the SPI layer. >>> Addressing the controller's limitations in the SPI-mem layer would >>> not be appropriate. >>>> Would it be possible to adapt spi_xfer() to handle such case? >> >> No, I don't think so. Its impossible to guess in the SPI driver layer, >> which parts of the message are TX data and which are RX data. So this >> message can't be split up into a half-duplex one here. This can only >> be done in the driver which constructs the SPI messages. >> >> Or did I miss something here? > > Actually I'm fine with it as SPI-mem only uses half-duplex. It's not > necessary to limit its use to SPI controllers (drivers) supporting > full-duplex. > > Stefan, shall I fold your changes in my series and resend? Or do I > resend only my original patches and Jagan/Tom will apply yours on top > of it? Do you need to re-send your original patches? If yes, then please do with my patches folded in. If a re-send is not necessary, then Jagan or Tom should be able to apply your latest patchset and mine on top of it. Thanks, Stefan
Hi Stefan, Stefan Roese <sr@denx.de> wrote on Thu, 9 Aug 2018 11:10:15 +0200: > Hi Miquel, > > On 09.08.2018 10:13, Miquel Raynal wrote: > > Stefan Roese <sr@denx.de> wrote on Thu, 9 Aug 2018 07:24:14 +0200: > > >> Hi Miquel, > >> > >> On 08.08.2018 10:56, Miquel Raynal wrote: > >>> Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 7 Aug 2018 > >>> 15:28:02 +0200: > >>>>> On Tue, 7 Aug 2018 14:16:52 +0200 > >>>> Stefan Roese <sr@denx.de> wrote: > >>>> >>>>> Some SPI controller do not support full-duplex SPI transfers. This patch > >>>>> changes the SPI transfer into 2 separate transfers - or 1, if no data is > >>>>> to transmitted. > >>>>> > >>>>> With this change, no buffers need to be allocated anymore. We use the > >>>>> TX and RX buffers that are passed to spi_mem_exec_op() directly. > >>>>> > >>>>> Signed-off-by: Stefan Roese <sr@denx.de> > >>>>> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> > >>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com> > >>>>> Cc: Boris Brezillon <boris.brezillon@bootlin.com> > >>>>> Cc: Jagan Teki <jagan@openedev.com> > >>>> > >>>> Looks good overall, just a few comments (that you might chose to ignore > >>>> if you disagree). > >>>> > >>>> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com> > >>>> > >>>> Sorry for being a bit late on the discussion, but while I do agree with > >>> the change, I'm not sure about its implementation : I think SPI > >>> controllers are supposed to be abstracted by the SPI layer. > >>> Addressing the controller's limitations in the SPI-mem layer would > >>> not be appropriate. > >>>> Would it be possible to adapt spi_xfer() to handle such case? > >> > >> No, I don't think so. Its impossible to guess in the SPI driver layer, > >> which parts of the message are TX data and which are RX data. So this > >> message can't be split up into a half-duplex one here. This can only > >> be done in the driver which constructs the SPI messages. > >> > >> Or did I miss something here? > > > Actually I'm fine with it as SPI-mem only uses half-duplex. It's not > > necessary to limit its use to SPI controllers (drivers) supporting > > full-duplex. > > > Stefan, shall I fold your changes in my series and resend? Or do I > > resend only my original patches and Jagan/Tom will apply yours on top > > of it? > > Do you need to re-send your original patches? If yes, then please do > with my patches folded in. If a re-send is not necessary, then Jagan > or Tom should be able to apply your latest patchset and mine on top > of it. I suppose Jagan prefers formal patches than a Github branch so basically I have to pull Boris fixes, squash them, do the same with yours and resend. As we missed the merge window, maybe I'll also take the time to rework mtdparts. I plan to do that next week. Thanks for your help, Miquèl
Hi Miquel, On 09.08.2018 16:50, Miquel Raynal wrote: > Stefan Roese <sr@denx.de> wrote on Thu, 9 Aug 2018 11:10:15 +0200: > >> Hi Miquel, >> >> On 09.08.2018 10:13, Miquel Raynal wrote: >>> Stefan Roese <sr@denx.de> wrote on Thu, 9 Aug 2018 07:24:14 +0200: >>>>> Hi Miquel, >>>> >>>> On 08.08.2018 10:56, Miquel Raynal wrote: >>>>> Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 7 Aug 2018 >>>>> 15:28:02 +0200: >>>>>>> On Tue, 7 Aug 2018 14:16:52 +0200 >>>>>> Stefan Roese <sr@denx.de> wrote: >>>>>> >>>>> Some SPI controller do not support full-duplex SPI transfers. This patch >>>>>>> changes the SPI transfer into 2 separate transfers - or 1, if no data is >>>>>>> to transmitted. >>>>>>> >>>>>>> With this change, no buffers need to be allocated anymore. We use the >>>>>>> TX and RX buffers that are passed to spi_mem_exec_op() directly. >>>>>>> >>>>>>> Signed-off-by: Stefan Roese <sr@denx.de> >>>>>>> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> >>>>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com> >>>>>>> Cc: Boris Brezillon <boris.brezillon@bootlin.com> >>>>>>> Cc: Jagan Teki <jagan@openedev.com> >>>>>> >>>>>> Looks good overall, just a few comments (that you might chose to ignore >>>>>> if you disagree). >>>>>> >>>>>> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com> >>>>>> >>>>>> Sorry for being a bit late on the discussion, but while I do agree with >>>>> the change, I'm not sure about its implementation : I think SPI >>>>> controllers are supposed to be abstracted by the SPI layer. >>>>> Addressing the controller's limitations in the SPI-mem layer would >>>>> not be appropriate. >>>>>> Would it be possible to adapt spi_xfer() to handle such case? >>>> >>>> No, I don't think so. Its impossible to guess in the SPI driver layer, >>>> which parts of the message are TX data and which are RX data. So this >>>> message can't be split up into a half-duplex one here. This can only >>>> be done in the driver which constructs the SPI messages. >>>> >>>> Or did I miss something here? >>>> Actually I'm fine with it as SPI-mem only uses half-duplex. It's not >>> necessary to limit its use to SPI controllers (drivers) supporting >>> full-duplex. >>>> Stefan, shall I fold your changes in my series and resend? Or do I >>> resend only my original patches and Jagan/Tom will apply yours on top >>> of it? >> >> Do you need to re-send your original patches? If yes, then please do >> with my patches folded in. If a re-send is not necessary, then Jagan >> or Tom should be able to apply your latest patchset and mine on top >> of it. > > I suppose Jagan prefers formal patches than a Github branch so > basically I have to pull Boris fixes, squash them, do the same with > yours and resend. I see. I was not aware, that the fixes from Boris have not been squashed yet. Then re-sending this whole series is probably the best solution. > As we missed the merge window, maybe I'll also take the time to rework > mtdparts. I plan to do that next week. That would be great. Thanks, Stefan
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index 07ce799170..84e33aa979 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -15,6 +15,8 @@ #include <spi-mem.h> #endif +#define OP_BUFFER_SIZE_MAX 16 /* Max size for cmd + addr + dummy */ + #ifndef __UBOOT__ /** * spi_controller_dma_map_mem_op_data() - DMA-map the buffer attached to a @@ -200,8 +202,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) bool tx_data = op->data.nbytes && (op->data.dir == SPI_MEM_DATA_OUT); struct udevice *bus = slave->dev->parent; struct dm_spi_ops *ops = spi_get_ops(bus); - unsigned int xfer_len, pos = 0; - u8 *tx_buf, *rx_buf = NULL; + unsigned int pos = 0; + const u8 *tx_buf; + u8 *rx_buf; + u8 op_buf[OP_BUFFER_SIZE_MAX]; + int op_len; + u32 flag; int ret; int i; @@ -330,67 +336,65 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) return -ENOTSUPP; } - xfer_len = sizeof(op->cmd.opcode) + op->addr.nbytes + - op->dummy.nbytes + op->data.nbytes; + tx_buf = op->data.buf.out; + rx_buf = op->data.buf.in; - /* - * Allocate a buffer to transmit the CMD, ADDR cycles with kmalloc() so - * we're guaranteed that this buffer is DMA-able, as required by the - * SPI layer. - */ - tx_buf = kzalloc(xfer_len, GFP_KERNEL); - if (!tx_buf) - return -ENOMEM; - - if (rx_data) { - rx_buf = kzalloc(xfer_len, GFP_KERNEL); - if (!rx_buf) - return -ENOMEM; + op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes; + if (op_len > OP_BUFFER_SIZE_MAX) { + printf("Length for cmd+addr+dummy too big (%d)\n", op_len); + return -EIO; } + memset(op_buf, 0x00, op_len); ret = spi_claim_bus(slave); if (ret < 0) return ret; - tx_buf[pos++] = op->cmd.opcode; + op_buf[pos++] = op->cmd.opcode; if (op->addr.nbytes) { for (i = 0; i < op->addr.nbytes; i++) - tx_buf[pos + i] = op->addr.val >> - (8 * (op->addr.nbytes - i - 1)); + op_buf[pos + i] = op->addr.val >> + (8 * (op->addr.nbytes - i - 1)); pos += op->addr.nbytes; } - if (op->dummy.nbytes) { - memset(tx_buf + pos, 0xff, op->dummy.nbytes); - pos += op->dummy.nbytes; + if (op->dummy.nbytes) + memset(op_buf + pos, 0xff, op->dummy.nbytes); + + /* 1st transfer: opcode + address + dummy cycles */ + flag = SPI_XFER_BEGIN; + /* Make sure to set END bit if no tx or rx data messages follow */ + if (!tx_data && !rx_data) + flag |= SPI_XFER_END; + ret = spi_xfer(slave, op_len * 8, op_buf, NULL, flag); + + if (tx_data) { + /* 2nd transfer a: tx data path */ + ret = spi_xfer(slave, op->data.nbytes * 8, tx_buf, NULL, + SPI_XFER_END); } - if (tx_data) - memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes); + if (rx_data) { + /* 2nd transfer b: rx data path */ + ret = spi_xfer(slave, op->data.nbytes * 8, NULL, rx_buf, + SPI_XFER_END); + } - ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf, - SPI_XFER_BEGIN | SPI_XFER_END); spi_release_bus(slave); for (i = 0; i < pos; i++) - debug("%02x ", tx_buf[i]); + debug("%02x ", op_buf[i]); debug("| [%dB %s] ", tx_data || rx_data ? op->data.nbytes : 0, tx_data || rx_data ? (tx_data ? "out" : "in") : "-"); - for (; i < xfer_len; i++) + for (i = 0; i < op->data.nbytes; i++) debug("%02x ", tx_data ? tx_buf[i] : rx_buf[i]); debug("[ret %d]\n", ret); if (ret < 0) return ret; - - if (rx_data) - memcpy(op->data.buf.in, rx_buf + pos, op->data.nbytes); - - kfree(tx_buf); - kfree(rx_buf); #endif /* __UBOOT__ */ return 0;
Some SPI controller do not support full-duplex SPI transfers. This patch changes the SPI transfer into 2 separate transfers - or 1, if no data is to transmitted. With this change, no buffers need to be allocated anymore. We use the TX and RX buffers that are passed to spi_mem_exec_op() directly. Signed-off-by: Stefan Roese <sr@denx.de> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> Cc: Miquel Raynal <miquel.raynal@bootlin.com> Cc: Boris Brezillon <boris.brezillon@bootlin.com> Cc: Jagan Teki <jagan@openedev.com> --- v2: - Replaces patch "spi: spi-mem: Add optional half-duplex SPI transfer mode" from first patchset version - No compile-time option but the default to use 2 separate SPI messages to transfer the command and data drivers/spi/spi-mem.c | 74 +++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 35 deletions(-)