Re: [PATCH v3 2/2] mmc: sdhci-of-dwcmshc: Add HPE GSC eMMC support

[Adrian Hunter <adrian.hunter@intel.com>]

On 13/03/2026 04:10, Shawn Lin wrote:
> 在 2026/03/13 星期五 3:25, Hawkins, Nick 写道:
>> Hi Shawn, thank you for your input.
>>
>> See below:
>>
>>> -----Original Message-----
>>> From: Shawn Lin <shawn.lin@linux.dev>
>>> Sent: Thursday, March 12, 2026 3:25 AM
>>> To: Hawkins, Nick <nick.hawkins@hpe.com>; ulf.hansson@linaro.org; adrian.hunter@intel.com; jszhang@kernel.org
>>> Cc: shawn.lin@linux.dev; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
>>> Subject: Re: [PATCH v3 2/2] mmc: sdhci-of-dwcmshc: Add HPE GSC eMMC support
>>
>>
>>>> +static int dwcmshc_hpe_gsc_init(struct device *dev, struct sdhci_host *host,
>>>> +                struct dwcmshc_priv *dwc_priv)
>>>> +{
>>>> +    struct regmap *soc_ctrl;
>>>> +    int ret;
>>>> +
>>>> +    /* Disable cmd conflict check and configure auto-tuning */
>>>> +    dwcmshc_hpe_vendor_specific(host);
>>>> +
>>>> +    /* Look up the GXP sysreg syscon for MSHCCS access */
>>>> +    soc_ctrl = syscon_regmap_lookup_by_phandle(dev->of_node, "hpe,gxp-sysreg");
>>>> +    if (IS_ERR(soc_ctrl)) {
>>>> +        dev_err(dev, "failed to get hpe,gxp-sysreg syscon\n");
>>>> +        return PTR_ERR(soc_ctrl);
>>>> +    }
>>>> +
>>>> +    /* Set SCGSyncDis (bit 18) to disable sync on HS200 RX delay lines */
>>>> +    ret = regmap_update_bits(soc_ctrl, HPE_GSC_MSHCCS_OFFSET,
>>>> +                HPE_GSC_MSHCCS_SCGSYNCDIS,
>>>> +                HPE_GSC_MSHCCS_SCGSYNCDIS);
>>>> +    if (ret) {
>>>> +        dev_err(dev, "failed to set SCGSyncDis in MSHCCS\n");
>>>> +        return ret;
>>>> +    }
>>>> +
>>>> +    sdhci_enable_v4_mode(host);
>>
>>> Sorry, I overlooked this part when in v2. But we enable it in
>>> dwcmshc_probe() based on the capabilities, isn't it? Unless your
>>> hardware didn't set SDHCI_CAN_64BIT_V4 but actually it does support
>>> it? Then it perhaps should be a quirk, although we in general would like
>>> to avoid more quirks... It depends on Adrian.
>>
>>> Btw, unlated to your patch, but th1520 did it the same way... Hmm
>>
>> The GSC's IP is configured with a 32-bit DMA AXI
>> interface, so SDHCI_CAN_64BIT_V4 (caps bit 27) is not set by the
>> hardware. However, the controller still requires Host V4 mode
>> (HOST_CTRL2_R bit 12) to be enabled. Since dwcmshc_probe()
>> only calls sdhci_enable_v4_mode() when SDHCI_CAN_64BIT_V4
>> is set, the explicit call in dwcmshc_hpe_gsc_init() is necessary.
> 
> Okay, this aligns with previous assumption. So, we should either handle it via a quirk in the probe function and remove the call here, or simply add a comment to explain the context. This depends on Adrian's preference. I have no further questions. Thanks

Since ->init() is needed anyway, adding extra logic to handle
sdhci_enable_v4_mode() separately doesn't reduce complexity,
so I would leave it.