Re: [PATCH v2 5/8] mmc: sdhci: add Black Sesame Technologies BST C1200 controller driver

[Albert Yang <yangzh0906@thundersoft.com>]

Hi Arnd,

Thank you for your detailed review and suggestions. I have addressed all the 
issues you raised in v2 of the patch series.

On Wed, Jul 2, 2025, at 11:44, Arnd Bergmann wrote:

> The description does not mention the actual device it's for
> but only DesignWare.
> 
> Try to keep this sorted alphabetically between the other
> CONFIG_MMC_SDHCI_* backends

Fixed. Updated the Kconfig description to mention "Black Sesame Technologies 
BST C1200 controller" and moved the config entry to the correct alphabetical 
position between MMC_SDHCI_BCM_KONA and MMC_SDHCI_F_SDH30.

> You are only using the first member here, the phy_crm_reg_base
> and phy_crm_reg_size are assigned during probe but not referenced
> later. devm_platform_ioremap_resource() should help simplify
> that code further.

Agreed. Removed the unused phy_crm_reg_base and phy_crm_reg_size fields from 
dwcmshc_priv structure and replaced the manual platform_get_resource() + 
ioremap() calls with devm_platform_ioremap_resource().

> You always pass priv->crm_reg_base into this helper, so
> it would be simpler to make it take the sdhci_pltfm_host
> pointer and the offset instead of the address.

Good suggestion. Replaced bst_write_phys_bst() and bst_read_phys_bst() with 
bst_crm_write() and bst_crm_read() that take sdhci_pltfm_host pointer and 
offset parameters for better encapsulation.

> The comment says 64K, but the size you use is 32K.

Fixed the comment to correctly state 32KB.

> Having an explanation here makes sense, but I don't think this
> captures what is actually going on, in particular:
> 
> - dma_alloc_coherent() being backed by an SRAM that is under
>   the 4GB boundary
> - the problem that the SoC is configured that all of DRAM
>   is outside of ZONE_DMA32
> - The type of hardware bug that leads to 64-bit DMA being
>   broken in this SoC.

You're absolutely right. I've enhanced the comment with a detailed explanation 
of our specific hardware constraints:

1. System memory uses 64-bit bus, eMMC controller uses 32-bit bus
2. eMMC controller cannot access memory through SMMU due to hardware bug
3. Our SRAM-based bounce buffer solution works within 32-bit address space

> I still have some hope that the hardware is not actually
> that broken and you can get it working normally, in one
> of these ways:
> - enabling 64-bit addressing in the parent bus
> - enabling SMMU translation for the parent bus
> - configuring the parent bus or the sdhci itself to
>   access the first 4GB of RAM, and describing the
>   offset in dma-ranges
> - moving the start of RAM in a global SoC config

I appreciate your optimism about finding alternative solutions. Unfortunately, 
we have thoroughly investigated these approaches:

Regarding these last two suggestions, I'm not very familiar with the implementation 
details. Could you provide more guidance on:

1. **dma-ranges approach**: We tried adding these properties to the device tree:
   ```
   dma-ranges = <0x00000000 0x00000000 0x100000000>;
   dma-mask = <0xffffffff>;
   ```
   However, we still encounter DMA addressing issues. Are there specific 
   examples in other drivers we could reference for similar hardware constraints?

2. **Moving RAM start position**: Which component would control this configuration? 
   Would this require bootloader parameter changes or SoC-level configuration?

We're certainly interested in exploring these alternatives if they could provide 
a more elegant solution than our current bounce buffer approach.

The v3 patch addresses all your code structure and documentation concerns 
while providing a clear explanation of why this approach is necessary for 
our platform. I have also fixed compilation warnings about unused variables 
that resulted from the refactoring.

**Current DMA Issues**: Despite setting DMA32_ZONE, we still encounter DMA 
addressing warnings. Key error messages include:
```
DMA addr 0x00000008113e2200+512 overflow (mask ffffffff, bus limit 0)
software IO TLB: swiotlb_memblock_alloc: Failed to allocate [various sizes]
```

This confirms our hardware limitation where the eMMC controller cannot access 
memory above 32-bit boundaries, even with ZONE_DMA32 configured.

The complete kernel log with detailed DMA allocation failures and warnings
is available at: https://pastebin.com/eJgtuHDh

Please let me know if you need any additional information or have suggestions
for alternative approaches.

Best regards,
Albert