All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] spi: bcm2835: enable dma modes for transfers meeting certain conditions
@ 2015-05-10 20:47 kernel-TqfNSX0MhmxHKSADF0wUEw
       [not found] ` <1431290849-11151-1-git-send-email-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2015-05-10 20:47 UTC (permalink / raw)
  To: Noralf Trønnes, Mark Brown, Stephen Warren, Lee Jones,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Martin Sperl

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

Conditions per spi_transfer are:
* transfer.len >= 96 bytes (to avoid mapping overhead costs)
* transfer.len < 65536 bytes (limitaion by spi-hw block - could get extended)
* an individual scatter/gather transfer length must be a multiple of 4
  for anything but the last transfer - spi-hw block limit.
  (some shortcut has been taken in can_dma to avoid unnecessary mapping of
   pages which, for which there is a chance that there is a split with a
   transfer length not a multiple of 4)

If it becomes a necessity these restrictions can get removed by additional
code.

Note that this patch requires a patch to dma-bcm2835.c by Noralf to
enable scatter-gather mode inside the dmaengine, which has not been
merged yet.

That is why no patch to arch/arm/boot/dts/bcm2835.dtsi is included - the
code works as before without dma when tx/rx are not set, but it writes
a message warning about dma not used:
spi-bcm2835 20204000.spi: no tx-dma configuration found - not using dma mode

To enable dma-mode add the following lines to the device-tree:
        dmas = <&dma 6>, <&dma 7>;
        dma-names = "tx", "rx";

Tested-by: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org>
(private communication)

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi-bcm2835.c |  303 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 301 insertions(+), 2 deletions(-)

Tested with:
* 2x mcp251x
* 1x enc28j60
* 1x fb_st7735r

Note that to make it work it requires the patch to dma-engine by 
Noralf Tronnes for scatter/gather DMA.

Also it is recommended that the patch for the race-condition in
spi_finalize_current_message is applied to avoid a free before unmap
situations (this only happens only in some configurations) 

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 830d99c..6ab43c8 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -23,15 +23,18 @@
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/of_irq.h>
-#include <linux/of_gpio.h>
+#include <linux/of_address.h>
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_irq.h>
 #include <linux/spi/spi.h>
 
 /* SPI register offsets */
@@ -70,6 +73,7 @@
 
 #define BCM2835_SPI_POLLING_LIMIT_US	30
 #define BCM2835_SPI_POLLING_JIFFIES	2
+#define BCM2835_SPI_DMA_MIN_LENGTH	96
 #define BCM2835_SPI_MODE_BITS	(SPI_CPOL | SPI_CPHA | SPI_CS_HIGH \
 				| SPI_NO_CS | SPI_3WIRE)
 
@@ -83,6 +87,7 @@ struct bcm2835_spi {
 	u8 *rx_buf;
 	int tx_len;
 	int rx_len;
+	bool dma_pending;
 };
 
 static inline u32 bcm2835_rd(struct bcm2835_spi *bs, unsigned reg)
@@ -128,12 +133,15 @@ static void bcm2835_spi_reset_hw(struct spi_master *master)
 	/* Disable SPI interrupts and transfer */
 	cs &= ~(BCM2835_SPI_CS_INTR |
 		BCM2835_SPI_CS_INTD |
+		BCM2835_SPI_CS_DMAEN |
 		BCM2835_SPI_CS_TA);
 	/* and reset RX/TX FIFOS */
 	cs |= BCM2835_SPI_CS_CLEAR_RX | BCM2835_SPI_CS_CLEAR_TX;
 
 	/* and reset the SPI_HW */
 	bcm2835_wr(bs, BCM2835_SPI_CS, cs);
+	/* as well as DLEN */
+	bcm2835_wr(bs, BCM2835_SPI_DLEN, 0);
 }
 
 static irqreturn_t bcm2835_spi_interrupt(int irq, void *dev_id)
@@ -193,6 +201,279 @@ static int bcm2835_spi_transfer_one_irq(struct spi_master *master,
 	return 1;
 }
 
+/*
+ * DMA support
+ *
+ * this implementation has currently a few issues in so far as it does
+ * not work arrount limitations of the HW.
+ *
+ * the main one being that DMA transfers are limited to 16 bit
+ * (so 0 to 65535 bytes) by the SPI HW due to BCM2835_SPI_DLEN
+ *
+ * also we currently assume that the scatter-gather fragments are
+ * all multiple of 4 (except the last) - otherwise we would need
+ * to reset the FIFO before subsequent transfers...
+ * this also means that tx/rx transfers sg's need to be of equal size!
+ *
+ * there may be a few more border-cases we may need to address as well
+ * but unfortunately this would mean splitting up the scatter-gather
+ * list making it slightly unpractical...
+ */
+static void bcm2835_spi_dma_done(void *data)
+{
+	struct spi_master *master = data;
+	struct bcm2835_spi *bs = spi_master_get_devdata(master);
+
+	/* reset fifo and HW */
+	bcm2835_spi_reset_hw(master);
+
+	/* and terminate tx-dma as we do not have an irq for it
+	 * because when the rx dma will terminate and this callback
+	 * is called the tx-dma must have finished - can't get to this
+	 * situation otherwise...
+	 */
+	dmaengine_terminate_all(master->dma_tx);
+
+	/* mark as no longer pending */
+	bs->dma_pending = 0;
+
+	/* and mark as completed */;
+	complete(&master->xfer_completion);
+}
+
+static int bcm2835_spi_prepare_sg(struct spi_master *master,
+				  struct spi_transfer *tfr,
+				  bool is_tx)
+{
+	struct dma_chan *chan;
+	struct scatterlist *sgl;
+	unsigned int nents;
+	enum dma_transfer_direction dir;
+	unsigned long flags;
+
+	struct dma_async_tx_descriptor *desc;
+	dma_cookie_t cookie;
+
+	if (is_tx) {
+		dir   = DMA_MEM_TO_DEV;
+		chan  = master->dma_tx;
+		nents = tfr->tx_sg.nents;
+		sgl   = tfr->tx_sg.sgl;
+		flags = 0 /* no  tx interrupt */;
+
+	} else {
+		dir   = DMA_DEV_TO_MEM;
+		chan  = master->dma_rx;
+		nents = tfr->rx_sg.nents;
+		sgl   = tfr->rx_sg.sgl;
+		flags = DMA_PREP_INTERRUPT;
+	}
+	/* prepare the channel */
+	desc = dmaengine_prep_slave_sg(chan, sgl, nents, dir, flags);
+	if (!desc)
+		return -EINVAL;
+
+	/* set callback for rx */
+	if (!is_tx) {
+		desc->callback = bcm2835_spi_dma_done;
+		desc->callback_param = master;
+	}
+
+	/* submit it to DMA-engine */
+	cookie = dmaengine_submit(desc);
+
+	return dma_submit_error(cookie);
+}
+
+static inline int bcm2835_check_sg_length(struct sg_table *sgt)
+{
+	int i;
+	struct scatterlist *sgl;
+
+	/* check that the sg entries are word-sized (except for last) */
+	for_each_sg(sgt->sgl, sgl, (int)sgt->nents - 1, i) {
+		if (sg_dma_len(sgl) % 4)
+			return -EFAULT;
+	}
+
+	return 0;
+}
+
+static int bcm2835_spi_transfer_one_dma(struct spi_master *master,
+					struct spi_device *spi,
+					struct spi_transfer *tfr,
+					u32 cs)
+{
+	struct bcm2835_spi *bs = spi_master_get_devdata(master);
+	int ret;
+
+	/* check that the scatter gather segments are all a multiple of 4 */
+	if (bcm2835_check_sg_length(&tfr->tx_sg) ||
+	    bcm2835_check_sg_length(&tfr->rx_sg)) {
+		dev_warn_once(&spi->dev,
+			      "scatter gather segment length is not a multiple of 4 - falling back to interrupt mode\n");
+		return bcm2835_spi_transfer_one_irq(master, spi, tfr, cs);
+	}
+
+	/* setup tx-DMA */
+	ret = bcm2835_spi_prepare_sg(master, tfr, true);
+	if (ret)
+		return ret;
+
+	/* start TX early */
+	dma_async_issue_pending(master->dma_tx);
+
+	/* mark as dma pending */
+	bs->dma_pending = 1;
+
+	/* set the DMA length */
+	bcm2835_wr(bs, BCM2835_SPI_DLEN, tfr->len);
+
+	/* start the HW */
+	bcm2835_wr(bs, BCM2835_SPI_CS,
+		   cs | BCM2835_SPI_CS_TA | BCM2835_SPI_CS_DMAEN);
+
+	/* setup rx-DMA late - to run transfers while
+	 * mapping of the rx buffers still takes place
+	 * this saves 10us or more.
+	 */
+	ret = bcm2835_spi_prepare_sg(master, tfr, false);
+	if (ret) {
+		/* need to reset on errors */
+		dmaengine_terminate_all(master->dma_tx);
+		bcm2835_spi_reset_hw(master);
+		return ret;
+	}
+
+	/* start rx dma late */
+	dma_async_issue_pending(master->dma_rx);
+
+	/* wait for wakeup in framework */
+	return 1;
+}
+
+static bool bcm2835_spi_can_dma(struct spi_master *master,
+				struct spi_device *spi,
+				struct spi_transfer *tfr)
+{
+	/* only run for gpio_cs */
+	if (!gpio_is_valid(spi->cs_gpio))
+		return false;
+
+	/* we start DMA efforts only on bigger transfers */
+	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 (((u32)tfr->tx_buf % 4 == 0) && ((u32)tfr->tx_buf % 4 == 0))
+		return true;
+
+	/* otherwise we only allow transfers within the same page
+	 * to avoid wasting time on dma_mapping when it is not practical
+	 */
+	if (((u32)tfr->tx_buf % SZ_4K) + tfr->len > SZ_4K) {
+		dev_warn_once(&spi->dev,
+			      "Unaligned spi tx-transfer bridging page\n");
+		return false;
+	}
+	if (((u32)tfr->rx_buf % SZ_4K) + tfr->len > SZ_4K) {
+		dev_warn_once(&spi->dev,
+			      "Unaligned spi tx-transfer bridging page\n");
+		return false;
+	}
+
+	/* return OK */
+	return true;
+}
+
+void bcm2835_dma_release(struct spi_master *master)
+{
+	if (master->dma_tx) {
+		dmaengine_terminate_all(master->dma_tx);
+		dma_release_channel(master->dma_tx);
+		master->dma_tx = NULL;
+	}
+	if (master->dma_rx) {
+		dmaengine_terminate_all(master->dma_rx);
+		dma_release_channel(master->dma_rx);
+		master->dma_rx = NULL;
+	}
+}
+
+void bcm2835_dma_init(struct spi_master *master, struct device *dev)
+{
+	struct dma_slave_config slave_config;
+	const __be32 *addr;
+	dma_addr_t dma_reg_base;
+	int ret;
+
+	/* base address in dma-space */
+	addr = of_get_address(master->dev.of_node, 0, NULL, NULL);
+	if (!addr) {
+		dev_err(dev, "could not get DMA-register address - not using dma mode\n");
+		goto err;
+	}
+	dma_reg_base = be32_to_cpup(addr);
+
+	/* get tx/rx dma */
+	master->dma_tx = dma_request_slave_channel(dev, "tx");
+	if (!master->dma_tx) {
+		dev_err(dev, "no tx-dma configuration found - not using dma mode\n");
+		goto err;
+	}
+	master->dma_rx = dma_request_slave_channel(dev, "rx");
+	if (!master->dma_rx) {
+		dev_err(dev, "no rx-dma configuration found - not using dma mode\n");
+		goto err_release;
+	}
+
+	/* configure DMAs */
+	slave_config.direction = DMA_MEM_TO_DEV;
+	slave_config.dst_addr = (u32)(dma_reg_base + BCM2835_SPI_FIFO);
+	slave_config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+
+	ret = dmaengine_slave_config(master->dma_tx, &slave_config);
+	if (ret)
+		goto err_config;
+
+	slave_config.direction = DMA_DEV_TO_MEM;
+	slave_config.src_addr = (u32)(dma_reg_base + BCM2835_SPI_FIFO);
+	slave_config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+
+	ret = dmaengine_slave_config(master->dma_rx, &slave_config);
+	if (ret)
+		goto err_config;
+
+	/* all went well, so set can_dma */
+	master->can_dma = bcm2835_spi_can_dma;
+	master->max_dma_len = 65535; /* limitation by BCM2835_SPI_DLEN */
+	/* need to do TX AND RX DMA, so we need dummy buffers */
+	master->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
+
+	return;
+
+err_config:
+	dev_err(dev, "issue configuring dma: %d - not using DMA mode\n",
+		ret);
+err_release:
+	bcm2835_dma_release(master);
+err:
+	return;
+}
+
 static int bcm2835_spi_transfer_one_poll(struct spi_master *master,
 					 struct spi_device *spi,
 					 struct spi_transfer *tfr,
@@ -301,12 +582,26 @@ static int bcm2835_spi_transfer_one(struct spi_master *master,
 		return bcm2835_spi_transfer_one_poll(master, spi, tfr,
 						     cs, xfer_time_us);
 
+	/* run in dma mode if conditions are right */
+	if (master->can_dma && bcm2835_spi_can_dma(master, spi, tfr))
+		return bcm2835_spi_transfer_one_dma(master, spi, tfr, cs);
+
+	/* run in interrupt-mode */
 	return bcm2835_spi_transfer_one_irq(master, spi, tfr, cs);
 }
 
 static void bcm2835_spi_handle_err(struct spi_master *master,
 				   struct spi_message *msg)
 {
+	struct bcm2835_spi *bs = spi_master_get_devdata(master);
+
+	/* if an error occurred and we have an active dma, then terminate */
+	if (bs->dma_pending) {
+		dmaengine_terminate_all(master->dma_tx);
+		dmaengine_terminate_all(master->dma_rx);
+		bs->dma_pending = 0;
+	}
+	/* and reset */
 	bcm2835_spi_reset_hw(master);
 }
 
@@ -476,6 +771,8 @@ static int bcm2835_spi_probe(struct platform_device *pdev)
 		goto out_clk_disable;
 	}
 
+	bcm2835_dma_init(master, &pdev->dev);
+
 	/* initialise the hardware with the default polarities */
 	bcm2835_wr(bs, BCM2835_SPI_CS,
 		   BCM2835_SPI_CS_CLEAR_RX | BCM2835_SPI_CS_CLEAR_TX);
@@ -506,6 +803,8 @@ static int bcm2835_spi_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(bs->clk);
 
+	bcm2835_dma_release(master);
+
 	return 0;
 }
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] spi: bcm2835: enable dma modes for transfers meeting certain conditions
       [not found] ` <1431290849-11151-1-git-send-email-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
@ 2015-05-11 16:30   ` Noralf Trønnes
  2015-05-11 18:24   ` Mark Brown
  1 sibling, 0 replies; 7+ messages in thread
From: Noralf Trønnes @ 2015-05-11 16:30 UTC (permalink / raw)
  To: kernel-TqfNSX0MhmxHKSADF0wUEw, Mark Brown, Stephen Warren,
	Lee Jones, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r


Den 10.05.2015 22:47, skrev kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>
> Conditions per spi_transfer are:
> * transfer.len >= 96 bytes (to avoid mapping overhead costs)
> * transfer.len < 65536 bytes (limitaion by spi-hw block - could get extended)
> * an individual scatter/gather transfer length must be a multiple of 4
>    for anything but the last transfer - spi-hw block limit.
>    (some shortcut has been taken in can_dma to avoid unnecessary mapping of
>     pages which, for which there is a chance that there is a split with a
>     transfer length not a multiple of 4)
>
> If it becomes a necessity these restrictions can get removed by additional
> code.
>
> Note that this patch requires a patch to dma-bcm2835.c by Noralf to
> enable scatter-gather mode inside the dmaengine, which has not been
> merged yet.
>
> That is why no patch to arch/arm/boot/dts/bcm2835.dtsi is included - the
> code works as before without dma when tx/rx are not set, but it writes
> a message warning about dma not used:
> spi-bcm2835 20204000.spi: no tx-dma configuration found - not using dma mode
>
> To enable dma-mode add the following lines to the device-tree:
>          dmas = <&dma 6>, <&dma 7>;
>          dma-names = "tx", "rx";
>
> Tested-by: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org>
> (private communication)
>
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> ---
>   drivers/spi/spi-bcm2835.c |  303 ++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 301 insertions(+), 2 deletions(-)
>
> Tested with:
> * 2x mcp251x
> * 1x enc28j60
> * 1x fb_st7735r
>
> Note that to make it work it requires the patch to dma-engine by
> Noralf Tronnes for scatter/gather DMA.
>
> Also it is recommended that the patch for the race-condition in
> spi_finalize_current_message is applied to avoid a free before unmap
> situations (this only happens only in some configurations)

A couple of throughput tests using the fbtft drivers:

I set up 2 SPI display devices running a movie on both framebuffers.
Since I don't have an easy way to hook up 2 displays, the test was run
without anything connected. Throughput was measured to ~2.5MB/s on one
device and ~3.0MB/s on the other in bursts of ~50ms with ~10ms delay
between frames. 'top' shows cpu util of ~25% for each mplayer process
and ~10% for system (30-35% idle).

Log details:
[    2.833071] graphics fb1: fb_ili9341 frame buffer, 320x240, 150 KiB 
video memory, 16 KiB DMA buffer memory, fps=50, spi32766.0 at 48 MHz
[    3.036245] graphics fb2: fb_ili9341 frame buffer, 320x240, 150 KiB 
video memory, 16 KiB DMA buffer memory, fps=50, spi32766.1 at 48 MHz

[   76.863674] fb_ili9341 spi32766.0: Display update: 2576 kB/s (58.217 
ms), fps=14 (69.997 ms)
[   76.894161] fb_ili9341 spi32766.1: Display update: 3077 kB/s (48.735 
ms), fps=16 (59.431 ms)


Only 1 display connected (35ms burst, 15ms delay):
[    2.826042] graphics fb1: fb_ili9341 frame buffer, 320x240, 150 KiB 
video memory, 16 KiB DMA buffer memory, fps=50, spi32766.0 at 48 MHz

[   51.000128] fb_ili9341 spi32766.0: Display update: 4429 kB/s (33.856 
ms), fps=20 (49.968 ms)

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] spi: bcm2835: enable dma modes for transfers meeting certain conditions
       [not found] ` <1431290849-11151-1-git-send-email-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
  2015-05-11 16:30   ` Noralf Trønnes
@ 2015-05-11 18:24   ` Mark Brown
       [not found]     ` <20150511182448.GW3458-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  1 sibling, 1 reply; 7+ messages in thread
From: Mark Brown @ 2015-05-11 18:24 UTC (permalink / raw)
  To: kernel-TqfNSX0MhmxHKSADF0wUEw
  Cc: Noralf Trønnes, Stephen Warren, Lee Jones,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]

On Sun, May 10, 2015 at 08:47:28PM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> 
> Conditions per spi_transfer are:
> * transfer.len >= 96 bytes (to avoid mapping overhead costs)
> * transfer.len < 65536 bytes (limitaion by spi-hw block - could get extended)

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] spi: bcm2835: enable dma modes for transfers meeting certain conditions
       [not found]     ` <20150511182448.GW3458-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2015-05-11 19:38       ` Martin Sperl
       [not found]         ` <F47F6B62-3DC6-47C9-AD72-BE7DDABD3AE6-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Martin Sperl @ 2015-05-11 19:38 UTC (permalink / raw)
  To: Mark Brown
  Cc: Noralf Trønnes, Stephen Warren, Lee Jones,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r


> On 11.05.2015, at 20:24, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> 
> Applied, thanks.

Found one typo with the logic - it only does one specific
check for TX twice instead of for rx_buf and tx_buf:
	if (((u32)tfr->tx_buf % 4 == 0) && ((u32)tfr->tx_buf % 4 == 0))

On top there have been some build-reports about SZ_4K not being defined
so I guess that there is an include <linux/sizes.h> missing.

as well as a warning about a pointer cast to u32 on 64 bit systems.
	if (((u32)tfr->tx_buf % 4 == 0) && ((u32)tfr->tx_buf % 4 == 0))
	if (((u32)tfr->rx_buf % SZ_4K) + tfr->len > SZ_4K) {
I assume fixable by using size_t instead of u32.

Do you want me to send a patch to fix those, or shall I resend the full
patch with those issues fixed?

Sorry for the inconvenience

Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] spi: bcm2835: enable dma modes for transfers meeting certain conditions
       [not found]         ` <F47F6B62-3DC6-47C9-AD72-BE7DDABD3AE6-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
@ 2015-05-12  2:45           ` Stephen Warren
       [not found]             ` <55516950.5070105-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Warren @ 2015-05-12  2:45 UTC (permalink / raw)
  To: Martin Sperl
  Cc: Mark Brown, Noralf Trønnes, Lee Jones,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 05/11/2015 01:38 PM, Martin Sperl wrote:
> 
>> On 11.05.2015, at 20:24, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>
>> Applied, thanks.
> 
> Found one typo with the logic - it only does one specific
> check for TX twice instead of for rx_buf and tx_buf:
> 	if (((u32)tfr->tx_buf % 4 == 0) && ((u32)tfr->tx_buf % 4 == 0))
> 
> On top there have been some build-reports about SZ_4K not being defined
> so I guess that there is an include <linux/sizes.h> missing.
> 
> as well as a warning about a pointer cast to u32 on 64 bit systems.
> 	if (((u32)tfr->tx_buf % 4 == 0) && ((u32)tfr->tx_buf % 4 == 0))
> 	if (((u32)tfr->rx_buf % SZ_4K) + tfr->len > SZ_4K) {
> I assume fixable by using size_t instead of u32.
> 
> Do you want me to send a patch to fix those, or shall I resend the full
> patch with those issues fixed?

Since the patch has been applied, you'll need to send an incremental patch.

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] spi: bcm2835: fix kbuild compile warnings/errors and a typo
       [not found]             ` <55516950.5070105-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2015-05-12 10:32               ` kernel-TqfNSX0MhmxHKSADF0wUEw
       [not found]                 ` <1431426729-2154-1-git-send-email-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2015-05-12 10:32 UTC (permalink / raw)
  To: Mark Brown, Stephen Warren, Lee Jones,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Stephen Rothwell
  Cc: Martin Sperl

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

fixes several warnings/error emmitted by the kbuild system:
* warn: cast from pointer to integer of different size
  using size_t instead of u32
* error: 'SZ_4K' undeclared
  moved to PAGE_SIZE and PAGE_MASK instead

Review showed also a typo in the same code where tx_buff
was checked twice instead of checking both rx and tx_buff.

Reported by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi-bcm2835.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 6ab43c8..ac1760e 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -20,6 +20,7 @@
  * GNU General Public License for more details.
  */
 
+#include <asm/page.h>
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
@@ -378,18 +379,19 @@ static bool bcm2835_spi_can_dma(struct spi_master *master,
 	}
 
 	/* if we run rx/tx_buf with word aligned addresses then we are OK */
-	if (((u32)tfr->tx_buf % 4 == 0) && ((u32)tfr->tx_buf % 4 == 0))
+	if ((((size_t)tfr->rx_buf & 3) == 0) &&
+	    (((size_t)tfr->tx_buf & 3) == 0))
 		return true;
 
 	/* otherwise we only allow transfers within the same page
 	 * to avoid wasting time on dma_mapping when it is not practical
 	 */
-	if (((u32)tfr->tx_buf % SZ_4K) + tfr->len > SZ_4K) {
+	if (((size_t)tfr->tx_buf & PAGE_MASK) + tfr->len > PAGE_SIZE) {
 		dev_warn_once(&spi->dev,
 			      "Unaligned spi tx-transfer bridging page\n");
 		return false;
 	}
-	if (((u32)tfr->rx_buf % SZ_4K) + tfr->len > SZ_4K) {
+	if (((size_t)tfr->rx_buf & PAGE_MASK) + tfr->len > PAGE_SIZE) {
 		dev_warn_once(&spi->dev,
 			      "Unaligned spi tx-transfer bridging page\n");
 		return false;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] spi: bcm2835: fix kbuild compile warnings/errors and a typo
       [not found]                 ` <1431426729-2154-1-git-send-email-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
@ 2015-05-12 10:42                   ` Mark Brown
  0 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2015-05-12 10:42 UTC (permalink / raw)
  To: kernel-TqfNSX0MhmxHKSADF0wUEw
  Cc: Stephen Warren, Lee Jones, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Stephen Rothwell

[-- Attachment #1: Type: text/plain, Size: 255 bytes --]

On Tue, May 12, 2015 at 10:32:08AM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> 
> fixes several warnings/error emmitted by the kbuild system:

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-05-12 10:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-10 20:47 [PATCH] spi: bcm2835: enable dma modes for transfers meeting certain conditions kernel-TqfNSX0MhmxHKSADF0wUEw
     [not found] ` <1431290849-11151-1-git-send-email-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
2015-05-11 16:30   ` Noralf Trønnes
2015-05-11 18:24   ` Mark Brown
     [not found]     ` <20150511182448.GW3458-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-05-11 19:38       ` Martin Sperl
     [not found]         ` <F47F6B62-3DC6-47C9-AD72-BE7DDABD3AE6-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
2015-05-12  2:45           ` Stephen Warren
     [not found]             ` <55516950.5070105-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2015-05-12 10:32               ` [PATCH] spi: bcm2835: fix kbuild compile warnings/errors and a typo kernel-TqfNSX0MhmxHKSADF0wUEw
     [not found]                 ` <1431426729-2154-1-git-send-email-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
2015-05-12 10:42                   ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.