RE: [PATCH] mmc: sdhci-pci-gli: fix GL9750 DMA write corruption

[BenChuang[莊智量]]

Hi Matthew,

> -----Original Message-----
> From: Matthew Schwartz <matthew.schwartz@xxxxxxxxx>
> Sent: Saturday, February 28, 2026 9:27 AM
> To: Adrian Hunter <adrian.hunter@xxxxxxxxx>; Ulf Hansson <ulf.hansson@xxxxxxxxxx>; BenChuang[莊智量]
> <Ben.Chuang@xxxxxxxxxxxxxxxxxxx>
> Cc: linux-mmc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] mmc: sdhci-pci-gli: fix GL9750 DMA write corruption
>
> On 2/27/26 1:16 AM, Adrian Hunter wrote:
> > On 27/02/2026 09:59, Matthew Schwartz wrote:
> >> The GL9750 SD host controller has intermittent data corruption during
> >> DMA write operations. The GM_BURST register's R_OSRC_Lmt field
> >> (bits 17:16), which limits outstanding DMA read requests from system
> >> memory, is not being cleared during initialization. The Windows driver
> >> sets R_OSRC_Lmt to zero, limiting requests to the smallest unit.
> >>
> >> Clear R_OSRC_Lmt to match the Windows driver behavior. This eliminates
> >> write corruption verified with f3write/f3read tests while maintaining
> >> DMA performance.
> >>
> >> Cc: stable@xxxxxxxxxxxxxxx
> >> Fixes: e51df6ce668a ("mmc: host: sdhci-pci: Add Genesys Logic GL975x support")
> >> Closes:
> https://lore.kernel.org/linux-mmc/33d12807-5c72-41c
> e-8679-57aa11831fad%40linux.dev%2F&data=05%7C02%7Cben.chuang%40genesyslogic.com.tw%7Cf7d89cd3b9ef4ee8f58
> 208de76687497%7C4e753840bf6b40a19645185818deeb52%7C0%7C0%7C639078388197698028%7CUnknown%7CTWFpb
> GZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%
> 7C%7C%7C&sdata=xdnJIB74XZ4LQYBgHseMZWvDSwO1mg4x0jCNxqMMoco%3D&reserved=0
> >> Signed-off-by: Matthew Schwartz <matthew.schwartz@xxxxxxxxx>
> >
> > Ben wrote "So I think your patch setting R_OSRC_Lmt to zero is reasonable."
> > Can be have a Reviewed-by tag also?
>
> Wasn't sure about the etiquette of adding a Reviewed-by without an explicit tag in an email,
> but happy to re-spin a v2 and add that if it's wanted.
>
> >
> > Nevertheless:
> >
> > Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> >
> >> ---
> >> Link to RFC:
> https://lore.kernel.org/all/20260117234800.931664-1
> -matthew.schwartz%40linux.dev%2F&data=05%7C02%7Cben.chuang%40genesyslogic.com.tw%7Cf7d89cd3b9ef4ee8f58208
> de76687497%7C4e753840bf6b40a19645185818deeb52%7C0%7C0%7C639078388197757693%7CUnknown%7CTWFpbGZsb
> 3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%
> 7C%7C&sdata=hG%2FsvJa9fEfEPXIcB81%2FG33pbxg54SxC2SX5WuKxCZw%3D&reserved=0
> >> Changes from RFC -> v1: use the proper name for the register field
> >> ---
> >>  drivers/mmc/host/sdhci-pci-gli.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
> >> index b0f91cc9e40e4..7a7be3f7bee6b 100644
> >> --- a/drivers/mmc/host/sdhci-pci-gli.c
> >> +++ b/drivers/mmc/host/sdhci-pci-gli.c
> >> @@ -26,6 +26,9 @@
> >>  #define   GLI_9750_WT_EN_ON           0x1
> >>  #define   GLI_9750_WT_EN_OFF          0x0
> >>
> >> +#define SDHCI_GLI_9750_GM_BURST_SIZE                0x510
> >> +#define   SDHCI_GLI_9750_GM_BURST_SIZE_R_OSRC_LMT     GENMASK(17, 16)
> >> +

Please move the definition of 0x510 register before the definition of 0x540 register.
i.e.

 #define   GLI_9750_MISC_TX1_DLY_VALUE    0x5
 #define   SDHCI_GLI_9750_MISC_SSC_OFF    BIT(26)

+#define        SDHCI_GLI_9750_GM_BURST_SIZE              0x510
+#define          SDHCI_GLI_9750_GM_BURST_SIZE_R_OSRC_LMT   GENMASK(17, 16)
+
 #define SDHCI_GLI_9750_TUNING_CONTROL            0x540
 #define   SDHCI_GLI_9750_TUNING_CONTROL_EN          BIT(4)
 #define   GLI_9750_TUNING_CONTROL_EN_ON             0x1

> >>  #define SDHCI_GLI_9750_CFG2          0x848
> >>  #define   SDHCI_GLI_9750_CFG2_L1DLY    GENMASK(28, 24)
> >>  #define   GLI_9750_CFG2_L1DLY_VALUE    0x1F
> >> @@ -629,6 +632,11 @@ static void gl9750_hw_setting(struct sdhci_host *host)
> >>
> >>    gl9750_wt_on(host);
> >>
> >> +  /* clear R_OSRC_Lmt to avoid DMA write corruption */
> >> +  value = sdhci_readl(host, SDHCI_GLI_9750_GM_BURST_SIZE);
> >> +  value &= ~SDHCI_GLI_9750_GM_BURST_SIZE_R_OSRC_LMT;
> >> +  sdhci_writel(host, value, SDHCI_GLI_9750_GM_BURST_SIZE);
> >> +

I recall that sdhci_reset() resets the 0x510 register to its default value.
So please test this by removing the card and reinserting the card again, and
see if the value still matches expectations. If not, perhaps the above code
can be added to gli_set_9750().

Best regards,
Ben Chuang

> >>    value = sdhci_readl(host, SDHCI_GLI_9750_CFG2);
> >>    value &= ~SDHCI_GLI_9750_CFG2_L1DLY;
> >>    /* set ASPM L1 entry delay to 7.9us */
> >

________________________________

Genesys Logic Email Confidentiality Notice:
This mail and any attachments may contain information that is confidential, proprietary, privileged or otherwise protected by law. The mail is intended solely for the named addressee (or a person responsible for delivering it to the addressee). If you are not the intended recipient of this mail, you are not authorized to read, print, copy or disseminate this mail.

If you have received this email in error, please notify us immediately by reply email and immediately delete this message and any attachments from your system. Please be noted that any unauthorized use, dissemination, distribution or copying of this email is strictly prohibited.
________________________________