| Message ID | 20211206131507.411-1-fred.ai@bayhubtech.com |
|---|---|
| State | New |
| Headers | show |
| Series | [[PATCH,V2] ] mmc:sdhci-pci-o2micro: Change implementation of tuning for SDR104 and HS200 mode | expand |
On 06/12/2021 15:15, fred wrote: > From: "fred.ai" <fred.ai@bayhubtech.com> > > Adjust the timing of an interface in SDR104 mode. After sending CMD6 > successfully to switch card mode,driver need to select DLL output > clock as SD clock.Setting DLL output clock phase value 0x9 when> select DLL output clock source. There are some kernel-style issues. Only single '[' in subject please e.g. should have been [PATCH V2 1/2] not [[PATCH V2]]. Also missing patch number and a space after mmc: i.e. should have been: [PATCH V2 1/2] mmc: sdhci-pci-o2micro: Change implementation of tuning for SDR104 and HS200 mode > > Signed-off-by: fred.ai <fred.ai@bayhubtech.com> Normally names are separated with space and capitalized, so perhaps Fred Ai rather than fred.ai ? > --- > Change in V2: > 1.Set register 0x354 bit 16 to select DLL clock > 2.Set register 0x354 bit [23:20] to select fixed output tuning phase 0x9 > 3.Driver need to clear 0x354 bit 16 and bit [23:20] to 0 when setting clock > --- > drivers/mmc/host/sdhci-pci-o2micro.c | 57 ++++++++++++++++++++++------ > 1 file changed, 45 insertions(+), 12 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-pci-o2micro.c b/drivers/mmc/host/sdhci-pci-o2micro.c > index f045c1ee4667..98c1a17eb619 100644 > --- a/drivers/mmc/host/sdhci-pci-o2micro.c > +++ b/drivers/mmc/host/sdhci-pci-o2micro.c > @@ -43,6 +43,7 @@ > #define O2_SD_CAP_REG0 0x334 > #define O2_SD_UHS1_CAP_SETTING 0x33C > #define O2_SD_DELAY_CTRL 0x350 > +#define O2_SD_OUTPUT_CLK_SOURCE_SWITCH 0x354 > #define O2_SD_UHS2_L1_CTRL 0x35C > #define O2_SD_FUNC_REG3 0x3E0 > #define O2_SD_FUNC_REG4 0x3E4 > @@ -301,9 +302,14 @@ static int sdhci_o2_dll_recovery(struct sdhci_host *host) > static int sdhci_o2_execute_tuning(struct mmc_host *mmc, u32 opcode) > { > struct sdhci_host *host = mmc_priv(mmc); > + struct sdhci_pci_slot *slot = sdhci_priv(host); > + struct sdhci_pci_chip *chip = slot->chip; > int current_bus_width = 0; > u32 scratch32 = 0; > u16 scratch = 0; > + u8 scratch_8 = 0; > + u32 reg_val; > + u8 dll_mode; > > /* > * This handler only implements the eMMC tuning that is specific to > @@ -322,6 +328,28 @@ static int sdhci_o2_execute_tuning(struct mmc_host *mmc, u32 opcode) > scratch |= O2_SD_PWR_FORCE_L0; > sdhci_writew(host, scratch, O2_SD_MISC_CTRL); > > + /* stop clk */ > + reg_val = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > + reg_val &= ~(SDHCI_CLOCK_CARD_EN); Parenthesis not needed > + sdhci_writew(host, reg_val, SDHCI_CLOCK_CONTROL); > + > + /* UnLock WP */ > + pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch_8); > + scratch_8 &= 0x7f; > + pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch_8); > + > + /* Set pcr 0x354[16] to choose dll clock, and set the default phase */ > + dll_mode = 0x9; > + pci_read_config_dword(chip->pdev, O2_SD_OUTPUT_CLK_SOURCE_SWITCH, ®_val); > + reg_val &= 0xFF0EFFFF; > + reg_val |= ((1 << 16) | (dll_mode << 20)); It would be nicer to define BIT() and GENMASK() and use FIELD_PREP here, refer include/linux/bitfield.h > + pci_write_config_dword(chip->pdev, O2_SD_OUTPUT_CLK_SOURCE_SWITCH, reg_val); > + > + /* start clk */ Indent should be a single tab. > + reg_val = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > + reg_val |= SDHCI_CLOCK_CARD_EN; > + sdhci_writew(host, reg_val, SDHCI_CLOCK_CONTROL); Other places in the code seem to pair "Unlock WP" with "Lock WP" but there is not one here. Is that correct? > + > /* wait DLL lock, timeout value 5ms */ > if (readx_poll_timeout(sdhci_o2_pll_dll_wdt_control, host, > scratch32, (scratch32 & O2_DLL_LOCK_STATUS), 1, 5000)) > @@ -520,11 +548,11 @@ static void sdhci_o2_enable_clk(struct sdhci_host *host, u16 clk) > > static void sdhci_pci_o2_set_clock(struct sdhci_host *host, unsigned int clock) > { > - u16 clk; > - u8 scratch; > - u32 scratch_32; > struct sdhci_pci_slot *slot = sdhci_priv(host); > struct sdhci_pci_chip *chip = slot->chip; > + u32 scratch_32; > + u8 scratch; > + u16 clk; Please do not make unrelated changes > > host->mmc->actual_clock = 0; > > @@ -533,22 +561,27 @@ static void sdhci_pci_o2_set_clock(struct sdhci_host *host, unsigned int clock) > if (clock == 0) > return; > > - if ((host->timing == MMC_TIMING_UHS_SDR104) && (clock == 200000000)) { > - pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch); > - > - scratch &= 0x7f; > - pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch); > + /* UnLock WP */ > + pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch); > + scratch &= 0x7f; > + pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch); > > + if ((host->timing == MMC_TIMING_UHS_SDR104) && (clock == 200000000)) { > pci_read_config_dword(chip->pdev, O2_SD_PLL_SETTING, &scratch_32); > > if ((scratch_32 & 0xFFFF0000) != 0x2c280000) > o2_pci_set_baseclk(chip, 0x2c280000); > + } > > - pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch); > + pci_read_config_dword(chip->pdev, O2_SD_OUTPUT_CLK_SOURCE_SWITCH, &scratch_32); > + scratch_32 &= ~(1 << 16); > + scratch_32 &= ~(0xf << 20); It would be nicer to define BIT() and GENMASK() and use them here, refer include/linux/bitfield.h > + pci_write_config_dword(chip->pdev, O2_SD_OUTPUT_CLK_SOURCE_SWITCH, scratch_32); > > - scratch |= 0x80; > - pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch); > - } > + /* Lock WP */ Indent a single tab > + pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch); > + scratch |= 0x80; > + pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch); > > clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock); > sdhci_o2_enable_clk(host, clk); >
diff --git a/drivers/mmc/host/sdhci-pci-o2micro.c b/drivers/mmc/host/sdhci-pci-o2micro.c index f045c1ee4667..98c1a17eb619 100644 --- a/drivers/mmc/host/sdhci-pci-o2micro.c +++ b/drivers/mmc/host/sdhci-pci-o2micro.c @@ -43,6 +43,7 @@ #define O2_SD_CAP_REG0 0x334 #define O2_SD_UHS1_CAP_SETTING 0x33C #define O2_SD_DELAY_CTRL 0x350 +#define O2_SD_OUTPUT_CLK_SOURCE_SWITCH 0x354 #define O2_SD_UHS2_L1_CTRL 0x35C #define O2_SD_FUNC_REG3 0x3E0 #define O2_SD_FUNC_REG4 0x3E4 @@ -301,9 +302,14 @@ static int sdhci_o2_dll_recovery(struct sdhci_host *host) static int sdhci_o2_execute_tuning(struct mmc_host *mmc, u32 opcode) { struct sdhci_host *host = mmc_priv(mmc); + struct sdhci_pci_slot *slot = sdhci_priv(host); + struct sdhci_pci_chip *chip = slot->chip; int current_bus_width = 0; u32 scratch32 = 0; u16 scratch = 0; + u8 scratch_8 = 0; + u32 reg_val; + u8 dll_mode; /* * This handler only implements the eMMC tuning that is specific to @@ -322,6 +328,28 @@ static int sdhci_o2_execute_tuning(struct mmc_host *mmc, u32 opcode) scratch |= O2_SD_PWR_FORCE_L0; sdhci_writew(host, scratch, O2_SD_MISC_CTRL); + /* stop clk */ + reg_val = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + reg_val &= ~(SDHCI_CLOCK_CARD_EN); + sdhci_writew(host, reg_val, SDHCI_CLOCK_CONTROL); + + /* UnLock WP */ + pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch_8); + scratch_8 &= 0x7f; + pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch_8); + + /* Set pcr 0x354[16] to choose dll clock, and set the default phase */ + dll_mode = 0x9; + pci_read_config_dword(chip->pdev, O2_SD_OUTPUT_CLK_SOURCE_SWITCH, ®_val); + reg_val &= 0xFF0EFFFF; + reg_val |= ((1 << 16) | (dll_mode << 20)); + pci_write_config_dword(chip->pdev, O2_SD_OUTPUT_CLK_SOURCE_SWITCH, reg_val); + + /* start clk */ + reg_val = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + reg_val |= SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, reg_val, SDHCI_CLOCK_CONTROL); + /* wait DLL lock, timeout value 5ms */ if (readx_poll_timeout(sdhci_o2_pll_dll_wdt_control, host, scratch32, (scratch32 & O2_DLL_LOCK_STATUS), 1, 5000)) @@ -520,11 +548,11 @@ static void sdhci_o2_enable_clk(struct sdhci_host *host, u16 clk) static void sdhci_pci_o2_set_clock(struct sdhci_host *host, unsigned int clock) { - u16 clk; - u8 scratch; - u32 scratch_32; struct sdhci_pci_slot *slot = sdhci_priv(host); struct sdhci_pci_chip *chip = slot->chip; + u32 scratch_32; + u8 scratch; + u16 clk; host->mmc->actual_clock = 0; @@ -533,22 +561,27 @@ static void sdhci_pci_o2_set_clock(struct sdhci_host *host, unsigned int clock) if (clock == 0) return; - if ((host->timing == MMC_TIMING_UHS_SDR104) && (clock == 200000000)) { - pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch); - - scratch &= 0x7f; - pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch); + /* UnLock WP */ + pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch); + scratch &= 0x7f; + pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch); + if ((host->timing == MMC_TIMING_UHS_SDR104) && (clock == 200000000)) { pci_read_config_dword(chip->pdev, O2_SD_PLL_SETTING, &scratch_32); if ((scratch_32 & 0xFFFF0000) != 0x2c280000) o2_pci_set_baseclk(chip, 0x2c280000); + } - pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch); + pci_read_config_dword(chip->pdev, O2_SD_OUTPUT_CLK_SOURCE_SWITCH, &scratch_32); + scratch_32 &= ~(1 << 16); + scratch_32 &= ~(0xf << 20); + pci_write_config_dword(chip->pdev, O2_SD_OUTPUT_CLK_SOURCE_SWITCH, scratch_32); - scratch |= 0x80; - pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch); - } + /* Lock WP */ + pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch); + scratch |= 0x80; + pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch); clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock); sdhci_o2_enable_clk(host, clk);