diff mbox series

[V3] mmc: sdhci-of_dwcmshc: Change to dwcmshc_phy_init for reusing codes

Message ID 20250131025406.1753513-1-jh80.chung@samsung.com
State New
Headers show
Series [V3] mmc: sdhci-of_dwcmshc: Change to dwcmshc_phy_init for reusing codes | expand

Commit Message

Jaehoon Chung Jan. 31, 2025, 2:54 a.m. UTC
dwcmshc_phy_1_8v_init and dwcmshc_phy_3_3v_init differ only by a few
lines of code. This allow us to reuse code depending on voltage.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
Changelog V3:
- Remove unnecessary condition
- Check rxsel value instead of checking entire condition
Changelog V2:
- Add more conditions to clarify if it's MMC_SIGNAL_VOLTAGE_180
- Order the local variable according to Adrian's comment
- Use local variable to make more readable
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 72 ++++++-----------------------
 1 file changed, 15 insertions(+), 57 deletions(-)

Comments

Adrian Hunter Feb. 7, 2025, 7:52 a.m. UTC | #1
On 31/01/25 04:54, Jaehoon Chung wrote:
> dwcmshc_phy_1_8v_init and dwcmshc_phy_3_3v_init differ only by a few
> lines of code. This allow us to reuse code depending on voltage.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>

Subject would be better with:

	sdhci-of_dwcmshc -> sdhci-of-dwcmshc

Nevertheless:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> Changelog V3:
> - Remove unnecessary condition
> - Check rxsel value instead of checking entire condition
> Changelog V2:
> - Add more conditions to clarify if it's MMC_SIGNAL_VOLTAGE_180
> - Order the local variable according to Adrian's comment
> - Use local variable to make more readable
> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 72 ++++++-----------------------
>  1 file changed, 15 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 7ea3da45db32..09b9ab15e499 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -328,12 +328,17 @@ static void dwcmshc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  	sdhci_request(mmc, mrq);
>  }
>  
> -static void dwcmshc_phy_1_8v_init(struct sdhci_host *host)
> +static void dwcmshc_phy_init(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	u32 rxsel = PHY_PAD_RXSEL_3V3;
>  	u32 val;
>  
> +	if (priv->flags & FLAG_IO_FIXED_1V8 ||
> +		host->mmc->ios.timing & MMC_SIGNAL_VOLTAGE_180)
> +		rxsel = PHY_PAD_RXSEL_1V8;
> +
>  	/* deassert phy reset & set tx drive strength */
>  	val = PHY_CNFG_RSTN_DEASSERT;
>  	val |= FIELD_PREP(PHY_CNFG_PAD_SP_MASK, PHY_CNFG_PAD_SP);
> @@ -353,7 +358,7 @@ static void dwcmshc_phy_1_8v_init(struct sdhci_host *host)
>  	sdhci_writeb(host, val, PHY_SDCLKDL_CNFG_R);
>  
>  	/* configure phy pads */
> -	val = PHY_PAD_RXSEL_1V8;
> +	val = rxsel;
>  	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLUP);
>  	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
>  	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
> @@ -365,65 +370,22 @@ static void dwcmshc_phy_1_8v_init(struct sdhci_host *host)
>  	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
>  	sdhci_writew(host, val, PHY_CLKPAD_CNFG_R);
>  
> -	val = PHY_PAD_RXSEL_1V8;
> +	val = rxsel;
>  	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLDOWN);
>  	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
>  	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
>  	sdhci_writew(host, val, PHY_STBPAD_CNFG_R);
>  
>  	/* enable data strobe mode */
> -	sdhci_writeb(host, FIELD_PREP(PHY_DLLDL_CNFG_SLV_INPSEL_MASK, PHY_DLLDL_CNFG_SLV_INPSEL),
> -		     PHY_DLLDL_CNFG_R);
> -
> -	/* enable phy dll */
> -	sdhci_writeb(host, PHY_DLL_CTRL_ENABLE, PHY_DLL_CTRL_R);
> -}
> -
> -static void dwcmshc_phy_3_3v_init(struct sdhci_host *host)
> -{
> -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -	struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
> -	u32 val;
> -
> -	/* deassert phy reset & set tx drive strength */
> -	val = PHY_CNFG_RSTN_DEASSERT;
> -	val |= FIELD_PREP(PHY_CNFG_PAD_SP_MASK, PHY_CNFG_PAD_SP);
> -	val |= FIELD_PREP(PHY_CNFG_PAD_SN_MASK, PHY_CNFG_PAD_SN);
> -	sdhci_writel(host, val, PHY_CNFG_R);
> -
> -	/* disable delay line */
> -	sdhci_writeb(host, PHY_SDCLKDL_CNFG_UPDATE, PHY_SDCLKDL_CNFG_R);
> -
> -	/* set delay line */
> -	sdhci_writeb(host, priv->delay_line, PHY_SDCLKDL_DC_R);
> -	sdhci_writeb(host, PHY_DLL_CNFG2_JUMPSTEP, PHY_DLL_CNFG2_R);
> -
> -	/* enable delay lane */
> -	val = sdhci_readb(host, PHY_SDCLKDL_CNFG_R);
> -	val &= ~(PHY_SDCLKDL_CNFG_UPDATE);
> -	sdhci_writeb(host, val, PHY_SDCLKDL_CNFG_R);
> +	if (rxsel == PHY_PAD_RXSEL_1V8) {
> +		u8 sel = FIELD_PREP(PHY_DLLDL_CNFG_SLV_INPSEL_MASK, PHY_DLLDL_CNFG_SLV_INPSEL);
>  
> -	/* configure phy pads */
> -	val = PHY_PAD_RXSEL_3V3;
> -	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLUP);
> -	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
> -	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
> -	sdhci_writew(host, val, PHY_CMDPAD_CNFG_R);
> -	sdhci_writew(host, val, PHY_DATAPAD_CNFG_R);
> -	sdhci_writew(host, val, PHY_RSTNPAD_CNFG_R);
> -
> -	val = FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
> -	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
> -	sdhci_writew(host, val, PHY_CLKPAD_CNFG_R);
> -
> -	val = PHY_PAD_RXSEL_3V3;
> -	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLDOWN);
> -	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
> -	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
> -	sdhci_writew(host, val, PHY_STBPAD_CNFG_R);
> +		sdhci_writeb(host, sel, PHY_DLLDL_CNFG_R);
> +	}
>  
>  	/* enable phy dll */
>  	sdhci_writeb(host, PHY_DLL_CTRL_ENABLE, PHY_DLL_CTRL_R);
> +
>  }
>  
>  static void th1520_sdhci_set_phy(struct sdhci_host *host)
> @@ -433,11 +395,7 @@ static void th1520_sdhci_set_phy(struct sdhci_host *host)
>  	u32 emmc_caps = MMC_CAP2_NO_SD | MMC_CAP2_NO_SDIO;
>  	u16 emmc_ctrl;
>  
> -	/* Before power on, set PHY configs */
> -	if (priv->flags & FLAG_IO_FIXED_1V8)
> -		dwcmshc_phy_1_8v_init(host);
> -	else
> -		dwcmshc_phy_3_3v_init(host);
> +	dwcmshc_phy_init(host);
>  
>  	if ((host->mmc->caps2 & emmc_caps) == emmc_caps) {
>  		emmc_ctrl = sdhci_readw(host, priv->vendor_specific_area1 + DWCMSHC_EMMC_CONTROL);
> @@ -1163,7 +1121,7 @@ static const struct sdhci_ops sdhci_dwcmshc_th1520_ops = {
>  	.get_max_clock		= dwcmshc_get_max_clock,
>  	.reset			= th1520_sdhci_reset,
>  	.adma_write_desc	= dwcmshc_adma_write_desc,
> -	.voltage_switch		= dwcmshc_phy_1_8v_init,
> +	.voltage_switch		= dwcmshc_phy_init,
>  	.platform_execute_tuning = th1520_execute_tuning,
>  };
>
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 7ea3da45db32..09b9ab15e499 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -328,12 +328,17 @@  static void dwcmshc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	sdhci_request(mmc, mrq);
 }
 
-static void dwcmshc_phy_1_8v_init(struct sdhci_host *host)
+static void dwcmshc_phy_init(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
+	u32 rxsel = PHY_PAD_RXSEL_3V3;
 	u32 val;
 
+	if (priv->flags & FLAG_IO_FIXED_1V8 ||
+		host->mmc->ios.timing & MMC_SIGNAL_VOLTAGE_180)
+		rxsel = PHY_PAD_RXSEL_1V8;
+
 	/* deassert phy reset & set tx drive strength */
 	val = PHY_CNFG_RSTN_DEASSERT;
 	val |= FIELD_PREP(PHY_CNFG_PAD_SP_MASK, PHY_CNFG_PAD_SP);
@@ -353,7 +358,7 @@  static void dwcmshc_phy_1_8v_init(struct sdhci_host *host)
 	sdhci_writeb(host, val, PHY_SDCLKDL_CNFG_R);
 
 	/* configure phy pads */
-	val = PHY_PAD_RXSEL_1V8;
+	val = rxsel;
 	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLUP);
 	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
 	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
@@ -365,65 +370,22 @@  static void dwcmshc_phy_1_8v_init(struct sdhci_host *host)
 	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
 	sdhci_writew(host, val, PHY_CLKPAD_CNFG_R);
 
-	val = PHY_PAD_RXSEL_1V8;
+	val = rxsel;
 	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLDOWN);
 	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
 	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
 	sdhci_writew(host, val, PHY_STBPAD_CNFG_R);
 
 	/* enable data strobe mode */
-	sdhci_writeb(host, FIELD_PREP(PHY_DLLDL_CNFG_SLV_INPSEL_MASK, PHY_DLLDL_CNFG_SLV_INPSEL),
-		     PHY_DLLDL_CNFG_R);
-
-	/* enable phy dll */
-	sdhci_writeb(host, PHY_DLL_CTRL_ENABLE, PHY_DLL_CTRL_R);
-}
-
-static void dwcmshc_phy_3_3v_init(struct sdhci_host *host)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
-	u32 val;
-
-	/* deassert phy reset & set tx drive strength */
-	val = PHY_CNFG_RSTN_DEASSERT;
-	val |= FIELD_PREP(PHY_CNFG_PAD_SP_MASK, PHY_CNFG_PAD_SP);
-	val |= FIELD_PREP(PHY_CNFG_PAD_SN_MASK, PHY_CNFG_PAD_SN);
-	sdhci_writel(host, val, PHY_CNFG_R);
-
-	/* disable delay line */
-	sdhci_writeb(host, PHY_SDCLKDL_CNFG_UPDATE, PHY_SDCLKDL_CNFG_R);
-
-	/* set delay line */
-	sdhci_writeb(host, priv->delay_line, PHY_SDCLKDL_DC_R);
-	sdhci_writeb(host, PHY_DLL_CNFG2_JUMPSTEP, PHY_DLL_CNFG2_R);
-
-	/* enable delay lane */
-	val = sdhci_readb(host, PHY_SDCLKDL_CNFG_R);
-	val &= ~(PHY_SDCLKDL_CNFG_UPDATE);
-	sdhci_writeb(host, val, PHY_SDCLKDL_CNFG_R);
+	if (rxsel == PHY_PAD_RXSEL_1V8) {
+		u8 sel = FIELD_PREP(PHY_DLLDL_CNFG_SLV_INPSEL_MASK, PHY_DLLDL_CNFG_SLV_INPSEL);
 
-	/* configure phy pads */
-	val = PHY_PAD_RXSEL_3V3;
-	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLUP);
-	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
-	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
-	sdhci_writew(host, val, PHY_CMDPAD_CNFG_R);
-	sdhci_writew(host, val, PHY_DATAPAD_CNFG_R);
-	sdhci_writew(host, val, PHY_RSTNPAD_CNFG_R);
-
-	val = FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
-	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
-	sdhci_writew(host, val, PHY_CLKPAD_CNFG_R);
-
-	val = PHY_PAD_RXSEL_3V3;
-	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLDOWN);
-	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
-	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N);
-	sdhci_writew(host, val, PHY_STBPAD_CNFG_R);
+		sdhci_writeb(host, sel, PHY_DLLDL_CNFG_R);
+	}
 
 	/* enable phy dll */
 	sdhci_writeb(host, PHY_DLL_CTRL_ENABLE, PHY_DLL_CTRL_R);
+
 }
 
 static void th1520_sdhci_set_phy(struct sdhci_host *host)
@@ -433,11 +395,7 @@  static void th1520_sdhci_set_phy(struct sdhci_host *host)
 	u32 emmc_caps = MMC_CAP2_NO_SD | MMC_CAP2_NO_SDIO;
 	u16 emmc_ctrl;
 
-	/* Before power on, set PHY configs */
-	if (priv->flags & FLAG_IO_FIXED_1V8)
-		dwcmshc_phy_1_8v_init(host);
-	else
-		dwcmshc_phy_3_3v_init(host);
+	dwcmshc_phy_init(host);
 
 	if ((host->mmc->caps2 & emmc_caps) == emmc_caps) {
 		emmc_ctrl = sdhci_readw(host, priv->vendor_specific_area1 + DWCMSHC_EMMC_CONTROL);
@@ -1163,7 +1121,7 @@  static const struct sdhci_ops sdhci_dwcmshc_th1520_ops = {
 	.get_max_clock		= dwcmshc_get_max_clock,
 	.reset			= th1520_sdhci_reset,
 	.adma_write_desc	= dwcmshc_adma_write_desc,
-	.voltage_switch		= dwcmshc_phy_1_8v_init,
+	.voltage_switch		= dwcmshc_phy_init,
 	.platform_execute_tuning = th1520_execute_tuning,
 };