+ Georg On Fri, 25 Oct 2024 at 08:01, Ben Chuang <benchuanggli@xxxxxxxxx> wrote: > > From: Ben Chuang <ben.chuang@xxxxxxxxxxxxxxxxxxx> > > On sdhci_gl9767_set_clock(), the vendor header space(VHS) is read-only > after calling gl9767_disable_ssc_pll() and gl9767_set_ssc_pll_205mhz(). > So the low power negotiation mode cannot be enabled again. > Introduce gl9767_set_low_power_negotiation() function to fix it. > > The explanation process is as below. > > static void sdhci_gl9767_set_clock() > { > ... > gl9767_vhs_write(); > ... > value |= PCIE_GLI_9767_CFG_LOW_PWR_OFF; > pci_write_config_dword(pdev, PCIE_GLI_9767_CFG, value); <--- (a) > > gl9767_disable_ssc_pll(); <--- (b) > sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL); > > if (clock == 0) > return; <-- (I) > > ... > if (clock == 200000000 && ios->timing == MMC_TIMING_UHS_SDR104) { > ... > gl9767_set_ssc_pll_205mhz(); <--- (c) > } > ... > value &= ~PCIE_GLI_9767_CFG_LOW_PWR_OFF; > pci_write_config_dword(pdev, PCIE_GLI_9767_CFG, value); <-- (II) > gl9767_vhs_read(); > } > > (a) disable low power negotiation mode. When return on (I), the low power > mode is disabled. After (b) and (c), VHS is read-only, the low power mode > cannot be enabled on (II). > > Fixes: d2754355512e ("mmc: sdhci-pci-gli: Set SDR104's clock to 205MHz and enable SSC for GL9767") Is this the same problem as being reported in https://lore.kernel.org/all/41c1c88a-b2c9-4c05-863a-467785027f49@xxxxxxxxxxxxxxxxxxx/ ? > Signed-off-by: Ben Chuang <benchuanggli@xxxxxxxxx> Not sure the above SoB makes sense. The below is perfectly sufficient, right? > Signed-off-by: Ben Chuang <ben.chuang@xxxxxxxxxxxxxxxxxxx> Kind regards Uffe > --- > drivers/mmc/host/sdhci-pci-gli.c | 35 +++++++++++++++++++------------- > 1 file changed, 21 insertions(+), 14 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > index 0f81586a19df..22a927ce2c88 100644 > --- a/drivers/mmc/host/sdhci-pci-gli.c > +++ b/drivers/mmc/host/sdhci-pci-gli.c > @@ -892,28 +892,40 @@ static void gl9767_disable_ssc_pll(struct pci_dev *pdev) > gl9767_vhs_read(pdev); > } > > +static void gl9767_set_low_power_negotiation(struct pci_dev *pdev, bool enable) > +{ > + u32 value; > + > + gl9767_vhs_write(pdev); > + > + pci_read_config_dword(pdev, PCIE_GLI_9767_CFG, &value); > + if (enable) > + value &= ~PCIE_GLI_9767_CFG_LOW_PWR_OFF; > + else > + value |= PCIE_GLI_9767_CFG_LOW_PWR_OFF; > + pci_write_config_dword(pdev, PCIE_GLI_9767_CFG, value); > + > + gl9767_vhs_read(pdev); > +} > + > static void sdhci_gl9767_set_clock(struct sdhci_host *host, unsigned int clock) > { > struct sdhci_pci_slot *slot = sdhci_priv(host); > struct mmc_ios *ios = &host->mmc->ios; > struct pci_dev *pdev; > - u32 value; > u16 clk; > > pdev = slot->chip->pdev; > host->mmc->actual_clock = 0; > > - gl9767_vhs_write(pdev); > - > - pci_read_config_dword(pdev, PCIE_GLI_9767_CFG, &value); > - value |= PCIE_GLI_9767_CFG_LOW_PWR_OFF; > - pci_write_config_dword(pdev, PCIE_GLI_9767_CFG, value); > - > + gl9767_set_low_power_negotiation(pdev, false); > gl9767_disable_ssc_pll(pdev); > sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL); > > - if (clock == 0) > + if (clock == 0) { > + gl9767_set_low_power_negotiation(pdev, true); > return; > + } > > clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock); > if (clock == 200000000 && ios->timing == MMC_TIMING_UHS_SDR104) { > @@ -922,12 +934,7 @@ static void sdhci_gl9767_set_clock(struct sdhci_host *host, unsigned int clock) > } > > sdhci_enable_clk(host, clk); > - > - pci_read_config_dword(pdev, PCIE_GLI_9767_CFG, &value); > - value &= ~PCIE_GLI_9767_CFG_LOW_PWR_OFF; > - pci_write_config_dword(pdev, PCIE_GLI_9767_CFG, value); > - > - gl9767_vhs_read(pdev); > + gl9767_set_low_power_negotiation(pdev, true); > } > > static void gli_set_9767(struct sdhci_host *host) > -- > 2.47.0 >