diff mbox series

[v1,1/1] drivers: mmc: iproc_sdhci: enable HS200 mode

Message ID 20210209043412.28934-1-rayagonda.kokatanur@broadcom.com
State Changes Requested
Delegated to: Peng Fan
Headers show
Series [v1,1/1] drivers: mmc: iproc_sdhci: enable HS200 mode | expand

Commit Message

Rayagonda Kokatanur Feb. 9, 2021, 4:34 a.m. UTC 
From: Bharat Gooty <bharat.gooty@broadcom.com>

Add tuning functionality which is needed for HS200 mode.
For HS200, program the correct needed 1.8 voltage

Signed-off-by: Bharat Gooty <bharat.gooty@broadcom.com>
Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
---
 drivers/mmc/iproc_sdhci.c | 88 +++++++++++++++++++++++++++++++++++----
 1 file changed, 79 insertions(+), 9 deletions(-)

Comments

Jaehoon Chung Feb. 9, 2021, 9:43 p.m. UTC | #1 
Hi Rayagonda,

On 2/9/21 1:34 PM, Rayagonda Kokatanur wrote:
> From: Bharat Gooty <bharat.gooty@broadcom.com>
> 
> 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 <bharat.gooty@broadcom.com>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> ---
>  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 <errno.h>
>  #include <malloc.h>
>  #include <sdhci.h>
> +#include "mmc_private.h"
>  #include <linux/delay.h>
>  
>  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_52MHz;
>  	host->index = fdtdec_get_uint(gd->fdt_blob, node, "index", 0);
>  	host->ops = &sdhci_platform_ops;
>
Rayagonda Kokatanur Feb. 26, 2021, 5:54 a.m. UTC | #2 
On Wed, Feb 10, 2021 at 3:12 AM Jaehoon Chung <jh80.chung@samsung.com> wrote:
>
> Hi Rayagonda,
>
> On 2/9/21 1:34 PM, Rayagonda Kokatanur wrote:
> > From: Bharat Gooty <bharat.gooty@broadcom.com>
> >
> > 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 <bharat.gooty@broadcom.com>
> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> > ---
> >  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 <errno.h>
> >  #include <malloc.h>
> >  #include <sdhci.h>
> > +#include "mmc_private.h"
> >  #include <linux/delay.h>
> >
> >  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_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_52MHz;
> >       host->index = fdtdec_get_uint(gd->fdt_blob, node, "index", 0);
> >       host->ops = &sdhci_platform_ops;
> >
>
diff mbox series

Patch

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 <errno.h>
 #include <malloc.h>
 #include <sdhci.h>
+#include "mmc_private.h"
 #include <linux/delay.h>
 
 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;
+	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
+	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)
+		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);
+
+		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);
+
+	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_52MHz;
 	host->index = fdtdec_get_uint(gd->fdt_blob, node, "index", 0);
 	host->ops = &sdhci_platform_ops;