public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: <hehuan1@eswincomputing.com>, <ulfh@kernel.org>,
	<linux-mmc@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: <ningyu@eswincomputing.com>, <linmin@eswincomputing.com>,
	<pinkesh.vaghela@einfochips.com>, <xuxiang@eswincomputing.com>
Subject: Re: [PATCH v1] mmc: sdhci-of-dwcmshc: Fix SDIO support for Eswin EIC7700
Date: Tue, 5 May 2026 15:55:16 +0300	[thread overview]
Message-ID: <1c3acc06-e9ff-4728-a4d7-2639e5dc4793@intel.com> (raw)
In-Reply-To: <20260424070907.1422-1-hehuan1@eswincomputing.com>

On 24/04/2026 10:09, hehuan1@eswincomputing.com wrote:
> From: Huan He <hehuan1@eswincomputing.com>
> 
> The EIC7700 code in sdhci-of-dwcmshc uses host->mmc->caps2 to select
> different configuration paths for different card types. The current logic
> distinguishes eMMC and SD, but does not handle SDIO separately.
> 
> Update the EIC7700 card-type checks so that eMMC, SD and SDIO are
> distinguished explicitly.
> 
> Switch the reset path to dwcmshc_reset() so that pending interrupt state
> is cleared consistently, and use sdhci_enable_clk() so the clock enable
> sequence follows the standard SDHCI flow.

The reset and clk changes seem like they are not only for SDIO, but
the subject only mentions SDIO.  Should the subject be more like below?

	Fix reset, clk, and SDIO support for Eswin EIC7700

> 
> Fixes: 32b2633219d3 ("mmc: sdhci-of-dwcmshc: Add support for Eswin EIC7700")
> Signed-off-by: Huan He <hehuan1@eswincomputing.com>
> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 45 +++++++++++++++--------------
>  1 file changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 0b2158a7e409..0c1b39d48502 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -277,6 +277,7 @@
>  #define PHY_DELAY_CODE_MAX		0x7f
>  #define PHY_DELAY_CODE_EMMC		0x17
>  #define PHY_DELAY_CODE_SD		0x55
> +#define PHY_DELAY_CODE_SDIO		0x29
>  
>  struct rk35xx_priv {
>  	struct reset_control *reset;
> @@ -1433,10 +1434,7 @@ static void sdhci_eic7700_set_clock(struct sdhci_host *host, unsigned int clock)
>  	clk_set_rate(pltfm_host->clk, clock);
>  
>  	clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> -	clk |= SDHCI_CLOCK_INT_EN;
> -	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> -
> -	dwcmshc_enable_card_clk(host);
> +	sdhci_enable_clk(host, clk);
>  }
>  
>  static void sdhci_eic7700_config_phy_delay(struct sdhci_host *host, int delay)
> @@ -1497,7 +1495,7 @@ static void sdhci_eic7700_config_phy(struct sdhci_host *host)
>  
>  static void sdhci_eic7700_reset(struct sdhci_host *host, u8 mask)
>  {
> -	sdhci_reset(host, mask);
> +	dwcmshc_reset(host, mask);
>  
>  	/* after reset all, the phy's config will be clear */
>  	if (mask == SDHCI_RESET_ALL)
> @@ -1594,18 +1592,18 @@ static int sdhci_eic7700_phase_code_tuning(struct sdhci_host *host, u32 opcode)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
> -	u32 sd_caps = MMC_CAP2_NO_MMC | MMC_CAP2_NO_SDIO;
> +	u32 emmc_caps = MMC_CAP2_NO_SD | MMC_CAP2_NO_SDIO;
>  	int phase_code = -1;
>  	int code_range = -1;
> -	bool is_sd = false;
> +	bool is_non_emmc = false;

It is nicer to avoid double negatives like !is_non_emmc
so prefer to use is_emmc

	is_emmc = (host->mmc->caps2 & emmc_caps) == emmc_caps;

>  	int code_min = -1;
>  	int code_max = -1;
>  	int cmd_error = 0;
>  	int ret = 0;
>  	int i = 0;
>  
> -	if ((host->mmc->caps2 & sd_caps) == sd_caps)
> -		is_sd = true;
> +	if ((host->mmc->caps2 & emmc_caps) != emmc_caps)
> +		is_non_emmc = true;
>  
>  	for (i = 0; i <= MAX_PHASE_CODE; i++) {
>  		/* Centered Phase code */
> @@ -1614,8 +1612,8 @@ static int sdhci_eic7700_phase_code_tuning(struct sdhci_host *host, u32 opcode)
>  		host->ops->reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
>  
>  		if (ret) {
> -			/* SD specific range tracking */
> -			if (is_sd && code_min != -1 && code_max != -1) {
> +			/* SD/SDIO specific range tracking */
> +			if (is_non_emmc && code_min != -1 && code_max != -1) {
>  				if (code_max - code_min > code_range) {
>  					code_range = code_max - code_min;
>  					phase_code = (code_min + code_max) / 2;
> @@ -1626,17 +1624,17 @@ static int sdhci_eic7700_phase_code_tuning(struct sdhci_host *host, u32 opcode)
>  				code_max = -1;
>  			}
>  			/* EMMC breaks after first valid range */
> -			if (!is_sd && code_min != -1 && code_max != -1)
> +			if (!is_non_emmc && code_min != -1 && code_max != -1)
>  				break;
>  		} else {
>  			/* Track valid phase code range */
>  			if (code_min == -1) {
>  				code_min = i;
> -				if (!is_sd)
> +				if (!is_non_emmc)
>  					continue;
>  			}
>  			code_max = i;
> -			if (is_sd && i == MAX_PHASE_CODE) {
> +			if (is_non_emmc && i == MAX_PHASE_CODE) {
>  				if (code_max - code_min > code_range) {
>  					code_range = code_max - code_min;
>  					phase_code = (code_min + code_max) / 2;
> @@ -1646,19 +1644,19 @@ static int sdhci_eic7700_phase_code_tuning(struct sdhci_host *host, u32 opcode)
>  	}
>  
>  	/* Handle tuning failure case */
> -	if ((is_sd && phase_code == -1) ||
> -	    (!is_sd && code_min == -1 && code_max == -1)) {
> +	if ((is_non_emmc && phase_code == -1) ||
> +	    (!is_non_emmc && code_min == -1 && code_max == -1)) {
>  		pr_err("%s: phase code tuning failed!\n", mmc_hostname(host->mmc));
>  		sdhci_writew(host, 0, priv->vendor_specific_area1 + DWCMSHC_AT_STAT);
>  		return -EIO;
>  	}
> -	if (!is_sd)
> +	if (!is_non_emmc)
>  		phase_code = (code_min + code_max) / 2;
>  
>  	sdhci_writew(host, phase_code, priv->vendor_specific_area1 + DWCMSHC_AT_STAT);
>  
> -	/* SD specific final verification */
> -	if (is_sd) {
> +	/* SD/SDIO specific final verification */
> +	if (is_non_emmc) {
>  		ret = mmc_send_tuning(host->mmc, opcode, &cmd_error);
>  		host->ops->reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
>  		if (ret) {
> @@ -1756,9 +1754,9 @@ static void sdhci_eic7700_set_uhs_signaling(struct sdhci_host *host, unsigned in
>  
>  static void sdhci_eic7700_set_uhs_wrapper(struct sdhci_host *host, unsigned int timing)
>  {
> -	u32 sd_caps = MMC_CAP2_NO_MMC | MMC_CAP2_NO_SDIO;
> +	u32 emmc_caps = MMC_CAP2_NO_SD | MMC_CAP2_NO_SDIO;
>  
> -	if ((host->mmc->caps2 & sd_caps) == sd_caps)
> +	if ((host->mmc->caps2 & emmc_caps) != emmc_caps)
>  		sdhci_set_uhs_signaling(host, timing);
>  	else
>  		sdhci_eic7700_set_uhs_signaling(host, timing);
> @@ -1767,6 +1765,7 @@ static void sdhci_eic7700_set_uhs_wrapper(struct sdhci_host *host, unsigned int
>  static int eic7700_init(struct device *dev, struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
>  {
>  	u32 emmc_caps = MMC_CAP2_NO_SD | MMC_CAP2_NO_SDIO;
> +	u32 sd_caps = MMC_CAP2_NO_MMC | MMC_CAP2_NO_SDIO;
>  	unsigned int val, hsp_int_status, hsp_pwr_ctrl;
>  	static const char * const clk_ids[] = {"axi"};
>  	struct of_phandle_args args;
> @@ -1821,8 +1820,10 @@ static int eic7700_init(struct device *dev, struct sdhci_host *host, struct dwcm
>  
>  	if ((host->mmc->caps2 & emmc_caps) == emmc_caps)
>  		dwc_priv->delay_line = PHY_DELAY_CODE_EMMC;
> -	else
> +	else if ((host->mmc->caps2 & sd_caps) == sd_caps)
>  		dwc_priv->delay_line = PHY_DELAY_CODE_SD;
> +	else
> +		dwc_priv->delay_line = PHY_DELAY_CODE_SDIO;
>  
>  	if (!of_property_read_u32(dev->of_node, "eswin,drive-impedance-ohms", &val))
>  		priv->drive_impedance = eic7700_convert_drive_impedance_ohm(dev, val);


      reply	other threads:[~2026-05-05 12:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-24  7:09 [PATCH v1] mmc: sdhci-of-dwcmshc: Fix SDIO support for Eswin EIC7700 hehuan1
2026-05-05 12:55 ` Adrian Hunter [this message]

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=1c3acc06-e9ff-4728-a4d7-2639e5dc4793@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=hehuan1@eswincomputing.com \
    --cc=linmin@eswincomputing.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=ningyu@eswincomputing.com \
    --cc=pinkesh.vaghela@einfochips.com \
    --cc=ulfh@kernel.org \
    --cc=xuxiang@eswincomputing.com \
    /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