All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.