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

Matthew Schwartz
2026-03-02T06:29:38Z

> On Mar 1, 2026, at 6:54 PM, BenChuang[莊智量] <Ben.Chuang@genesyslogic.com.tw> wrote:
>
> Hi Matthew,
>
>> -----Original Message-----
>> From: Matthew Schwartz <matthew.schwartz@linux.dev>
>> Sent: Saturday, February 28, 2026 9:27 AM
>> To: Adrian Hunter <adrian.hunter@intel.com>; Ulf Hansson <ulf.hansson@linaro.org>; BenChuang[莊智量]
>> <Ben.Chuang@genesyslogic.com.tw>
>> Cc: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
>> 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@vger.kernel.org
>>>> 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@linux.dev>
>>>
>>> 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@intel.com>
>>>
>>>> ---
>>>> 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.

Sure, I can move it.

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

I will double check this before sending out a V2 that addresses your earlier comment.

Thanks for the review,
Matt

>
> 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.
> ________________________________