mbox series

[0/4] mmc: xenon: add AC5 support

Message ID 20231227123257.1170590-1-enachman@marvell.com
Headers show
Series mmc: xenon: add AC5 support | expand

Message

Elad Nachman Dec. 27, 2023, 12:32 p.m. UTC
From: Elad Nachman <enachman@marvell.com>

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(-)

Comments

Krzysztof Kozlowski Dec. 27, 2023, 12:38 p.m. UTC | #1
On 27/12/2023 13:32, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add dt bindings for Marvell ac5 eMMC controller
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  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 3a8e74894ae0..50c6de8bf0bc 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

Please make earlier const as enum and add it there.

> +          - const: marvell,armada-ap806-sdhci

You also missed here blank line, but that won't matter after above change.


Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 27, 2023, 12:40 p.m. UTC | #2
On 27/12/2023 13:32, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add mmc and mmc clock nodes to ac5 and ac5x device tree files
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  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
Krzysztof Kozlowski Dec. 27, 2023, 12:41 p.m. UTC | #3
On 27/12/2023 13:32, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add dt bindings for Marvell ac5 eMMC controller

Driver change says it is not fully compatible. Your commit msg here
explains nothing, except what subject is saying. You have entire commit
msg to explain such cases.


Best regards,
Krzysztof
Andrew Lunn Dec. 29, 2023, 9:56 p.m. UTC | #4
On Wed, Dec 27, 2023 at 02:32:53PM +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> 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