| Message ID | E13569A5-33E5-48ED-B25F-EC7374E144C9@martin.sperl.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/31/2015 08:49 AM, Martin Sperl wrote: > >> On 28.03.2015, at 05:09, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> As for testing: I have also tried to test with mmc_spi, but I have not >>> been able to make that driver work reliably in any recent kernel >>> versions. >>> Most of the time I see timeouts - and with lots of different SD-cards... >>> >>> IIRC the last time I tested it successfully was with 3.12. >> >> It'd be great if you could use "git bisect" to track down the change >> that broke this. ... > Bisection between these shows the responsible commit is: ... > commit 0589342c27944e50ebd7a54f5215002b6598b748 > Author: Rob Herring <rob.herring@calxeda.com> > Date: Tue Oct 29 23:36:46 2013 -0500 > > of: set dma_mask to point to coherent_dma_mask > > Platform devices created by DT code don't initialize dma_mask pointer to > anything. Set it to coherent_dma_mask by default if the architecture > code has not set it. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> ... > Now I went back to 4.0.rc5+ and patched it as follows: > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -179,9 +179,10 @@ static void of_dma_configure(struct device *dev) > * Set it to coherent_dma_mask by default if the architecture > * code has not set it. > */ > +/* > if (!dev->dma_mask) > dev->dma_mask = &dev->coherent_dma_mask; > - > +*/ > ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); > if (ret < 0) { > dma_addr = offset = 0; > > And now it works - the SD card gets detected immediately, > I can mount it and everything... Does that change cause the SDHCI core to use vs. not-use any of the SDHCI DMA modes? If so, I wonder if this is because the upstream kernel doesn't deal with the bcm2835's ARM physical address <-> SoC bus address conversions, which in turn can cause DMA and CPU access to not use the same caching settings and become incoherent, which in turn can cause DMA data corruption. See the following link for some background: https://www.mail-archive.com/u-boot@lists.denx.de/msg167376.html [PATCH 2/3] ARM: bcm2835: implement phys_to_bus/bus_to_phys (you'll want to read all of patches 1-3 for the complete background I think, but patch 2 describes the HW issue) -- 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 01.04.2015, at 17:20, Stephen Warren <swarren@wwwdotorg.org> wrote: > > Does that change cause the SDHCI core to use vs. not-use any of the SDHCI DMA modes? If so, I wonder if this is because the upstream kernel doesn't deal with the bcm2835's ARM physical address <-> SoC bus address conversions, which in turn can cause DMA and CPU access to not use the same caching settings and become incoherent, which in turn can cause DMA data corruption. See the following link for some background: > > https://www.mail-archive.com/u-boot@lists.denx.de/msg167376.html > [PATCH 2/3] ARM: bcm2835: implement phys_to_bus/bus_to_phys > > (you'll want to read all of patches 1-3 for the complete background I think, but patch 2 describes the HW issue) I can also just patch out the assignement to dev_dma here and it works as well: if (spi->master->dev.parent->dma_mask) { struct device *dev = spi->master->dev.parent; host->dma_dev = dev; host->ones_dma = dma_map_single(dev, ones, MMC_SPI_BLOCKSIZE, DMA_TO_DEVICE); host->data_dma = dma_map_single(dev, host->data, sizeof(*host->data), DMA_BIDIRECTIONAL); So DMA there is not an issue with SDHCI - it is just when using the "normal" spi-bcm2835 that I can reproduce it. If you read the mmc_spi driver there is a lot of places where we have constructs like this: if (host->dma_dev) { host->m.is_dma_mapped = 1; dma_sync_single_for_device(host->dma_dev, host->data_dma, sizeof(*host->data), DMA_BIDIRECTIONAL); } status = spi_sync_locked(host->spi, &host->m); if (host->dma_dev) dma_sync_single_for_cpu(host->dma_dev, host->data_dma, sizeof(*host->data), DMA_BIDIRECTIONAL); Similar constructs exist for: dma_map_page and dma_unmap_page. As far as I can tell these are the "typical" dma-related calls. But the point is that the spi-bcm2835 driver does not implement DMA (yet), so some things may not be fully set up for mapping inside spi->master->dev.parent and some mapping options may fail/not work as expected. This may results in possibly "bad" behavior, just because dma_mask is set. So i am not sure if this "mapping" issue you are mentioning is really the root-cause - also I test on a RPI1, but have also tried on a RPI2 with the foundation kernel. On top my reading/experience with DMA on the bcm2835 is that I was always using 0xC0000000 as the bus-address, as the L1/L2 addresses in the documentation are only related to VideoCore caches - and that is I guess why the "L2 preload" is explicitly mentioned in the documentation - probably to speed up the GPU by prefetching the necessary data via DMA and thus avoiding to wait for data to get read from SRAM. Unfortunately I can not test if the the mmc_spi issue exists on a beaglebone, as the mmc_spi driver can not even get compiled there because of CONFIG_HIGHMEM being set which conflicts with KConfig portion for mmc_spi... Ciao, Martin -- 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 Fri, Apr 03, 2015 at 12:17:31PM +0200, Martin Sperl wrote: > > On 01.04.2015, at 17:20, Stephen Warren <swarren@wwwdotorg.org> wrote: It looks like something rewrote Stephen's message to have no word wrapping... > If you read the mmc_spi driver there is a lot of places where we have > constructs like this: > if (host->dma_dev) { > host->m.is_dma_mapped = 1; > dma_sync_single_for_device(host->dma_dev, > host->data_dma, sizeof(*host->data), > DMA_BIDIRECTIONAL); > } > status = spi_sync_locked(host->spi, &host->m); We probably need to have a good, hard look at this stuff - in general we shouldn't be using this sort of mapping in clients, and for some systems this will actually end up broken. > Unfortunately I can not test if the the mmc_spi issue exists on a > beaglebone, as the mmc_spi driver can not even get compiled there because > of CONFIG_HIGHMEM being set which conflicts with KConfig portion for > mmc_spi... This is a bit alarming and probably also indicates that the code needs looking at.
--- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -179,9 +179,10 @@ static void of_dma_configure(struct device *dev) * Set it to coherent_dma_mask by default if the architecture * code has not set it. */ +/* if (!dev->dma_mask) dev->dma_mask = &dev->coherent_dma_mask; - +*/ ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size); if (ret < 0) { dma_addr = offset = 0; And now it works - the SD card gets detected immediately, I can mount it and everything... Is this something we need to fix in bcm2835 by using the "correct" coherent_dma_mask? Still I do not understand why we need dma_mask not set for the mmc_spi driver and how it influences the behaviour... Martin P.s: Note that I have built like this: make bcm2835_defconfig echo "CONFIG_MMC_SPI=y" >> .config using the following diff to enable mmc_spi in the device-tree: --- a/arch/arm/boot/dts/bcm2835-rpi-b.dts +++ b/arch/arm/boot/dts/bcm2835-rpi-b.dts @@ -50,3 +50,14 @@ status = "okay"; bus-width = <4>; }; + +&spi { + status = "okay"; + sd1: sd@1 { + reg = <1>; + status = "okay"; + compatible = "spi,mmc_spi"; + voltage-ranges = <3200 3500>; + spi-max-frequency = <8000000>; + }; +};