AM335x BeagleBone SPI Issues

Hi,

On Mon, Dec 10, 2012 at 03:19:15PM +0000, Jack Mitchell wrote:
> On 10/12/12 14:59, Felipe Balbi wrote:
> >Hi,
> >
> >On Mon, Dec 10, 2012 at 02:50:16PM +0000, Jack Mitchell wrote:
> >>>On Mon, Dec 10, 2012 at 01:23:09PM +0000, Jack Mitchell wrote:
> >>>>Hi,
> >>>>
> >>>>I am currently having issues with the SPI driver on the beaglebone
> >>>>using the 3.7-rc8 kernel[1]. I have probed the SPI pins and I have
> >>>>found that writing works however reading doesn't. When using DMA the
> >>>>program seems to lock hard and no data is sent on the bus. I am
> >>>>testing the bus using spidev and the spidev_test[2] application,
> >>>>however I first came across spi issues with a custom spi driver which
> >>>>stopped working when I transitioned from 3.2-psp to 3.7-rc8.
> >>>>
> >>>>The current output I am seeing from the spidev_test program is just a
> >>>>series of 0x00 data, which looks to me like no data is getting in at
> >>>>all. The spidev_test program is not using DMA as the buffer size is
> >>>>too low, so I forced the dma on when buffer size is > 1 and the
> >>>>program hangs hard with the system still responding to other
> >>>>commands.I have briged the pins 18 and 21 on the BeagleBone P9
> >>>>header.
> >>>>
> >>>>Has anyone seen issues like this, or if not if someone could please
> >>>>test the latest 3.7-rc8 from [1] and let me know if it works for them
> >>>>and the issue is at my end.
> >>>>
> >>>>To get spidev working with devicetree I applied the patch from [3]
> >>>>and changed the dtb as in the patch pasted below.
> >>>>
> >>>>[1] https://github.com/beagleboard/kernel/tree/3.7
> >>>>[2] http://lxr.linux.no/#linux+v3.6.9/Documentation/spi/spidev_test.c
> >>>>[3] http://www.mail-archive.com/spi-devel-general@lists.sourceforge.net/msg09958.html
> >>>do you have any debugging output from that driver ? It would be cool to
> >>>see if DMA is at least being kicked properly for small transfers.
> >>When I run the spidev program with dma for transfers > 1, the program
> >>hangs and the only output in dmesg is:
> >>
> >>[   12.613952] libphy: 4a101000.mdio:00 - Link is Up - 100/Full <----
> >>Last line from initial log in [2]
> >>[   47.669202] spidev spi1.0: setup: speed 24000000, sample leading
> >>edge, clk normal
> >>[   47.669246] spidev spi1.0: setup mode 0, 8 bits/w, 24000000 Hz max --> 0
> >>[   47.669260] spidev spi1.0: spi mode 00
> >>[   47.669283] spidev spi1.0: setup: speed 24000000, sample leading
> >>edge, clk normal
> >>[   47.669300] spidev spi1.0: setup mode 0, 16 bits/w, 24000000 Hz max --> 0
> >>[   47.669312] spidev spi1.0: 16 bits per word
> >>[   47.669330] spidev spi1.0: setup: speed 24000000, sample leading
> >>edge, clk normal
> >>[   47.669347] spidev spi1.0: setup mode 0, 16 bits/w, 24000000 Hz max --> 0
> >>[   47.669358] spidev spi1.0: 24000000 Hz (max)
> >>[   47.673811] spidev spi1.0: setup: speed 24000000, sample leading
> >>edge, clk normal
> >>
> >>The initial dmesg statup log is at [2].
> >can you apply the debugging patch below to spi driver and show me the
> >output again ?
> >
> >Note that I'm allowing DMA for as small as 1-byte transfers (as you
> >needed) and I'm also disabling DMA Request line before calling
> >complete() as I think the current way could race, but likely wouldn't
> >cause issues. Anyway, please show me the output.
> >
> >diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
> >index 3542fdc..d2b1af2 100644
> >--- a/drivers/spi/spi-omap2-mcspi.c
> >+++ b/drivers/spi/spi-omap2-mcspi.c
> >@@ -108,7 +108,7 @@ struct omap2_mcspi_dma {
> >  /* use PIO for small transfers, avoiding DMA setup/teardown overhead and
> >   * cache operations; better heuristics consider wordsize and bitrate.
> >   */
> >-#define DMA_MIN_BYTES			160
> >+#define DMA_MIN_BYTES			1
> >  /*
> >@@ -298,10 +298,11 @@ static void omap2_mcspi_rx_callback(void *data)
> >  	struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
> >  	struct omap2_mcspi_dma *mcspi_dma = &mcspi->dma_channels[spi->chip_select];
> >-	complete(&mcspi_dma->dma_rx_completion);
> >-
> >  	/* We must disable the DMA RX request */
> >+	dev_info(&spi->dev, "Disabling RX Request Line\n");
> >  	omap2_mcspi_set_dma_req(spi, 1, 0);
> >+
> >+	complete(&mcspi_dma->dma_rx_completion);
> >  }
> >  static void omap2_mcspi_tx_callback(void *data)
> >@@ -310,10 +311,11 @@ static void omap2_mcspi_tx_callback(void *data)
> >  	struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
> >  	struct omap2_mcspi_dma *mcspi_dma = &mcspi->dma_channels[spi->chip_select];
> >-	complete(&mcspi_dma->dma_tx_completion);
> >-
> >  	/* We must disable the DMA TX request */
> >+	dev_info(&spi->dev, "Disabling TX Request Line\n");
> >  	omap2_mcspi_set_dma_req(spi, 0, 0);
> >+
> >+	complete(&mcspi_dma->dma_tx_completion);
> >  }
> >  static void omap2_mcspi_tx_dma(struct spi_device *spi,
> >@@ -328,6 +330,7 @@ static void omap2_mcspi_tx_dma(struct spi_device *spi,
> >  	void __iomem		*chstat_reg;
> >  	struct omap2_mcspi_cs	*cs = spi->controller_state;
> >+	dev_info(&spi->dev, "kicking TX DMA\n");
> >  	mcspi = spi_master_get_devdata(spi->master);
> >  	mcspi_dma = &mcspi->dma_channels[spi->chip_select];
> >  	count = xfer->len;
> >@@ -359,7 +362,9 @@ static void omap2_mcspi_tx_dma(struct spi_device *spi,
> >  	dma_async_issue_pending(mcspi_dma->dma_tx);
> >  	omap2_mcspi_set_dma_req(spi, 0, 1);
> >+	dev_info(&spi->dev, "Waiting for TX Completion\n");
> >  	wait_for_completion(&mcspi_dma->dma_tx_completion);
> >+	dev_info(&spi->dev, "TX Completed\n");
> >  	dma_unmap_single(mcspi->dev, xfer->tx_dma, count,
> >  			 DMA_TO_DEVICE);
> >@@ -392,6 +397,7 @@ omap2_mcspi_rx_dma(struct spi_device *spi, struct spi_transfer *xfer,
> >  	word_len = cs->word_len;
> >  	l = mcspi_cached_chconf0(spi);
> >+	dev_info(&spi->dev, "kicking RX DMA\n");
> >  	if (word_len <= 8)
> >  		element_count = count;
> >  	else if (word_len <= 16)
> >@@ -428,7 +434,9 @@ omap2_mcspi_rx_dma(struct spi_device *spi, struct spi_transfer *xfer,
> >  	dma_async_issue_pending(mcspi_dma->dma_rx);
> >  	omap2_mcspi_set_dma_req(spi, 1, 1);
> >+	dev_info(&spi->dev, "Waiting for RX Completion\n");
> >  	wait_for_completion(&mcspi_dma->dma_rx_completion);
> >+	dev_info(&spi->dev, "RX Completed\n");
> >  	dma_unmap_single(mcspi->dev, xfer->rx_dma, count,
> >  			 DMA_FROM_DEVICE);
> >  	omap2_mcspi_set_enable(spi, 0);
> >
> >>>It would also be nice to have a clear picture of what "custom spi
> >>>driver" you're talking about.
> >>The custom SPI driver is for connecting and reading registers from an
> >>in house FPGA design and can be found at [1]. It's fairly rudimentary
> >>and also in the development stages, I'm very new to Linux kernel
> >>programming so please take that into account :)
> >no problem ;-)
> >
> >>However it did work flawlessly with 3.2-psp.
> >yeah, it could be some regression introduced somewhere, or just a bugfix
> >which was done on 3.2-psp and was missed upstream... annoying when those
> >happen :-p
> >
> 
> Your patch didn't apply cleanly but I hacked it in and got the following:
> 
> [   40.434566] spidev spi1.0: setup: speed 24000000, sample leading
> edge, clk normal
> [   40.434609] spidev spi1.0: setup mode 0, 8 bits/w, 24000000 Hz max --> 0
> [   40.434622] spidev spi1.0: spi mode 00
> [   40.434645] spidev spi1.0: setup: speed 24000000, sample leading
> edge, clk normal
> [   40.434661] spidev spi1.0: setup mode 0, 16 bits/w, 24000000 Hz max --> 0
> [   40.434673] spidev spi1.0: 16 bits per word
> [   40.434692] spidev spi1.0: setup: speed 24000000, sample leading
> edge, clk normal
> [   40.434708] spidev spi1.0: setup mode 0, 16 bits/w, 24000000 Hz max --> 0
> [   40.434720] spidev spi1.0: 24000000 Hz (max)
> [   40.439300] spidev spi1.0: setup: speed 24000000, sample leading
> edge, clk normal
> [   40.439397] spidev spi1.0: kicking TX DMA
> [   40.443797] spidev spi1.0: Waiting for TX Completion

so TX dma never completes... perhaps all you need is Shubhro's commit
reordering TX and RX calls ??

commit e47a682ace0cd56eb8e09b806c2b0f034b491520
Author: Shubhrajyoti D <shubhrajyoti@ti.com>
Date:   Tue Nov 6 14:30:19 2012 +0530

    spi: omap2-mcspi: Reorder the wait_for_completion for tx

    The commit  d7b4394e[Cleanup the omap2_mcspi_txrx_dma function]
    changed the wait_for_completion order. Move the wait so that the
    rx doesnot wait for the tx to complete.

    Reported-and-tested-by: Sørensen, Stefan <Sorensen@polycom.com>
    Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
    Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Shubhro, what do you think ? could this be the case ?

AM335x BeagleBone SPI Issues

Commit Message

Comments

Patch