* [PATCH 2/2] mmc: sdhci-pci-gli: GL9767: Fix initializing the UHS-II interface during a power-on
@ 2025-09-01 9:42 Ben Chuang
2025-09-01 17:01 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Ben Chuang @ 2025-09-01 9:42 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson
Cc: victor.shih, ben.chuang, HL.Liu, SeanHY.Chen, benchuanggli,
victorshihgli, linux-mmc, linux-kernel, stable
From: Ben Chuang <ben.chuang@genesyslogic.com.tw>
According to the power structure of IC hardware design for UHS-II
interface, reset control and timing must be added to the initialization
process of powering on the UHS-II interface.
Fixes: 27dd3b82557a ("mmc: sdhci-pci-gli: enable UHS-II mode for GL9767")
Cc: stable@vger.kernel.org # v6.13+
Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw>
---
drivers/mmc/host/sdhci-pci-gli.c | 71 +++++++++++++++++++++++++++++++-
1 file changed, 70 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
index 3a1de477e9af..85d0d7e6169c 100644
--- a/drivers/mmc/host/sdhci-pci-gli.c
+++ b/drivers/mmc/host/sdhci-pci-gli.c
@@ -283,6 +283,8 @@
#define PCIE_GLI_9767_UHS2_CTL2_ZC_VALUE 0xb
#define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL BIT(6)
#define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL_VALUE 0x1
+#define PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN BIT(13)
+#define PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE BIT(14)
#define GLI_MAX_TUNING_LOOP 40
@@ -1179,6 +1181,69 @@ static void gl9767_set_low_power_negotiation(struct pci_dev *pdev, bool enable)
gl9767_vhs_read(pdev);
}
+static void sdhci_gl9767_uhs2_phy_reset_assert(struct sdhci_host *host)
+{
+ struct sdhci_pci_slot *slot = sdhci_priv(host);
+ struct pci_dev *pdev = slot->chip->pdev;
+ u32 value;
+
+ gl9767_vhs_write(pdev);
+ pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value);
+ value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN;
+ pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value);
+ value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE;
+ pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value);
+ gl9767_vhs_read(pdev);
+}
+
+static void sdhci_gl9767_uhs2_phy_reset_deassert(struct sdhci_host *host)
+{
+ struct sdhci_pci_slot *slot = sdhci_priv(host);
+ struct pci_dev *pdev = slot->chip->pdev;
+ u32 value;
+
+ gl9767_vhs_write(pdev);
+ pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value);
+ value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE;
+ pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value);
+ value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN;
+ pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value);
+ gl9767_vhs_read(pdev);
+}
+
+static void __gl9767_uhs2_set_power(struct sdhci_host *host, unsigned char mode, unsigned short vdd)
+{
+ u8 pwr = 0;
+
+ if (mode != MMC_POWER_OFF) {
+ pwr = sdhci_get_vdd_value(vdd);
+ if (!pwr)
+ WARN(1, "%s: Invalid vdd %#x\n",
+ mmc_hostname(host->mmc), vdd);
+ pwr |= SDHCI_VDD2_POWER_180;
+ }
+
+ if (host->pwr == pwr)
+ return;
+
+ host->pwr = pwr;
+
+ if (pwr == 0) {
+ sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
+ } else {
+ sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
+
+ pwr |= SDHCI_POWER_ON;
+ sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL);
+ mdelay(5);
+
+ sdhci_gl9767_uhs2_phy_reset_assert(host);
+ pwr |= SDHCI_VDD2_POWER_ON;
+ sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
+ mdelay(5);
+ }
+}
+
static void sdhci_gl9767_set_clock(struct sdhci_host *host, unsigned int clock)
{
struct sdhci_pci_slot *slot = sdhci_priv(host);
@@ -1205,6 +1270,10 @@ static void sdhci_gl9767_set_clock(struct sdhci_host *host, unsigned int clock)
}
sdhci_enable_clk(host, clk);
+
+ if (mmc_card_uhs2(host->mmc))
+ sdhci_gl9767_uhs2_phy_reset_deassert(host);
+
gl9767_set_low_power_negotiation(pdev, true);
}
@@ -1476,7 +1545,7 @@ static void sdhci_gl9767_set_power(struct sdhci_host *host, unsigned char mode,
gl9767_vhs_read(pdev);
sdhci_gli_overcurrent_event_enable(host, false);
- sdhci_uhs2_set_power(host, mode, vdd);
+ __gl9767_uhs2_set_power(host, mode, vdd);
sdhci_gli_overcurrent_event_enable(host, true);
} else {
gl9767_vhs_write(pdev);
--
2.51.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] mmc: sdhci-pci-gli: GL9767: Fix initializing the UHS-II interface during a power-on 2025-09-01 9:42 [PATCH 2/2] mmc: sdhci-pci-gli: GL9767: Fix initializing the UHS-II interface during a power-on Ben Chuang @ 2025-09-01 17:01 ` Adrian Hunter 2025-09-02 6:33 ` Ben Chuang 0 siblings, 1 reply; 6+ messages in thread From: Adrian Hunter @ 2025-09-01 17:01 UTC (permalink / raw) To: Ben Chuang, ulf.hansson Cc: victor.shih, ben.chuang, HL.Liu, SeanHY.Chen, victorshihgli, linux-mmc, linux-kernel, stable On 01/09/2025 12:42, Ben Chuang wrote: > From: Ben Chuang <ben.chuang@genesyslogic.com.tw> > > According to the power structure of IC hardware design for UHS-II > interface, reset control and timing must be added to the initialization > process of powering on the UHS-II interface. > > Fixes: 27dd3b82557a ("mmc: sdhci-pci-gli: enable UHS-II mode for GL9767") > Cc: stable@vger.kernel.org # v6.13+ > Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw> > --- > drivers/mmc/host/sdhci-pci-gli.c | 71 +++++++++++++++++++++++++++++++- > 1 file changed, 70 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > index 3a1de477e9af..85d0d7e6169c 100644 > --- a/drivers/mmc/host/sdhci-pci-gli.c > +++ b/drivers/mmc/host/sdhci-pci-gli.c > @@ -283,6 +283,8 @@ > #define PCIE_GLI_9767_UHS2_CTL2_ZC_VALUE 0xb > #define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL BIT(6) > #define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL_VALUE 0x1 > +#define PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN BIT(13) > +#define PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE BIT(14) > > #define GLI_MAX_TUNING_LOOP 40 > > @@ -1179,6 +1181,69 @@ static void gl9767_set_low_power_negotiation(struct pci_dev *pdev, bool enable) > gl9767_vhs_read(pdev); > } > > +static void sdhci_gl9767_uhs2_phy_reset_assert(struct sdhci_host *host) > +{ > + struct sdhci_pci_slot *slot = sdhci_priv(host); > + struct pci_dev *pdev = slot->chip->pdev; > + u32 value; > + > + gl9767_vhs_write(pdev); > + pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > + value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > + value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > + gl9767_vhs_read(pdev); > +} > + > +static void sdhci_gl9767_uhs2_phy_reset_deassert(struct sdhci_host *host) > +{ > + struct sdhci_pci_slot *slot = sdhci_priv(host); > + struct pci_dev *pdev = slot->chip->pdev; > + u32 value; > + > + gl9767_vhs_write(pdev); > + pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > + value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; Maybe add a small comment about PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE and PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN being updated separately. > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > + value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > + gl9767_vhs_read(pdev); > +} sdhci_gl9767_uhs2_phy_reset_assert() and sdhci_gl9767_uhs2_phy_reset_deassert() are fairly similar. Maybe consider: static void sdhci_gl9767_uhs2_phy_reset(struct sdhci_host *host, bool assert) { struct sdhci_pci_slot *slot = sdhci_priv(host); struct pci_dev *pdev = slot->chip->pdev; u32 value, set, clr; if (assert) { set = PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; clr = PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; } else { set = PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; clr = PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; } gl9767_vhs_write(pdev); pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); value |= set; pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); value &= ~clr; pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); gl9767_vhs_read(pdev); } > + > +static void __gl9767_uhs2_set_power(struct sdhci_host *host, unsigned char mode, unsigned short vdd) > +{ > + u8 pwr = 0; > + > + if (mode != MMC_POWER_OFF) { > + pwr = sdhci_get_vdd_value(vdd); > + if (!pwr) > + WARN(1, "%s: Invalid vdd %#x\n", > + mmc_hostname(host->mmc), vdd); > + pwr |= SDHCI_VDD2_POWER_180; > + } > + > + if (host->pwr == pwr) > + return; > + > + host->pwr = pwr; > + > + if (pwr == 0) { > + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > + } else { > + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > + > + pwr |= SDHCI_POWER_ON; > + sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL); > + mdelay(5); Can be mmc_delay(5) > + > + sdhci_gl9767_uhs2_phy_reset_assert(host); > + pwr |= SDHCI_VDD2_POWER_ON; > + sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL); > + mdelay(5); Can be mmc_delay(5) > + } > +} > + > static void sdhci_gl9767_set_clock(struct sdhci_host *host, unsigned int clock) > { > struct sdhci_pci_slot *slot = sdhci_priv(host); > @@ -1205,6 +1270,10 @@ static void sdhci_gl9767_set_clock(struct sdhci_host *host, unsigned int clock) > } > > sdhci_enable_clk(host, clk); > + > + if (mmc_card_uhs2(host->mmc)) > + sdhci_gl9767_uhs2_phy_reset_deassert(host); > + > gl9767_set_low_power_negotiation(pdev, true); > } > > @@ -1476,7 +1545,7 @@ static void sdhci_gl9767_set_power(struct sdhci_host *host, unsigned char mode, > gl9767_vhs_read(pdev); > > sdhci_gli_overcurrent_event_enable(host, false); > - sdhci_uhs2_set_power(host, mode, vdd); > + __gl9767_uhs2_set_power(host, mode, vdd); > sdhci_gli_overcurrent_event_enable(host, true); > } else { > gl9767_vhs_write(pdev); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci-pci-gli: GL9767: Fix initializing the UHS-II interface during a power-on 2025-09-01 17:01 ` Adrian Hunter @ 2025-09-02 6:33 ` Ben Chuang 2025-09-02 7:12 ` Ben Chuang 0 siblings, 1 reply; 6+ messages in thread From: Ben Chuang @ 2025-09-02 6:33 UTC (permalink / raw) To: Adrian Hunter Cc: ulf.hansson, victor.shih, ben.chuang, HL.Liu, SeanHY.Chen, linux-mmc, linux-kernel, stable On Tue, Sep 2, 2025 at 1:02 AM Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 01/09/2025 12:42, Ben Chuang wrote: > > From: Ben Chuang <ben.chuang@genesyslogic.com.tw> > > > > According to the power structure of IC hardware design for UHS-II > > interface, reset control and timing must be added to the initialization > > process of powering on the UHS-II interface. > > > > Fixes: 27dd3b82557a ("mmc: sdhci-pci-gli: enable UHS-II mode for GL9767") > > Cc: stable@vger.kernel.org # v6.13+ > > Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw> > > --- > > drivers/mmc/host/sdhci-pci-gli.c | 71 +++++++++++++++++++++++++++++++- > > 1 file changed, 70 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > > index 3a1de477e9af..85d0d7e6169c 100644 > > --- a/drivers/mmc/host/sdhci-pci-gli.c > > +++ b/drivers/mmc/host/sdhci-pci-gli.c > > @@ -283,6 +283,8 @@ > > #define PCIE_GLI_9767_UHS2_CTL2_ZC_VALUE 0xb > > #define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL BIT(6) > > #define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL_VALUE 0x1 > > +#define PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN BIT(13) > > +#define PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE BIT(14) > > > > #define GLI_MAX_TUNING_LOOP 40 > > > > @@ -1179,6 +1181,69 @@ static void gl9767_set_low_power_negotiation(struct pci_dev *pdev, bool enable) > > gl9767_vhs_read(pdev); > > } > > > > +static void sdhci_gl9767_uhs2_phy_reset_assert(struct sdhci_host *host) > > +{ > > + struct sdhci_pci_slot *slot = sdhci_priv(host); > > + struct pci_dev *pdev = slot->chip->pdev; > > + u32 value; > > + > > + gl9767_vhs_write(pdev); > > + pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > > + value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > > + value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > > + gl9767_vhs_read(pdev); > > +} > > + > > +static void sdhci_gl9767_uhs2_phy_reset_deassert(struct sdhci_host *host) > > +{ > > + struct sdhci_pci_slot *slot = sdhci_priv(host); > > + struct pci_dev *pdev = slot->chip->pdev; > > + u32 value; > > + > > + gl9767_vhs_write(pdev); > > + pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > > + value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > > Maybe add a small comment about PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE > and PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN being updated separately. > > > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > > + value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > > + gl9767_vhs_read(pdev); > > +} > > sdhci_gl9767_uhs2_phy_reset_assert() and sdhci_gl9767_uhs2_phy_reset_deassert() > are fairly similar. Maybe consider: > > static void sdhci_gl9767_uhs2_phy_reset(struct sdhci_host *host, bool assert) > { > struct sdhci_pci_slot *slot = sdhci_priv(host); > struct pci_dev *pdev = slot->chip->pdev; > u32 value, set, clr; > > if (assert) { > set = PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > clr = PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > } else { > set = PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > clr = PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > } > > gl9767_vhs_write(pdev); > pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > value |= set; > pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > value &= ~clr; > pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > gl9767_vhs_read(pdev); > } > OK, I will update it. Thank you. > > > + > > +static void __gl9767_uhs2_set_power(struct sdhci_host *host, unsigned char mode, unsigned short vdd) > > +{ > > + u8 pwr = 0; > > + > > + if (mode != MMC_POWER_OFF) { > > + pwr = sdhci_get_vdd_value(vdd); > > + if (!pwr) > > + WARN(1, "%s: Invalid vdd %#x\n", > > + mmc_hostname(host->mmc), vdd); > > + pwr |= SDHCI_VDD2_POWER_180; > > + } > > + > > + if (host->pwr == pwr) > > + return; > > + > > + host->pwr = pwr; > > + > > + if (pwr == 0) { > > + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > > + } else { > > + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > > + > > + pwr |= SDHCI_POWER_ON; > > + sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL); > > + mdelay(5); > > Can be mmc_delay(5) > > > + > > + sdhci_gl9767_uhs2_phy_reset_assert(host); > > + pwr |= SDHCI_VDD2_POWER_ON; > > + sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL); > > + mdelay(5); > > Can be mmc_delay(5) I may not modify it now. mmc_delay() is in "drivers/mmc/core/core.h". If sdhci-pci-gli.c only includes "../core/core.h", the compiler will report some errors. <snippet> IIn file included from host/sdhci-pci-gli.c:17: host/../core/core.h:131:52: warning: ‘struct mmc_ctx’ declared inside parameter list will not be visible outside of this definition or declaration 131 | int __mmc_claim_host(struct mmc_host *host, struct mmc_ctx *ctx, | ^~~~~~~ host/../core/core.h:134:49: warning: ‘struct mmc_ctx’ declared inside parameter list will not be visible outside of this definition or declaration 134 | void mmc_get_card(struct mmc_card *card, struct mmc_ctx *ctx); | ^~~~~~~ host/../core/core.h:135:49: warning: ‘struct mmc_ctx’ declared inside parameter list will not be visible outside of this definition or declaration 135 | void mmc_put_card(struct mmc_card *card, struct mmc_ctx *ctx); | ^~~~~~~ host/../core/core.h: In function ‘mmc_pre_req’: host/../core/core.h:165:17: error: invalid use of undefined type ‘struct mmc_host’ 165 | if (host->ops->pre_req) | ^~ > > > + } > > +} > > + > > static void sdhci_gl9767_set_clock(struct sdhci_host *host, unsigned int clock) > > { > > struct sdhci_pci_slot *slot = sdhci_priv(host); > > @@ -1205,6 +1270,10 @@ static void sdhci_gl9767_set_clock(struct sdhci_host *host, unsigned int clock) > > } > > > > sdhci_enable_clk(host, clk); > > + > > + if (mmc_card_uhs2(host->mmc)) > > + sdhci_gl9767_uhs2_phy_reset_deassert(host); > > + > > gl9767_set_low_power_negotiation(pdev, true); > > } > > > > @@ -1476,7 +1545,7 @@ static void sdhci_gl9767_set_power(struct sdhci_host *host, unsigned char mode, > > gl9767_vhs_read(pdev); > > > > sdhci_gli_overcurrent_event_enable(host, false); > > - sdhci_uhs2_set_power(host, mode, vdd); > > + __gl9767_uhs2_set_power(host, mode, vdd); > > sdhci_gli_overcurrent_event_enable(host, true); > > } else { > > gl9767_vhs_write(pdev); > Best regards, Ben Chuang ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci-pci-gli: GL9767: Fix initializing the UHS-II interface during a power-on 2025-09-02 6:33 ` Ben Chuang @ 2025-09-02 7:12 ` Ben Chuang 2025-09-02 7:47 ` Adrian Hunter 0 siblings, 1 reply; 6+ messages in thread From: Ben Chuang @ 2025-09-02 7:12 UTC (permalink / raw) To: Adrian Hunter Cc: ulf.hansson, victor.shih, ben.chuang, HL.Liu, SeanHY.Chen, linux-mmc, linux-kernel, stable On Tue, Sep 2, 2025 at 2:33 PM Ben Chuang <benchuanggli@gmail.com> wrote: > > On Tue, Sep 2, 2025 at 1:02 AM Adrian Hunter <adrian.hunter@intel.com> wrote: > > > > On 01/09/2025 12:42, Ben Chuang wrote: > > > From: Ben Chuang <ben.chuang@genesyslogic.com.tw> > > > > > > According to the power structure of IC hardware design for UHS-II > > > interface, reset control and timing must be added to the initialization > > > process of powering on the UHS-II interface. > > > > > > Fixes: 27dd3b82557a ("mmc: sdhci-pci-gli: enable UHS-II mode for GL9767") > > > Cc: stable@vger.kernel.org # v6.13+ > > > Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw> > > > --- > > > drivers/mmc/host/sdhci-pci-gli.c | 71 +++++++++++++++++++++++++++++++- > > > 1 file changed, 70 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > > > index 3a1de477e9af..85d0d7e6169c 100644 > > > --- a/drivers/mmc/host/sdhci-pci-gli.c > > > +++ b/drivers/mmc/host/sdhci-pci-gli.c > > > @@ -283,6 +283,8 @@ > > > #define PCIE_GLI_9767_UHS2_CTL2_ZC_VALUE 0xb > > > #define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL BIT(6) > > > #define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL_VALUE 0x1 > > > +#define PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN BIT(13) > > > +#define PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE BIT(14) > > > > > > #define GLI_MAX_TUNING_LOOP 40 > > > > > > @@ -1179,6 +1181,69 @@ static void gl9767_set_low_power_negotiation(struct pci_dev *pdev, bool enable) > > > gl9767_vhs_read(pdev); > > > } > > > > > > +static void sdhci_gl9767_uhs2_phy_reset_assert(struct sdhci_host *host) > > > +{ > > > + struct sdhci_pci_slot *slot = sdhci_priv(host); > > > + struct pci_dev *pdev = slot->chip->pdev; > > > + u32 value; > > > + > > > + gl9767_vhs_write(pdev); > > > + pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > > > + value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > > > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > > > + value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > > > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > > > + gl9767_vhs_read(pdev); > > > +} > > > + > > > +static void sdhci_gl9767_uhs2_phy_reset_deassert(struct sdhci_host *host) > > > +{ > > > + struct sdhci_pci_slot *slot = sdhci_priv(host); > > > + struct pci_dev *pdev = slot->chip->pdev; > > > + u32 value; > > > + > > > + gl9767_vhs_write(pdev); > > > + pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > > > + value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > > > > Maybe add a small comment about PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE > > and PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN being updated separately. > > > > > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > > > + value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > > > + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > > > + gl9767_vhs_read(pdev); > > > +} > > > > sdhci_gl9767_uhs2_phy_reset_assert() and sdhci_gl9767_uhs2_phy_reset_deassert() > > are fairly similar. Maybe consider: > > > > static void sdhci_gl9767_uhs2_phy_reset(struct sdhci_host *host, bool assert) > > { > > struct sdhci_pci_slot *slot = sdhci_priv(host); > > struct pci_dev *pdev = slot->chip->pdev; > > u32 value, set, clr; > > > > if (assert) { > > set = PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > > clr = PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > > } else { > > set = PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > > clr = PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > > } > > > > gl9767_vhs_write(pdev); > > pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > > value |= set; > > pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > > value &= ~clr; > > pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > > gl9767_vhs_read(pdev); > > } > > > > OK, I will update it. Thank you. > > > > > > + > > > +static void __gl9767_uhs2_set_power(struct sdhci_host *host, unsigned char mode, unsigned short vdd) > > > +{ > > > + u8 pwr = 0; > > > + > > > + if (mode != MMC_POWER_OFF) { > > > + pwr = sdhci_get_vdd_value(vdd); > > > + if (!pwr) > > > + WARN(1, "%s: Invalid vdd %#x\n", > > > + mmc_hostname(host->mmc), vdd); > > > + pwr |= SDHCI_VDD2_POWER_180; > > > + } > > > + > > > + if (host->pwr == pwr) > > > + return; > > > + > > > + host->pwr = pwr; > > > + > > > + if (pwr == 0) { > > > + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > > > + } else { > > > + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > > > + > > > + pwr |= SDHCI_POWER_ON; > > > + sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL); > > > + mdelay(5); > > > > Can be mmc_delay(5) > > > > > + > > > + sdhci_gl9767_uhs2_phy_reset_assert(host); > > > + pwr |= SDHCI_VDD2_POWER_ON; > > > + sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL); > > > + mdelay(5); > > > > Can be mmc_delay(5) > > I may not modify it now. > mmc_delay() is in "drivers/mmc/core/core.h". > If sdhci-pci-gli.c only includes "../core/core.h", the compiler will > report some errors. Ah, just add another headersand it will build. --- a/drivers/mmc/host/sdhci-pci-gli.c +++ b/drivers/mmc/host/sdhci-pci-gli.c @@ -14,6 +14,8 @@ #include <linux/delay.h> #include <linux/of.h> #include <linux/iopoll.h> +#include <linux/mmc/host.h> +#include "../core/core.h" #include "sdhci.h" #include "sdhci-cqhci.h" #include "sdhci-pci.h" @@ -968,10 +970,10 @@ static void gl9755_set_power(struct sdhci_host *host, unsigned char mode, sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL); /* wait stable */ - mdelay(5); + mmc_delay(5); sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL); /* wait stable */ - mdelay(5); + mmc_delay(5); sdhci_gli_overcurrent_event_enable(host, true); } > > <snippet> > IIn file included from host/sdhci-pci-gli.c:17: > host/../core/core.h:131:52: warning: ‘struct mmc_ctx’ declared inside > parameter list will not be visible outside of this definition or > declaration > 131 | int __mmc_claim_host(struct mmc_host *host, struct mmc_ctx *ctx, > | ^~~~~~~ > host/../core/core.h:134:49: warning: ‘struct mmc_ctx’ declared inside > parameter list will not be visible outside of this definition or > declaration > 134 | void mmc_get_card(struct mmc_card *card, struct mmc_ctx *ctx); > | ^~~~~~~ > host/../core/core.h:135:49: warning: ‘struct mmc_ctx’ declared inside > parameter list will not be visible outside of this definition or > declaration > 135 | void mmc_put_card(struct mmc_card *card, struct mmc_ctx *ctx); > | ^~~~~~~ > host/../core/core.h: In function ‘mmc_pre_req’: > host/../core/core.h:165:17: error: invalid use of undefined type > ‘struct mmc_host’ > 165 | if (host->ops->pre_req) > | ^~ > > > > > > + } > > > +} > > > + > > > static void sdhci_gl9767_set_clock(struct sdhci_host *host, unsigned int clock) > > > { > > > struct sdhci_pci_slot *slot = sdhci_priv(host); > > > @@ -1205,6 +1270,10 @@ static void sdhci_gl9767_set_clock(struct sdhci_host *host, unsigned int clock) > > > } > > > > > > sdhci_enable_clk(host, clk); > > > + > > > + if (mmc_card_uhs2(host->mmc)) > > > + sdhci_gl9767_uhs2_phy_reset_deassert(host); > > > + > > > gl9767_set_low_power_negotiation(pdev, true); > > > } > > > > > > @@ -1476,7 +1545,7 @@ static void sdhci_gl9767_set_power(struct sdhci_host *host, unsigned char mode, > > > gl9767_vhs_read(pdev); > > > > > > sdhci_gli_overcurrent_event_enable(host, false); > > > - sdhci_uhs2_set_power(host, mode, vdd); > > > + __gl9767_uhs2_set_power(host, mode, vdd); > > > sdhci_gli_overcurrent_event_enable(host, true); > > > } else { > > > gl9767_vhs_write(pdev); > > > > Best regards, > Ben Chuang ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci-pci-gli: GL9767: Fix initializing the UHS-II interface during a power-on 2025-09-02 7:12 ` Ben Chuang @ 2025-09-02 7:47 ` Adrian Hunter 2025-09-02 8:54 ` Ben Chuang 0 siblings, 1 reply; 6+ messages in thread From: Adrian Hunter @ 2025-09-02 7:47 UTC (permalink / raw) To: Ben Chuang Cc: ulf.hansson, victor.shih, ben.chuang, HL.Liu, SeanHY.Chen, linux-mmc, linux-kernel, stable On 02/09/2025 10:12, Ben Chuang wrote: > On Tue, Sep 2, 2025 at 2:33 PM Ben Chuang <benchuanggli@gmail.com> wrote: >> >> On Tue, Sep 2, 2025 at 1:02 AM Adrian Hunter <adrian.hunter@intel.com> wrote: >>> >>> On 01/09/2025 12:42, Ben Chuang wrote: >>>> From: Ben Chuang <ben.chuang@genesyslogic.com.tw> >>>> >>>> According to the power structure of IC hardware design for UHS-II >>>> interface, reset control and timing must be added to the initialization >>>> process of powering on the UHS-II interface. >>>> >>>> Fixes: 27dd3b82557a ("mmc: sdhci-pci-gli: enable UHS-II mode for GL9767") >>>> Cc: stable@vger.kernel.org # v6.13+ >>>> Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw> >>>> --- >>>> drivers/mmc/host/sdhci-pci-gli.c | 71 +++++++++++++++++++++++++++++++- >>>> 1 file changed, 70 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c >>>> index 3a1de477e9af..85d0d7e6169c 100644 >>>> --- a/drivers/mmc/host/sdhci-pci-gli.c >>>> +++ b/drivers/mmc/host/sdhci-pci-gli.c >>>> @@ -283,6 +283,8 @@ >>>> #define PCIE_GLI_9767_UHS2_CTL2_ZC_VALUE 0xb >>>> #define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL BIT(6) >>>> #define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL_VALUE 0x1 >>>> +#define PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN BIT(13) >>>> +#define PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE BIT(14) >>>> >>>> #define GLI_MAX_TUNING_LOOP 40 >>>> >>>> @@ -1179,6 +1181,69 @@ static void gl9767_set_low_power_negotiation(struct pci_dev *pdev, bool enable) >>>> gl9767_vhs_read(pdev); >>>> } >>>> >>>> +static void sdhci_gl9767_uhs2_phy_reset_assert(struct sdhci_host *host) >>>> +{ >>>> + struct sdhci_pci_slot *slot = sdhci_priv(host); >>>> + struct pci_dev *pdev = slot->chip->pdev; >>>> + u32 value; >>>> + >>>> + gl9767_vhs_write(pdev); >>>> + pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); >>>> + value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; >>>> + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); >>>> + value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; >>>> + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); >>>> + gl9767_vhs_read(pdev); >>>> +} >>>> + >>>> +static void sdhci_gl9767_uhs2_phy_reset_deassert(struct sdhci_host *host) >>>> +{ >>>> + struct sdhci_pci_slot *slot = sdhci_priv(host); >>>> + struct pci_dev *pdev = slot->chip->pdev; >>>> + u32 value; >>>> + >>>> + gl9767_vhs_write(pdev); >>>> + pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); >>>> + value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; >>> >>> Maybe add a small comment about PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE >>> and PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN being updated separately. >>> >>>> + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); >>>> + value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; >>>> + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); >>>> + gl9767_vhs_read(pdev); >>>> +} >>> >>> sdhci_gl9767_uhs2_phy_reset_assert() and sdhci_gl9767_uhs2_phy_reset_deassert() >>> are fairly similar. Maybe consider: >>> >>> static void sdhci_gl9767_uhs2_phy_reset(struct sdhci_host *host, bool assert) >>> { >>> struct sdhci_pci_slot *slot = sdhci_priv(host); >>> struct pci_dev *pdev = slot->chip->pdev; >>> u32 value, set, clr; >>> >>> if (assert) { >>> set = PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; >>> clr = PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; >>> } else { >>> set = PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; >>> clr = PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; >>> } >>> >>> gl9767_vhs_write(pdev); >>> pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); >>> value |= set; >>> pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); >>> value &= ~clr; >>> pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); >>> gl9767_vhs_read(pdev); >>> } >>> >> >> OK, I will update it. Thank you. >> >>> >>>> + >>>> +static void __gl9767_uhs2_set_power(struct sdhci_host *host, unsigned char mode, unsigned short vdd) >>>> +{ >>>> + u8 pwr = 0; >>>> + >>>> + if (mode != MMC_POWER_OFF) { >>>> + pwr = sdhci_get_vdd_value(vdd); >>>> + if (!pwr) >>>> + WARN(1, "%s: Invalid vdd %#x\n", >>>> + mmc_hostname(host->mmc), vdd); >>>> + pwr |= SDHCI_VDD2_POWER_180; >>>> + } >>>> + >>>> + if (host->pwr == pwr) >>>> + return; >>>> + >>>> + host->pwr = pwr; >>>> + >>>> + if (pwr == 0) { >>>> + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); >>>> + } else { >>>> + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); >>>> + >>>> + pwr |= SDHCI_POWER_ON; >>>> + sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL); >>>> + mdelay(5); >>> >>> Can be mmc_delay(5) >>> >>>> + >>>> + sdhci_gl9767_uhs2_phy_reset_assert(host); >>>> + pwr |= SDHCI_VDD2_POWER_ON; >>>> + sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL); >>>> + mdelay(5); >>> >>> Can be mmc_delay(5) >> >> I may not modify it now. >> mmc_delay() is in "drivers/mmc/core/core.h". >> If sdhci-pci-gli.c only includes "../core/core.h", the compiler will >> report some errors. > > Ah, just add another headersand it will build. > > --- a/drivers/mmc/host/sdhci-pci-gli.c > +++ b/drivers/mmc/host/sdhci-pci-gli.c > @@ -14,6 +14,8 @@ > #include <linux/delay.h> > #include <linux/of.h> > #include <linux/iopoll.h> > +#include <linux/mmc/host.h> > +#include "../core/core.h" > #include "sdhci.h" > #include "sdhci-cqhci.h" > #include "sdhci-pci.h" > @@ -968,10 +970,10 @@ static void gl9755_set_power(struct sdhci_host > *host, unsigned char mode, > > sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL); > /* wait stable */ > - mdelay(5); > + mmc_delay(5); > sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL); > /* wait stable */ > - mdelay(5); > + mmc_delay(5); > sdhci_gli_overcurrent_event_enable(host, true); > } It seems mmc_delay() is for core mmc code only, not host drivers. But the issue with mdelay is that it does not sleep. Other options are msleep() or usleep_range(). ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci-pci-gli: GL9767: Fix initializing the UHS-II interface during a power-on 2025-09-02 7:47 ` Adrian Hunter @ 2025-09-02 8:54 ` Ben Chuang 0 siblings, 0 replies; 6+ messages in thread From: Ben Chuang @ 2025-09-02 8:54 UTC (permalink / raw) To: Adrian Hunter Cc: ulf.hansson, victor.shih, ben.chuang, HL.Liu, SeanHY.Chen, linux-mmc, linux-kernel, stable On Tue, Sep 2, 2025 at 3:47 PM Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 02/09/2025 10:12, Ben Chuang wrote: > > On Tue, Sep 2, 2025 at 2:33 PM Ben Chuang <benchuanggli@gmail.com> wrote: > >> > >> On Tue, Sep 2, 2025 at 1:02 AM Adrian Hunter <adrian.hunter@intel.com> wrote: > >>> > >>> On 01/09/2025 12:42, Ben Chuang wrote: > >>>> From: Ben Chuang <ben.chuang@genesyslogic.com.tw> > >>>> > >>>> According to the power structure of IC hardware design for UHS-II > >>>> interface, reset control and timing must be added to the initialization > >>>> process of powering on the UHS-II interface. > >>>> > >>>> Fixes: 27dd3b82557a ("mmc: sdhci-pci-gli: enable UHS-II mode for GL9767") > >>>> Cc: stable@vger.kernel.org # v6.13+ > >>>> Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw> > >>>> --- > >>>> drivers/mmc/host/sdhci-pci-gli.c | 71 +++++++++++++++++++++++++++++++- > >>>> 1 file changed, 70 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > >>>> index 3a1de477e9af..85d0d7e6169c 100644 > >>>> --- a/drivers/mmc/host/sdhci-pci-gli.c > >>>> +++ b/drivers/mmc/host/sdhci-pci-gli.c > >>>> @@ -283,6 +283,8 @@ > >>>> #define PCIE_GLI_9767_UHS2_CTL2_ZC_VALUE 0xb > >>>> #define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL BIT(6) > >>>> #define PCIE_GLI_9767_UHS2_CTL2_ZC_CTL_VALUE 0x1 > >>>> +#define PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN BIT(13) > >>>> +#define PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE BIT(14) > >>>> > >>>> #define GLI_MAX_TUNING_LOOP 40 > >>>> > >>>> @@ -1179,6 +1181,69 @@ static void gl9767_set_low_power_negotiation(struct pci_dev *pdev, bool enable) > >>>> gl9767_vhs_read(pdev); > >>>> } > >>>> > >>>> +static void sdhci_gl9767_uhs2_phy_reset_assert(struct sdhci_host *host) > >>>> +{ > >>>> + struct sdhci_pci_slot *slot = sdhci_priv(host); > >>>> + struct pci_dev *pdev = slot->chip->pdev; > >>>> + u32 value; > >>>> + > >>>> + gl9767_vhs_write(pdev); > >>>> + pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > >>>> + value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > >>>> + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > >>>> + value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > >>>> + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > >>>> + gl9767_vhs_read(pdev); > >>>> +} > >>>> + > >>>> +static void sdhci_gl9767_uhs2_phy_reset_deassert(struct sdhci_host *host) > >>>> +{ > >>>> + struct sdhci_pci_slot *slot = sdhci_priv(host); > >>>> + struct pci_dev *pdev = slot->chip->pdev; > >>>> + u32 value; > >>>> + > >>>> + gl9767_vhs_write(pdev); > >>>> + pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > >>>> + value |= PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > >>> > >>> Maybe add a small comment about PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE > >>> and PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN being updated separately. > >>> > >>>> + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > >>>> + value &= ~PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > >>>> + pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > >>>> + gl9767_vhs_read(pdev); > >>>> +} > >>> > >>> sdhci_gl9767_uhs2_phy_reset_assert() and sdhci_gl9767_uhs2_phy_reset_deassert() > >>> are fairly similar. Maybe consider: > >>> > >>> static void sdhci_gl9767_uhs2_phy_reset(struct sdhci_host *host, bool assert) > >>> { > >>> struct sdhci_pci_slot *slot = sdhci_priv(host); > >>> struct pci_dev *pdev = slot->chip->pdev; > >>> u32 value, set, clr; > >>> > >>> if (assert) { > >>> set = PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > >>> clr = PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > >>> } else { > >>> set = PCIE_GLI_9767_UHS2_CTL2_FORCE_RESETN_VALUE; > >>> clr = PCIE_GLI_9767_UHS2_CTL2_FORCE_PHY_RESETN; > >>> } > >>> > >>> gl9767_vhs_write(pdev); > >>> pci_read_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, &value); > >>> value |= set; > >>> pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > >>> value &= ~clr; > >>> pci_write_config_dword(pdev, PCIE_GLI_9767_UHS2_CTL2, value); > >>> gl9767_vhs_read(pdev); > >>> } > >>> > >> > >> OK, I will update it. Thank you. > >> > >>> > >>>> + > >>>> +static void __gl9767_uhs2_set_power(struct sdhci_host *host, unsigned char mode, unsigned short vdd) > >>>> +{ > >>>> + u8 pwr = 0; > >>>> + > >>>> + if (mode != MMC_POWER_OFF) { > >>>> + pwr = sdhci_get_vdd_value(vdd); > >>>> + if (!pwr) > >>>> + WARN(1, "%s: Invalid vdd %#x\n", > >>>> + mmc_hostname(host->mmc), vdd); > >>>> + pwr |= SDHCI_VDD2_POWER_180; > >>>> + } > >>>> + > >>>> + if (host->pwr == pwr) > >>>> + return; > >>>> + > >>>> + host->pwr = pwr; > >>>> + > >>>> + if (pwr == 0) { > >>>> + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > >>>> + } else { > >>>> + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > >>>> + > >>>> + pwr |= SDHCI_POWER_ON; > >>>> + sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL); > >>>> + mdelay(5); > >>> > >>> Can be mmc_delay(5) > >>> > >>>> + > >>>> + sdhci_gl9767_uhs2_phy_reset_assert(host); > >>>> + pwr |= SDHCI_VDD2_POWER_ON; > >>>> + sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL); > >>>> + mdelay(5); > >>> > >>> Can be mmc_delay(5) > >> > >> I may not modify it now. > >> mmc_delay() is in "drivers/mmc/core/core.h". > >> If sdhci-pci-gli.c only includes "../core/core.h", the compiler will > >> report some errors. > > > > Ah, just add another headersand it will build. > > > > --- a/drivers/mmc/host/sdhci-pci-gli.c > > +++ b/drivers/mmc/host/sdhci-pci-gli.c > > @@ -14,6 +14,8 @@ > > #include <linux/delay.h> > > #include <linux/of.h> > > #include <linux/iopoll.h> > > +#include <linux/mmc/host.h> > > +#include "../core/core.h" > > #include "sdhci.h" > > #include "sdhci-cqhci.h" > > #include "sdhci-pci.h" > > @@ -968,10 +970,10 @@ static void gl9755_set_power(struct sdhci_host > > *host, unsigned char mode, > > > > sdhci_writeb(host, pwr & 0xf, SDHCI_POWER_CONTROL); > > /* wait stable */ > > - mdelay(5); > > + mmc_delay(5); > > sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL); > > /* wait stable */ > > - mdelay(5); > > + mmc_delay(5); > > sdhci_gli_overcurrent_event_enable(host, true); > > } > > It seems mmc_delay() is for core mmc code only, not host drivers. > But the issue with mdelay is that it does not sleep. Other options > are msleep() or usleep_range(). > Ok, I will use usleep_range() instead of mdelay(). Thanks for the suggestion. Best regards, Ben Chuang ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-02 8:55 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-09-01 9:42 [PATCH 2/2] mmc: sdhci-pci-gli: GL9767: Fix initializing the UHS-II interface during a power-on Ben Chuang 2025-09-01 17:01 ` Adrian Hunter 2025-09-02 6:33 ` Ben Chuang 2025-09-02 7:12 ` Ben Chuang 2025-09-02 7:47 ` Adrian Hunter 2025-09-02 8:54 ` Ben Chuang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®