public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Shawn Lin <shawn.lin@linux.dev>
To: 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
Date: Thu, 12 Mar 2026 16:25:22 +0800	[thread overview]
Message-ID: <c4271da0-87c9-d7c2-db46-e7ed7f5e4fdf@linux.dev> (raw)
In-Reply-To: <20260311181112.1700667-3-nick.hawkins@hpe.com>

在 2026/03/12 星期四 2:11, nick.hawkins@hpe.com 写道:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add support for the eMMC controller integrated in the HPE GSC (ARM64
> Cortex-A53) BMC SoC under the new 'hpe,gsc-dwcmshc' compatible
> string.
> 
> The HPE GSC eMMC controller is based on the DesignWare Cores MSHC IP
> but requires several platform-specific adjustments:
> 
> Clock mux (dwcmshc_hpe_set_clock):
>    The GSC SoC wires SDHCI_CLOCK_CONTROL.freq_sel directly to a clock
>    mux rather than a divider.  Forcing freq_sel = 1 when the requested
>    clock is 200 MHz (HS200) selects the correct high-speed clock source.
>    Using the generic sdhci_set_clock() would otherwise leave the mux on
>    the wrong source after tuning.
> 
> Auto-tuning / vendor config (dwcmshc_hpe_vendor_specific):
>    Disables the command-conflict check (DWCMSHC_HOST_CTRL3 BIT(0)) and
>    programs the ATCTRL register using existing AT_CTRL_* macros:
>      AT_CTRL_AT_EN           auto-tuning circuit enable
>      AT_CTRL_SWIN_TH_EN      sampling window threshold enable
>      AT_CTRL_TUNE_CLK_STOP_EN tune-clock-stop enable
>      PRE_CHANGE_DLY  = 3     pre-change delay
>      POST_CHANGE_DLY = 3     post-change delay
>      SWIN_TH_VAL    = 2      sampling window threshold
>    This combination is required for reliable HS200 signal integrity on
>    the GSC PCB trace topology.
> 
> Reset (dwcmshc_hpe_reset):
>    Calls sdhci_reset(), re-applies the vendor config above, and then
>    sets DWCMSHC_CARD_IS_EMMC unconditionally.  The GSC controller
>    clears this bit on every reset; leaving it clear causes card-detect
>    mis-identification on an eMMC-only slot.
> 
> UHS signaling (dwcmshc_hpe_set_uhs_signaling):
>    Wraps dwcmshc_set_uhs_signaling() and unconditionally sets
>    CARD_IS_EMMC for all timing modes, not just HS400.
> 
> Init (dwcmshc_hpe_gsc_init):
>    Obtains the SoC register block via the 'hpe,gxp-sysreg' syscon
>    phandle and sets SCGSyncDis (BIT(18)) in MSHCCS (offset 0x110)
>    to allow the HS200 RX delay lines to settle while the card clock
>    is stopped during auto-tuning.  Enables SDHCI v4 mode.
> 
> Quirks:
>    SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN:  base clock not advertised in
>      capabilities; must be obtained from the DTS 'clocks' property.
>    SDHCI_QUIRK2_PRESET_VALUE_BROKEN:  preset-value registers are not
>      populated in the GSC ROM.
> 
> All HPE-specific code is isolated to the new hpe_gsc_init / hpe_ops /
> hpe_gsc_pdata symbols.  No existing platform (Rockchip, T-Head, sg2042,
> etc.) is affected.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> ---
>   drivers/mmc/host/sdhci-of-dwcmshc.c | 149 ++++++++++++++++++++++++++++
>   1 file changed, 149 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 2b75a36c096b..46662071cc61 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -1245,6 +1245,132 @@ static int sg2042_init(struct device *dev, struct sdhci_host *host,
>   					     ARRAY_SIZE(clk_ids), clk_ids);
>   }
>   
> +/* HPE GSC-specific vendor configuration: disable command conflict check
> + * and program Auto-Tuning Control register.
> + */
> +static void dwcmshc_hpe_vendor_specific(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
> +	u32 atctrl;
> +	u8 extra;
> +
> +	extra = sdhci_readb(host, dwc_priv->vendor_specific_area1 + DWCMSHC_HOST_CTRL3);
> +	extra &= ~BIT(0);
> +	sdhci_writeb(host, extra, dwc_priv->vendor_specific_area1 + DWCMSHC_HOST_CTRL3);
> +
> +	atctrl = AT_CTRL_AT_EN | AT_CTRL_SWIN_TH_EN | AT_CTRL_TUNE_CLK_STOP_EN |
> +		FIELD_PREP(AT_CTRL_PRE_CHANGE_DLY_MASK, 3) |
> +		FIELD_PREP(AT_CTRL_POST_CHANGE_DLY_MASK, AT_CTRL_POST_CHANGE_DLY) |
> +		FIELD_PREP(AT_CTRL_SWIN_TH_VAL_MASK, 2);
> +	sdhci_writel(host, atctrl, dwc_priv->vendor_specific_area1 + DWCMSHC_EMMC_ATCTRL);
> +}
> +
> +static void dwcmshc_hpe_reset(struct sdhci_host *host, u8 mask)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
> +	u16 ctrl;
> +
> +	dwcmshc_reset(host, mask);
> +
> +	dwcmshc_hpe_vendor_specific(host);
> +
> +	/* HPE GSC eMMC always needs CARD_IS_EMMC set after reset */
> +	ctrl = sdhci_readw(host, dwc_priv->vendor_specific_area1 + DWCMSHC_EMMC_CONTROL);
> +	ctrl |= DWCMSHC_CARD_IS_EMMC;
> +	sdhci_writew(host, ctrl, dwc_priv->vendor_specific_area1 + DWCMSHC_EMMC_CONTROL);
> +}
> +
> +static void dwcmshc_hpe_set_uhs_signaling(struct sdhci_host *host,
> +					  unsigned int timing)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	u16 ctrl;
> +
> +	dwcmshc_set_uhs_signaling(host, timing);
> +
> +	/* HPE GSC: always set CARD_IS_EMMC for all timing modes */
> +	ctrl = sdhci_readw(host, priv->vendor_specific_area1 + DWCMSHC_EMMC_CONTROL);
> +	ctrl |= DWCMSHC_CARD_IS_EMMC;
> +	sdhci_writew(host, ctrl, priv->vendor_specific_area1 + DWCMSHC_EMMC_CONTROL);
> +}
> +
> +/*
> + * HPE GSC eMMC controller clock setup.
> + *
> + * The GSC SoC wires the freq_sel field of SDHCI_CLOCK_CONTROL directly to a
> + * clock mux rather than a divider. Force freq_sel = 1 when running at
> + * 200 MHz (HS200) so the mux selects the correct clock source.
> + */
> +static void dwcmshc_hpe_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +	u16 clk;
> +
> +	host->mmc->actual_clock = 0;
> +
> +	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> +
> +	if (clock == 0)
> +		return;
> +
> +	clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock);
> +
> +	if (host->mmc->actual_clock == 200000000)
> +		clk |= (1 << SDHCI_DIVIDER_SHIFT);
> +
> +	sdhci_enable_clk(host, clk);
> +}
> +
> +/*
> + * HPE GSC eMMC controller init.
> + *
> + * The GSC SoC requires configuring MSHCCS.  Bit 18 (SCGSyncDis) disables clock
> + * synchronisation for phase-select values going to the HS200 RX delay lines,
> + * allowing the card clock to be stopped while the delay selection settles and
> + * the phase shift is applied.  This must be used together with the ATCTRL
> + * settings programmed in dwcmshc_hpe_vendor_specific():
> + *   AT_CTRL_R.TUNE_CLK_STOP_EN  = 0x1
> + *   AT_CTRL_R.POST_CHANGE_DLY   = 0x3
> + *   AT_CTRL_R.PRE_CHANGE_DLY    = 0x3
> + *
> + * The DTS node provides a syscon phandle ('hpe,gxp-sysreg') to access
> + * this register at offset 0x110 within the SoC control block.
> + */
> +#define HPE_GSC_MSHCCS_OFFSET		0x110
> +#define HPE_GSC_MSHCCS_SCGSYNCDIS	BIT(18)
> +

I guess you should move these macros to the beginning of this c file.

> +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

> +
> +	return 0;
> +}
> +
>   static void sdhci_eic7700_set_clock(struct sdhci_host *host, unsigned int clock)
>   {
>   	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1834,6 +1960,25 @@ static const struct dwcmshc_pltfm_data sdhci_dwcmshc_eic7700_pdata = {
>   	.init = eic7700_init,
>   };
>   
> +static const struct sdhci_ops sdhci_dwcmshc_hpe_ops = {
> +	.set_clock		= dwcmshc_hpe_set_clock,
> +	.set_bus_width		= sdhci_set_bus_width,
> +	.set_uhs_signaling	= dwcmshc_hpe_set_uhs_signaling,
> +	.get_max_clock		= dwcmshc_get_max_clock,
> +	.reset			= dwcmshc_hpe_reset,
> +	.adma_write_desc	= dwcmshc_adma_write_desc,
> +	.irq			= dwcmshc_cqe_irq_handler,
> +};
> +
> +static const struct dwcmshc_pltfm_data sdhci_dwcmshc_hpe_gsc_pdata = {
> +	.pdata = {
> +		.ops = &sdhci_dwcmshc_hpe_ops,
> +		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +	},
> +	.init = dwcmshc_hpe_gsc_init,
> +};
> +
>   static const struct cqhci_host_ops dwcmshc_cqhci_ops = {
>   	.enable		= dwcmshc_sdhci_cqe_enable,
>   	.disable	= sdhci_cqe_disable,
> @@ -1942,6 +2087,10 @@ static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
>   		.compatible = "eswin,eic7700-dwcmshc",
>   		.data = &sdhci_dwcmshc_eic7700_pdata,
>   	},
> +	{
> +		.compatible = "hpe,gsc-dwcmshc",
> +		.data = &sdhci_dwcmshc_hpe_gsc_pdata,
> +	},
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);
> 

  reply	other threads:[~2026-03-12  8:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-11 18:11 [PATCH v3 0/2] mmc: sdhci-of-dwcmshc: Add HPE GSC eMMC support nick.hawkins
2026-03-11 18:11 ` [PATCH v3 1/2] dt-bindings: mmc: snps,dwcmshc-sdhci: add HPE GSC dwcmshc compatible nick.hawkins
2026-03-13  8:07   ` Krzysztof Kozlowski
2026-03-11 18:11 ` [PATCH v3 2/2] mmc: sdhci-of-dwcmshc: Add HPE GSC eMMC support nick.hawkins
2026-03-12  8:25   ` Shawn Lin [this message]
2026-03-12 19:25     ` Hawkins, Nick
2026-03-13  2:10       ` Shawn Lin
2026-03-13  6:35         ` Adrian Hunter
2026-03-13  7:10   ` Adrian Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c4271da0-87c9-d7c2-db46-e7ed7f5e4fdf@linux.dev \
    --to=shawn.lin@linux.dev \
    --cc=adrian.hunter@intel.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jszhang@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=nick.hawkins@hpe.com \
    --cc=robh@kernel.org \
    --cc=shawn.lin@rock-chips.com \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox