2023-12-27 12:34:30

by Elad Nachman

[permalink] [raw]
Subject: [PATCH 0/4] mmc: xenon: add AC5 support

From: Elad Nachman <[email protected]>

This patch series adds support for the Marvell AC5/X/IM series of SOCs.
The main hurdles in supporting these SOCs are the following limitations:
1. DDR starts at offset 0x2_0000_0000
2. mmc controller has only 31-bit path on the crossbar to the DDR.

Point number one is solved by the first patch, which targets the
arm64 subsystem, by taking into account the DDR start address when
calculating the DMA and DMA32 zones.

This yields the correct split between DMA, DMA32 and NORMAL zones
according to the device tree CPU address limitations.

Point number two is solved in the mmc xenon driver by detecting the memory
size, and when it is more than 2GB, disable ADMA and 64-bit DMA, which
effectively enables SDMA with a bounce buffer.
DMA mask is then set manually to 34 bit to account for the DDR starting
at offset 0x2_0000_0000 .

Elad Nachman (4):
arm64: mm: Fix SOCs with DDR starting above zero
dt-bindings: mmc: add Marvell ac5
arm64: dts: ac5: add mmc node and clock
mmc: xenon: Add ac5 support via bounce buffer

.../bindings/mmc/marvell,xenon-sdhci.yaml | 3 ++
arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 33 ++++++++++++++++++-
arch/arm64/mm/init.c | 20 ++++++++---
drivers/mmc/host/sdhci-xenon.c | 33 ++++++++++++++++++-
drivers/mmc/host/sdhci-xenon.h | 3 +-
5 files changed, 84 insertions(+), 8 deletions(-)

--
2.25.1



2023-12-27 12:34:31

by Elad Nachman

[permalink] [raw]
Subject: [PATCH 1/4] arm64: mm: Fix SOCs with DDR starting above zero

From: Elad Nachman <[email protected]>

Some SOCs, like the Marvell AC5/X/IM, have a combination
of DDR starting at 0x2_0000_0000 coupled with DMA controllers
limited to 31 and 32 bit of addressing.
This requires to properly arrange ZONE_DMA and ZONE_DMA32 for
these SOCs, so swiotlb and coherent DMA allocation would work
properly.
Change initialization so device tree dma zone bits are taken as
function of offset from DRAM start, and when calculating the
maximal zone physical RAM address for physical DDR starting above
32-bit, combine the physical address start plus the zone mask
passed as parameter.
This creates the proper zone splitting for these SOCs:
0..2GB for ZONE_DMA
2GB..4GB for ZONE_DMA32
4GB..8GB for ZONE_NORMAL

Signed-off-by: Elad Nachman <[email protected]>
---
arch/arm64/mm/init.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 74c1db8ce271..8288c778916e 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -115,20 +115,21 @@ static void __init arch_reserve_crashkernel(void)

/*
* Return the maximum physical address for a zone accessible by the given bits
- * limit. If DRAM starts above 32-bit, expand the zone to the maximum
- * available memory, otherwise cap it at 32-bit.
+ * limit. If DRAM starts above 32-bit, expand the zone to the available memory
+ * start limited by the zone bits mask, otherwise cap it at 32-bit.
*/
static phys_addr_t __init max_zone_phys(unsigned int zone_bits)
{
phys_addr_t zone_mask = DMA_BIT_MASK(zone_bits);
phys_addr_t phys_start = memblock_start_of_DRAM();
+ phys_addr_t phys_end = memblock_end_of_DRAM();

if (phys_start > U32_MAX)
- zone_mask = PHYS_ADDR_MAX;
+ zone_mask = phys_start | zone_mask;
else if (phys_start > zone_mask)
zone_mask = U32_MAX;

- return min(zone_mask, memblock_end_of_DRAM() - 1) + 1;
+ return min(zone_mask, phys_end - 1) + 1;
}

static void __init zone_sizes_init(void)
@@ -140,7 +141,16 @@ static void __init zone_sizes_init(void)

#ifdef CONFIG_ZONE_DMA
acpi_zone_dma_bits = fls64(acpi_iort_dma_get_max_cpu_address());
- dt_zone_dma_bits = fls64(of_dma_get_max_cpu_address(NULL));
+ /*
+ * When calculating the dma zone bits from the device tree, subtract
+ * the DRAM start address, in case it does not start from address
+ * zero. This way. we pass only the zone size related bits to
+ * max_zone_phys(), which will add them to the base of the DRAM.
+ * This prevents miscalculations on arm64 SOCs which combines
+ * DDR starting above 4GB with memory controllers limited to
+ * 32-bits or less:
+ */
+ dt_zone_dma_bits = fls64(of_dma_get_max_cpu_address(NULL) - memblock_start_of_DRAM());
zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits);
arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
--
2.25.1


2023-12-27 12:34:32

by Elad Nachman

[permalink] [raw]
Subject: [PATCH 4/4] mmc: xenon: Add ac5 support via bounce buffer

From: Elad Nachman <[email protected]>

AC5/X/IM SOCs has a variant of the Xenon eMMC controller,
in which only 31-bit of addressing pass from the controller
on the AXI bus.
Since we cannot guarantee that only buffers from the first 2GB
of memory will reach the driver, the driver is configured for
SDMA mode, without 64-bit mode, overriding the DMA mask to 34-bit
to support the DDR memory mapping, which starts at offset 8GB.

Signed-off-by: Elad Nachman <[email protected]>
---
drivers/mmc/host/sdhci-xenon.c | 33 ++++++++++++++++++++++++++++++++-
drivers/mmc/host/sdhci-xenon.h | 3 ++-
2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
index 25ba7aecc3be..4d6df1815da1 100644
--- a/drivers/mmc/host/sdhci-xenon.c
+++ b/drivers/mmc/host/sdhci-xenon.c
@@ -18,6 +18,8 @@
#include <linux/of.h>
#include <linux/pm.h>
#include <linux/pm_runtime.h>
+#include <linux/mm.h>
+#include <linux/dma-mapping.h>

#include "sdhci-pltfm.h"
#include "sdhci-xenon.h"
@@ -422,6 +424,7 @@ static int xenon_probe_params(struct platform_device *pdev)
struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
u32 sdhc_id, nr_sdhc;
u32 tuning_count;
+ struct sysinfo si;

/* Disable HS200 on Armada AP806 */
if (priv->hw_version == XENON_AP806)
@@ -450,6 +453,23 @@ static int xenon_probe_params(struct platform_device *pdev)
}
priv->tuning_count = tuning_count;

+ /*
+ * AC5/X/IM HW has only 31-bits passed in the crossbar switch.
+ * If we have more than 2GB of memory, this means we might pass
+ * memory pointers which are above 2GB and which cannot be properly
+ * represented. In this case, disable ADMA, 64-bit DMA and allow only SDMA.
+ * This effectively will enable bounce buffer quirk in the
+ * generic SDHCI driver, which will make sure DMA is only done
+ * from supported memory regions:
+ */
+ if (priv->hw_version == XENON_AC5) {
+ si_meminfo(&si);
+ if (si.totalram * si.mem_unit > SZ_2G) {
+ host->quirks |= SDHCI_QUIRK_BROKEN_ADMA;
+ host->quirks2 |= SDHCI_QUIRK2_BROKEN_64_BIT_DMA;
+ }
+ }
+
return xenon_phy_parse_params(dev, host);
}

@@ -562,7 +582,17 @@ static int xenon_probe(struct platform_device *pdev)
goto remove_sdhc;

pm_runtime_put_autosuspend(&pdev->dev);
-
+ /*
+ * If we previously detected AC5 with over 2GB of memory,
+ * then we disable ADMA and 64-bit DMA.
+ * This means generic SDHCI driver has set the DMA mask to
+ * 32-bit. Since DDR starts at 0x2_0000_0000, we must use
+ * 34-bit DMA mask to access this DDR memory:
+ */
+ if (priv->hw_version == XENON_AC5) {
+ if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA)
+ dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(34));
+ }
return 0;

remove_sdhc:
@@ -680,6 +710,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 {
--
2.25.1


2023-12-27 12:34:51

by Elad Nachman

[permalink] [raw]
Subject: [PATCH 3/4] arm64: dts: ac5: add mmc node and clock

From: Elad Nachman <[email protected]>

Add mmc and mmc clock nodes to ac5 and ac5x device tree files

Signed-off-by: Elad Nachman <[email protected]>
---
arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 33 ++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
index b5e042b8e929..decad14d0db8 100644
--- a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
@@ -77,7 +77,6 @@ soc {
#address-cells = <2>;
#size-cells = <2>;
ranges;
- dma-ranges;

internal-regs@7f000000 {
#address-cells = <1>;
@@ -204,6 +203,31 @@ gpio1: gpio@18140 {
};
};

+ mmc_dma: mmc-dma-peripherals@80500000 {
+ compatible = "simple-bus";
+ #address-cells = <0x2>;
+ #size-cells = <0x2>;
+ ranges;
+ dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>;
+ dma-coherent;
+
+ sdhci: mmc@805c0000 {
+ compatible = "marvell,ac5-sdhci",
+ "marvell,armada-ap806-sdhci";
+ reg = <0x0 0x805c0000 0x0 0x1000>;
+ interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&emmc_clock>, <&cnm_clock>;
+ clock-names = "core", "axi";
+ status = "okay";
+ bus-width = <8>;
+ /*marvell,xenon-phy-slow-mode;*/
+ non-removable;
+ mmc-ddr-1_8v;
+ mmc-hs200-1_8v;
+ mmc-hs400-1_8v;
+ };
+ };
+
/*
* Dedicated section for devices behind 32bit controllers so we
* can configure specific DMA mapping for them
@@ -335,5 +359,12 @@ nand_clock: nand-clock {
#clock-cells = <0>;
clock-frequency = <400000000>;
};
+
+ emmc_clock: emmc_clock {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <400000000>;
+ };
+
};
};
--
2.25.1


2023-12-27 12:41:14

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/4] arm64: dts: ac5: add mmc node and clock

On 27/12/2023 13:32, Elad Nachman wrote:
> From: Elad Nachman <[email protected]>
>
> Add mmc and mmc clock nodes to ac5 and ac5x device tree files
>
> Signed-off-by: Elad Nachman <[email protected]>
> ---
> arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 33 ++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
> index b5e042b8e929..decad14d0db8 100644
> --- a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
> @@ -77,7 +77,6 @@ soc {
> #address-cells = <2>;
> #size-cells = <2>;
> ranges;
> - dma-ranges;
>
> internal-regs@7f000000 {
> #address-cells = <1>;
> @@ -204,6 +203,31 @@ gpio1: gpio@18140 {
> };
> };
>
> + mmc_dma: mmc-dma-peripherals@80500000 {

Generic node name, so bus@?

> + compatible = "simple-bus";
> + #address-cells = <0x2>;
> + #size-cells = <0x2>;
> + ranges;

ranges is second.

You have address/size cells, so are you sure dtbs W=1 does not complain?


> + dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>;
> + dma-coherent;
> +
> + sdhci: mmc@805c0000 {
> + compatible = "marvell,ac5-sdhci",
> + "marvell,armada-ap806-sdhci";
> + reg = <0x0 0x805c0000 0x0 0x1000>;
> + interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&emmc_clock>, <&cnm_clock>;
> + clock-names = "core", "axi";
> + status = "okay";

Drop

> + bus-width = <8>;
> + /*marvell,xenon-phy-slow-mode;*/

Drop or explain why commented code should be here.

> + non-removable;
> + mmc-ddr-1_8v;
> + mmc-hs200-1_8v;
> + mmc-hs400-1_8v;
> + };
> + };
> +
> /*
> * Dedicated section for devices behind 32bit controllers so we
> * can configure specific DMA mapping for them
> @@ -335,5 +359,12 @@ nand_clock: nand-clock {
> #clock-cells = <0>;
> clock-frequency = <400000000>;
> };
> +
> + emmc_clock: emmc_clock {

No underscores in node names. I think you got such feedback before.

But anyway, this looks like a fake clock.

> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <400000000>;
> + };
> +

Drop

> };
> };

Best regards,
Krzysztof


2023-12-29 21:57:06

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 0/4] mmc: xenon: add AC5 support

On Wed, Dec 27, 2023 at 02:32:53PM +0200, Elad Nachman wrote:
> From: Elad Nachman <[email protected]>
>
> This patch series adds support for the Marvell AC5/X/IM series of SOCs.
> The main hurdles in supporting these SOCs are the following limitations:
> 1. DDR starts at offset 0x2_0000_0000
> 2. mmc controller has only 31-bit path on the crossbar to the DDR.
>
> Point number one is solved by the first patch, which targets the
> arm64 subsystem, by taking into account the DDR start address when
> calculating the DMA and DMA32 zones.
>
> This yields the correct split between DMA, DMA32 and NORMAL zones
> according to the device tree CPU address limitations.
>
> Point number two is solved in the mmc xenon driver by detecting the memory
> size, and when it is more than 2GB, disable ADMA and 64-bit DMA, which
> effectively enables SDMA with a bounce buffer.
> DMA mask is then set manually to 34 bit to account for the DDR starting
> at offset 0x2_0000_0000 .

You probably need to split this patchset up since the first patch will
get merged via the arm64 core maintainers, the MMC driver change via
the MMC maintainers, and maybe the DT changes for the
ac5-98dx25xx.dtsi via the mvebu maintainers, or the MMC maintainer?

Andrew

2024-01-04 10:51:01

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 4/4] mmc: xenon: Add ac5 support via bounce buffer

On 27/12/23 14:32, Elad Nachman wrote:
> From: Elad Nachman <[email protected]>
>
> AC5/X/IM SOCs has a variant of the Xenon eMMC controller,
> in which only 31-bit of addressing pass from the controller
> on the AXI bus.
> Since we cannot guarantee that only buffers from the first 2GB
> of memory will reach the driver, the driver is configured for
> SDMA mode, without 64-bit mode, overriding the DMA mask to 34-bit
> to support the DDR memory mapping, which starts at offset 8GB.
>
> Signed-off-by: Elad Nachman <[email protected]>

One minor comment below otherwise:

Acked-by: Adrian Hunter <[email protected]>

> ---
> drivers/mmc/host/sdhci-xenon.c | 33 ++++++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci-xenon.h | 3 ++-
> 2 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> index 25ba7aecc3be..4d6df1815da1 100644
> --- a/drivers/mmc/host/sdhci-xenon.c
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -18,6 +18,8 @@
> #include <linux/of.h>
> #include <linux/pm.h>
> #include <linux/pm_runtime.h>
> +#include <linux/mm.h>
> +#include <linux/dma-mapping.h>
>
> #include "sdhci-pltfm.h"
> #include "sdhci-xenon.h"
> @@ -422,6 +424,7 @@ static int xenon_probe_params(struct platform_device *pdev)
> struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> u32 sdhc_id, nr_sdhc;
> u32 tuning_count;
> + struct sysinfo si;
>
> /* Disable HS200 on Armada AP806 */
> if (priv->hw_version == XENON_AP806)
> @@ -450,6 +453,23 @@ static int xenon_probe_params(struct platform_device *pdev)
> }
> priv->tuning_count = tuning_count;
>
> + /*
> + * AC5/X/IM HW has only 31-bits passed in the crossbar switch.
> + * If we have more than 2GB of memory, this means we might pass
> + * memory pointers which are above 2GB and which cannot be properly
> + * represented. In this case, disable ADMA, 64-bit DMA and allow only SDMA.
> + * This effectively will enable bounce buffer quirk in the
> + * generic SDHCI driver, which will make sure DMA is only done
> + * from supported memory regions:
> + */
> + if (priv->hw_version == XENON_AC5) {
> + si_meminfo(&si);
> + if (si.totalram * si.mem_unit > SZ_2G) {
> + host->quirks |= SDHCI_QUIRK_BROKEN_ADMA;
> + host->quirks2 |= SDHCI_QUIRK2_BROKEN_64_BIT_DMA;
> + }
> + }
> +
> return xenon_phy_parse_params(dev, host);
> }
>
> @@ -562,7 +582,17 @@ static int xenon_probe(struct platform_device *pdev)
> goto remove_sdhc;
>
> pm_runtime_put_autosuspend(&pdev->dev);
> -
> + /*
> + * If we previously detected AC5 with over 2GB of memory,
> + * then we disable ADMA and 64-bit DMA.
> + * This means generic SDHCI driver has set the DMA mask to
> + * 32-bit. Since DDR starts at 0x2_0000_0000, we must use
> + * 34-bit DMA mask to access this DDR memory:
> + */
> + if (priv->hw_version == XENON_AC5) {
> + if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA)

Kernel style is to avoid nested if-statements i.e.

if (priv->hw_version == XENON_AC5 &&
host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA)
dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(34));

> + dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(34));
> + }
> return 0;
>
> remove_sdhc:
> @@ -680,6 +710,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 {