diff mbox series

mmc: sdhci-msm: Disable broken 64-bit DMA on MSM8916

Message ID 20230518-msm8916-64bit-v1-1-5694b0f35211@gerhold.net
State New
Headers show
Series mmc: sdhci-msm: Disable broken 64-bit DMA on MSM8916 | expand

Commit Message

Stephan Gerhold May 18, 2023, 9:39 a.m. UTC 
While SDHCI claims to support 64-bit DMA on MSM8916 it does not seem to
be properly functional. It is not immediately obvious because SDHCI is
usually used with IOMMU bypassed on this SoC, and all physical memory
has 32-bit addresses. But when trying to enable the IOMMU it quickly
fails with an error such as the following:

  arm-smmu 1e00000.iommu: Unhandled context fault:
    fsr=0x402, iova=0xfffff200, fsynr=0xe0000, cbfrsynra=0x140, cb=3
  mmc1: ADMA error: 0x02000000
  mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
  mmc1: sdhci: Sys addr:  0x00000000 | Version:  0x00002e02
  mmc1: sdhci: Blk size:  0x00000008 | Blk cnt:  0x00000000
  mmc1: sdhci: Argument:  0x00000000 | Trn mode: 0x00000013
  mmc1: sdhci: Present:   0x03f80206 | Host ctl: 0x00000019
  mmc1: sdhci: Power:     0x0000000f | Blk gap:  0x00000000
  mmc1: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
  mmc1: sdhci: Timeout:   0x0000000a | Int stat: 0x00000001
  mmc1: sdhci: Int enab:  0x03ff900b | Sig enab: 0x03ff100b
  mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
  mmc1: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x00008007
  mmc1: sdhci: Cmd:       0x0000333a | Max curr: 0x00000000
  mmc1: sdhci: Resp[0]:   0x00000920 | Resp[1]:  0x5b590000
  mmc1: sdhci: Resp[2]:   0xe6487f80 | Resp[3]:  0x0a404094
  mmc1: sdhci: Host ctl2: 0x00000008
  mmc1: sdhci: ADMA Err:  0x00000001 | ADMA Ptr: 0x0000000ffffff224
  mmc1: sdhci_msm: ----------- VENDOR REGISTER DUMP -----------
  mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:  0x60006400 | DLL cfg2: 0x00000000
  mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:  0x00000000 | DDR cfg: 0x00000000
  mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 : 0xf88018a8 Vndr func3: 0x00000000
  mmc1: sdhci: ============================================
  mmc1: sdhci: fffffffff200: DMA 0x0000ffffffffe100, LEN 0x0008, Attr=0x21
  mmc1: sdhci: fffffffff20c: DMA 0x0000000000000000, LEN 0x0000, Attr=0x03

Looking closely it's obvious that only the 32-bit part of the address
(0xfffff200) arrives at the SMMU, the higher 16-bit (0xffff...) get
lost somewhere. This might not be a limitation of the SDHCI itself but
perhaps the bus/interconnect it is connected to, or even the connection
to the SMMU.

Work around this by setting SDHCI_QUIRK2_BROKEN_64_BIT_DMA to avoid
using 64-bit addresses.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 drivers/mmc/host/sdhci-msm.c | 3 +++
 1 file changed, 3 insertions(+)


---
base-commit: d4ebc9419afbac330e9ec0d2936108742aa4d97a
change-id: 20230518-msm8916-64bit-f5bcf6af7679

Best regards,

Comments

Konrad Dybcio May 18, 2023, 9:48 a.m. UTC | #1 
On 18.05.2023 11:39, Stephan Gerhold wrote:
> While SDHCI claims to support 64-bit DMA on MSM8916 it does not seem to
> be properly functional. It is not immediately obvious because SDHCI is
> usually used with IOMMU bypassed on this SoC, and all physical memory
> has 32-bit addresses. But when trying to enable the IOMMU it quickly
> fails with an error such as the following:
> 
>   arm-smmu 1e00000.iommu: Unhandled context fault:
>     fsr=0x402, iova=0xfffff200, fsynr=0xe0000, cbfrsynra=0x140, cb=3
>   mmc1: ADMA error: 0x02000000
>   mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
>   mmc1: sdhci: Sys addr:  0x00000000 | Version:  0x00002e02
>   mmc1: sdhci: Blk size:  0x00000008 | Blk cnt:  0x00000000
>   mmc1: sdhci: Argument:  0x00000000 | Trn mode: 0x00000013
>   mmc1: sdhci: Present:   0x03f80206 | Host ctl: 0x00000019
>   mmc1: sdhci: Power:     0x0000000f | Blk gap:  0x00000000
>   mmc1: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
>   mmc1: sdhci: Timeout:   0x0000000a | Int stat: 0x00000001
>   mmc1: sdhci: Int enab:  0x03ff900b | Sig enab: 0x03ff100b
>   mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
>   mmc1: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x00008007
>   mmc1: sdhci: Cmd:       0x0000333a | Max curr: 0x00000000
>   mmc1: sdhci: Resp[0]:   0x00000920 | Resp[1]:  0x5b590000
>   mmc1: sdhci: Resp[2]:   0xe6487f80 | Resp[3]:  0x0a404094
>   mmc1: sdhci: Host ctl2: 0x00000008
>   mmc1: sdhci: ADMA Err:  0x00000001 | ADMA Ptr: 0x0000000ffffff224
>   mmc1: sdhci_msm: ----------- VENDOR REGISTER DUMP -----------
>   mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:  0x60006400 | DLL cfg2: 0x00000000
>   mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:  0x00000000 | DDR cfg: 0x00000000
>   mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 : 0xf88018a8 Vndr func3: 0x00000000
>   mmc1: sdhci: ============================================
>   mmc1: sdhci: fffffffff200: DMA 0x0000ffffffffe100, LEN 0x0008, Attr=0x21
>   mmc1: sdhci: fffffffff20c: DMA 0x0000000000000000, LEN 0x0000, Attr=0x03
> 
> Looking closely it's obvious that only the 32-bit part of the address
> (0xfffff200) arrives at the SMMU, the higher 16-bit (0xffff...) get
> lost somewhere. This might not be a limitation of the SDHCI itself but
> perhaps the bus/interconnect it is connected to, or even the connection
> to the SMMU.
> 
> Work around this by setting SDHCI_QUIRK2_BROKEN_64_BIT_DMA to avoid
> using 64-bit addresses.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
Would using 64bit address translation not require you to use (dma-)ranges
with a greater-than-default size, like we do on newer platforms? Did you
test that by chance?

Konrad
>  drivers/mmc/host/sdhci-msm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 8ac81d57a3df..1877d583fe8c 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -2479,6 +2479,9 @@ static inline void sdhci_msm_get_of_property(struct platform_device *pdev,
>  		msm_host->ddr_config = DDR_CONFIG_POR_VAL;
>  
>  	of_property_read_u32(node, "qcom,dll-config", &msm_host->dll_config);
> +
> +	if (of_device_is_compatible(node, "qcom,msm8916-sdhci"))
> +		host->quirks2 |= SDHCI_QUIRK2_BROKEN_64_BIT_DMA;
>  }
>  
>  static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
> 
> ---
> base-commit: d4ebc9419afbac330e9ec0d2936108742aa4d97a
> change-id: 20230518-msm8916-64bit-f5bcf6af7679
> 
> Best regards,
Stephan Gerhold May 18, 2023, 11:10 a.m. UTC | #2 
On Thu, May 18, 2023 at 11:48:55AM +0200, Konrad Dybcio wrote:
> On 18.05.2023 11:39, Stephan Gerhold wrote:
> > While SDHCI claims to support 64-bit DMA on MSM8916 it does not seem to
> > be properly functional. It is not immediately obvious because SDHCI is
> > usually used with IOMMU bypassed on this SoC, and all physical memory
> > has 32-bit addresses. But when trying to enable the IOMMU it quickly
> > fails with an error such as the following:
> > 
> >   arm-smmu 1e00000.iommu: Unhandled context fault:
> >     fsr=0x402, iova=0xfffff200, fsynr=0xe0000, cbfrsynra=0x140, cb=3
> >   mmc1: ADMA error: 0x02000000
> >   mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
> >   mmc1: sdhci: Sys addr:  0x00000000 | Version:  0x00002e02
> >   mmc1: sdhci: Blk size:  0x00000008 | Blk cnt:  0x00000000
> >   mmc1: sdhci: Argument:  0x00000000 | Trn mode: 0x00000013
> >   mmc1: sdhci: Present:   0x03f80206 | Host ctl: 0x00000019
> >   mmc1: sdhci: Power:     0x0000000f | Blk gap:  0x00000000
> >   mmc1: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
> >   mmc1: sdhci: Timeout:   0x0000000a | Int stat: 0x00000001
> >   mmc1: sdhci: Int enab:  0x03ff900b | Sig enab: 0x03ff100b
> >   mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
> >   mmc1: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x00008007
> >   mmc1: sdhci: Cmd:       0x0000333a | Max curr: 0x00000000
> >   mmc1: sdhci: Resp[0]:   0x00000920 | Resp[1]:  0x5b590000
> >   mmc1: sdhci: Resp[2]:   0xe6487f80 | Resp[3]:  0x0a404094
> >   mmc1: sdhci: Host ctl2: 0x00000008
> >   mmc1: sdhci: ADMA Err:  0x00000001 | ADMA Ptr: 0x0000000ffffff224
> >   mmc1: sdhci_msm: ----------- VENDOR REGISTER DUMP -----------
> >   mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:  0x60006400 | DLL cfg2: 0x00000000
> >   mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:  0x00000000 | DDR cfg: 0x00000000
> >   mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 : 0xf88018a8 Vndr func3: 0x00000000
> >   mmc1: sdhci: ============================================
> >   mmc1: sdhci: fffffffff200: DMA 0x0000ffffffffe100, LEN 0x0008, Attr=0x21
> >   mmc1: sdhci: fffffffff20c: DMA 0x0000000000000000, LEN 0x0000, Attr=0x03
> > 
> > Looking closely it's obvious that only the 32-bit part of the address
> > (0xfffff200) arrives at the SMMU, the higher 16-bit (0xffff...) get
> > lost somewhere. This might not be a limitation of the SDHCI itself but
> > perhaps the bus/interconnect it is connected to, or even the connection
> > to the SMMU.
> > 
> > Work around this by setting SDHCI_QUIRK2_BROKEN_64_BIT_DMA to avoid
> > using 64-bit addresses.
> > 
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> Would using 64bit address translation not require you to use (dma-)ranges
> with a greater-than-default size, like we do on newer platforms? Did you
> test that by chance?
> 

No, there is no "dma-ranges" in msm8916.dtsi. It seems to use 64-bit
(virtual) addresses by default for SDHCI, limited to 48-bit by the SMMU.

I tried limiting dma-ranges to 32-bit in msm8916.dtsi /soc@0 like
dma-ranges = <0 0 0 0xffffffff>; This seems to work as well as an
alternative to this patch, although it causes several annoying
"Invalid size 0xffffffff for dma-range(s)" warnings. I know the
0xffffffff isn't valid but I don't see how I would actually specify
2^32 there without rewriting the entire msm8916.dtsi to use
#address/size-cells = <2> instead of <1>.

It's not entirely clear to me where the actual limitation is coming from
here. It could be specific to the SDHCI (in which case this patch makes
more sense) or a general limitation of the bus/interconnect (in which
case dma-ranges might make more sense).

In any case 64-bit DMA is broken for SDHCI on MSM8916 so I would say
this patch is still the right thing to do. It's pointless to waste extra
cycles and memory to manage 64-bit pointers for SDHCI ADMA 64-bit, when
they can never work successfully on this SoC. The dma-ranges approach
doesn't change that, while SDHCI_QUIRK2_BROKEN_64_BIT_DMA actually makes
it use ADMA in 32-bit mode.

Thanks,
Stephan
Adrian Hunter May 29, 2023, 2:10 p.m. UTC | #3 
On 18/05/23 12:39, Stephan Gerhold wrote:
> While SDHCI claims to support 64-bit DMA on MSM8916 it does not seem to
> be properly functional. It is not immediately obvious because SDHCI is
> usually used with IOMMU bypassed on this SoC, and all physical memory
> has 32-bit addresses. But when trying to enable the IOMMU it quickly
> fails with an error such as the following:
> 
>   arm-smmu 1e00000.iommu: Unhandled context fault:
>     fsr=0x402, iova=0xfffff200, fsynr=0xe0000, cbfrsynra=0x140, cb=3
>   mmc1: ADMA error: 0x02000000
>   mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
>   mmc1: sdhci: Sys addr:  0x00000000 | Version:  0x00002e02
>   mmc1: sdhci: Blk size:  0x00000008 | Blk cnt:  0x00000000
>   mmc1: sdhci: Argument:  0x00000000 | Trn mode: 0x00000013
>   mmc1: sdhci: Present:   0x03f80206 | Host ctl: 0x00000019
>   mmc1: sdhci: Power:     0x0000000f | Blk gap:  0x00000000
>   mmc1: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
>   mmc1: sdhci: Timeout:   0x0000000a | Int stat: 0x00000001
>   mmc1: sdhci: Int enab:  0x03ff900b | Sig enab: 0x03ff100b
>   mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
>   mmc1: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x00008007
>   mmc1: sdhci: Cmd:       0x0000333a | Max curr: 0x00000000
>   mmc1: sdhci: Resp[0]:   0x00000920 | Resp[1]:  0x5b590000
>   mmc1: sdhci: Resp[2]:   0xe6487f80 | Resp[3]:  0x0a404094
>   mmc1: sdhci: Host ctl2: 0x00000008
>   mmc1: sdhci: ADMA Err:  0x00000001 | ADMA Ptr: 0x0000000ffffff224
>   mmc1: sdhci_msm: ----------- VENDOR REGISTER DUMP -----------
>   mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:  0x60006400 | DLL cfg2: 0x00000000
>   mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:  0x00000000 | DDR cfg: 0x00000000
>   mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 : 0xf88018a8 Vndr func3: 0x00000000
>   mmc1: sdhci: ============================================
>   mmc1: sdhci: fffffffff200: DMA 0x0000ffffffffe100, LEN 0x0008, Attr=0x21
>   mmc1: sdhci: fffffffff20c: DMA 0x0000000000000000, LEN 0x0000, Attr=0x03
> 
> Looking closely it's obvious that only the 32-bit part of the address
> (0xfffff200) arrives at the SMMU, the higher 16-bit (0xffff...) get
> lost somewhere. This might not be a limitation of the SDHCI itself but
> perhaps the bus/interconnect it is connected to, or even the connection
> to the SMMU.
> 
> Work around this by setting SDHCI_QUIRK2_BROKEN_64_BIT_DMA to avoid
> using 64-bit addresses.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-msm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 8ac81d57a3df..1877d583fe8c 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -2479,6 +2479,9 @@ static inline void sdhci_msm_get_of_property(struct platform_device *pdev,
>  		msm_host->ddr_config = DDR_CONFIG_POR_VAL;
>  
>  	of_property_read_u32(node, "qcom,dll-config", &msm_host->dll_config);
> +
> +	if (of_device_is_compatible(node, "qcom,msm8916-sdhci"))
> +		host->quirks2 |= SDHCI_QUIRK2_BROKEN_64_BIT_DMA;
>  }
>  
>  static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
> 
> ---
> base-commit: d4ebc9419afbac330e9ec0d2936108742aa4d97a
> change-id: 20230518-msm8916-64bit-f5bcf6af7679
> 
> Best regards,
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ac81d57a3df..1877d583fe8c 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -2479,6 +2479,9 @@  static inline void sdhci_msm_get_of_property(struct platform_device *pdev,
 		msm_host->ddr_config = DDR_CONFIG_POR_VAL;
 
 	of_property_read_u32(node, "qcom,dll-config", &msm_host->dll_config);
+
+	if (of_device_is_compatible(node, "qcom,msm8916-sdhci"))
+		host->quirks2 |= SDHCI_QUIRK2_BROKEN_64_BIT_DMA;
 }
 
 static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)