[PATCH 0/3] mmc: xenon-sdhci: Try to fix 2G address limitation on AC5 SoC

[PATCH 0/3] mmc: xenon-sdhci: Try to fix 2G address limitation on AC5 SoC

[Vadym Kochan]

There is a limitation on AC5 SoC that Xenon SDHC can address only
first 2GB of memory. Turning to the SDMA mode to use the bounce_buffer
causes ext_csd recognition to fail on init.

Using of swiotlb=force also does not help as it is allocated at the
end of the memory.

So it was decided to use reserved-memory as a bounce buffer in case
if the board has more than 2G of memory, or turn on the PIO
mode if such memory region does not exist in the device-tree.
There is a custom property which is used to attach bounce memory region
to the device on demand (only if more 2G memory is used). This is a software
property only so it was not described in the dt-bindings.

Separate DTSI is added to reserve memory only for boards with eMMC cards.

It was tested that this approach is 1.5 times faster than PIO.

Vadym Kochan (3):
  dt-bindings: mmc: xenon: Add compatible string for Xenon SDHCI on AC5
    SoC
  mmc: sdhci-xenon: Try to fix 2G address limitation on AC5 SoC
  arm64: dts: marvell: Add DTSI to fix Xenon SDHCI 2G limitation on AC5
    SoC

 .../bindings/mmc/marvell,xenon-sdhci.yaml     |  3 ++
 .../boot/dts/marvell/ac5-98dx25xx-sdhci.dtsi  | 40 +++++++++++++++++++
 drivers/mmc/host/sdhci-xenon.c                | 36 +++++++++++++++++
 3 files changed, 79 insertions(+)
 create mode 100644 arch/arm64/boot/dts/marvell/ac5-98dx25xx-sdhci.dtsi

-- 
2.17.1

[PATCH 1/3] dt-bindings: mmc: xenon: Add compatible string for Xenon SDHCI on AC5 SoC

[Vadym Kochan]

It indicates the Xenon SDHCI on AC5 SoC so the driver will try to fixup
the 2G address space limitation issue.

Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
---
 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
index 3ee758886558..17cda847e0a3 100644
--- a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
@@ -34,6 +34,9 @@ properties:
           - const: marvell,armada-3700-sdhci
           - const: marvell,sdhci-xenon
 
+      - items:
+          - const: marvell,ac5-sdhci
+
   reg:
     minItems: 1
     maxItems: 2
-- 
2.17.1

Re: [PATCH 1/3] dt-bindings: mmc: xenon: Add compatible string for Xenon SDHCI on AC5 SoC

[Krzysztof Kozlowski]

On 06/08/2022 11:58, Vadym Kochan wrote:
> It indicates the Xenon SDHCI on AC5 SoC so the driver will try to fixup
> the 2G address space limitation issue.
> 
> Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
> ---
>  Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> index 3ee758886558..17cda847e0a3 100644
> --- a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> @@ -34,6 +34,9 @@ properties:
>            - const: marvell,armada-3700-sdhci
>            - const: marvell,sdhci-xenon
>  
> +      - items:
> +          - const: marvell,ac5-sdhci

These are not items. Should be part of existing enum.

You missed updating allOf:if:then.

Best regards,
Krzysztof

[PATCH 2/3] mmc: sdhci-xenon: Try to fix 2G address limitation on AC5 SoC

[Vadym Kochan]

There is a limitation on AC5 SoC that Xenon SDHC can address only
first 2GB of memory. Turning to the SDMA mode to use the bounce_buffer
causes ext_csd recognition to fail on init.

Using of swiotlb=force also does not help as it is allocated at the
end of the memory.

So it was decided to use reserved-memory as a bounce buffer in case
if the board has more than 2G of memory, or turn on the PIO
mode if such memory region does not exist in the device-tree.

It was tested that this approach is 1.5 times faster than PIO.

Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
---
 drivers/mmc/host/sdhci-xenon.c | 36 ++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
index 08e838400b52..fbbd1a045002 100644
--- a/drivers/mmc/host/sdhci-xenon.c
+++ b/drivers/mmc/host/sdhci-xenon.c
@@ -14,8 +14,10 @@
 #include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/ktime.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 
@@ -486,6 +488,31 @@ 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 device *dev = mmc_dev(host->mmc);
+	struct sysinfo si;
+	int err;
+
+	si_meminfo(&si);
+
+	if ((si.totalram * si.mem_unit) > SZ_2G) {
+		struct device_node *dma_bounce_np;
+
+		dma_bounce_np = of_parse_phandle(dev->of_node,
+						 "marvell,ac5-sdhci-dma-bounce-pool", 0);
+		err = of_reserved_mem_device_init_by_idx(dev, dma_bounce_np, 0);
+		of_node_put(dma_bounce_np);
+		if (err) {
+			dev_warn(dev, "Disabling DMA because of 2GB DMA access limit and missing DMA bounce region\n");
+			host->quirks |= SDHCI_QUIRK_BROKEN_ADMA;
+			host->quirks |= SDHCI_QUIRK_BROKEN_DMA;
+		}
+	}
+
+	return 0;
+}
+
 static int xenon_probe(struct platform_device *pdev)
 {
 	struct sdhci_pltfm_host *pltfm_host;
@@ -531,6 +558,12 @@ static int xenon_probe(struct platform_device *pdev)
 			if (err)
 				goto err_clk;
 		}
+
+		if (of_device_is_compatible(dev->of_node, "marvell,ac5-sdhci")) {
+			err = xenon_ac5_probe(host);
+			if (err)
+				goto err_clk_axi;
+		}
 	}
 
 	err = mmc_of_parse(host->mmc);
@@ -570,6 +603,7 @@ static int xenon_probe(struct platform_device *pdev)
 	pm_runtime_put_noidle(&pdev->dev);
 	xenon_sdhc_unprepare(host);
 err_clk_axi:
+	of_reserved_mem_device_release(&pdev->dev);
 	clk_disable_unprepare(priv->axi_clk);
 err_clk:
 	clk_disable_unprepare(pltfm_host->clk);
@@ -591,6 +625,7 @@ static int xenon_remove(struct platform_device *pdev)
 	sdhci_remove_host(host, 0);
 
 	xenon_sdhc_unprepare(host);
+	of_reserved_mem_device_release(&pdev->dev);
 	clk_disable_unprepare(priv->axi_clk);
 	clk_disable_unprepare(pltfm_host->clk);
 
@@ -682,6 +717,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_AP806},
 	{}
 };
 MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
-- 
2.17.1

Re: [PATCH 2/3] mmc: sdhci-xenon: Try to fix 2G address limitation on AC5 SoC

[Krzysztof Kozlowski]

On 06/08/2022 11:58, Vadym Kochan wrote:
> There is a limitation on AC5 SoC that Xenon SDHC can address only
> first 2GB of memory. Turning to the SDMA mode to use the bounce_buffer
> causes ext_csd recognition to fail on init.
> 
> Using of swiotlb=force also does not help as it is allocated at the
> end of the memory.
> 
> So it was decided to use reserved-memory as a bounce buffer in case
> if the board has more than 2G of memory, or turn on the PIO
> mode if such memory region does not exist in the device-tree.
> 
> It was tested that this approach is 1.5 times faster than PIO.
> 
> Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
> ---
>  drivers/mmc/host/sdhci-xenon.c | 36 ++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> index 08e838400b52..fbbd1a045002 100644
> --- a/drivers/mmc/host/sdhci-xenon.c
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -14,8 +14,10 @@
>  #include <linux/acpi.h>
>  #include <linux/delay.h>
>  #include <linux/ktime.h>
> +#include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_reserved_mem.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
>  
> @@ -486,6 +488,31 @@ 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 device *dev = mmc_dev(host->mmc);
> +	struct sysinfo si;
> +	int err;
> +
> +	si_meminfo(&si);
> +
> +	if ((si.totalram * si.mem_unit) > SZ_2G) {
> +		struct device_node *dma_bounce_np;
> +
> +		dma_bounce_np = of_parse_phandle(dev->of_node,
> +						 "marvell,ac5-sdhci-dma-bounce-pool", 0);

No. Undocumented property. Please document all bindings, not only pieces.

> +		err = of_reserved_mem_device_init_by_idx(dev, dma_bounce_np, 0);
> +		of_node_put(dma_bounce_np);
> +		if (err) {
> +			dev_warn(dev, "Disabling DMA because of 2GB DMA access limit and missing DMA bounce region\n");
> +			host->quirks |= SDHCI_QUIRK_BROKEN_ADMA;
> +			host->quirks |= SDHCI_QUIRK_BROKEN_DMA;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int xenon_probe(struct platform_device *pdev)
>  {
>  	struct sdhci_pltfm_host *pltfm_host;
> @@ -531,6 +558,12 @@ static int xenon_probe(struct platform_device *pdev)
>  			if (err)
>  				goto err_clk;
>  		}
> +
> +		if (of_device_is_compatible(dev->of_node, "marvell,ac5-sdhci")) {

Don't code compatible comparing in the driver. This is done by matching,
only once. You should use driver data/variant type.

> +			err = xenon_ac5_probe(host);
> +			if (err)
> +				goto err_clk_axi;
> +		}
>  	}
>  
>  	err = mmc_of_parse(host->mmc);
> @@ -570,6 +603,7 @@ static int xenon_probe(struct platform_device *pdev)
>  	pm_runtime_put_noidle(&pdev->dev);
>  	xenon_sdhc_unprepare(host);
>  err_clk_axi:
> +	of_reserved_mem_device_release(&pdev->dev);
>  	clk_disable_unprepare(priv->axi_clk);
>  err_clk:
>  	clk_disable_unprepare(pltfm_host->clk);
> @@ -591,6 +625,7 @@ static int xenon_remove(struct platform_device *pdev)
>  	sdhci_remove_host(host, 0);
>  
>  	xenon_sdhc_unprepare(host);
> +	of_reserved_mem_device_release(&pdev->dev);
>  	clk_disable_unprepare(priv->axi_clk);
>  	clk_disable_unprepare(pltfm_host->clk);
>  
> @@ -682,6 +717,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_AP806},

It's not the same.

>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);


Best regards,
Krzysztof

[PATCH 3/3] arm64: dts: marvell: Add DTSI to fix Xenon SDHCI 2G limitation on AC5 SoC

[Vadym Kochan]

There is a limitation in Xenon SDHCI on AC5 SoC that it can address only
the first 2G of memory, so it was decided to use reserved-memory as
bounce buffer because swiotlb=force and SDMA mode did not help.

There is a custom property which is used to attach bounce memory region
to the device on demand (only if more 2G memory is used). This is a software
property only so it was not described in the dt-bindings.

Separate DTSI is added to reserve memory only for boards with eMMC cards.

Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
---
 .../boot/dts/marvell/ac5-98dx25xx-sdhci.dtsi  | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 arch/arm64/boot/dts/marvell/ac5-98dx25xx-sdhci.dtsi

diff --git a/arch/arm64/boot/dts/marvell/ac5-98dx25xx-sdhci.dtsi b/arch/arm64/boot/dts/marvell/ac5-98dx25xx-sdhci.dtsi
new file mode 100644
index 000000000000..cc5e53b64473
--- /dev/null
+++ b/arch/arm64/boot/dts/marvell/ac5-98dx25xx-sdhci.dtsi
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Device Tree Include file for AC5 with SDHCI support
+ *
+ * Copyright (C) 2022 Marvell
+ */
+
+/ {
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		/*
+		 * SDHCI controller in AC5 SoC has 2G address space
+		 * limitation, so in case if RAM has more 2G then use
+		 * this dedicated memory region.
+		 */
+		sdhci_dma_restricted_region: sdhci-dma-restricted-region@0 {
+			compatible = "restricted-dma-pool";
+			reg = <0x2 0x0 0x0 0x200000>;
+		};
+	};
+
+	sdhci_dma_bounce_pool: sdhci-dma-bounce-pool@0 {
+		memory-region = <&sdhci_dma_restricted_region>;
+	};
+
+	soc {
+		sdhci: sdhci@805c0000 {
+			compatible = "marvell,ac5-sdhci";
+			reg = <0x0 0x805c0000 0x0 0x300>;
+			interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cnm_clock>;
+			clock-names = "core";
+			marvell,ac5-sdhci-dma-bounce-pool = <&sdhci_dma_bounce_pool>;
+			status = "okay";
+		};
+	};
+};
-- 
2.17.1

Re: [PATCH 3/3] arm64: dts: marvell: Add DTSI to fix Xenon SDHCI 2G limitation on AC5 SoC

[Krzysztof Kozlowski]

On 06/08/2022 11:58, Vadym Kochan wrote:
> +
> +	soc {
> +		sdhci: sdhci@805c0000 {
> +			compatible = "marvell,ac5-sdhci";
> +			reg = <0x0 0x805c0000 0x0 0x300>;
> +			interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cnm_clock>;
> +			clock-names = "core";
> +			marvell,ac5-sdhci-dma-bounce-pool = <&sdhci_dma_bounce_pool>;

Undocumented property.

Best regards,
Krzysztof