Message ID | 1376243970-6489-11-git-send-email-tomasz.figa@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Sun, Aug 11, 2013 at 07:59:22PM +0200, Tomasz Figa wrote: > With support for amba-pl08x driver, on S3C64xx the generic DMA engine > API can be used instead of the private s3c-dma interface. I'm still getting issues for SPI with this: [ 50.952845] s3c64xx-spi s3c6410-spi.0: Failed to get RX DMA channel [ 50.953520] spi_master spi0: failed to prepare transfer hardware when I test (with -next with this series applied on top), this is from the first transfer that tries to DMA - there's actually no RX data. No errors are reported by the DMA controller itself. I'm also getting crashes setting up the audio DMA while derferencecing the channel. This is without DT - are there any hookups missing because of that?
On Tuesday 13 of August 2013 00:36:41 Mark Brown wrote: > On Sun, Aug 11, 2013 at 07:59:22PM +0200, Tomasz Figa wrote: > > With support for amba-pl08x driver, on S3C64xx the generic DMA engine > > API can be used instead of the private s3c-dma interface. > > I'm still getting issues for SPI with this: > > [ 50.952845] s3c64xx-spi s3c6410-spi.0: Failed to get RX DMA channel > [ 50.953520] spi_master spi0: failed to prepare transfer hardware > > when I test (with -next with this series applied on top), this is from > the first transfer that tries to DMA - there's actually no RX data. No > errors are reported by the DMA controller itself. I'm also getting > crashes setting up the audio DMA while derferencecing the channel. > > This is without DT - are there any hookups missing because of that? Hmm, strange. I've been testing this without any significant patches, like PWM, CCF or DT support, just everything from the branch I pointed to and some board- specific patches to help in debugging. Anyway, this is the exact branch I've been using: git://github.com/tom3q/linux.git v3.12-pl080-devel Best regards, Tomasz
On Tue, Aug 13, 2013 at 01:49:53AM +0200, Tomasz Figa wrote: > I've been testing this without any significant patches, like PWM, CCF or > DT support, just everything from the branch I pointed to and some board- > specific patches to help in debugging. I retested with the SPI patches you sent separately yesterday which didn't make a difference for the SPI issue. > Anyway, this is the exact branch I've been using: > git://github.com/tom3q/linux.git v3.12-pl080-devel I can't test this directly as it has the DAPM bugs I fixed yesterday in it (those patches will hit -next tomorrow), however it does look like there's nothing else relevant in there...
On Tue, Aug 13, 2013 at 01:49:53AM +0200, Tomasz Figa wrote: > I've been testing this without any significant patches, like PWM, CCF or > DT support, just everything from the branch I pointed to and some board- > specific patches to help in debugging. Done some more digging. It's not failing the first time it requests the channel, it's failing sometime later on (after we've been running for a while but when we've had a substantial idle period). However it is the first time we're actually intending to use the channel, previously to this the transfers would always be too small to trigger DMA. I'm not sure that's relevant though as we're not getting far enough to actually request the channel. I'm still debugging what's going on here - the basic refcounting all looks OK in the SPI driver, I can see it requesting and releasing with the refcounts all going back to zero in the DMA driver as expected but when we come back to the device later on dmaengine is deciding the device is unavailable quite early on in the process.
On Tue, Aug 13, 2013 at 07:55:47PM +0100, Mark Brown wrote: > I'm still debugging what's going on here - the basic refcounting all > looks OK in the SPI driver, I can see it requesting and releasing with > the refcounts all going back to zero in the DMA driver as expected but > when we come back to the device later on dmaengine is deciding the > device is unavailable quite early on in the process. The failure is happening because this check is failing: /* devices with multiple channels need special handling as we need to * ensure that all channels are either private or public. */ if (dev->chancnt > 1 && !dma_has_cap(DMA_PRIVATE, dev->cap_mask)) list_for_each_entry(chan, &dev->channels, device_node) { /* some channels are already publicly allocated */ if (chan->client_count) { which is happening because dma1chan0 (which is on the same DMA controller as the SPI controller) and in fact every other DMA channel had references grabbed by the network stack dmaengine helpers which I'd enabled in config. The fact that they do that is unhelpful, it renders the API mostly useless, but is nothing to do with this series. Having tweaked the config everything appears to work so: Tested-by: Mark Brown <broonie@linaro.org> though the whole thing with the filter function is as I say a bit fun from a code review point of view.
On Tuesday 13 of August 2013 21:18:16 Mark Brown wrote: > On Tue, Aug 13, 2013 at 07:55:47PM +0100, Mark Brown wrote: > > I'm still debugging what's going on here - the basic refcounting all > > looks OK in the SPI driver, I can see it requesting and releasing with > > the refcounts all going back to zero in the DMA driver as expected but > > when we come back to the device later on dmaengine is deciding the > > device is unavailable quite early on in the process. > > The failure is happening because this check is failing: > > /* devices with multiple channels need special handling as we need to > * ensure that all channels are either private or public. > */ > if (dev->chancnt > 1 && !dma_has_cap(DMA_PRIVATE, dev->cap_mask)) > list_for_each_entry(chan, &dev->channels, device_node) { > /* some channels are already publicly allocated */ > if (chan->client_count) { > > which is happening because dma1chan0 (which is on the same DMA > controller as the SPI controller) and in fact every other DMA channel > had references grabbed by the network stack dmaengine helpers which I'd > enabled in config. The fact that they do that is unhelpful, it renders > the API mostly useless, but is nothing to do with this series. > > Having tweaked the config everything appears to work so: > > Tested-by: Mark Brown <broonie@linaro.org> > > though the whole thing with the filter function is as I say a bit fun > from a code review point of view. I believe you just found another brokenness of current DMA channel matching. Thanks for testing. Best regards, Tomasz
On Tue, Aug 13, 2013 at 10:20:19PM +0200, Tomasz Figa wrote: > On Tuesday 13 of August 2013 21:18:16 Mark Brown wrote: > > if (dev->chancnt > 1 && !dma_has_cap(DMA_PRIVATE, dev->cap_mask)) > > list_for_each_entry(chan, &dev->channels, device_node) { > > /* some channels are already publicly allocated */ > > if (chan->client_count) { > > which is happening because dma1chan0 (which is on the same DMA > > controller as the SPI controller) and in fact every other DMA channel > > had references grabbed by the network stack dmaengine helpers which I'd > > enabled in config. The fact that they do that is unhelpful, it renders > > the API mostly useless, but is nothing to do with this series. > I believe you just found another brokenness of current DMA channel > matching. Yeah, at first glance the reference grabbing thing seems a bit peculiar. I didn't research why it's done that way yet.
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 89cbbab..241a049 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -366,7 +366,7 @@ config SPI_S3C24XX_FIQ config SPI_S3C64XX tristate "Samsung S3C64XX series type SPI" depends on (ARCH_S3C24XX || ARCH_S3C64XX || ARCH_S5P64X0 || ARCH_EXYNOS) - select S3C64XX_DMA if ARCH_S3C64XX + select S3C64XX_DMA if ARCH_S3C64XX && !S3C64XX_PL080 help SPI driver for Samsung S3C64XX and newer SoCs.