Hi Adrian, On Mon, 12 Dec 2022 10:42:36 +0200, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > On 9/12/22 15:27, Vadym Kochan wrote: > > On Fri, 9 Dec 2022 14:13:06 +0200, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > >> On 9/12/22 14:10, Vadym Kochan wrote: > >>> Hi Adrian, > >>> > >>> On Fri, 9 Dec 2022 13:53:58 +0200, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > >>>> On 9/12/22 13:39, Vadym Kochan wrote: > >>>>> Hi Adrian, > >>>>> > >>>>> On Fri, 9 Dec 2022 09:23:05 +0200, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > >>>>>> On 5/12/22 12:59, Vadym Kochan wrote: > >>>>>>> There is a limitation on AC5 SoC that mmc controller > >>>>>>> can't have DMA access over 2G memory, so use SDMA with > >>>>>>> a bounce buffer. Swiotlb can't help because on arm64 arch > >>>>>>> it reserves memblock's at the end of the memory. > >>>>>>> > >>>>>>> Additionally set mask to 34 bit since on AC5 SoC RAM starts > >>>>>>> at 0x2_00000000. > >>>>>> > >>>>>> Can you explain more about how a 34-bit DMA mask works when > >>>>>> SDMA only supports 32-bit addresses? > >>>>>> > >>>>> > >>>>> So, after I set > >>>>> > >>>>>>> + host->flags &= ~SDHCI_USE_64_BIT_DMA; > >>>>> > >>>>> then sdhc core sets mask to 32 bit, but then dma_map fails to map > >>>>> bounce buffer because the base address is higher than 32bit - 0x2_00000000, > >>>>> and 34bit mask fixed it. > >>>> > >>>> What happens if the bounce buffer gets mapped in the range > >>>> 0x1_00000000 to 0x1_ffffffff ? > >>>> > >>> > >>> From my understanding, on the AC5 SoC RAM starts at 0x2_00000000 so the bounce > >>> buffer can be mapped in the range 0x2_00000000..0x2_ffffffff > >> > >> Right but I guess I meant what about 0x3_00000000..0x3_ffffffff ? > >> Isn't that also in DMA_BIT_MASK(34) > > > > Yes, you are right. > > So it would fail in that case? Is it possible to use devicetree > reserved memory or some such, to set aside 64k for the bounce > buffer DMA mapping? > The main restriction is that only lower 2GB can be used for DMA. I already did send solution based on reserved memory, I can send it again in context of this series. Also what about the solution which Linus suggested ? [cut] Let's just create a new quirk: SDHCI_QUIRK_31BIT_DMA_ROOF Define the semantics such that this will allow DMA for buffers that are below the 31st bit, but does not have the semantics to limit scatter-gather buffers to be 32-bit aligned. [/cut] Thanks, > > > >> > >>> > >>>>> > >>>>>>> > >>>>>>> Co-developed-by: Elad Nachman <enachman@xxxxxxxxxxx> > >>>>>>> Signed-off-by: Elad Nachman <enachman@xxxxxxxxxxx> > >>>>>>> Signed-off-by: Vadym Kochan <vadym.kochan@xxxxxxxxxxx> > >>>>>>> --- > >>>>>>> drivers/mmc/host/sdhci-xenon.c | 38 ++++++++++++++++++++++++++++++++++ > >>>>>>> drivers/mmc/host/sdhci-xenon.h | 3 ++- > >>>>>>> 2 files changed, 40 insertions(+), 1 deletion(-) > >>>>>>> > >>>>>>> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c > >>>>>>> index 08e838400b52..5f3db0425674 100644 > >>>>>>> --- a/drivers/mmc/host/sdhci-xenon.c > >>>>>>> +++ b/drivers/mmc/host/sdhci-xenon.c > >>>>>>> @@ -13,7 +13,9 @@ > >>>>>>> > >>>>>>> #include <linux/acpi.h> > >>>>>>> #include <linux/delay.h> > >>>>>>> +#include <linux/dma-mapping.h> > >>>>>>> #include <linux/ktime.h> > >>>>>>> +#include <linux/mm.h> > >>>>>>> #include <linux/module.h> > >>>>>>> #include <linux/of.h> > >>>>>>> #include <linux/pm.h> > >>>>>>> @@ -253,6 +255,22 @@ static unsigned int xenon_get_max_clock(struct sdhci_host *host) > >>>>>>> return pltfm_host->clock; > >>>>>>> } > >>>>>>> > >>>>>>> +static int xenon_set_dma_mask(struct sdhci_host *host) > >>>>>>> +{ > >>>>>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > >>>>>>> + struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host); > >>>>>>> + struct mmc_host *mmc = host->mmc; > >>>>>>> + struct device *dev = mmc_dev(mmc); > >>>>>>> + > >>>>>>> + if (priv->hw_version == XENON_AC5) { > >>>>>>> + host->flags &= ~SDHCI_USE_64_BIT_DMA; > >>>>>>> + > >>>>>>> + return dma_set_mask_and_coherent(dev, DMA_BIT_MASK(34)); > >>>>>>> + } > >>>>>>> + > >>>>>>> + return sdhci_set_dma_mask(host); > >>>>>>> +} > >>>>>>> + > >>>>>>> static const struct sdhci_ops sdhci_xenon_ops = { > >>>>>>> .voltage_switch = xenon_voltage_switch, > >>>>>>> .set_clock = sdhci_set_clock, > >>>>>>> @@ -261,6 +279,7 @@ static const struct sdhci_ops sdhci_xenon_ops = { > >>>>>>> .reset = xenon_reset, > >>>>>>> .set_uhs_signaling = xenon_set_uhs_signaling, > >>>>>>> .get_max_clock = xenon_get_max_clock, > >>>>>>> + .set_dma_mask = xenon_set_dma_mask, > >>>>>>> }; > >>>>>>> > >>>>>>> static const struct sdhci_pltfm_data sdhci_xenon_pdata = { > >>>>>>> @@ -486,6 +505,18 @@ static void xenon_sdhc_unprepare(struct sdhci_host *host) > >>>>>>> xenon_disable_sdhc(host, sdhc_id); > >>>>>>> } > >>>>>>> > >>>>>>> +static int xenon_ac5_probe(struct sdhci_host *host) > >>>>>>> +{ > >>>>>>> + struct sysinfo si; > >>>>>>> + > >>>>>>> + si_meminfo(&si); > >>>>>>> + > >>>>>>> + if ((si.totalram * si.mem_unit) > SZ_2G) > >>>>>>> + host->quirks |= SDHCI_QUIRK_BROKEN_ADMA; > >>>>>>> + > >>>>>>> + return 0; > >>>>>>> +} > >>>>>>> + > >>>>>>> static int xenon_probe(struct platform_device *pdev) > >>>>>>> { > >>>>>>> struct sdhci_pltfm_host *pltfm_host; > >>>>>>> @@ -533,6 +564,12 @@ static int xenon_probe(struct platform_device *pdev) > >>>>>>> } > >>>>>>> } > >>>>>>> > >>>>>>> + if (priv->hw_version == XENON_AC5) { > >>>>>>> + err = xenon_ac5_probe(host); > >>>>>>> + if (err) > >>>>>>> + goto err_clk_axi; > >>>>>>> + } > >>>>>>> + > >>>>>>> err = mmc_of_parse(host->mmc); > >>>>>>> if (err) > >>>>>>> goto err_clk_axi; > >>>>>>> @@ -682,6 +719,7 @@ static const struct of_device_id sdhci_xenon_dt_ids[] = { > >>>>>>> { .compatible = "marvell,armada-ap807-sdhci", .data = (void *)XENON_AP807}, > >>>>>>> { .compatible = "marvell,armada-cp110-sdhci", .data = (void *)XENON_CP110}, > >>>>>>> { .compatible = "marvell,armada-3700-sdhci", .data = (void *)XENON_A3700}, > >>>>>>> + { .compatible = "marvell,ac5-sdhci", .data = (void *)XENON_AC5}, > >>>>>>> {} > >>>>>>> }; > >>>>>>> MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids); > >>>>>>> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h > >>>>>>> index 3e9c6c908a79..0460d97aad26 100644 > >>>>>>> --- a/drivers/mmc/host/sdhci-xenon.h > >>>>>>> +++ b/drivers/mmc/host/sdhci-xenon.h > >>>>>>> @@ -57,7 +57,8 @@ enum xenon_variant { > >>>>>>> XENON_A3700, > >>>>>>> XENON_AP806, > >>>>>>> XENON_AP807, > >>>>>>> - XENON_CP110 > >>>>>>> + XENON_CP110, > >>>>>>> + XENON_AC5 > >>>>>>> }; > >>>>>>> > >>>>>>> struct xenon_priv { > >>>>>> > >>>>> > >>>>> Regards, > >>>> > >> >