* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates [not found] ` <20191015131750.GV25745@shell.armlinux.org.uk> @ 2019-10-23 5:42 ` Michael Ellerman 2019-10-23 6:41 ` Benjamin Herrenschmidt 2019-10-23 14:20 ` Christian Zigotzky 0 siblings, 2 replies; 12+ messages in thread From: Michael Ellerman @ 2019-10-23 5:42 UTC (permalink / raw) To: Russell King - ARM Linux admin, Christian Zigotzky, Benjamin Herrenschmidt, Paul Mackerras Cc: ulf.hansson, R.T.Dickinson, linux-mmc, Rob Herring, contact, mad skateman, linuxppc-dev, Christian Zigotzky Russell King - ARM Linux admin <linux@armlinux.org.uk> writes: > On Tue, Oct 15, 2019 at 03:12:49PM +0200, Christian Zigotzky wrote: >> Hello Russell, >> >> You asked me about "dma-coherent" in the Cyrus device tree. Unfortunately I >> don't find the property "dma-coherent" in the dtb source files. >> >> Output of "fdtdump cyrus_p5020_eth_poweroff.dtb | grep dma": >> >> dma0 = "/soc@ffe000000/dma@100300"; >> dma1 = "/soc@ffe000000/dma@101300"; >> dma@100300 { >> compatible = "fsl,eloplus-dma"; >> dma-channel@0 { >> compatible = "fsl,eloplus-dma-channel"; >> dma-channel@80 { >> compatible = "fsl,eloplus-dma-channel"; >> dma-channel@100 { >> compatible = "fsl,eloplus-dma-channel"; >> dma-channel@180 { >> compatible = "fsl,eloplus-dma-channel"; >> dma@101300 { >> compatible = "fsl,eloplus-dma"; >> dma-channel@0 { >> compatible = "fsl,eloplus-dma-channel"; >> dma-channel@80 { >> compatible = "fsl,eloplus-dma-channel"; >> dma-channel@100 { >> compatible = "fsl,eloplus-dma-channel"; >> dma-channel@180 { >> compatible = "fsl,eloplus-dma-channel"; > > Hmm, so it looks like PowerPC doesn't mark devices that are dma > coherent with a property that describes them as such. > > I think this opens a wider question - what should of_dma_is_coherent() > return for PowerPC? It seems right now that it returns false for > devices that are DMA coherent, which seems to me to be a recipe for > future mistakes. Right, it seems of_dma_is_coherent() has baked in the assumption that devices are non-coherent unless explicitly marked as coherent. Which is wrong on all or at least most existing powerpc systems according to Ben. > Any ideas from the PPC maintainers? Fixing it at the source seems like the best option to prevent future breakage. So I guess that would mean making of_dma_is_coherent() return true/false based on CONFIG_NOT_COHERENT_CACHE on powerpc. We could do it like below, which would still allow the dma-coherent property to work if it ever makes sense on a future powerpc platform. I don't really know any of this embedded stuff well, so happy to take other suggestions on how to handle this mess. cheers diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index 25aaa3903000..b96c9010acb6 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -760,6 +760,22 @@ static int __init check_cache_coherency(void) late_initcall(check_cache_coherency); #endif /* CONFIG_CHECK_CACHE_COHERENCY */ +#ifndef CONFIG_NOT_COHERENT_CACHE +/* + * For historical reasons powerpc kernels are built with hard wired knowledge of + * whether or not DMA accesses are cache coherent. Additionally device trees on + * powerpc do not typically support the dma-coherent property. + * + * So when we know that DMA is coherent, override arch_of_dma_is_coherent() to + * tell the drivers/of code that all devices are coherent regardless of whether + * they have a dma-coherent property. + */ +bool arch_of_dma_is_coherent(struct device_node *np) +{ + return true; +} +#endif + #ifdef CONFIG_DEBUG_FS struct dentry *powerpc_debugfs_root; EXPORT_SYMBOL(powerpc_debugfs_root); diff --git a/drivers/of/address.c b/drivers/of/address.c index 978427a9d5e6..3a4b2949a322 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -993,6 +993,14 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz } EXPORT_SYMBOL_GPL(of_dma_get_range); +/* + * arch_of_dma_is_coherent - Arch hook to determine if device is coherent for DMA + */ +bool __weak arch_of_dma_is_coherent(struct device_node *np) +{ + return false; +} + /** * of_dma_is_coherent - Check if device is coherent * @np: device node @@ -1002,8 +1010,12 @@ EXPORT_SYMBOL_GPL(of_dma_get_range); */ bool of_dma_is_coherent(struct device_node *np) { - struct device_node *node = of_node_get(np); + struct device_node *node; + + if (arch_of_dma_is_coherent(np)) + return true; + np = of_node_get(np); while (node) { if (of_property_read_bool(node, "dma-coherent")) { of_node_put(node); ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates 2019-10-23 5:42 ` Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates Michael Ellerman @ 2019-10-23 6:41 ` Benjamin Herrenschmidt 2019-10-23 13:52 ` Rob Herring 2019-10-23 14:20 ` Christian Zigotzky 1 sibling, 1 reply; 12+ messages in thread From: Benjamin Herrenschmidt @ 2019-10-23 6:41 UTC (permalink / raw) To: Michael Ellerman, Russell King - ARM Linux admin, Christian Zigotzky, Paul Mackerras Cc: ulf.hansson, R.T.Dickinson, linux-mmc, Rob Herring, contact, mad skateman, linuxppc-dev, Christian Zigotzky On Wed, 2019-10-23 at 16:42 +1100, Michael Ellerman wrote: > > Right, it seems of_dma_is_coherent() has baked in the assumption that > devices are non-coherent unless explicitly marked as coherent. > > Which is wrong on all or at least most existing powerpc systems > according to Ben. This is probably broken on sparc(64) as well and whatever else uses DT and is an intrinsicly coherent architecture (did we ever have DT enabled x86s ? Wasn't OLPC such a beast ?). I think this should have been done the other way around and default to coherent since most traditional OF platforms are coherent, and you can't just require those DTs to change. > > Any ideas from the PPC maintainers? > > Fixing it at the source seems like the best option to prevent future > breakage. > > So I guess that would mean making of_dma_is_coherent() return true/false > based on CONFIG_NOT_COHERENT_CACHE on powerpc. > > We could do it like below, which would still allow the dma-coherent > property to work if it ever makes sense on a future powerpc platform. > > I don't really know any of this embedded stuff well, so happy to take > other suggestions on how to handle this mess. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates 2019-10-23 6:41 ` Benjamin Herrenschmidt @ 2019-10-23 13:52 ` Rob Herring 2019-10-23 14:12 ` Russell King - ARM Linux admin 2019-10-23 14:31 ` Russell King - ARM Linux admin 0 siblings, 2 replies; 12+ messages in thread From: Rob Herring @ 2019-10-23 13:52 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Ulf Hansson, mad skateman, linux-mmc, Russell King - ARM Linux admin, Paul Mackerras, R.T.Dickinson, Christian Zigotzky, contact, linuxppc-dev, Christian Zigotzky On Wed, Oct 23, 2019 at 1:41 AM Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > On Wed, 2019-10-23 at 16:42 +1100, Michael Ellerman wrote: > > > > Right, it seems of_dma_is_coherent() has baked in the assumption that > > devices are non-coherent unless explicitly marked as coherent. > > > > Which is wrong on all or at least most existing powerpc systems > > according to Ben. > > This is probably broken on sparc(64) as well and whatever else uses > DT and is an intrinsicly coherent architecture (did we ever have > DT enabled x86s ? Wasn't OLPC such a beast ?). Only if those platforms use one of the 5 drivers that call this function: drivers/ata/ahci_qoriq.c: qoriq_priv->is_dmacoherent = of_dma_is_coherent(np); drivers/crypto/ccree/cc_driver.c: new_drvdata->coherent = of_dma_is_coherent(np); drivers/iommu/arm-smmu-v3.c: if (of_dma_is_coherent(dev->of_node)) drivers/iommu/arm-smmu.c: if (of_dma_is_coherent(dev->of_node)) drivers/mmc/host/sdhci-of-esdhc.c: if (of_dma_is_coherent(dev->of_node)) Curious that a PPC specific driver (ahci_qoriq) calls it... Note that the value is also passed to arch_setup_dma_ops(), but only arc, arm, arm64, and mips implement arch_setup_dma_ops. > I think this should have been done the other way around and default to > coherent since most traditional OF platforms are coherent, and you > can't just require those DTs to change. You can blame me. This was really only intended for cases where coherency is configurable on a per peripheral basis and can't be determined in other ways. The simple solution here is simply to use the compatible string for the device to determine coherent or not. Rob ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates 2019-10-23 13:52 ` Rob Herring @ 2019-10-23 14:12 ` Russell King - ARM Linux admin 2019-10-23 14:31 ` Russell King - ARM Linux admin 1 sibling, 0 replies; 12+ messages in thread From: Russell King - ARM Linux admin @ 2019-10-23 14:12 UTC (permalink / raw) To: Rob Herring Cc: Ulf Hansson, mad skateman, linux-mmc, Paul Mackerras, R.T.Dickinson, Christian Zigotzky, contact, linuxppc-dev, Christian Zigotzky On Wed, Oct 23, 2019 at 08:52:33AM -0500, Rob Herring wrote: > On Wed, Oct 23, 2019 at 1:41 AM Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > > > On Wed, 2019-10-23 at 16:42 +1100, Michael Ellerman wrote: > > > > > > Right, it seems of_dma_is_coherent() has baked in the assumption that > > > devices are non-coherent unless explicitly marked as coherent. > > > > > > Which is wrong on all or at least most existing powerpc systems > > > according to Ben. > > > > This is probably broken on sparc(64) as well and whatever else uses > > DT and is an intrinsicly coherent architecture (did we ever have > > DT enabled x86s ? Wasn't OLPC such a beast ?). > > Only if those platforms use one of the 5 drivers that call this function: > > drivers/ata/ahci_qoriq.c: qoriq_priv->is_dmacoherent = > of_dma_is_coherent(np); > drivers/crypto/ccree/cc_driver.c: new_drvdata->coherent = > of_dma_is_coherent(np); > drivers/iommu/arm-smmu-v3.c: if (of_dma_is_coherent(dev->of_node)) > drivers/iommu/arm-smmu.c: if (of_dma_is_coherent(dev->of_node)) > drivers/mmc/host/sdhci-of-esdhc.c: if (of_dma_is_coherent(dev->of_node)) > > Curious that a PPC specific driver (ahci_qoriq) calls it... Maybe because it is not PPC specific: arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi: compatible = "fsl,lx2160a-ahci"; arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi: compatible = "fsl,lx2160a-ahci"; arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi: compatible = "fsl,lx2160a-ahci"; arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi: compatible = "fsl,lx2160a-ahci"; drivers/ata/ahci_qoriq.c: { .compatible = "fsl,lx2160a-ahci", .data = (void *)AHCI_LX2160A}, -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates 2019-10-23 13:52 ` Rob Herring 2019-10-23 14:12 ` Russell King - ARM Linux admin @ 2019-10-23 14:31 ` Russell King - ARM Linux admin 2019-10-25 22:28 ` Rob Herring 1 sibling, 1 reply; 12+ messages in thread From: Russell King - ARM Linux admin @ 2019-10-23 14:31 UTC (permalink / raw) To: Rob Herring Cc: Ulf Hansson, mad skateman, linux-mmc, Paul Mackerras, R.T.Dickinson, Christian Zigotzky, contact, linuxppc-dev, Christian Zigotzky On Wed, Oct 23, 2019 at 08:52:33AM -0500, Rob Herring wrote: > > I think this should have been done the other way around and default to > > coherent since most traditional OF platforms are coherent, and you > > can't just require those DTs to change. > > You can blame me. This was really only intended for cases where > coherency is configurable on a per peripheral basis and can't be > determined in other ways. > > The simple solution here is simply to use the compatible string for > the device to determine coherent or not. It really isn't that simple. There are two aspects to coherency, both of which must match: 1) The configuration of the device 2) The configuration of the kernel's DMA API (1) is controlled by the driver, which can make the decision any way it pleases. (2) on ARM64 is controlled depending on whether or not "dma-coherent" is specified in the device tree, since ARM64 can have a mixture of DMA coherent and non-coherent devices. A mismatch between (1) and (2) results in data corruption, potentially eating your filesystem. So, it's very important that the two match. These didn't match for the LX2160A, but, due to the way CMA was working, we sort of got away with it, but it was very dangerous as far as data safety went. Then, a change to CMA happened which moved where it was located, which caused a regression. Reverting the CMA changes didn't seem to be an option, so another solution had to be found. I started a discussion on how best to solve this: https://archive.armlinux.org.uk/lurker/thread/20190919.041320.1e53541f.en.html and the solution that the discussion came out with was the one that has been merged - which we now know caused a regression on PPC. Using compatible strings doesn't solve the issue: there is no way to tell the DMA API from the driver that the device is coherent. The only way to do that is via the "dma-coherent" property in DT on ARM64. To say that this is a mess is an under-statement, but we seem to have ended up here because of a series of piece-meal changes that don't seem to have been thought through enough. So, what's the right way to solve this, and ensure that the DMA API and device match as far as their coherency expectations go? Revert all the changes for sdhci-of-esdhc and CMA back to 5.0 or 5.1 state? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates 2019-10-23 14:31 ` Russell King - ARM Linux admin @ 2019-10-25 22:28 ` Rob Herring 2019-10-26 6:39 ` Christoph Hellwig 2019-10-28 8:45 ` Benjamin Herrenschmidt 0 siblings, 2 replies; 12+ messages in thread From: Rob Herring @ 2019-10-25 22:28 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Ulf Hansson, mad skateman, linux-mmc, Paul Mackerras, R.T.Dickinson, Christian Zigotzky, contact, linuxppc-dev, Christian Zigotzky On Wed, Oct 23, 2019 at 9:32 AM Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > > On Wed, Oct 23, 2019 at 08:52:33AM -0500, Rob Herring wrote: > > > I think this should have been done the other way around and default to > > > coherent since most traditional OF platforms are coherent, and you > > > can't just require those DTs to change. > > > > You can blame me. This was really only intended for cases where > > coherency is configurable on a per peripheral basis and can't be > > determined in other ways. > > > > The simple solution here is simply to use the compatible string for > > the device to determine coherent or not. > > It really isn't that simple. This doesn't work?: if (IS_ENABLED(CONFIG_PPC) || of_dma_is_coherent(dev->of_node)) value |= ESDHC_DMA_SNOOP; else value &= ~ESDHC_DMA_SNOOP; While I said use the compatibles, using the kconfig symbol is easier than sorting out which compatibles are PPC SoCs. Though if that's already done elsewhere in the driver, you could set a flag and use that here. I'd be surprised if this was the only difference between ARM and PPC SoCs for this block. > There are two aspects to coherency, both of which must match: > > 1) The configuration of the device > 2) The configuration of the kernel's DMA API > > (1) is controlled by the driver, which can make the decision any way > it pleases. > > (2) on ARM64 is controlled depending on whether or not "dma-coherent" > is specified in the device tree, since ARM64 can have a mixture of > DMA coherent and non-coherent devices. > > A mismatch between (1) and (2) results in data corruption, potentially > eating your filesystem. So, it's very important that the two match. > > These didn't match for the LX2160A, but, due to the way CMA was working, > we sort of got away with it, but it was very dangerous as far as data > safety went. > > Then, a change to CMA happened which moved where it was located, which > caused a regression. Reverting the CMA changes didn't seem to be an > option, so another solution had to be found. > > I started a discussion on how best to solve this: > > https://archive.armlinux.org.uk/lurker/thread/20190919.041320.1e53541f.en.html > > and the solution that the discussion came out with was the one that has > been merged - which we now know caused a regression on PPC. > > Using compatible strings doesn't solve the issue: there is no way to > tell the DMA API from the driver that the device is coherent. The > only way to do that is via the "dma-coherent" property in DT on ARM64. > > To say that this is a mess is an under-statement, but we seem to have > ended up here because of a series of piece-meal changes that don't seem > to have been thought through enough. > > So, what's the right way to solve this, and ensure that the DMA API and > device match as far as their coherency expectations go? Revert all the > changes for sdhci-of-esdhc and CMA back to 5.0 or 5.1 state? The other option is similar to earlier in the thread and just add to of_dma_is_coherent(): /* Powerpc is normally cache coherent DMA */ if (IS_ENABLED(CONFIG_PPC) && !IS_ENABLED(CONFIG_NOT_COHERENT_CACHE)) return true; We could do the all the weak arch hooks, but that seems like overkill to me at this point. Rob ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates 2019-10-25 22:28 ` Rob Herring @ 2019-10-26 6:39 ` Christoph Hellwig 2019-10-28 8:47 ` Benjamin Herrenschmidt 2019-10-28 8:45 ` Benjamin Herrenschmidt 1 sibling, 1 reply; 12+ messages in thread From: Christoph Hellwig @ 2019-10-26 6:39 UTC (permalink / raw) To: Rob Herring Cc: Ulf Hansson, mad skateman, linux-mmc, Russell King - ARM Linux admin, Paul Mackerras, R.T.Dickinson, Christian Zigotzky, contact, linuxppc-dev, Christian Zigotzky On Fri, Oct 25, 2019 at 05:28:45PM -0500, Rob Herring wrote: > This doesn't work?: > > if (IS_ENABLED(CONFIG_PPC) || of_dma_is_coherent(dev->of_node)) > value |= ESDHC_DMA_SNOOP; > else > value &= ~ESDHC_DMA_SNOOP; > > While I said use the compatibles, using the kconfig symbol is easier > than sorting out which compatibles are PPC SoCs. Though if that's > already done elsewhere in the driver, you could set a flag and use > that here. I'd be surprised if this was the only difference between > ARM and PPC SoCs for this block. I think the right thing is a Kconfig variable that the architectures selects which says if OF is by default coherent or incoherent, and then use that in of_dma_is_coherent. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates 2019-10-26 6:39 ` Christoph Hellwig @ 2019-10-28 8:47 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 12+ messages in thread From: Benjamin Herrenschmidt @ 2019-10-28 8:47 UTC (permalink / raw) To: Christoph Hellwig, Rob Herring Cc: Ulf Hansson, mad skateman, linux-mmc, Russell King - ARM Linux admin, Paul Mackerras, R.T.Dickinson, Christian Zigotzky, contact, linuxppc-dev, Christian Zigotzky On Fri, 2019-10-25 at 23:39 -0700, Christoph Hellwig wrote: > On Fri, Oct 25, 2019 at 05:28:45PM -0500, Rob Herring wrote: > > This doesn't work?: > > > > if (IS_ENABLED(CONFIG_PPC) || of_dma_is_coherent(dev- > > >of_node)) > > value |= ESDHC_DMA_SNOOP; > > else > > value &= ~ESDHC_DMA_SNOOP; > > > > While I said use the compatibles, using the kconfig symbol is > > easier > > than sorting out which compatibles are PPC SoCs. Though if that's > > already done elsewhere in the driver, you could set a flag and use > > that here. I'd be surprised if this was the only difference between > > ARM and PPC SoCs for this block. > > I think the right thing is a Kconfig variable that the architectures > selects which says if OF is by default coherent or incoherent, and > then use that in of_dma_is_coherent. That too. We could also define properties for both ways so we can override the default either way. Cheers, Ben. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates 2019-10-25 22:28 ` Rob Herring 2019-10-26 6:39 ` Christoph Hellwig @ 2019-10-28 8:45 ` Benjamin Herrenschmidt 1 sibling, 0 replies; 12+ messages in thread From: Benjamin Herrenschmidt @ 2019-10-28 8:45 UTC (permalink / raw) To: Rob Herring, Russell King - ARM Linux admin Cc: Ulf Hansson, mad skateman, linux-mmc, Paul Mackerras, R.T.Dickinson, Christian Zigotzky, contact, linuxppc-dev, Christian Zigotzky On Fri, 2019-10-25 at 17:28 -0500, Rob Herring wrote: > This doesn't work?: > > if (IS_ENABLED(CONFIG_PPC) || of_dma_is_coherent(dev- > >of_node)) > value |= ESDHC_DMA_SNOOP; > else > value &= ~ESDHC_DMA_SNOOP; CONFIG_PPC is restrictive. What about sparc64 ? There could be others .. .we can't suddenly requests people to add new properties for what was implied behaviours before hand, esp since it's not in the base 1275 spec, no ? I would suggest of_dma_is_coherent is *true* by default unless overriden to do something else. Cheers, Ben. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates 2019-10-23 5:42 ` Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates Michael Ellerman 2019-10-23 6:41 ` Benjamin Herrenschmidt @ 2019-10-23 14:20 ` Christian Zigotzky 2019-11-04 14:44 ` Christian Zigotzky 1 sibling, 1 reply; 12+ messages in thread From: Christian Zigotzky @ 2019-10-23 14:20 UTC (permalink / raw) To: Michael Ellerman Cc: ulf.hansson, mad skateman, linux-mmc, Russell King - ARM Linux admin, Rob Herring, Paul Mackerras, R.T.Dickinson, contact, linuxppc-dev, Christian Zigotzky Hello, The patch below works. I compiled the RC4 of kernel 5.4 with this patch today and the onboard SD card works without any problems. Thanks! Christian > On 23. Oct 2019, at 07:42, Michael Ellerman <mpe@ellerman.id.au> wrote: > > Russell King - ARM Linux admin <linux@armlinux.org.uk> writes: >>> On Tue, Oct 15, 2019 at 03:12:49PM +0200, Christian Zigotzky wrote: >>> Hello Russell, >>> >>> You asked me about "dma-coherent" in the Cyrus device tree. Unfortunately I >>> don't find the property "dma-coherent" in the dtb source files. >>> >>> Output of "fdtdump cyrus_p5020_eth_poweroff.dtb | grep dma": >>> >>> dma0 = "/soc@ffe000000/dma@100300"; >>> dma1 = "/soc@ffe000000/dma@101300"; >>> dma@100300 { >>> compatible = "fsl,eloplus-dma"; >>> dma-channel@0 { >>> compatible = "fsl,eloplus-dma-channel"; >>> dma-channel@80 { >>> compatible = "fsl,eloplus-dma-channel"; >>> dma-channel@100 { >>> compatible = "fsl,eloplus-dma-channel"; >>> dma-channel@180 { >>> compatible = "fsl,eloplus-dma-channel"; >>> dma@101300 { >>> compatible = "fsl,eloplus-dma"; >>> dma-channel@0 { >>> compatible = "fsl,eloplus-dma-channel"; >>> dma-channel@80 { >>> compatible = "fsl,eloplus-dma-channel"; >>> dma-channel@100 { >>> compatible = "fsl,eloplus-dma-channel"; >>> dma-channel@180 { >>> compatible = "fsl,eloplus-dma-channel"; >> >> Hmm, so it looks like PowerPC doesn't mark devices that are dma >> coherent with a property that describes them as such. >> >> I think this opens a wider question - what should of_dma_is_coherent() >> return for PowerPC? It seems right now that it returns false for >> devices that are DMA coherent, which seems to me to be a recipe for >> future mistakes. > > Right, it seems of_dma_is_coherent() has baked in the assumption that > devices are non-coherent unless explicitly marked as coherent. > > Which is wrong on all or at least most existing powerpc systems > according to Ben. > >> Any ideas from the PPC maintainers? > > Fixing it at the source seems like the best option to prevent future > breakage. > > So I guess that would mean making of_dma_is_coherent() return true/false > based on CONFIG_NOT_COHERENT_CACHE on powerpc. > > We could do it like below, which would still allow the dma-coherent > property to work if it ever makes sense on a future powerpc platform. > > I don't really know any of this embedded stuff well, so happy to take > other suggestions on how to handle this mess. > > cheers > > > diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c > index 25aaa3903000..b96c9010acb6 100644 > --- a/arch/powerpc/kernel/setup-common.c > +++ b/arch/powerpc/kernel/setup-common.c > @@ -760,6 +760,22 @@ static int __init check_cache_coherency(void) > late_initcall(check_cache_coherency); > #endif /* CONFIG_CHECK_CACHE_COHERENCY */ > > +#ifndef CONFIG_NOT_COHERENT_CACHE > +/* > + * For historical reasons powerpc kernels are built with hard wired knowledge of > + * whether or not DMA accesses are cache coherent. Additionally device trees on > + * powerpc do not typically support the dma-coherent property. > + * > + * So when we know that DMA is coherent, override arch_of_dma_is_coherent() to > + * tell the drivers/of code that all devices are coherent regardless of whether > + * they have a dma-coherent property. > + */ > +bool arch_of_dma_is_coherent(struct device_node *np) > +{ > + return true; > +} > +#endif > + > #ifdef CONFIG_DEBUG_FS > struct dentry *powerpc_debugfs_root; > EXPORT_SYMBOL(powerpc_debugfs_root); > diff --git a/drivers/of/address.c b/drivers/of/address.c > index 978427a9d5e6..3a4b2949a322 100644 > --- a/drivers/of/address.c > +++ b/drivers/of/address.c > @@ -993,6 +993,14 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz > } > EXPORT_SYMBOL_GPL(of_dma_get_range); > > +/* > + * arch_of_dma_is_coherent - Arch hook to determine if device is coherent for DMA > + */ > +bool __weak arch_of_dma_is_coherent(struct device_node *np) > +{ > + return false; > +} > + > /** > * of_dma_is_coherent - Check if device is coherent > * @np: device node > @@ -1002,8 +1010,12 @@ EXPORT_SYMBOL_GPL(of_dma_get_range); > */ > bool of_dma_is_coherent(struct device_node *np) > { > - struct device_node *node = of_node_get(np); > + struct device_node *node; > + > + if (arch_of_dma_is_coherent(np)) > + return true; > > + np = of_node_get(np); > while (node) { > if (of_property_read_bool(node, "dma-coherent")) { > of_node_put(node); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates 2019-10-23 14:20 ` Christian Zigotzky @ 2019-11-04 14:44 ` Christian Zigotzky 2019-11-05 7:50 ` Bug 205201 - overflow of DMA mask and bus mask Christian Zigotzky 0 siblings, 1 reply; 12+ messages in thread From: Christian Zigotzky @ 2019-11-04 14:44 UTC (permalink / raw) To: Michael Ellerman Cc: ulf.hansson, mad skateman, linux-mmc, Russell King - ARM Linux admin, Rob Herring, Paul Mackerras, R.T.Dickinson, contact, linuxppc-dev, Christian Zigotzky FYI: The onboard SD card works also with the RC6 of kernel 5.4 with the patch below. -- Christian On 23 October 2019 at 4:20pm, Christian Zigotzky wrote: > Hello, > > The patch below works. I compiled the RC4 of kernel 5.4 with this patch today and the onboard SD card works without any problems. > > Thanks! > > Christian > >> On 23. Oct 2019, at 07:42, Michael Ellerman <mpe@ellerman.id.au> wrote: >> >> Russell King - ARM Linux admin <linux@armlinux.org.uk> writes: >>>> On Tue, Oct 15, 2019 at 03:12:49PM +0200, Christian Zigotzky wrote: >>>> Hello Russell, >>>> >>>> You asked me about "dma-coherent" in the Cyrus device tree. Unfortunately I >>>> don't find the property "dma-coherent" in the dtb source files. >>>> >>>> Output of "fdtdump cyrus_p5020_eth_poweroff.dtb | grep dma": >>>> >>>> dma0 = "/soc@ffe000000/dma@100300"; >>>> dma1 = "/soc@ffe000000/dma@101300"; >>>> dma@100300 { >>>> compatible = "fsl,eloplus-dma"; >>>> dma-channel@0 { >>>> compatible = "fsl,eloplus-dma-channel"; >>>> dma-channel@80 { >>>> compatible = "fsl,eloplus-dma-channel"; >>>> dma-channel@100 { >>>> compatible = "fsl,eloplus-dma-channel"; >>>> dma-channel@180 { >>>> compatible = "fsl,eloplus-dma-channel"; >>>> dma@101300 { >>>> compatible = "fsl,eloplus-dma"; >>>> dma-channel@0 { >>>> compatible = "fsl,eloplus-dma-channel"; >>>> dma-channel@80 { >>>> compatible = "fsl,eloplus-dma-channel"; >>>> dma-channel@100 { >>>> compatible = "fsl,eloplus-dma-channel"; >>>> dma-channel@180 { >>>> compatible = "fsl,eloplus-dma-channel"; >>> Hmm, so it looks like PowerPC doesn't mark devices that are dma >>> coherent with a property that describes them as such. >>> >>> I think this opens a wider question - what should of_dma_is_coherent() >>> return for PowerPC? It seems right now that it returns false for >>> devices that are DMA coherent, which seems to me to be a recipe for >>> future mistakes. >> Right, it seems of_dma_is_coherent() has baked in the assumption that >> devices are non-coherent unless explicitly marked as coherent. >> >> Which is wrong on all or at least most existing powerpc systems >> according to Ben. >> >>> Any ideas from the PPC maintainers? >> Fixing it at the source seems like the best option to prevent future >> breakage. >> >> So I guess that would mean making of_dma_is_coherent() return true/false >> based on CONFIG_NOT_COHERENT_CACHE on powerpc. >> >> We could do it like below, which would still allow the dma-coherent >> property to work if it ever makes sense on a future powerpc platform. >> >> I don't really know any of this embedded stuff well, so happy to take >> other suggestions on how to handle this mess. >> >> cheers >> >> >> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c >> index 25aaa3903000..b96c9010acb6 100644 >> --- a/arch/powerpc/kernel/setup-common.c >> +++ b/arch/powerpc/kernel/setup-common.c >> @@ -760,6 +760,22 @@ static int __init check_cache_coherency(void) >> late_initcall(check_cache_coherency); >> #endif /* CONFIG_CHECK_CACHE_COHERENCY */ >> >> +#ifndef CONFIG_NOT_COHERENT_CACHE >> +/* >> + * For historical reasons powerpc kernels are built with hard wired knowledge of >> + * whether or not DMA accesses are cache coherent. Additionally device trees on >> + * powerpc do not typically support the dma-coherent property. >> + * >> + * So when we know that DMA is coherent, override arch_of_dma_is_coherent() to >> + * tell the drivers/of code that all devices are coherent regardless of whether >> + * they have a dma-coherent property. >> + */ >> +bool arch_of_dma_is_coherent(struct device_node *np) >> +{ >> + return true; >> +} >> +#endif >> + >> #ifdef CONFIG_DEBUG_FS >> struct dentry *powerpc_debugfs_root; >> EXPORT_SYMBOL(powerpc_debugfs_root); >> diff --git a/drivers/of/address.c b/drivers/of/address.c >> index 978427a9d5e6..3a4b2949a322 100644 >> --- a/drivers/of/address.c >> +++ b/drivers/of/address.c >> @@ -993,6 +993,14 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz >> } >> EXPORT_SYMBOL_GPL(of_dma_get_range); >> >> +/* >> + * arch_of_dma_is_coherent - Arch hook to determine if device is coherent for DMA >> + */ >> +bool __weak arch_of_dma_is_coherent(struct device_node *np) >> +{ >> + return false; >> +} >> + >> /** >> * of_dma_is_coherent - Check if device is coherent >> * @np: device node >> @@ -1002,8 +1010,12 @@ EXPORT_SYMBOL_GPL(of_dma_get_range); >> */ >> bool of_dma_is_coherent(struct device_node *np) >> { >> - struct device_node *node = of_node_get(np); >> + struct device_node *node; >> + >> + if (arch_of_dma_is_coherent(np)) >> + return true; >> >> + np = of_node_get(np); >> while (node) { >> if (of_property_read_bool(node, "dma-coherent")) { >> of_node_put(node); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Bug 205201 - overflow of DMA mask and bus mask 2019-11-04 14:44 ` Christian Zigotzky @ 2019-11-05 7:50 ` Christian Zigotzky 0 siblings, 0 replies; 12+ messages in thread From: Christian Zigotzky @ 2019-11-05 7:50 UTC (permalink / raw) To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Darren Stevens, contact, R.T.Dickinson, mad skateman, Rob Herring, linuxppc-dev Hi All, We still have DMA problems with some PCI devices. Since the PowerPC updates 4.21-1 [1] we need to decrease the RAM to 3500MB (mem=3500M) if we want to work with our PCI devices. The FSL P5020 and P5040 have these problems currently. Error message: [ 25.654852] bttv 1000:04:05.0: overflow 0x00000000fe077000+4096 of DMA mask ffffffff bus mask df000000 All 5.x Linux kernels can't initialize a SCSI PCI card anymore so booting of a Linux userland isn't possible. PLEASE check the DMA changes in the PowerPC updates 4.21-1 [1]. The kernel 4.20 works with all PCI devices without limitation of RAM. We created a bug report a month ago. [2] Thanks, Christian [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8d6973327ee84c2f40dd9efd8928d4a1186c96e2 [2] https://bugzilla.kernel.org/show_bug.cgi?id=205201 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-11-05 7:53 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <7b549219-a2e1-08c7-331b-9c3e4fdb8a8f@xenosoft.de> [not found] ` <3aeae0d8-e9be-2585-cbbd-70263cb495f1@xenosoft.de> [not found] ` <20191015125105.GU25745@shell.armlinux.org.uk> [not found] ` <5611f3bc-68aa-78ec-182a-1cb414202314@xenosoft.de> [not found] ` <20191015131750.GV25745@shell.armlinux.org.uk> 2019-10-23 5:42 ` Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates Michael Ellerman 2019-10-23 6:41 ` Benjamin Herrenschmidt 2019-10-23 13:52 ` Rob Herring 2019-10-23 14:12 ` Russell King - ARM Linux admin 2019-10-23 14:31 ` Russell King - ARM Linux admin 2019-10-25 22:28 ` Rob Herring 2019-10-26 6:39 ` Christoph Hellwig 2019-10-28 8:47 ` Benjamin Herrenschmidt 2019-10-28 8:45 ` Benjamin Herrenschmidt 2019-10-23 14:20 ` Christian Zigotzky 2019-11-04 14:44 ` Christian Zigotzky 2019-11-05 7:50 ` Bug 205201 - overflow of DMA mask and bus mask Christian Zigotzky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).