Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1194971 > unrolled thread

[PATCH v3 0/6] mmc: imx: a few fixes and new feature

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/6] mmc: imx: a few fixes and new feature Haibo Chen <haibo.chen@freescale.com> - 2015-07-29 11:10 +0200
    [PATCH v3 6/6] mmc: sdhci-esdhc-imx: set back the burst_length_enable bit to 1 Haibo Chen <haibo.chen@freescale.com> - 2015-07-29 11:10 +0200
    [PATCH v3 5/6] mmc: sdhci-esdhc-imx: config watermark level and burst length Haibo Chen <haibo.chen@freescale.com> - 2015-07-29 11:10 +0200
      Re: [PATCH v3 5/6] mmc: sdhci-esdhc-imx: config watermark level and  burst length Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-07-31 17:20 +0200
    Re: [PATCH v3 0/6] mmc: imx: a few fixes and new feature Dong Aisheng <b29396@freescale.com> - 2015-07-29 11:20 +0200
      Re: [PATCH v3 0/6] mmc: imx: a few fixes and new feature Holger Schurig <holgerschurig@gmail.com> - 2015-07-30 11:50 +0200

#1194971 — [PATCH v3 0/6] mmc: imx: a few fixes and new feature

Changes for v3:
-Add property describe in binding doc.

Haibo Chen (6):
  mmc: sdhci-esdhc-imx: add imx7d support and support HS400
  mmc: sdhci-esdhc-imx: add tuning-step seting support
  ARM: dts: imx7d-sdb: add eMMC5.0 support
  mmc: sdhci-esdhc-imx: add compatible string in bingding doc
  mmc: sdhci-esdhc-imx: config watermark level and burst length
  mmc: sdhci-esdhc-imx: set back the burst_length_enable bit to 1

 .../devicetree/bindings/mmc/fsl-imx-esdhc.txt      |  2 +
 arch/arm/boot/dts/imx7d-sdb.dts                    | 13 +++
 drivers/mmc/host/sdhci-esdhc-imx.c                 | 97 +++++++++++++++++++++-
 include/linux/platform_data/mmc-esdhc-imx.h        |  1 +
 4 files changed, 112 insertions(+), 1 deletion(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1194975 — [PATCH v3 6/6] mmc: sdhci-esdhc-imx: set back the burst_length_enable bit to 1

Currently we find that if a usdhc is choosed to boot system, then ROM
code will set the burst length enable bit of this usdhc as 0.

This will make performance drop a lot if this usdhc's burst length is
16. So this patch set back the burst_length_enable bit as 1, which is
the default value, and means burst length is enabled for INCR.

Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 37d0095..dd945e5 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -32,6 +32,7 @@
 #include "sdhci-esdhc.h"
 
 #define	ESDHC_CTRL_D3CD			0x08
+#define ESDHC_BURST_LEN_EN_INCR		(1 << 27)
 /* VENDOR SPEC register */
 #define ESDHC_VENDOR_SPEC		0xc0
 #define  ESDHC_VENDOR_SPEC_SDIO_QUIRK	(1 << 1)
@@ -1158,6 +1159,16 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 		host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
 		host->mmc->caps |= MMC_CAP_1_8V_DDR;
 
+		/*
+		 * ROM code will change the burst_length_enable setting to
+		 * zero if this usdhc is choosed to boot system. Change it
+		 * back here, otherwise it will impact the performance a
+		 * lot if the burst length is 16.
+		 */
+		writel(readl(host->ioaddr + SDHCI_HOST_CONTROL)
+			| ESDHC_BURST_LEN_EN_INCR,
+			host->ioaddr + SDHCI_HOST_CONTROL);
+
 		if (!(imx_data->socdata->flags & ESDHC_FLAG_HS200))
 			host->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1194976 — [PATCH v3 5/6] mmc: sdhci-esdhc-imx: config watermark level and burst length

i.MX7D support eMMC HS400 mode, this mode can run in 8 bit,200MHZ
DDR mode. So the I/O speed improve a lot compare to SD3.0

The default burst length is 8, if we don't change this value, in
HS400 mode, when we do eMMC read operation, we can find that the
clock signal will stop for a period of time. This means the speed
of data moving on AHB bus is slower than I/O speed. So we should
improve the speed of data moving on AHB bus.

For imx7d usdhc, this patch set the burst length as 16, and set
watermark level as 64. The test result is the clock signal has
no stop during the eMMC HS400 operation. For other imx usdhc, remain
the default value: burst length as 8, watermark level as 16.

Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 158f93b..37d0095 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -239,6 +239,11 @@ static inline int is_imx6q_usdhc(struct pltfm_imx_data *data)
 	return data->socdata == &usdhc_imx6q_data;
 }
 
+static inline int is_imx7d_usdhc(struct pltfm_imx_data *data)
+{
+	return data->socdata == &usdhc_imx7d_data;
+}
+
 static inline int esdhc_is_usdhc(struct pltfm_imx_data *data)
 {
 	return !!(data->socdata->flags & ESDHC_FLAG_USDHC);
@@ -1145,7 +1150,11 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	 * to something insane.  Change it back here.
 	 */
 	if (esdhc_is_usdhc(imx_data)) {
-		writel(0x08100810, host->ioaddr + ESDHC_WTMK_LVL);
+		if (is_imx7d_usdhc(imx_data))
+			writel(0x10401040, host->ioaddr + ESDHC_WTMK_LVL);
+		else
+			writel(0x08100810, host->ioaddr + ESDHC_WTMK_LVL);
+
 		host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
 		host->mmc->caps |= MMC_CAP_1_8V_DDR;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1197108 — Re: [PATCH v3 5/6] mmc: sdhci-esdhc-imx: config watermark level and burst length

On Fri, Jul 31, 2015 at 10:51:41PM +0800, Dong Aisheng wrote:
> On Wed, Jul 29, 2015 at 05:03:56PM +0800, Haibo Chen wrote:
> > i.MX7D support eMMC HS400 mode, this mode can run in 8 bit,200MHZ
> > DDR mode. So the I/O speed improve a lot compare to SD3.0
> > 
> > The default burst length is 8, if we don't change this value, in
> > HS400 mode, when we do eMMC read operation, we can find that the
> > clock signal will stop for a period of time. This means the speed
> > of data moving on AHB bus is slower than I/O speed. So we should
> > improve the speed of data moving on AHB bus.
> > 
> > For imx7d usdhc, this patch set the burst length as 16, and set
> > watermark level as 64. The test result is the clock signal has
> > no stop during the eMMC HS400 operation. For other imx usdhc, remain
> > the default value: burst length as 8, watermark level as 16.
> > 
> > Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 158f93b..37d0095 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -239,6 +239,11 @@ static inline int is_imx6q_usdhc(struct pltfm_imx_data *data)
> >  	return data->socdata == &usdhc_imx6q_data;
> >  }
> >  
> > +static inline int is_imx7d_usdhc(struct pltfm_imx_data *data)
> > +{
> > +	return data->socdata == &usdhc_imx7d_data;
> > +}
> 
> Can we using flag to check instead of adding more is_imx_usdhc()?

No, not more flags.  Do the job properly and parameterise the differences.

> >  static inline int esdhc_is_usdhc(struct pltfm_imx_data *data)
> >  {
> >  	return !!(data->socdata->flags & ESDHC_FLAG_USDHC);
> > @@ -1145,7 +1150,11 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
> >  	 * to something insane.  Change it back here.
> >  	 */
> >  	if (esdhc_is_usdhc(imx_data)) {
> > -		writel(0x08100810, host->ioaddr + ESDHC_WTMK_LVL);
> > +		if (is_imx7d_usdhc(imx_data))
> > +			writel(0x10401040, host->ioaddr + ESDHC_WTMK_LVL);
> > +		else
> > +			writel(0x08100810, host->ioaddr + ESDHC_WTMK_LVL);

So the value to be written to this register should come from the
driver data, which is already used as a struct esdhc_soc_data to
extrapolate out the differences.

Going down the flag path will lead you to an even more of a stinking
shitpile than sdhci already is - if another version of the SoC requires
a different value there, what are you going to do?  Add yet another
flag for the next value?  What are you going to do when you have 16
different values?  Use 16 different flags?  Clearly that path is insane.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1194983

On Wed, Jul 29, 2015 at 05:03:51PM +0800, Haibo Chen wrote:
> Changes for v3:
> -Add property describe in binding doc.
> 
> Haibo Chen (6):
>   mmc: sdhci-esdhc-imx: add imx7d support and support HS400
>   mmc: sdhci-esdhc-imx: add tuning-step seting support
>   ARM: dts: imx7d-sdb: add eMMC5.0 support
>   mmc: sdhci-esdhc-imx: add compatible string in bingding doc
>   mmc: sdhci-esdhc-imx: config watermark level and burst length
>   mmc: sdhci-esdhc-imx: set back the burst_length_enable bit to 1
> 

Hi Haibo,

I'm a little busy these days.
Will help review it ASAP, maybe can do it tomorrow.

Regards
Dong Aisheng

>  .../devicetree/bindings/mmc/fsl-imx-esdhc.txt      |  2 +
>  arch/arm/boot/dts/imx7d-sdb.dts                    | 13 +++
>  drivers/mmc/host/sdhci-esdhc-imx.c                 | 97 +++++++++++++++++++++-
>  include/linux/platform_data/mmc-esdhc-imx.h        |  1 +
>  4 files changed, 112 insertions(+), 1 deletion(-)
> 
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1195872

Can you also fix the driver to NOT use mdelay(1) while in spinlock irqsafe?

A driver with such a design should have gotten a NAK in the first place ...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web