| Message ID | 26064ccd4a898549fcdecbd5d0a3f32fb0a3ba55.1520691344.git.meghana.madhyastha@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Meghana, On Sat, Mar 10, 2018 at 4:51 PM, Meghana Madhyastha <meghana.madhyastha@gmail.com> wrote: > Split spi messages into chunks of <65535 in the spi subsystem and remove the message > length warning in bcm2835_spi_can_dma. This is so that the messages can be transferred > via dma and that the tinydrm drivers need not split it. > > Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com> Thanks for your patch! > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -1242,6 +1242,14 @@ static void __spi_pump_messages(struct spi_controller *ctlr, bool in_kthread) > trace_spi_message_start(ctlr->cur_msg); > > if (ctlr->prepare_message) { > + gfp_t gfp_flags = GFP_KERNEL | GFP_DMA; > + size_t max_transfer_size = 32000; Do you really want to impose this arbitrary limit (which BTW doesn't match the value in the patch description) to each and every SPI controller driver? > + ret = spi_split_transfers_maxsize(ctlr, ctlr->cur_msg, max_transfer_size, gfp_flags); > + if (ret) { > + dev_err(&ctlr->dev, > + "failed to split message\n"); > + goto out; > + } > ret = ctlr->prepare_message(ctlr, ctlr->cur_msg); > if (ret) { > dev_err(&ctlr->dev, "failed to prepare message: %d\n", Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Den 10.03.2018 16.51, skrev Meghana Madhyastha: > Split spi messages into chunks of <65535 in the spi subsystem and remove the message > length warning in bcm2835_spi_can_dma. This is so that the messages can be transferred > via dma and that the tinydrm drivers need not split it. > > Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com> > --- > drivers/spi/spi-bcm2835.c | 13 ------------- > drivers/spi/spi.c | 8 ++++++++ > 2 files changed, 8 insertions(+), 13 deletions(-) > > diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c > index f35cc10772f6..0dcc45f158b8 100644 > --- a/drivers/spi/spi-bcm2835.c > +++ b/drivers/spi/spi-bcm2835.c > @@ -365,19 +365,6 @@ static bool bcm2835_spi_can_dma(struct spi_master *master, > if (tfr->len < BCM2835_SPI_DMA_MIN_LENGTH) > return false; > > - /* BCM2835_SPI_DLEN has defined a max transfer size as > - * 16 bit, so max is 65535 > - * we can revisit this by using an alternative transfer > - * method - ideally this would get done without any more > - * interaction... > - */ > - if (tfr->len > 65535) { > - dev_warn_once(&spi->dev, > - "transfer size of %d too big for dma-transfer\n", > - tfr->len); > - return false; > - } > - > /* if we run rx/tx_buf with word aligned addresses then we are OK */ > if ((((size_t)tfr->rx_buf & 3) == 0) && > (((size_t)tfr->tx_buf & 3) == 0)) > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index b33a727a0158..e8e2c366a93b 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -1242,6 +1242,14 @@ static void __spi_pump_messages(struct spi_controller *ctlr, bool in_kthread) > trace_spi_message_start(ctlr->cur_msg); > > if (ctlr->prepare_message) { > + gfp_t gfp_flags = GFP_KERNEL | GFP_DMA; > + size_t max_transfer_size = 32000; What number is this? As Geert says, this shouldn't be in the core but in the driver callback bcm2835_spi_prepare_message(). Noralf. > + ret = spi_split_transfers_maxsize(ctlr, ctlr->cur_msg, max_transfer_size, gfp_flags); > + if (ret) { > + dev_err(&ctlr->dev, > + "failed to split message\n"); > + goto out; > + } > ret = ctlr->prepare_message(ctlr, ctlr->cur_msg); > if (ret) { > dev_err(&ctlr->dev, "failed to prepare message: %d\n", -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Mar 10, 2018 at 08:12:29PM +0100, Geert Uytterhoeven wrote: > > + gfp_t gfp_flags = GFP_KERNEL | GFP_DMA; > > + size_t max_transfer_size = 32000; > Do you really want to impose this arbitrary limit (which BTW doesn't match > the value in the patch description) to each and every SPI controller driver? Drivers can advertise any limits they have on their transfer sizes using the max_transfer_size() operation, we should be using that as the limit.
On Sun, Mar 11, 2018 at 02:33:32PM +0100, Noralf Trønnes wrote: > Den 10.03.2018 16.51, skrev Meghana Madhyastha: > > trace_spi_message_start(ctlr->cur_msg); > > if (ctlr->prepare_message) { > > + gfp_t gfp_flags = GFP_KERNEL | GFP_DMA; > > + size_t max_transfer_size = 32000; > What number is this? > As Geert says, this shouldn't be in the core but in the driver callback > bcm2835_spi_prepare_message(). No, this definitely does belong in the core - the limit should come from the driver but if there is a limit then we shouldn't require each driver to open code using that limit.
diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index f35cc10772f6..0dcc45f158b8 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -365,19 +365,6 @@ static bool bcm2835_spi_can_dma(struct spi_master *master, if (tfr->len < BCM2835_SPI_DMA_MIN_LENGTH) return false; - /* BCM2835_SPI_DLEN has defined a max transfer size as - * 16 bit, so max is 65535 - * we can revisit this by using an alternative transfer - * method - ideally this would get done without any more - * interaction... - */ - if (tfr->len > 65535) { - dev_warn_once(&spi->dev, - "transfer size of %d too big for dma-transfer\n", - tfr->len); - return false; - } - /* if we run rx/tx_buf with word aligned addresses then we are OK */ if ((((size_t)tfr->rx_buf & 3) == 0) && (((size_t)tfr->tx_buf & 3) == 0)) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index b33a727a0158..e8e2c366a93b 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1242,6 +1242,14 @@ static void __spi_pump_messages(struct spi_controller *ctlr, bool in_kthread) trace_spi_message_start(ctlr->cur_msg); if (ctlr->prepare_message) { + gfp_t gfp_flags = GFP_KERNEL | GFP_DMA; + size_t max_transfer_size = 32000; + ret = spi_split_transfers_maxsize(ctlr, ctlr->cur_msg, max_transfer_size, gfp_flags); + if (ret) { + dev_err(&ctlr->dev, + "failed to split message\n"); + goto out; + } ret = ctlr->prepare_message(ctlr, ctlr->cur_msg); if (ret) { dev_err(&ctlr->dev, "failed to prepare message: %d\n",
Split spi messages into chunks of <65535 in the spi subsystem and remove the message length warning in bcm2835_spi_can_dma. This is so that the messages can be transferred via dma and that the tinydrm drivers need not split it. Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com> --- drivers/spi/spi-bcm2835.c | 13 ------------- drivers/spi/spi.c | 8 ++++++++ 2 files changed, 8 insertions(+), 13 deletions(-)