Re: [PATCH v1 1/1] drivers: mmc: iproc_sdhci: enable HS200 mode
On Wed, Feb 10, 2021 at 3:12 AM Jaehoon Chung wrote: > > Hi Rayagonda, > > On 2/9/21 1:34 PM, Rayagonda Kokatanur wrote: > > From: Bharat Gooty > > > > Add tuning functionality which is needed for HS200 mode. > > For HS200, program the correct needed 1.8 voltage > > I didn't test with this on target. But how did you use HS200 mode? > In this patch, there is no set to HS200 mode. Is there any other patch. It can be enabled from a config file (configs/bcm_ns3_defconfig) based on requirement. Hence not added config file changes. > > > > > Signed-off-by: Bharat Gooty > > Signed-off-by: Rayagonda Kokatanur > > --- > > drivers/mmc/iproc_sdhci.c | 88 +++ > > 1 file changed, 79 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/mmc/iproc_sdhci.c b/drivers/mmc/iproc_sdhci.c > > index f931e4b3c1..360ea01e21 100644 > > --- a/drivers/mmc/iproc_sdhci.c > > +++ b/drivers/mmc/iproc_sdhci.c > > @@ -9,6 +9,7 @@ > > #include > > #include > > #include > > +#include "mmc_private.h" > > #include > > > > DECLARE_GLOBAL_DATA_PTR; > > @@ -139,17 +140,87 @@ static void sdhci_iproc_writeb(struct sdhci_host > > *host, u8 val, int reg) > > > > static int sdhci_iproc_set_ios_post(struct sdhci_host *host) > > { > > - u32 ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); > > + struct mmc *mmc = (struct mmc *)host->mmc; > > + u32 ctrl; > > > > - /* Reset UHS mode bits */ > > - ctrl &= ~SDHCI_CTRL_UHS_MASK; > > + if (mmc->signal_voltage == MMC_SIGNAL_VOLTAGE_180) { > > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); > > + ctrl |= SDHCI_CTRL_VDD_180; > > + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); > > + } > > > > - if (host->mmc->ddr_mode) > > - ctrl |= UHS_DDR50_BUS_SPEED; > > Doesn't need to remove this? If someone want to use DDR mode, doesn't need to > set this bit? Supported speeds can come from the capabilities registers > > > + sdhci_set_uhs_timing(host); > > + return 0; > > +} > > > > +static void sdhci_start_tuning(struct sdhci_host *host) > > +{ > > + u32 ctrl; > > + > > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); > > + ctrl |= SDHCI_CTRL_EXEC_TUNING; > > sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); > > > > - return 0; > > + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE); > > + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE); > > +} > > + > > +static void sdhci_end_tuning(struct sdhci_host *host) > > +{ > > + /* Enable only interrupts served by the SD controller */ > > + sdhci_writel(host, SDHCI_INT_DATA_MASK | SDHCI_INT_CMD_MASK, > > + SDHCI_INT_ENABLE); > > + /* Mask all sdhci interrupt sources */ > > + sdhci_writel(host, 0x0, SDHCI_SIGNAL_ENABLE); > > +} > > + > > +static int sdhci_iproc_execute_tuning(struct mmc *mmc, u8 opcode) > > +{ > > +#define MAX_TUNING_LOOP 40 > > Move to top. Thank you, will fix it. > > > + struct mmc_cmd cmd; > > + u32 ctrl; > > + u32 blocksize = SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG, 64); > > + struct sdhci_host *host = dev_get_priv(mmc->dev); > > + char tuning_loop_counter = MAX_TUNING_LOOP; > > + int ret = 0; > > + > > + sdhci_start_tuning(host); > > + > > + cmd.cmdidx = opcode; > > + cmd.resp_type = MMC_RSP_R1; > > + cmd.cmdarg = 0; > > + > > + if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && mmc->bus_width == 8) > > According to spec, HS200 can be used with 4/8 bit buswidth. Iproc SDHCI controller supports HS200 in 8bit mode only. > > > > + blocksize = SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG, 128); > > + > > + sdhci_writew(host, blocksize, SDHCI_BLOCK_SIZE); > > + sdhci_writew(host, 1, SDHCI_BLOCK_COUNT); > > + sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE); > > + > > + do { > > + mmc_send_cmd(mmc, &cmd, NULL); > > + if (opcode == MMC_CMD_SEND_TUNING_BLOCK) > > + udelay(1); > > Add the comment to add udelay(1). Sure, thank you. > > > + > > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); > > + > > + if (tuning_loop_counter-- == 0) > > + break; > > + > > + } while (ctrl & SDHCI_CTRL_EXEC_TUNING); > > + > > + if (tuning_loop_counter < 0 || (!(ctrl & SDHCI_CTRL_TUNED_CLK))) { > > + ctrl &= ~(SDHCI_CTRL_TUNED_CLK | SDHCI_CTRL_EXEC_TUNING); > > + sdhci_writel(host, ctrl, SDHCI_HOST_CONTROL2); > > + printf("%s:Tuning failed, opcode = 0x%02x\n", __func__, > > opcode); > > + ret = -EIO; > > + } > > + > > + udelay(1); > > Doesn't need to add udelay(1) at here? okay, will remove it. > > Best Regards, > Jaehoon Chung > > > + > > + sdhci_end_tuning(host); > > + > > + return ret; > > } > > > > static struct sdhci_ops sdhci_platform_ops = { > > @@ -162,6 +233,7 @@ static struct sdhci_ops sdhci_pla
Re: [PATCH v1 1/1] drivers: mmc: iproc_sdhci: enable HS200 mode
Hi Rayagonda, On 2/9/21 1:34 PM, Rayagonda Kokatanur wrote: > From: Bharat Gooty > > Add tuning functionality which is needed for HS200 mode. > For HS200, program the correct needed 1.8 voltage I didn't test with this on target. But how did you use HS200 mode? In this patch, there is no set to HS200 mode. Is there any other patch. > > Signed-off-by: Bharat Gooty > Signed-off-by: Rayagonda Kokatanur > --- > drivers/mmc/iproc_sdhci.c | 88 +++ > 1 file changed, 79 insertions(+), 9 deletions(-) > > diff --git a/drivers/mmc/iproc_sdhci.c b/drivers/mmc/iproc_sdhci.c > index f931e4b3c1..360ea01e21 100644 > --- a/drivers/mmc/iproc_sdhci.c > +++ b/drivers/mmc/iproc_sdhci.c > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include "mmc_private.h" > #include > > DECLARE_GLOBAL_DATA_PTR; > @@ -139,17 +140,87 @@ static void sdhci_iproc_writeb(struct sdhci_host *host, > u8 val, int reg) > > static int sdhci_iproc_set_ios_post(struct sdhci_host *host) > { > - u32 ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + struct mmc *mmc = (struct mmc *)host->mmc; > + u32 ctrl; > > - /* Reset UHS mode bits */ > - ctrl &= ~SDHCI_CTRL_UHS_MASK; > + if (mmc->signal_voltage == MMC_SIGNAL_VOLTAGE_180) { > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + ctrl |= SDHCI_CTRL_VDD_180; > + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); > + } > > - if (host->mmc->ddr_mode) > - ctrl |= UHS_DDR50_BUS_SPEED; Doesn't need to remove this? If someone want to use DDR mode, doesn't need to set this bit? > + sdhci_set_uhs_timing(host); > + return 0; > +} > > +static void sdhci_start_tuning(struct sdhci_host *host) > +{ > + u32 ctrl; > + > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + ctrl |= SDHCI_CTRL_EXEC_TUNING; > sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); > > - return 0; > + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE); > + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE); > +} > + > +static void sdhci_end_tuning(struct sdhci_host *host) > +{ > + /* Enable only interrupts served by the SD controller */ > + sdhci_writel(host, SDHCI_INT_DATA_MASK | SDHCI_INT_CMD_MASK, > + SDHCI_INT_ENABLE); > + /* Mask all sdhci interrupt sources */ > + sdhci_writel(host, 0x0, SDHCI_SIGNAL_ENABLE); > +} > + > +static int sdhci_iproc_execute_tuning(struct mmc *mmc, u8 opcode) > +{ > +#define MAX_TUNING_LOOP 40 Move to top. > + struct mmc_cmd cmd; > + u32 ctrl; > + u32 blocksize = SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG, 64); > + struct sdhci_host *host = dev_get_priv(mmc->dev); > + char tuning_loop_counter = MAX_TUNING_LOOP; > + int ret = 0; > + > + sdhci_start_tuning(host); > + > + cmd.cmdidx = opcode; > + cmd.resp_type = MMC_RSP_R1; > + cmd.cmdarg = 0; > + > + if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && mmc->bus_width == 8) According to spec, HS200 can be used with 4/8 bit buswidth. > + blocksize = SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG, 128); > + > + sdhci_writew(host, blocksize, SDHCI_BLOCK_SIZE); > + sdhci_writew(host, 1, SDHCI_BLOCK_COUNT); > + sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE); > + > + do { > + mmc_send_cmd(mmc, &cmd, NULL); > + if (opcode == MMC_CMD_SEND_TUNING_BLOCK) > + udelay(1); Add the comment to add udelay(1). > + > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + > + if (tuning_loop_counter-- == 0) > + break; > + > + } while (ctrl & SDHCI_CTRL_EXEC_TUNING); > + > + if (tuning_loop_counter < 0 || (!(ctrl & SDHCI_CTRL_TUNED_CLK))) { > + ctrl &= ~(SDHCI_CTRL_TUNED_CLK | SDHCI_CTRL_EXEC_TUNING); > + sdhci_writel(host, ctrl, SDHCI_HOST_CONTROL2); > + printf("%s:Tuning failed, opcode = 0x%02x\n", __func__, opcode); > + ret = -EIO; > + } > + > + udelay(1); Doesn't need to add udelay(1) at here? Best Regards, Jaehoon Chung > + > + sdhci_end_tuning(host); > + > + return ret; > } > > static struct sdhci_ops sdhci_platform_ops = { > @@ -162,6 +233,7 @@ static struct sdhci_ops sdhci_platform_ops = { > .write_b = sdhci_iproc_writeb, > #endif > .set_ios_post = sdhci_iproc_set_ios_post, > + .platform_execute_tuning = sdhci_iproc_execute_tuning, > }; > > struct iproc_sdhci_plat { > @@ -189,9 +261,7 @@ static int iproc_sdhci_probe(struct udevice *dev) > > host->name = dev->name; > host->ioaddr = dev_read_addr_ptr(dev); > - host->voltages = MMC_VDD_165_195 | > - MMC_VDD_32_33 | MMC_VDD_33_34; > - host->quirks = SDHCI_QUIRK_BROKEN_VOLTAGE | SDHCI_QUIRK_BROKEN_R1B; > + host->quirks = SDHCI_QUIRK_BROKEN_R1B; > host->host_caps = MMC_MODE_DDR_52