Linux MultiMedia Card development
 help / color / mirror / Atom feed
* Re: [PATCH v4] mmc: sdhci-msm: Add pm_runtime and system PM support
From: Ulf Hansson @ 2016-10-19  7:14 UTC (permalink / raw)
  To: Pramod Gurav
  Cc: Adrian Hunter, linux-mmc, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org,
	Harjani Ritesh
In-Reply-To: <20161018101624.27946-1-pramod.gurav@linaro.org>

On 18 October 2016 at 12:16, Pramod Gurav <pramod.gurav@linaro.org> wrote:
> Provides runtime PM callbacks to enable and disable clock resources
> when idle. Also support system PM callbacks to be called during system
> suspend and resume.
>
> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>

Looks good to me. I hold it for a while to allow people to test/ack.

Kind regards
Uffe

> ---
>
> Tested on DB410C.
>
> Changes in v4:
> - Remove calls to sdhci_runtime_resume_host/sdhci_runtime_suspend_host
>   from runtime callbacks as sdhc msm controller is capable of restoring
>   it's register values after clocks are disabled and re-enabled.
>
> Changes in v3:
> - Added CONFIG_PM around runtime pm function.
> - Replaced msm suspend/resume with generic function directly
> - Use SET_SYSTEM_SLEEP_PM_OPS instead of late version
>
> Changes in v2:
> - Moved pm_rutime enabling before adding host
> - Handled pm_rutime in remove
> - Changed runtime handling with reference from sdhci-of-at91.c
>
>  drivers/mmc/host/sdhci-msm.c | 66 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 8ef44a2a..33ec809 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -18,6 +18,7 @@
>  #include <linux/of_device.h>
>  #include <linux/delay.h>
>  #include <linux/mmc/mmc.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>
>  #include "sdhci-pltfm.h"
> @@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>                 goto clk_disable;
>         }
>
> +       pm_runtime_get_noresume(&pdev->dev);
> +       pm_runtime_set_active(&pdev->dev);
> +       pm_runtime_enable(&pdev->dev);
> +       pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> +       pm_runtime_use_autosuspend(&pdev->dev);
> +
>         ret = sdhci_add_host(host);
>         if (ret)
> -               goto clk_disable;
> +               goto pm_runtime_disable;
> +
> +       platform_set_drvdata(pdev, host);
> +
> +       pm_runtime_put_autosuspend(&pdev->dev);
>
>         return 0;
>
> +pm_runtime_disable:
> +       pm_runtime_disable(&pdev->dev);
> +       pm_runtime_set_suspended(&pdev->dev);
> +       pm_runtime_put_noidle(&pdev->dev);
>  clk_disable:
>         clk_disable_unprepare(msm_host->clk);
>  pclk_disable:
> @@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
>                     0xffffffff);
>
>         sdhci_remove_host(host, dead);
> +
> +       pm_runtime_get_sync(&pdev->dev);
> +       pm_runtime_disable(&pdev->dev);
> +       pm_runtime_put_noidle(&pdev->dev);
> +
>         clk_disable_unprepare(msm_host->clk);
>         clk_disable_unprepare(msm_host->pclk);
>         if (!IS_ERR(msm_host->bus_clk))
> @@ -693,12 +713,56 @@ static int sdhci_msm_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +#ifdef CONFIG_PM
> +static int sdhci_msm_runtime_suspend(struct device *dev)
> +{
> +       struct sdhci_host *host = dev_get_drvdata(dev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> +       clk_disable_unprepare(msm_host->clk);
> +       clk_disable_unprepare(msm_host->pclk);
> +
> +       return 0;
> +}
> +
> +static int sdhci_msm_runtime_resume(struct device *dev)
> +{
> +       struct sdhci_host *host = dev_get_drvdata(dev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +       int ret;
> +
> +       ret = clk_prepare_enable(msm_host->clk);
> +       if (ret) {
> +               dev_err(dev, "clk_enable failed: %d\n", ret);
> +               return ret;
> +       }
> +       ret = clk_prepare_enable(msm_host->pclk);
> +       if (ret) {
> +               dev_err(dev, "clk_enable failed: %d\n", ret);
> +               clk_disable_unprepare(msm_host->clk);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
> +       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +                                       pm_runtime_force_resume)
> +       SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend, sdhci_msm_runtime_resume,
> +                               NULL)
> +};
> +
>  static struct platform_driver sdhci_msm_driver = {
>         .probe = sdhci_msm_probe,
>         .remove = sdhci_msm_remove,
>         .driver = {
>                    .name = "sdhci_msm",
>                    .of_match_table = sdhci_msm_dt_match,
> +                  .pm = &sdhci_msm_pm_ops,
>         },
>  };
>
> --
> 2.9.3
>

^ permalink raw reply

* Re: [RFC 2/2] mmc: sdhci: Ignore capability register when it comes to speeds and use DT binding instead when sdhci-cap-speed-modes-broken is set.
From: Ulf Hansson @ 2016-10-19  7:10 UTC (permalink / raw)
  To: Zach Brown
  Cc: Adrian Hunter, Rob Herring, Mark Rutland, linux-mmc,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1476824736-9337-3-git-send-email-zach.brown-acOepvfBmUk@public.gmane.org>

On 18 October 2016 at 23:05, Zach Brown <zach.brown-acOepvfBmUk@public.gmane.org> wrote:
> When the sdhci-cap-speed-modes-broken DT property is set, the driver
> will ignore the bits of the capability registers that correspond to
> speed modes and will read the of properties of the device to determine
> which speeds are supported.

To me this seems like a great idea. Yeah, I proposed it so I guess
that's why. :-)

Anyway, it's Adrian call to decide how he want to go with this. He
might consider the other option [1] to be better.

Some more comments below.

>
> Signed-off-by: Zach Brown <zach.brown-acOepvfBmUk@public.gmane.org>
> ---
>  drivers/mmc/host/sdhci.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 1e25b01..100649a 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -22,6 +22,7 @@
>  #include <linux/scatterlist.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/of.h>
>
>  #include <linux/leds.h>
>
> @@ -3020,6 +3021,32 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
>  }
>  EXPORT_SYMBOL_GPL(__sdhci_read_caps);
>
> +void sdhci_get_speed_caps_from_of(struct sdhci_host *host)
> +{
> +       struct mmc_host *mmc = host->mmc;
> +
> +       host->caps &= ~SDHCI_CAN_DO_HISPD;
> +
> +       if (of_property_read_bool(mmc_dev(mmc)->of_node, "cap-sd-highspeed"))
> +               host->caps |= SDHCI_CAN_DO_HISPD;
> +
> +       if (host->version < SDHCI_SPEC_300)
> +               return;
> +
> +       host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_SDR104 |
> +                        SDHCI_SUPPORT_DDR50);
> +
> +       if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-sdr50"))
> +               host->caps1 |= SDHCI_SUPPORT_SDR50;
> +
> +       if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-sdr104"))
> +               host->caps1 |= SDHCI_SUPPORT_SDR104;
> +
> +       if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-ddr50"))
> +               host->caps1 |= SDHCI_SUPPORT_DDR50;
> +

I don't think you need a separate OF parsing function. Instead the
SDHCI variant drivers may call mmc_of_parse() to parse the generic mmc
OF properties and then read the SDHCI caps registers (in some way or
the other).

As reading the SDHCI caps registers is done in __sdhci_read_caps(),
you could instead check for the OF property
"sdhci-cap-speed-modes-broken" in there,  and if it's set, clear the
related bits. I think that's all you need.

Note, the above code considers only SD speed modes, I assume we should
include eMMC speed modes to be broken as well!?

> +}
> +
>  int sdhci_setup_host(struct sdhci_host *host)
>  {
>         struct mmc_host *mmc;
> @@ -3046,6 +3073,10 @@ int sdhci_setup_host(struct sdhci_host *host)
>                 return ret;
>
>         sdhci_read_caps(host);
> +       if (of_property_read_bool(mmc_dev(mmc)->of_node,
> +                                 "sdhci-cap-speed-modes-broken"))
> +               sdhci_get_speed_caps_from_of(host);
> +
>
>         override_timeout_clk = host->timeout_clk;
>
> --
> 2.7.4
>

Kind regards
Uffe

[1]
http://www.spinics.net/lists/devicetree/msg146401.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [v12, 0/8] Fix eSDHC host version register bug
From: Y.B. Lu @ 2016-10-19  2:47 UTC (permalink / raw)
  To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org
  Cc: Mark Rutland, Ulf Hansson, X.B. Xie, M.H. Lian,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-clk,
	Qiang Zhao, Russell King, Bhupesh Sharma, Jochen Friedrich,
	Claudiu Manoil,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Arnd Bergmann,
	Scott Wood, Rob Herring, Santosh Shilimkar,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mmc,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Leo Li <le>
In-Reply-To: <CAPDyKFoit0sTPpfi2=AHcdZsDwOJxbBX68O-1jBFL=5JN0m6_w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

+ Greg

Hi Greg,

I submitted this patchset for a MMC bug fix, and introduce the below patch which needs your ACK.
> > Arnd Bergmann (1):
> >   base: soc: introduce soc_device_match() interface
https://patchwork.kernel.org/patch/9342913/

Could you help to review it and give some comments or ACK.
Thank you very much.



Best regards,
Yangbo Lu


> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org]
> Sent: Tuesday, October 18, 2016 6:48 PM
> To: Y.B. Lu
> Cc: linux-mmc; Scott Wood; Arnd Bergmann; linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org;
> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-clk; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Mark Rutland;
> Rob Herring; Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil;
> Bhupesh Sharma; Qiang Zhao; Kumar Gala; Santosh Shilimkar; Leo Li; X.B.
> Xie; M.H. Lian
> Subject: Re: [v12, 0/8] Fix eSDHC host version register bug
> 
> On 21 September 2016 at 08:57, Yangbo Lu <yangbo.lu-3arQi8VN3Tc@public.gmane.org> wrote:
> > This patchset is used to fix a host version register bug in the
> > T4240-R1.0-R2.0 eSDHC controller. To match the SoC version and
> > revision, 10 previous version patchsets had tried many methods but all
> of them were rejected by reviewers.
> > Such as
> >         - dts compatible method
> >         - syscon method
> >         - ifdef PPC method
> >         - GUTS driver getting SVR method Anrd suggested a
> > soc_device_match method in v10, and this is the only available method
> > left now. This v11 patchset introduces the soc_device_match interface
> > in soc driver.
> >
> > The first six patches of Yangbo are to add the GUTS driver. This is
> > used to register a soc device which contain soc version and revision
> information.
> > The other two patches introduce the soc_device_match method in soc
> > driver and apply it on esdhc driver to fix this bug.
> >
> > Arnd Bergmann (1):
> >   base: soc: introduce soc_device_match() interface
> >
> > Yangbo Lu (7):
> >   dt: bindings: update Freescale DCFG compatible
> >   ARM64: dts: ls2080a: add device configuration node
> >   dt: bindings: move guts devicetree doc out of powerpc directory
> >   powerpc/fsl: move mpc85xx.h to include/linux/fsl
> >   soc: fsl: add GUTS driver for QorIQ platforms
> >   MAINTAINERS: add entry for Freescale SoC drivers
> >   mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0
> >
> >  Documentation/devicetree/bindings/arm/fsl.txt      |   6 +-
> >  .../bindings/{powerpc => soc}/fsl/guts.txt         |   3 +
> >  MAINTAINERS                                        |  11 +-
> >  arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi     |   6 +
> >  arch/powerpc/kernel/cpu_setup_fsl_booke.S          |   2 +-
> >  arch/powerpc/sysdev/fsl_pci.c                      |   2 +-
> >  drivers/base/Kconfig                               |   1 +
> >  drivers/base/soc.c                                 |  66 ++++++
> >  drivers/clk/clk-qoriq.c                            |   3 +-
> >  drivers/i2c/busses/i2c-mpc.c                       |   2 +-
> >  drivers/iommu/fsl_pamu.c                           |   3 +-
> >  drivers/mmc/host/Kconfig                           |   1 +
> >  drivers/mmc/host/sdhci-of-esdhc.c                  |  20 ++
> >  drivers/net/ethernet/freescale/gianfar.c           |   2 +-
> >  drivers/soc/Kconfig                                |   2 +-
> >  drivers/soc/fsl/Kconfig                            |  19 ++
> >  drivers/soc/fsl/Makefile                           |   1 +
> >  drivers/soc/fsl/guts.c                             | 257
> +++++++++++++++++++++
> >  include/linux/fsl/guts.h                           | 125 ++++++----
> >  .../asm/mpc85xx.h => include/linux/fsl/svr.h       |   4 +-
> >  include/linux/sys_soc.h                            |   3 +
> >  21 files changed, 478 insertions(+), 61 deletions(-)  rename
> > Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt (91%)
> > create mode 100644 drivers/soc/fsl/Kconfig  create mode 100644
> > drivers/soc/fsl/guts.c  rename arch/powerpc/include/asm/mpc85xx.h =>
> > include/linux/fsl/svr.h (97%)
> >
> > --
> > 2.1.0.27.g96db324
> >
> 
> This looks good to me! I am not sure which tree you want this to be
> picked up through, but unless no other volunteers I can take it through
> my mmc tree.
> 
> Although, before considering to apply, I need an ack from Scott/Arnd for
> the guts driver in patch 5/8 and I need an ack from Greg for patch 7/8,
> where the soc_device_match() interface is added (seems like you didn't
> add him on cc/to).
> 
> Kind regards
> Uffe

^ permalink raw reply

* RE: [v12, 0/8] Fix eSDHC host version register bug
From: Y.B. Lu @ 2016-10-19  2:40 UTC (permalink / raw)
  To: Ulf Hansson, Scott Wood, Arnd Bergmann
  Cc: Mark Rutland, X.B. Xie, M.H. Lian,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-clk,
	Qiang Zhao, Russell King, Bhupesh Sharma, Jochen Friedrich,
	Claudiu Manoil,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
	Santosh Shilimkar,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mmc,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Leo Li,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	Kumar
In-Reply-To: <CAPDyKFoit0sTPpfi2=AHcdZsDwOJxbBX68O-1jBFL=5JN0m6_w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org]
> Sent: Tuesday, October 18, 2016 6:48 PM
> To: Y.B. Lu
> Cc: linux-mmc; Scott Wood; Arnd Bergmann; linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org;
> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-clk; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Mark Rutland;
> Rob Herring; Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil;
> Bhupesh Sharma; Qiang Zhao; Kumar Gala; Santosh Shilimkar; Leo Li; X.B.
> Xie; M.H. Lian
> Subject: Re: [v12, 0/8] Fix eSDHC host version register bug
> 
> On 21 September 2016 at 08:57, Yangbo Lu <yangbo.lu-3arQi8VN3Tc@public.gmane.org> wrote:
> > This patchset is used to fix a host version register bug in the
> > T4240-R1.0-R2.0 eSDHC controller. To match the SoC version and
> > revision, 10 previous version patchsets had tried many methods but all
> of them were rejected by reviewers.
> > Such as
> >         - dts compatible method
> >         - syscon method
> >         - ifdef PPC method
> >         - GUTS driver getting SVR method Anrd suggested a
> > soc_device_match method in v10, and this is the only available method
> > left now. This v11 patchset introduces the soc_device_match interface
> > in soc driver.
> >
> > The first six patches of Yangbo are to add the GUTS driver. This is
> > used to register a soc device which contain soc version and revision
> information.
> > The other two patches introduce the soc_device_match method in soc
> > driver and apply it on esdhc driver to fix this bug.
> >
> > Arnd Bergmann (1):
> >   base: soc: introduce soc_device_match() interface
> >
> > Yangbo Lu (7):
> >   dt: bindings: update Freescale DCFG compatible
> >   ARM64: dts: ls2080a: add device configuration node
> >   dt: bindings: move guts devicetree doc out of powerpc directory
> >   powerpc/fsl: move mpc85xx.h to include/linux/fsl
> >   soc: fsl: add GUTS driver for QorIQ platforms
> >   MAINTAINERS: add entry for Freescale SoC drivers
> >   mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0
> >
> >  Documentation/devicetree/bindings/arm/fsl.txt      |   6 +-
> >  .../bindings/{powerpc => soc}/fsl/guts.txt         |   3 +
> >  MAINTAINERS                                        |  11 +-
> >  arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi     |   6 +
> >  arch/powerpc/kernel/cpu_setup_fsl_booke.S          |   2 +-
> >  arch/powerpc/sysdev/fsl_pci.c                      |   2 +-
> >  drivers/base/Kconfig                               |   1 +
> >  drivers/base/soc.c                                 |  66 ++++++
> >  drivers/clk/clk-qoriq.c                            |   3 +-
> >  drivers/i2c/busses/i2c-mpc.c                       |   2 +-
> >  drivers/iommu/fsl_pamu.c                           |   3 +-
> >  drivers/mmc/host/Kconfig                           |   1 +
> >  drivers/mmc/host/sdhci-of-esdhc.c                  |  20 ++
> >  drivers/net/ethernet/freescale/gianfar.c           |   2 +-
> >  drivers/soc/Kconfig                                |   2 +-
> >  drivers/soc/fsl/Kconfig                            |  19 ++
> >  drivers/soc/fsl/Makefile                           |   1 +
> >  drivers/soc/fsl/guts.c                             | 257
> +++++++++++++++++++++
> >  include/linux/fsl/guts.h                           | 125 ++++++----
> >  .../asm/mpc85xx.h => include/linux/fsl/svr.h       |   4 +-
> >  include/linux/sys_soc.h                            |   3 +
> >  21 files changed, 478 insertions(+), 61 deletions(-)  rename
> > Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt (91%)
> > create mode 100644 drivers/soc/fsl/Kconfig  create mode 100644
> > drivers/soc/fsl/guts.c  rename arch/powerpc/include/asm/mpc85xx.h =>
> > include/linux/fsl/svr.h (97%)
> >
> > --
> > 2.1.0.27.g96db324
> >
> 
> This looks good to me! I am not sure which tree you want this to be
> picked up through, but unless no other volunteers I can take it through
> my mmc tree.
> 
> Although, before considering to apply, I need an ack from Scott/Arnd for
> the guts driver in patch 5/8 and I need an ack from Greg for patch 7/8,
> where the soc_device_match() interface is added (seems like you didn't
> add him on cc/to).
> 

[Lu Yangbo-B47093] Thanks a lot for your clarifying, Uffe.
This patchset was based on mmc tree, and needed your picking up.
But I think it needs to be rebased now since I saw qbman driver was in drivers/soc/fsl/ now.
I will do that after collecting others' ACKs or comments.

Hi Scott and Arnd,
Could I get your ACTs if there're no other changes needed?
Thanks a lot.

> Kind regards
> Uffe

^ permalink raw reply

* Re: Regression after "do not use CMD13 to get status after speed mode switch"
From: Stephen Boyd @ 2016-10-19  1:03 UTC (permalink / raw)
  To: Ritesh Harjani, Linus Walleij, linux-mmc@vger.kernel.org,
	Ulf Hansson, Chaotian Jing
  Cc: linux-arm-msm@vger.kernel.org, Bjorn Andersson, Andy Gross,
	Georgi Djakov
In-Reply-To: <5f261465-9cc6-4403-5e1b-23df1b347dcb@codeaurora.org>

On 10/17/2016 09:06 PM, Ritesh Harjani wrote:
> Hi Linus,
>
> Recently for the same patch below was submitted for Qcom sdhci-msm
> driver.
>
> https://patchwork.kernel.org/patch/9237679/
>
> You mentioned yours is mmci driver.
> See if it provides any hints.
>

I still see an mmc1 error on apq8074 dragonboard, but I don't see the
long 10 minute timeout. Maybe this is a different problem?

[    5.440702] mmc1: card never left busy state
[    5.440729] mmc1: error -110 whilst initialising SD card
[    5.546527] mmc1: Reset 0x1 never completed.
[    5.546548] sdhci: =========== REGISTER DUMP (mmc1)===========
[    5.549854] sdhci: Sys addr: 0x00000000 | Version:  0x00003802
[    5.555497] sdhci: Blk size: 0x00004000 | Blk cnt:  0x00000000
[    5.561310] sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
[    5.567127] sdhci: Present:  0x01f80000 | Host ctl: 0x00000000
[    5.572944] sdhci: Power:    0x00000000 | Blk gap:  0x00000000
[    5.578759] sdhci: Wake-up:  0x00000000 | Clock:    0x00000003
[    5.584575] sdhci: Timeout:  0x00000000 | Int stat: 0x00000000
[    5.590391] sdhci: Int enab: 0x00000000 | Sig enab: 0x00000000
[    5.596207] sdhci: AC12 err: 0x00000000 | Slot int: 0x00000000
[    5.602023] sdhci: Caps:     0x2329c8b2 | Caps_1:   0x00008007
[    5.607838] sdhci: Cmd:      0x00000000 | Max curr: 0x00000000
[    5.613653] sdhci: Host ctl2: 0x00000000
[    5.619468] sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x00000000
[    5.623549] sdhci: ===========================================


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply

* [RFC 2/2] mmc: sdhci: Ignore capability register when it comes to speeds and use DT binding instead when sdhci-cap-speed-modes-broken is set.
From: Zach Brown @ 2016-10-18 21:05 UTC (permalink / raw)
  To: ulf.hansson
  Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree,
	linux-kernel, zach.brown
In-Reply-To: <1476824736-9337-1-git-send-email-zach.brown@ni.com>

When the sdhci-cap-speed-modes-broken DT property is set, the driver
will ignore the bits of the capability registers that correspond to
speed modes and will read the of properties of the device to determine
which speeds are supported.

Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/mmc/host/sdhci.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1e25b01..100649a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -22,6 +22,7 @@
 #include <linux/scatterlist.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pm_runtime.h>
+#include <linux/of.h>
 
 #include <linux/leds.h>
 
@@ -3020,6 +3021,32 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
 }
 EXPORT_SYMBOL_GPL(__sdhci_read_caps);
 
+void sdhci_get_speed_caps_from_of(struct sdhci_host *host)
+{
+	struct mmc_host *mmc = host->mmc;
+
+	host->caps &= ~SDHCI_CAN_DO_HISPD;
+
+	if (of_property_read_bool(mmc_dev(mmc)->of_node, "cap-sd-highspeed"))
+		host->caps |= SDHCI_CAN_DO_HISPD;
+
+	if (host->version < SDHCI_SPEC_300)
+		return;
+
+	host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_SDR104 |
+			 SDHCI_SUPPORT_DDR50);
+
+	if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-sdr50"))
+		host->caps1 |= SDHCI_SUPPORT_SDR50;
+
+	if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-sdr104"))
+		host->caps1 |= SDHCI_SUPPORT_SDR104;
+
+	if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-ddr50"))
+		host->caps1 |= SDHCI_SUPPORT_DDR50;
+
+}
+
 int sdhci_setup_host(struct sdhci_host *host)
 {
 	struct mmc_host *mmc;
@@ -3046,6 +3073,10 @@ int sdhci_setup_host(struct sdhci_host *host)
 		return ret;
 
 	sdhci_read_caps(host);
+	if (of_property_read_bool(mmc_dev(mmc)->of_node,
+				  "sdhci-cap-speed-modes-broken"))
+		sdhci_get_speed_caps_from_of(host);
+
 
 	override_timeout_clk = host->timeout_clk;
 
-- 
2.7.4


^ permalink raw reply related

* [RFC 1/2] mmc: sdhci: dt: Add device tree property sdhci-cap-speed-modes-broken
From: Zach Brown @ 2016-10-18 21:05 UTC (permalink / raw)
  To: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A
  Cc: adrian.hunter-ral2JQCrhuEAvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, zach.brown-acOepvfBmUk
In-Reply-To: <1476824736-9337-1-git-send-email-zach.brown-acOepvfBmUk@public.gmane.org>

On some systems the sdhci capabilty registers are incorrect for one
reason or another.

The sdhci-cap-speed-modes-broken property will let the driver know that
the sdhci capability registers should not be relied on for speed modes.
Instead the driver should check the mmc generic DT bindings.

Signed-off-by: Zach Brown <zach.brown-acOepvfBmUk@public.gmane.org>
---
 Documentation/devicetree/bindings/mmc/mmc.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 8a37782..671d6c0 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -52,6 +52,9 @@ Optional properties:
 - no-sdio: controller is limited to send sdio cmd during initialization
 - no-sd: controller is limited to send sd cmd during initialization
 - no-mmc: controller is limited to send mmc cmd during initialization
+- sdhci-cap-speed-modes-broken: One or more of the bits in the sdhci
+  capabilities registers representing speed modes are incorrect. All the bits
+  representing speed modes should be ignored.
 
 *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
 polarity properties, we have to fix the meaning of the "normal" and "inverted"
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [RFC 0/2] Ignore capability registers when it comes to speeds and use DT binding instead.
From: Zach Brown @ 2016-10-18 21:05 UTC (permalink / raw)
  To: ulf.hansson
  Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree,
	linux-kernel, zach.brown

The first patch add documentation about a new devicetree property
sdhci-cap-speed-modes-broken.

The second patch makes the sdhci use the DT binding instead of the caps
register for determining which speed modes are supported by the controller.

This RFC is an alternative to another patch set I sent up.
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1251944.html

Zach Brown (2):
  mmc: sdhci: dt: Add device tree property sdhci-cap-speed-modes-broken
  mmc: sdhci: Ignore capability register when it comes to speeds and use
    DT     binding instead when sdhci-cap-speed-modes-broken is set.

 Documentation/devicetree/bindings/mmc/mmc.txt |  3 +++
 drivers/mmc/host/sdhci.c                      | 31 +++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

--
2.7.4

^ permalink raw reply

* Re: [PATCH 1/2] mmc: sdhci-iproc: Add brcm,sdhci-iproc compat string in bindings document
From: Scott Branden @ 2016-10-18 20:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: Ulf Hansson, Mark Rutland, Ray Jui, Scott Branden, Adrian Hunter,
	BCM Kernel Feedback, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Anup Patel
In-Reply-To: <20161018131614.lskxbdir3ghun5md@rob-hp-laptop>

Hi Rob,

On 16-10-18 06:16 AM, Rob Herring wrote:
> On Wed, Oct 12, 2016 at 11:35:51AM -0700, Scott Branden wrote:
>> Adds brcm,sdhci-iproc compat string to DT bindings document for
>> the iProc SDHCI driver.
>>
>> Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
>> index be56d2b..aa58b94 100644
>> --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
>> @@ -7,6 +7,7 @@ Required properties:
>>  - compatible : Should be one of the following
>>  	       "brcm,bcm2835-sdhci"
>>  	       "brcm,sdhci-iproc-cygnus"
>> +	       "brcm,sdhci-iproc"
>
> Seems kind of generic. SoC specific compatible strings please.
>
The compatibility string is generic on purpose as it is not intended to 
be SoC specific but work on all new iproc SoCs that have the proper 
fixes in place for this block (unlike bcm2835 and cygnus class devices 
which can only do 32-bit accesses).  I could call it brcm,sdhci-iproc-v2 
if that is better or leave it as is.  Please let me know your preferences.

Regards,
Scott
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v4] MMC: meson: initial support for GX platforms
From: Kevin Hilman @ 2016-10-18 19:56 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: linux-amlogic, linux-arm-kernel

Initial support for the SD/eMMC controller in the Amlogic S905/GX*
family of SoCs.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
Changes from v3:
- better handling of clock error paths
- rename to meson-gx to reflect support for newer SoCs
- has now been tested with SDIO

 .../devicetree/bindings/mmc/amlogic,meson-gxbb.txt |  33 +
 MAINTAINERS                                        |   1 +
 drivers/mmc/host/Kconfig                           |  10 +
 drivers/mmc/host/Makefile                          |   1 +
 drivers/mmc/host/meson-gx.c                        | 853 +++++++++++++++++++++
 5 files changed, 898 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt
 create mode 100644 drivers/mmc/host/meson-gx.c

diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt b/Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt
new file mode 100644
index 000000000000..a2fa9a1c26ae
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-gxbb.txt
@@ -0,0 +1,33 @@
+Amlogic SD / eMMC controller for S905/GXBB family SoCs
+
+The MMC 5.1 compliant host controller on Amlogic provides the
+interface for SD, eMMC and SDIO devices.
+
+This file documents the properties in addition to those available in
+the MMC core bindings, documented by mmc.txt.
+
+Required properties:
+- compatible : contains one of:
+  - "amlogic,meson-gx-mmc"
+  - "amlogic,meson-gxbb-mmc"
+  - "amlogic,meson-gxl-mmc"
+  - "amlogic,meson-gxm-mmc"
+- clocks     : A list of phandle + clock-specifier pairs for the clocks listed in clock-names.
+- clock-names: Should contain the following:
+	"core" - Main peripheral bus clock
+	"clkin0" - Parent clock of internal mux
+	"clkin1" - Other parent clock of internal mux
+  The driver has an interal mux clock which switches between clkin0 and clkin1 depending on the
+  clock rate requested by the MMC core.
+
+Example:
+
+	sd_emmc_a: mmc@70000 {
+        	compatible = "amlogic,meson-gxbb-mmc";
+		reg = <0x0 0x70000 0x0 0x2000>;
+                interrupts = < GIC_SPI 216 IRQ_TYPE_EDGE_RISING>;
+		clocks = <&clkc CLKID_SD_EMMC_A>, <&xtal>, <&clkc CLKID_FCLK_DIV2>;
+		clock-names = "core", "clkin0", "clkin1";
+		pinctrl-0 = <&emmc_pins>;
+	};
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 1cd38a7e0064..73e8d64ec28c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1036,6 +1036,7 @@ F:	arch/arm/mach-meson/
 F:	arch/arm/boot/dts/meson*
 F:	arch/arm64/boot/dts/amlogic/
 F: 	drivers/pinctrl/meson/
+F:	drivers/mmc/host/meson*
 N:	meson
 
 ARM/Annapurna Labs ALPINE ARCHITECTURE
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 5274f503a39a..5cf7ebaf1e8b 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -322,6 +322,16 @@ config MMC_SDHCI_IPROC
 
 	  If unsure, say N.
 
+config MMC_MESON_GX
+	tristate "Amlogic S905/GX* SD/MMC Host Controller support"
+	depends on ARCH_MESON && MMC
+	help
+	  This selects support for the Amlogic SD/MMC Host Controller
+	  found on the S905/GX* family of SoCs.  This controller is
+	  MMC 5.1 compliant and supports SD, eMMC and SDIO interfaces.
+
+	  If you have a controller with this interface, say Y here.
+
 config MMC_MOXART
 	tristate "MOXART SD/MMC Host Controller support"
 	depends on ARCH_MOXART && MMC
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index e2bdaaf43184..1c4852999ae4 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
 obj-$(CONFIG_MMC_VUB300)	+= vub300.o
 obj-$(CONFIG_MMC_USHC)		+= ushc.o
 obj-$(CONFIG_MMC_WMT)		+= wmt-sdmmc.o
+obj-$(CONFIG_MMC_MESON_GX)	+= meson-gx.o
 obj-$(CONFIG_MMC_MOXART)	+= moxart-mmc.o
 obj-$(CONFIG_MMC_SUNXI)		+= sunxi-mmc.o
 obj-$(CONFIG_MMC_USDHI6ROL0)	+= usdhi6rol0.o
diff --git a/drivers/mmc/host/meson-gx.c b/drivers/mmc/host/meson-gx.c
new file mode 100644
index 000000000000..fd3c40322b2d
--- /dev/null
+++ b/drivers/mmc/host/meson-gx.c
@@ -0,0 +1,853 @@
+/*
+ * Amlogic SD/eMMC driver for the GX/S905 family SoCs
+ *
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Kevin Hilman <khilman@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ * The full GNU General Public License is included in this distribution
+ * in the file called COPYING.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/ioport.h>
+#include <linux/spinlock.h>
+#include <linux/dma-mapping.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/sdio.h>
+#include <linux/mmc/slot-gpio.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/regulator/consumer.h>
+
+#define DRIVER_NAME "meson-gxbb-mmc"
+
+#define SD_EMMC_CLOCK 0x0
+#define   CLK_DIV_SHIFT 0
+#define   CLK_DIV_WIDTH 6
+#define   CLK_DIV_MASK 0x3f
+#define   CLK_DIV_MAX 63
+#define   CLK_SRC_SHIFT 6
+#define   CLK_SRC_WIDTH 2
+#define   CLK_SRC_MASK 0x3
+#define   CLK_SRC_XTAL 0   /* external crystal */
+#define   CLK_SRC_XTAL_RATE 24000000
+#define   CLK_SRC_PLL 1    /* FCLK_DIV2 */
+#define   CLK_SRC_PLL_RATE 1000000000
+#define   CLK_PHASE_SHIFT 8
+#define   CLK_PHASE_MASK 0x3
+#define   CLK_PHASE_0 0
+#define   CLK_PHASE_90 1
+#define   CLK_PHASE_180 2
+#define   CLK_PHASE_270 3
+#define   CLK_ALWAYS_ON BIT(24)
+
+#define SD_EMMC_DElAY 0x4
+#define SD_EMMC_ADJUST 0x8
+#define SD_EMMC_CALOUT 0x10
+#define SD_EMMC_START 0x40
+#define   START_DESC_INIT BIT(0)
+#define   START_DESC_BUSY BIT(1)
+#define   START_DESC_ADDR_SHIFT 2
+#define   START_DESC_ADDR_MASK (~0x3)
+
+#define SD_EMMC_CFG 0x44
+#define   CFG_BUS_WIDTH_SHIFT 0
+#define   CFG_BUS_WIDTH_MASK 0x3
+#define   CFG_BUS_WIDTH_1 0x0
+#define   CFG_BUS_WIDTH_4 0x1
+#define   CFG_BUS_WIDTH_8 0x2
+#define   CFG_DDR BIT(2)
+#define   CFG_BLK_LEN_SHIFT 4
+#define   CFG_BLK_LEN_MASK 0xf
+#define   CFG_RESP_TIMEOUT_SHIFT 8
+#define   CFG_RESP_TIMEOUT_MASK 0xf
+#define   CFG_RC_CC_SHIFT 12
+#define   CFG_RC_CC_MASK 0xf
+#define   CFG_STOP_CLOCK BIT(22)
+#define   CFG_CLK_ALWAYS_ON BIT(18)
+#define   CFG_AUTO_CLK BIT(23)
+
+#define SD_EMMC_STATUS 0x48
+#define   STATUS_BUSY BIT(31)
+
+#define SD_EMMC_IRQ_EN 0x4c
+#define   IRQ_EN_MASK 0x3fff
+#define   IRQ_RXD_ERR_SHIFT 0
+#define   IRQ_RXD_ERR_MASK 0xff
+#define   IRQ_TXD_ERR BIT(8)
+#define   IRQ_DESC_ERR BIT(9)
+#define   IRQ_RESP_ERR BIT(10)
+#define   IRQ_RESP_TIMEOUT BIT(11)
+#define   IRQ_DESC_TIMEOUT BIT(12)
+#define   IRQ_END_OF_CHAIN BIT(13)
+#define   IRQ_RESP_STATUS BIT(14)
+#define   IRQ_SDIO BIT(15)
+
+#define SD_EMMC_CMD_CFG 0x50
+#define SD_EMMC_CMD_ARG 0x54
+#define SD_EMMC_CMD_DAT 0x58
+#define SD_EMMC_CMD_RSP 0x5c
+#define SD_EMMC_CMD_RSP1 0x60
+#define SD_EMMC_CMD_RSP2 0x64
+#define SD_EMMC_CMD_RSP3 0x68
+
+#define SD_EMMC_RXD 0x94
+#define SD_EMMC_TXD 0x94
+#define SD_EMMC_LAST_REG SD_EMMC_TXD
+
+#define SD_EMMC_CFG_BLK_SIZE 512 /* internal buffer max: 512 bytes */
+#define SD_EMMC_CFG_RESP_TIMEOUT 256 /* in clock cycles */
+#define SD_EMMC_CFG_CMD_GAP 16 /* in clock cycles */
+#define MUX_CLK_NUM_PARENTS 2
+
+struct meson_host {
+	struct	device		*dev;
+	struct	mmc_host	*mmc;
+	struct	mmc_request	*mrq;
+	struct	mmc_command	*cmd;
+
+	spinlock_t lock;
+	void __iomem *regs;
+	int irq;
+	u32 ocr_mask;
+	struct clk *core_clk;
+	struct clk_mux mux;
+	struct clk *mux_clk;
+	struct clk *mux_parent[MUX_CLK_NUM_PARENTS];
+	unsigned long mux_parent_rate[MUX_CLK_NUM_PARENTS];
+
+	struct clk_divider cfg_div;
+	struct clk *cfg_div_clk;
+
+	unsigned int bounce_buf_size;
+	void *bounce_buf;
+	dma_addr_t bounce_dma_addr;
+
+	bool vqmmc_enabled;
+};
+
+struct sd_emmc_desc {
+	u32 cmd_cfg;
+	u32 cmd_arg;
+	u32 cmd_data;
+	u32 cmd_resp;
+};
+#define CMD_CFG_LENGTH_SHIFT 0
+#define CMD_CFG_LENGTH_MASK 0x1ff
+#define CMD_CFG_BLOCK_MODE BIT(9)
+#define CMD_CFG_R1B BIT(10)
+#define CMD_CFG_END_OF_CHAIN BIT(11)
+#define CMD_CFG_TIMEOUT_SHIFT 12
+#define CMD_CFG_TIMEOUT_MASK 0xf
+#define CMD_CFG_NO_RESP BIT(16)
+#define CMD_CFG_NO_CMD BIT(17)
+#define CMD_CFG_DATA_IO BIT(18)
+#define CMD_CFG_DATA_WR BIT(19)
+#define CMD_CFG_RESP_NOCRC BIT(20)
+#define CMD_CFG_RESP_128 BIT(21)
+#define CMD_CFG_RESP_NUM BIT(22)
+#define CMD_CFG_DATA_NUM BIT(23)
+#define CMD_CFG_CMD_INDEX_SHIFT 24
+#define CMD_CFG_CMD_INDEX_MASK 0x3f
+#define CMD_CFG_ERROR BIT(30)
+#define CMD_CFG_OWNER BIT(31)
+
+#define CMD_DATA_MASK (~0x3)
+#define CMD_DATA_BIG_ENDIAN BIT(1)
+#define CMD_DATA_SRAM BIT(0)
+#define CMD_RESP_MASK (~0x1)
+#define CMD_RESP_SRAM BIT(0)
+
+static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
+{
+	struct mmc_host *mmc = host->mmc;
+	int ret = 0;
+	u32 cfg;
+
+	if (clk_rate) {
+		if (WARN_ON(clk_rate > mmc->f_max))
+			clk_rate = mmc->f_max;
+		else if (WARN_ON(clk_rate < mmc->f_min))
+			clk_rate = mmc->f_min;
+	}
+
+	if (clk_rate == mmc->actual_clock)
+		return 0;
+
+	/* stop clock */
+	cfg = readl(host->regs + SD_EMMC_CFG);
+	if (!(cfg & CFG_STOP_CLOCK)) {
+		cfg |= CFG_STOP_CLOCK;
+		writel(cfg, host->regs + SD_EMMC_CFG);
+	}
+
+	dev_dbg(host->dev, "change clock rate %u -> %lu\n",
+		mmc->actual_clock, clk_rate);
+
+	if (clk_rate == 0) {
+		mmc->actual_clock = 0;
+		return 0;
+	}
+
+	ret = clk_set_rate(host->cfg_div_clk, clk_rate);
+	if (ret)
+		dev_warn(host->dev, "Unable to set cfg_div_clk to %lu. ret=%d\n",
+			 clk_rate, ret);
+	else if (clk_rate && clk_rate != clk_get_rate(host->cfg_div_clk))
+		dev_warn(host->dev, "divider requested rate %lu != actual rate %lu: ret=%d\n",
+			 clk_rate, clk_get_rate(host->cfg_div_clk), ret);
+	else
+		mmc->actual_clock = clk_rate;
+
+	/* (re)start clock, if non-zero */
+	if (!ret && clk_rate) {
+		cfg = readl(host->regs + SD_EMMC_CFG);
+		cfg &= ~CFG_STOP_CLOCK;
+		writel(cfg, host->regs + SD_EMMC_CFG);
+	}
+
+	return ret;
+}
+
+/*
+ * The SD/eMMC IP block has an internal mux and divider used for
+ * generating the MMC clock.  Use the clock framework to create and
+ * manage these clocks.
+ */
+static int meson_mmc_clk_init(struct meson_host *host)
+{
+	struct clk_init_data init;
+	char clk_name[32];
+	int i, ret = 0;
+	const char *mux_parent_names[MUX_CLK_NUM_PARENTS];
+	unsigned int mux_parent_count = 0;
+	const char *clk_div_parents[1];
+	unsigned int f_min = UINT_MAX;
+	u32 clk_reg, cfg;
+
+	/* get the mux parents */
+	for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
+		char name[16];
+
+		snprintf(name, sizeof(name), "clkin%d", i);
+		host->mux_parent[i] = devm_clk_get(host->dev, name);
+		if (IS_ERR(host->mux_parent[i])) {
+			ret = PTR_ERR(host->mux_parent[i]);
+			if (PTR_ERR(host->mux_parent[i]) != -EPROBE_DEFER)
+				dev_err(host->dev, "Missing clock %s\n", name);
+			host->mux_parent[i] = NULL;
+			return ret;
+		}
+
+		host->mux_parent_rate[i] = clk_get_rate(host->mux_parent[i]);
+		mux_parent_names[i] = __clk_get_name(host->mux_parent[i]);
+		mux_parent_count++;
+		if (host->mux_parent_rate[i] < f_min)
+			f_min = host->mux_parent_rate[i];
+	}
+
+	/* cacluate f_min based on input clocks, and max divider value */
+	if (f_min != UINT_MAX)
+		f_min = DIV_ROUND_UP(CLK_SRC_XTAL_RATE, CLK_DIV_MAX);
+	else
+		f_min = 4000000;  /* default min: 400 MHz */
+	host->mmc->f_min = f_min;
+
+	/* create the mux */
+	snprintf(clk_name, sizeof(clk_name), "%s#mux", dev_name(host->dev));
+	init.name = clk_name;
+	init.ops = &clk_mux_ops;
+	init.flags = 0;
+	init.parent_names = mux_parent_names;
+	init.num_parents = mux_parent_count;
+
+	host->mux.reg = host->regs + SD_EMMC_CLOCK;
+	host->mux.shift = CLK_SRC_SHIFT;
+	host->mux.mask = CLK_SRC_MASK;
+	host->mux.flags = 0;
+	host->mux.table = NULL;
+	host->mux.hw.init = &init;
+
+	host->mux_clk = devm_clk_register(host->dev, &host->mux.hw);
+	if (WARN_ON(IS_ERR(host->mux_clk)))
+		return PTR_ERR(host->mux_clk);
+
+	/* create the divider */
+	snprintf(clk_name, sizeof(clk_name), "%s#div", dev_name(host->dev));
+	init.name = devm_kstrdup(host->dev, clk_name, GFP_KERNEL);
+	init.ops = &clk_divider_ops;
+	init.flags = CLK_SET_RATE_PARENT;
+	clk_div_parents[0] = __clk_get_name(host->mux_clk);
+	init.parent_names = clk_div_parents;
+	init.num_parents = ARRAY_SIZE(clk_div_parents);
+
+	host->cfg_div.reg = host->regs + SD_EMMC_CLOCK;
+	host->cfg_div.shift = CLK_DIV_SHIFT;
+	host->cfg_div.width = CLK_DIV_WIDTH;
+	host->cfg_div.hw.init = &init;
+	host->cfg_div.flags = CLK_DIVIDER_ONE_BASED |
+		CLK_DIVIDER_ROUND_CLOSEST | CLK_DIVIDER_ALLOW_ZERO;
+
+	host->cfg_div_clk = devm_clk_register(host->dev, &host->cfg_div.hw);
+	if (WARN_ON(PTR_ERR_OR_ZERO(host->cfg_div_clk)))
+		return PTR_ERR(host->cfg_div_clk);
+
+	/* init SD_EMMC_CLOCK to sane defaults w/min clock rate */
+	clk_reg = 0;
+	clk_reg |= CLK_PHASE_180 << CLK_PHASE_SHIFT;
+	clk_reg |= CLK_SRC_XTAL << CLK_SRC_SHIFT;
+	clk_reg |= CLK_DIV_MAX << CLK_DIV_SHIFT;
+	clk_reg &= ~CLK_ALWAYS_ON;
+	writel(clk_reg, host->regs + SD_EMMC_CLOCK);
+
+	/* Ensure clock starts in "auto" mode, not "always on" */
+	cfg = readl(host->regs + SD_EMMC_CFG);
+	cfg &= ~CFG_CLK_ALWAYS_ON;
+	cfg |= CFG_AUTO_CLK;
+	writel(cfg, host->regs + SD_EMMC_CFG);
+
+	ret = clk_prepare_enable(host->cfg_div_clk);
+	if (!ret)
+		ret = meson_mmc_clk_set(host, f_min);
+
+	if (!ret)
+		clk_disable_unprepare(host->cfg_div_clk);
+
+	return ret;
+}
+
+static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	struct meson_host *host = mmc_priv(mmc);
+	u32 bus_width;
+	u32 val, orig;
+
+	/*
+	 * GPIO regulator, only controls switching between 1v8 and
+	 * 3v3, doesn't support MMC_POWER_OFF, MMC_POWER_ON.
+	 */
+	switch (ios->power_mode) {
+	case MMC_POWER_OFF:
+		if (!IS_ERR(mmc->supply.vmmc))
+			mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+		if (!IS_ERR(mmc->supply.vqmmc) && host->vqmmc_enabled) {
+			regulator_disable(mmc->supply.vqmmc);
+			host->vqmmc_enabled = false;
+		}
+
+		break;
+
+	case MMC_POWER_UP:
+		if (!IS_ERR(mmc->supply.vmmc))
+			mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, ios->vdd);
+		break;
+
+	case MMC_POWER_ON:
+		if (!IS_ERR(mmc->supply.vqmmc) && !host->vqmmc_enabled) {
+			int ret = regulator_enable(mmc->supply.vqmmc);
+
+			if (ret < 0)
+				dev_err(mmc_dev(mmc),
+					"failed to enable vqmmc regulator\n");
+			else
+				host->vqmmc_enabled = true;
+		}
+
+		break;
+	}
+
+
+	meson_mmc_clk_set(host, ios->clock);
+
+	/* Bus width */
+	val = readl(host->regs + SD_EMMC_CFG);
+	switch (ios->bus_width) {
+	case MMC_BUS_WIDTH_1:
+		bus_width = CFG_BUS_WIDTH_1;
+		break;
+	case MMC_BUS_WIDTH_4:
+		bus_width = CFG_BUS_WIDTH_4;
+		break;
+	case MMC_BUS_WIDTH_8:
+		bus_width = CFG_BUS_WIDTH_8;
+		break;
+	default:
+		dev_err(host->dev, "Invalid ios->bus_width: %u.  Setting to 4.\n",
+			ios->bus_width);
+		bus_width = CFG_BUS_WIDTH_4;
+		return;
+	}
+
+	val = readl(host->regs + SD_EMMC_CFG);
+	orig = val;
+
+	val &= ~(CFG_BUS_WIDTH_MASK << CFG_BUS_WIDTH_SHIFT);
+	val |= bus_width << CFG_BUS_WIDTH_SHIFT;
+
+	val &= ~(CFG_BLK_LEN_MASK << CFG_BLK_LEN_SHIFT);
+	val |= ilog2(SD_EMMC_CFG_BLK_SIZE) << CFG_BLK_LEN_SHIFT;
+
+	val &= ~(CFG_RESP_TIMEOUT_MASK << CFG_RESP_TIMEOUT_SHIFT);
+	val |= ilog2(SD_EMMC_CFG_RESP_TIMEOUT) << CFG_RESP_TIMEOUT_SHIFT;
+
+	val &= ~(CFG_RC_CC_MASK << CFG_RC_CC_SHIFT);
+	val |= ilog2(SD_EMMC_CFG_CMD_GAP) << CFG_RC_CC_SHIFT;
+
+	writel(val, host->regs + SD_EMMC_CFG);
+
+	if (val != orig)
+		dev_dbg(host->dev, "%s: SD_EMMC_CFG: 0x%08x -> 0x%08x\n",
+			__func__, orig, val);
+}
+
+static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	struct meson_host *host = mmc_priv(mmc);
+
+	WARN_ON(host->mrq != mrq);
+
+	host->mrq = NULL;
+	host->cmd = NULL;
+	mmc_request_done(host->mmc, mrq);
+
+	return 0;
+}
+
+static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
+{
+	struct meson_host *host = mmc_priv(mmc);
+	struct sd_emmc_desc *desc, desc_tmp;
+	u32 cfg;
+	u8 blk_len, cmd_cfg_timeout;
+	unsigned int xfer_bytes = 0;
+
+	/* Setup descriptors */
+	dma_rmb();
+	desc = &desc_tmp;
+	memset(desc, 0, sizeof(struct sd_emmc_desc));
+
+	desc->cmd_cfg |= (cmd->opcode & CMD_CFG_CMD_INDEX_MASK)	<<
+		CMD_CFG_CMD_INDEX_SHIFT;
+	desc->cmd_cfg |= CMD_CFG_OWNER;  /* owned by CPU */
+	desc->cmd_arg = cmd->arg;
+
+	/* Response */
+	if (cmd->flags & MMC_RSP_PRESENT) {
+		desc->cmd_cfg &= ~CMD_CFG_NO_RESP;
+		if (cmd->flags & MMC_RSP_136)
+			desc->cmd_cfg |= CMD_CFG_RESP_128;
+		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
+		desc->cmd_resp = 0;
+
+		if (!(cmd->flags & MMC_RSP_CRC))
+			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
+
+		if (cmd->flags & MMC_RSP_BUSY)
+			desc->cmd_cfg |= CMD_CFG_R1B;
+	} else {
+		desc->cmd_cfg |= CMD_CFG_NO_RESP;
+	}
+
+	/* data? */
+	if (cmd->data) {
+		desc->cmd_cfg |= CMD_CFG_DATA_IO;
+		if (cmd->data->blocks > 1) {
+			desc->cmd_cfg |= CMD_CFG_BLOCK_MODE;
+			desc->cmd_cfg |=
+				(cmd->data->blocks & CMD_CFG_LENGTH_MASK) <<
+				CMD_CFG_LENGTH_SHIFT;
+
+			/* check if block-size matches, if not update */
+			cfg = readl(host->regs + SD_EMMC_CFG);
+			blk_len = cfg & (CFG_BLK_LEN_MASK << CFG_BLK_LEN_SHIFT);
+			blk_len >>= CFG_BLK_LEN_SHIFT;
+			if (blk_len != ilog2(cmd->data->blksz)) {
+				dev_warn(host->dev, "%s: update blk_len %d -> %d\n",
+					__func__, blk_len,
+					 ilog2(cmd->data->blksz));
+				blk_len = ilog2(cmd->data->blksz);
+				cfg &= ~(CFG_BLK_LEN_MASK << CFG_BLK_LEN_SHIFT);
+				cfg |= blk_len << CFG_BLK_LEN_SHIFT;
+				writel(cfg, host->regs + SD_EMMC_CFG);
+			}
+		} else {
+			desc->cmd_cfg &= ~CMD_CFG_BLOCK_MODE;
+			desc->cmd_cfg |=
+				(cmd->data->blksz & CMD_CFG_LENGTH_MASK) <<
+				CMD_CFG_LENGTH_SHIFT;
+		}
+
+		cmd->data->bytes_xfered = 0;
+		xfer_bytes = cmd->data->blksz * cmd->data->blocks;
+		if (cmd->data->flags & MMC_DATA_WRITE) {
+			desc->cmd_cfg |= CMD_CFG_DATA_WR;
+			WARN_ON(xfer_bytes > host->bounce_buf_size);
+			sg_copy_to_buffer(cmd->data->sg, cmd->data->sg_len,
+					  host->bounce_buf, xfer_bytes);
+			cmd->data->bytes_xfered = xfer_bytes;
+			dma_wmb();
+		} else {
+			desc->cmd_cfg &= ~CMD_CFG_DATA_WR;
+		}
+
+		if (xfer_bytes > 0) {
+			desc->cmd_cfg &= ~CMD_CFG_DATA_NUM;
+			desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
+		} else {
+			/* write data to data_addr */
+			desc->cmd_cfg |= CMD_CFG_DATA_NUM;
+			desc->cmd_data = 0;
+		}
+
+		cmd_cfg_timeout = 12;
+	} else {
+		desc->cmd_cfg &= ~CMD_CFG_DATA_IO;
+		cmd_cfg_timeout = 10;
+	}
+	desc->cmd_cfg |= (cmd_cfg_timeout & CMD_CFG_TIMEOUT_MASK) <<
+		CMD_CFG_TIMEOUT_SHIFT;
+
+	host->cmd = cmd;
+
+	/* Last descriptor */
+	desc->cmd_cfg |= CMD_CFG_END_OF_CHAIN;
+	writel(desc->cmd_cfg, host->regs + SD_EMMC_CMD_CFG);
+	writel(desc->cmd_data, host->regs + SD_EMMC_CMD_DAT);
+	writel(desc->cmd_resp, host->regs + SD_EMMC_CMD_RSP);
+	wmb(); /* ensure descriptor is written before kicked */
+	writel(desc->cmd_arg, host->regs + SD_EMMC_CMD_ARG);
+}
+
+static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	struct meson_host *host = mmc_priv(mmc);
+
+	WARN_ON(host->mrq != NULL);
+
+	/* Stop execution */
+	writel(0, host->regs + SD_EMMC_START);
+
+	/* clear, ack, enable all interrupts */
+	writel(0, host->regs + SD_EMMC_IRQ_EN);
+	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
+	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
+
+	host->mrq = mrq;
+
+	if (mrq->sbc)
+		meson_mmc_start_cmd(mmc, mrq->sbc);
+	else
+		meson_mmc_start_cmd(mmc, mrq->cmd);
+}
+
+static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
+{
+	struct meson_host *host = mmc_priv(mmc);
+
+	if (cmd->flags & MMC_RSP_136) {
+		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP3);
+		cmd->resp[1] = readl(host->regs + SD_EMMC_CMD_RSP2);
+		cmd->resp[2] = readl(host->regs + SD_EMMC_CMD_RSP1);
+		cmd->resp[3] = readl(host->regs + SD_EMMC_CMD_RSP);
+	} else if (cmd->flags & MMC_RSP_PRESENT) {
+		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP);
+	}
+
+	return 0;
+}
+
+static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
+{
+	struct meson_host *host = dev_id;
+	struct mmc_request *mrq;
+	struct mmc_command *cmd = host->cmd;
+	u32 irq_en, status, raw_status;
+	irqreturn_t ret = IRQ_HANDLED;
+
+	if (WARN_ON(!host))
+		return IRQ_NONE;
+
+	mrq = host->mrq;
+
+	if (WARN_ON(!mrq))
+		return IRQ_NONE;
+
+	if (WARN_ON(!cmd))
+		return IRQ_NONE;
+
+	spin_lock(&host->lock);
+	irq_en = readl(host->regs + SD_EMMC_IRQ_EN);
+	raw_status = readl(host->regs + SD_EMMC_STATUS);
+	status = raw_status & irq_en;
+
+	if (!status) {
+		dev_warn(host->dev, "Spurious IRQ! status=0x%08x, irq_en=0x%08x\n",
+			 raw_status, irq_en);
+		ret = IRQ_NONE;
+		goto out;
+	}
+
+	cmd->error = 0;
+	if (status & IRQ_RXD_ERR_MASK) {
+		dev_dbg(host->dev, "Unhandled IRQ: RXD error\n");
+		cmd->error = -EILSEQ;
+	}
+	if (status & IRQ_TXD_ERR) {
+		dev_dbg(host->dev, "Unhandled IRQ: TXD error\n");
+		cmd->error = -EILSEQ;
+	}
+	if (status & IRQ_DESC_ERR)
+		dev_dbg(host->dev, "Unhandled IRQ: Descriptor error\n");
+	if (status & IRQ_RESP_ERR) {
+		dev_dbg(host->dev, "Unhandled IRQ: Response error\n");
+		cmd->error = -EILSEQ;
+	}
+	if (status & IRQ_RESP_TIMEOUT) {
+		dev_dbg(host->dev, "Unhandled IRQ: Response timeout\n");
+		cmd->error = -ETIMEDOUT;
+	}
+	if (status & IRQ_DESC_TIMEOUT) {
+		dev_dbg(host->dev, "Unhandled IRQ: Descriptor timeout\n");
+		cmd->error = -ETIMEDOUT;
+	}
+	if (status & IRQ_SDIO)
+		dev_dbg(host->dev, "Unhandled IRQ: SDIO.\n");
+
+	if (status & (IRQ_END_OF_CHAIN | IRQ_RESP_STATUS))
+		ret = IRQ_WAKE_THREAD;
+	else  {
+		dev_warn(host->dev, "Unknown IRQ! status=0x%04x: MMC CMD%u arg=0x%08x flags=0x%08x stop=%d\n",
+			 status, cmd->opcode, cmd->arg,
+			 cmd->flags, mrq->stop ? 1 : 0);
+		if (cmd->data) {
+			struct mmc_data *data = cmd->data;
+
+			dev_warn(host->dev, "\tblksz %u blocks %u flags 0x%08x (%s%s)",
+				 data->blksz, data->blocks, data->flags,
+				 data->flags & MMC_DATA_WRITE ? "write" : "",
+				 data->flags & MMC_DATA_READ ? "read" : "");
+		}
+	}
+
+out:
+	/* ack all (enabled) interrupts */
+	writel(status, host->regs + SD_EMMC_STATUS);
+
+	if (ret == IRQ_HANDLED) {
+		meson_mmc_read_resp(host->mmc, cmd);
+		meson_mmc_request_done(host->mmc, cmd->mrq);
+	}
+
+	spin_unlock(&host->lock);
+	return ret;
+}
+
+static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
+{
+	struct meson_host *host = dev_id;
+	struct mmc_request *mrq = host->mrq;
+	struct mmc_command *cmd = host->cmd;
+	struct mmc_data *data;
+	unsigned int xfer_bytes;
+	int ret = IRQ_HANDLED;
+
+	if (WARN_ON(!mrq))
+		ret = IRQ_NONE;
+
+	if (WARN_ON(!cmd))
+		ret = IRQ_NONE;
+
+	data = cmd->data;
+	if (data) {
+		xfer_bytes = data->blksz * data->blocks;
+		if (data->flags & MMC_DATA_READ) {
+			WARN_ON(xfer_bytes > host->bounce_buf_size);
+			sg_copy_from_buffer(data->sg, data->sg_len,
+					    host->bounce_buf, xfer_bytes);
+			data->bytes_xfered = xfer_bytes;
+		}
+	}
+
+	meson_mmc_read_resp(host->mmc, cmd);
+	if (!data || !data->stop || mrq->sbc)
+		meson_mmc_request_done(host->mmc, mrq);
+	else
+		meson_mmc_start_cmd(host->mmc, data->stop);
+
+	return ret;
+}
+
+/*
+ * NOTE: we only need this until the GPIO/pinctrl driver can handle
+ * interrupts.  For now, the MMC core will use this for polling.
+ */
+static int meson_mmc_get_cd(struct mmc_host *mmc)
+{
+	int status = mmc_gpio_get_cd(mmc);
+
+	if (status == -ENOSYS)
+		return 1; /* assume present */
+
+	return status;
+}
+
+static const struct mmc_host_ops meson_mmc_ops = {
+	.request	= meson_mmc_request,
+	.set_ios	= meson_mmc_set_ios,
+	.get_cd         = meson_mmc_get_cd,
+};
+
+static int meson_mmc_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct meson_host *host;
+	struct mmc_host *mmc;
+	int ret;
+
+	mmc = mmc_alloc_host(sizeof(struct meson_host), &pdev->dev);
+	if (!mmc)
+		return -ENOMEM;
+	host = mmc_priv(mmc);
+	host->mmc = mmc;
+	host->dev = &pdev->dev;
+	dev_set_drvdata(&pdev->dev, host);
+
+	spin_lock_init(&host->lock);
+
+	/* Get regulators and the supported OCR mask */
+	host->vqmmc_enabled = false;
+	ret = mmc_regulator_get_supply(mmc);
+	if (ret == -EPROBE_DEFER)
+		goto free_host;
+
+	ret = mmc_of_parse(mmc);
+	if (ret) {
+		dev_warn(&pdev->dev, "error parsing DT: %d\n", ret);
+		goto free_host;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	host->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(host->regs)) {
+		ret = PTR_ERR(host->regs);
+		goto free_host;
+	}
+
+	host->irq = platform_get_irq(pdev, 0);
+	if (host->irq == 0) {
+		dev_err(&pdev->dev, "failed to get interrupt resource.\n");
+		ret = -EINVAL;
+		goto free_host;
+	}
+
+	host->core_clk = devm_clk_get(&pdev->dev, "core");
+	if (IS_ERR(host->core_clk)) {
+		ret = PTR_ERR(host->core_clk);
+		goto free_host;
+	}
+
+	ret = clk_prepare_enable(host->core_clk);
+	if (ret)
+		goto free_host;
+
+	ret = meson_mmc_clk_init(host);
+	if (ret)
+		goto free_host;
+
+	/* Stop execution */
+	writel(0, host->regs + SD_EMMC_START);
+
+	/* clear, ack, enable all interrupts */
+	writel(0, host->regs + SD_EMMC_IRQ_EN);
+	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
+
+	ret = devm_request_threaded_irq(&pdev->dev, host->irq,
+					meson_mmc_irq, meson_mmc_irq_thread,
+					IRQF_SHARED, DRIVER_NAME, host);
+	if (ret)
+		goto free_host;
+
+	/* data bounce buffer */
+	host->bounce_buf_size = SZ_512K;
+	host->bounce_buf =
+		dma_alloc_coherent(host->dev, host->bounce_buf_size,
+				   &host->bounce_dma_addr, GFP_KERNEL);
+	if (host->bounce_buf == NULL) {
+		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
+		ret = -ENOMEM;
+		goto free_host;
+	}
+
+	mmc->ops = &meson_mmc_ops;
+	mmc_add_host(mmc);
+
+	return 0;
+
+free_host:
+	clk_disable_unprepare(host->cfg_div_clk);
+	clk_disable_unprepare(host->core_clk);
+	mmc_free_host(mmc);
+	return ret;
+}
+
+static int meson_mmc_remove(struct platform_device *pdev)
+{
+	struct meson_host *host = dev_get_drvdata(&pdev->dev);
+
+	if (WARN_ON(!host))
+		return 0;
+
+	if (host->bounce_buf)
+		dma_free_coherent(host->dev, host->bounce_buf_size,
+				  host->bounce_buf, host->bounce_dma_addr);
+
+	clk_disable_unprepare(host->cfg_div_clk);
+	clk_disable_unprepare(host->core_clk);
+
+	mmc_free_host(host->mmc);
+	return 0;
+}
+
+static const struct of_device_id meson_mmc_of_match[] = {
+	{ .compatible = "amlogic,meson-gx-mmc", },
+	{ .compatible = "amlogic,meson-gxbb-mmc", },
+	{ .compatible = "amlogic,meson-gxl-mmc", },
+	{ .compatible = "amlogic,meson-gxm-mmc", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, meson_mmc_of_match);
+
+static struct platform_driver meson_mmc_driver = {
+	.probe		= meson_mmc_probe,
+	.remove		= meson_mmc_remove,
+	.driver		= {
+		.name = DRIVER_NAME,
+		.of_match_table = of_match_ptr(meson_mmc_of_match),
+	},
+};
+
+module_platform_driver(meson_mmc_driver);
+
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_DESCRIPTION("Amlogic S905/GXBB SD/eMMC driver");
+MODULE_AUTHOR("Kevin Hilman <khilman@baylibre.com>");
+MODULE_LICENSE("GPL v2");
+
-- 
2.9.3


^ permalink raw reply related

* [PATCH] mmc: sdhci-esdhc-imx: make sure usdhc clock enabled while doing suspend
From: Haibo Chen @ 2016-10-18  7:39 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, dongas86; +Cc: linux-mmc, aisheng.dong, haibo.chen

When suspend usdhc, it will access usdhc register. So usdhc clock
should be enabled, otherwise the access usdhc register will return
error or cause system hung.

Take this into consideration, if system enable a usdhc and do not
connect any SD/SDIO/MMC card, after system boot up, this usdhc
will do runtime suspend, and close all usdhc clock. At this time,
if suspend the system, due to no card persent, usdhc runtime resume
will not be called. So usdhc clock still closed, then in suspend,
once access usdhc register, system hung or bus error return.

This patch make sure usdhc clock always enabled while doing usdhc
suspend.

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

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 7123ef9..1df3846 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1322,17 +1322,28 @@ static int sdhci_esdhc_suspend(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
 
+#ifdef CONFIG_PM
+	pm_runtime_get_sync(host->mmc->parent);
+#endif
+
 	return sdhci_suspend_host(host);
 }
 
 static int sdhci_esdhc_resume(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
+	int ret;
 
 	/* re-initialize hw state in case it's lost in low power mode */
 	sdhci_esdhc_imx_hwinit(host);
+	ret = sdhci_resume_host(host);
 
-	return sdhci_resume_host(host);
+#ifdef CONFIG_PM
+	pm_runtime_mark_last_busy(host->mmc->parent);
+	pm_runtime_put_autosuspend(host->mmc->parent);
+#endif
+
+	return ret;
 }
 #endif
 
-- 
1.9.1


^ permalink raw reply related

* [PATCH 0/2] Add device tree property and quirk for supporting sdhci
From: Zach Brown @ 2016-10-18 15:37 UTC (permalink / raw)
  To: ulf.hansson
  Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree,
	linux-kernel, zach.brown

Some board configurations can not support sd highspeed mode due to the distance
between the card slot and the controller. The card and controller report that
they are capable of highspeed however, so we need a mechanism for specifying
that the setup is incapable of supporting highspeed mode.

The first patch adds documentation about a new devicetree property
broken-highspeed.

The second patch keeps the sd controller and card from going into highspeed
mode when the property is set.


Zach Brown (2):
  sdhci: Add device tree property broken-highspeed
  sdhci: Prevent SD from doing high-speed timing when broken-highspeed
    property is set

 Documentation/devicetree/bindings/mmc/mmc.txt | 2 ++
 drivers/mmc/host/sdhci.c                      | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.7.4


^ permalink raw reply

* [PATCH 2/2] sdhci: Prevent SD from doing high-speed timing when broken-highspeed property is set
From: Zach Brown @ 2016-10-18 15:37 UTC (permalink / raw)
  To: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A
  Cc: adrian.hunter-ral2JQCrhuEAvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, zach.brown-acOepvfBmUk
In-Reply-To: <1476805026-677-1-git-send-email-zach.brown-acOepvfBmUk@public.gmane.org>

When the broken-highspeed property is set the sdhci driver will not
go into highspeed mode even if the controller and card appear to
otherwise support highspeed mode.

This is useful in cases where the controller and card support highspeed,
but the board configuration or some other issue make highspeed
impossible. For example, we send the SDIO lines through a fpga so we
need the data to change on the falling edge of the clock or there will
be issues with hold time.

Signed-off-by: Zach Brown <zach.brown-acOepvfBmUk@public.gmane.org>
Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/mmc/host/sdhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1e25b01..4ec44fd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -22,6 +22,7 @@
 #include <linux/scatterlist.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pm_runtime.h>
+#include <linux/of.h>
 
 #include <linux/leds.h>
 
@@ -3269,7 +3270,8 @@ int sdhci_setup_host(struct sdhci_host *host)
 	if (host->quirks2 & SDHCI_QUIRK2_HOST_NO_CMD23)
 		mmc->caps &= ~MMC_CAP_CMD23;
 
-	if (host->caps & SDHCI_CAN_DO_HISPD)
+	if ((host->caps & SDHCI_CAN_DO_HISPD) &&
+	    !(of_property_read_bool(mmc_dev(mmc)->of_node, "broken-highspeed")))
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
 
 	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 1/2] sdhci: Add device tree property broken-highspeed
From: Zach Brown @ 2016-10-18 15:37 UTC (permalink / raw)
  To: ulf.hansson
  Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree,
	linux-kernel, zach.brown
In-Reply-To: <1476805026-677-1-git-send-email-zach.brown@ni.com>

Certain board configurations can make highspeed malfuction due to timing
issues. In these cases a way is needed to force the controller and card
into standard speed even if they otherwise appear to be capable of
highspeed.

The broken-highspeed property will let the sdhci driver know that
highspeed will not work.

Signed-off-by: Zach Brown <zach.brown@ni.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mmc/mmc.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 8a37782..a2b298c 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -52,6 +52,8 @@ Optional properties:
 - no-sdio: controller is limited to send sdio cmd during initialization
 - no-sd: controller is limited to send sd cmd during initialization
 - no-mmc: controller is limited to send mmc cmd during initialization
+- broken-highspeed: Highspeed is broken, even if the controller and card
+  themselves claim they support highspeed.
 
 *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
 polarity properties, we have to fix the meaning of the "normal" and "inverted"
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Gregory CLEMENT @ 2016-10-18 13:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Rob Herring, Ziji Hu, Ulf Hansson, Adrian Hunter, linux-mmc,
	Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, devicetree,
	Thomas Petazzoni, linux-arm-kernel, Jack(SH) Zhu, Jimmy Xu,
	Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones, Shiwu Zhang,
	Victor Gu, Wei(SOCP) Liu, Wilson Ding
In-Reply-To: <d95c3035-e3cd-a93c-d408-27f4c8dba589@marvell.com>

Hi Rob,
 
 On mar., oct. 11 2016, Ziji Hu <huziji@marvell.com> wrote:
[...]

>>> +  Different Xenon SDHC release has different register set size.
>>> +  The specific size should also refer to the SOC implementation.
>>> +
>>> +Optional Properties:
>>> +- Slot Index
>>> +  A single Xenon IP can support multiple slots.
>>> +  During initialization, each slot should set corresponding setting bit in
>>> +  some Xenon-specific registers. The corresponding bit is determined by
>>> +  this property.
>>> +  - xenon,slotno = <slot_index>;
>> 
>> Slots should probably be represented as child nodes with the reg 
>> property being the slot number.
>
> 	Since each SDHC slot is independent, I find it is more
> convenient to implement each one as independent SD host/MMC host
> instant.
> 	Otherwise, a main function should loop and initialize each
> slot, like sdhci-pci. I prefer to avoiding such a unnecessary main
> function.
>
> 	It is very hard to determine the slot number by reg property.
> 	Xenon slots are likely to be different types. 1st slot might
> be eMMC and 2nd one might be SD. They might have different register
> size.
> 	The register size might also varies in different Xenon versions.
>

Something that took me a while to figure out is that even it is the same
hardware block which handle multiple SoCs.

Each slots is managed by its own set of register. From the point of view
of the OS, it is as if we have an independent controller for each
slot.

But for an obscure reason, some command need to know which slot is
used. That's why we ended with this property.

With some example what you had in mind was something like that:
sdhci@aa0000 {
		compatible = "marvell,armada-3700-sdhci";
		reg = <0xaa0000 0x1000>;
[...]
                slot0 {
                      /* slot0 is an eMMC */
                      reg = <0>;
                      bus-width = <8>;
                      xenon,pad-type = "fixed-1-8v";

                }
                slot1 {
                      /* slot1 is an SD Card */
                      reg = <1>;
                      bus-width = <4>;
                      xenon,pad-type = "fixed-1-8v";

                }
	};

But it won't work as each slot uses its own address registers, that why we
ended with this:
sdhci@aa0000 {
                /* slot0 is an eMMC */
		compatible = "marvell,armada-3700-sdhci";
		reg = <0xaa0000 0x1000>;
[...]
                xenon,slotno = <0>;
                bus-width = <8>;
                xenon,pad-type = "fixed-1-8v";
	};
sdhci@bb0000 {
                /* slot1 is an SD Card */
		compatible = "marvell,armada-3700-sdhci";
		reg = <0xbb0000 0x1000>;
[...]
                xenon,slotno = <1>;
                bus-width = <4>;
                xenon,pad-type = "fixed-1-8v";
	};

I hope it is more clear now.

Gregory

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: Regression after "do not use CMD13 to get status after speed mode switch"
From: Adrian Hunter @ 2016-10-18 13:23 UTC (permalink / raw)
  To: Linus Walleij, linux-mmc@vger.kernel.org, Ulf Hansson,
	Chaotian Jing
  Cc: linux-arm-msm@vger.kernel.org, Bjorn Andersson, Stephen Boyd,
	Andy Gross
In-Reply-To: <CACRpkdaUf+YpUEhi-njseR1GQXvM3GuA97Q5GuEpQ3NtFdkoHw@mail.gmail.com>

On 18/10/16 11:36, Linus Walleij wrote:
> On Mon, Oct 17, 2016 at 4:32 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> 
>> Before this patch the eMMC is detected and all partitions enumerated
>> immediately, but after the patch it doesn't come up at all, except
>> sometimes, when it appears minutes (!) after boot, all of a sudden.
> 
> FYI this is what it looks like when it eventually happens:
> root@msm8660:/ [  627.710175] mmc0: new high speed MMC card at address 0001
> [  627.711641] mmcblk0: mmc0:0001 SEM04G 3.69 GiB
> [  627.715485] mmcblk0boot0: mmc0:0001 SEM04G partition 1 1.00 MiB
> [  627.736654] mmcblk0boot1: mmc0:0001 SEM04G partition 2 1.00 MiB
> [  627.747397] mmcblk0rpmb: mmc0:0001 SEM04G partition 3 128 KiB
> [  627.756326]  mmcblk0: p1 p2 p3 p4 < p5 p6 p7 p8 p9 p10 p11 p12 p13
> p14 p15 p16 p17 p18 p19 p20 p21 >
> 
> So after 627 seconds, a bit hard for users to wait this long for their
> root filesystem.

If the driver does not support busy detection and the eMMC card provides
zero as the cmd6 generic timeout (which it may especially as cmd6 generic
timeout wasn't added until eMMCv4.5), then __mmc_switch() defaults to
waiting 10 minutes i.e.

#define MMC_OPS_TIMEOUT_MS	(10 * 60 * 1000) /* 10 minute timeout */

So removal of CMD13 polling for HS mode (as per commit
08573eaf1a70104f83fdbee9b84e5be03480e9ed) is going to be a problem for some
combinations of eMMC cards and host drivers.


^ permalink raw reply

* Re: [PATCH v2 0/9] Init runtime PM support for dw_mmc
From: Heiko Stübner @ 2016-10-18 13:26 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Ulf Hansson, Jaehoon Chung, linux-mmc, Doug Anderson,
	open list:ARM/Rockchip SoC...
In-Reply-To: <5be33ac5-7b5e-4b53-67bf-1c38822832ea@rock-chips.com>

Am Dienstag, 18. Oktober 2016, 19:35:31 schrieb Shawn Lin:
> 在 2016/10/18 16:46, Ulf Hansson 写道:
> > + Heiko
> > 
> > On 12 October 2016 at 04:50, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> >> Hi Jaehoon and Ulf,
> >> 
> >>    This patch is gonna support runtime PM for dw_mmc.
> >> 
> >> It could support to disable ciu_clk by default and disable
> >> biu_clk if the devices are non-removeable, or removeable
> >> with gpio-base card detect.
> >> 
> >>    Then I remove the system PM since the runtime PM actually
> >> 
> >> does the same thing as it. So I help migrate the dw_mmc variant
> >> drivers to use runtime PM pairs and pm_runtime_force_*. Note
> >> that I only enable runtime PM for dw_mmc-rockchip as I will
> >> leave the decision to the owners of the corresponding drivers.
> >> I just tested it on my RK3288 platform with linux-next to make
> >> the runtime PM and system PM work fine for my emmc, sd card and
> >> sdio. But I don't have hardware to help test other variant drivers.
> >> But in theory it should work fine as I mentioned that the runtime
> >> PM does the same thing as system PM except for disabling ciu_clk
> >> aggressively which should not be related to the variant hosts.
> >> 
> >>    As you could see that I just extend the slot-gpio a bit, so the
> >> 
> >> ideal way is Ulf could pick them up with Jaehoon's ack. :)
> > 
> > The mmc core change looks fine to me, so I will wait for a pull
> > request from Jaehoon.
> > 
> >> Changes in v2:
> >> - use struct device as argument for runtime callback
> >> - use dw_mci_runtime_* directly
> >> - use dw_mci_runtime_* directly
> >> - minor fix since I change the argument for dw_mci_runtime_*
> >> - use dw_mci_runtime_* directly
> >> - use dw_mci_runtime_* directly
> >> 
> >> Shawn Lin (9):
> >>   mmc: dw_mmc: add runtime PM callback
> >>   mmc: dw_mmc-rockchip: add runtime PM support
> >>   mmc: core: expose the capability of gpio card detect
> >>   mmc: dw_mmc: disable biu clk if possible
> >>   mmc: dw_mmc-k3: deploy runtime PM facilities
> >>   mmc: dw_mmc-exynos: deploy runtime PM facilities
> >>   mmc: dw_mmc-pci: deploy runtime PM facilities
> >>   mmc: dw_mmc-pltfm: deploy runtime PM facilities
> >>   mmc: dw_mmc: remove system PM callback
> >>  
> >>  drivers/mmc/core/slot-gpio.c       |  8 +++++++
> >>  drivers/mmc/host/dw_mmc-exynos.c   | 24 +++++++++-----------
> >>  drivers/mmc/host/dw_mmc-k3.c       | 39 ++++++++------------------------
> >>  drivers/mmc/host/dw_mmc-pci.c      | 29 ++++++++----------------
> >>  drivers/mmc/host/dw_mmc-pltfm.c    | 28 +++++++----------------
> >>  drivers/mmc/host/dw_mmc-rockchip.c | 42
> >>  +++++++++++++++++++++++++++++++---
> >>  drivers/mmc/host/dw_mmc.c          | 46
> >>  +++++++++++++++++++++++++------------- drivers/mmc/host/dw_mmc.h       
> >>    |  6 ++---
> >>  include/linux/mmc/slot-gpio.h      |  1 +
> >>  9 files changed, 117 insertions(+), 106 deletions(-)
> > 
> > Overall these changes looks good to me, so I am ready to accept the PR
> > from Jaehoon!!
> > 
> > 
> > Although, highly related to this patchset, I am worried that there is
> > a misunderstanding on how MMC_PM_KEEP_POWER (DT binding
> > "keep-power-in-suspend") is being used for dw_mmc. Perhaps I am wrong,
> > but I would appreciate if you could elaborate a bit for my
> > understanding.
> > 
> > First, this cap is solely intended to be used for controllers which
> > may have SDIO cards attached, as it indicates those cards may be
> > configured to be powered on while the system enters suspend state. By
> > looking at some DTS files, for example
> > arch/arm64/boot/dts/rockchip/rk3368-orion-r68-meta.dts which uses it
> > for an eMMC slot, this is clearly being abused.
> 
> Indeed. In general, it should be copy-paste issues as folks maybe write
> their dts referring to the exist dts there. So yes, I will do some
> cleanup work for them in prevent of further spread of abused code.

late to the party, but a copy-paste mistake were my thoughts as well.
Thanks Shawn for providing the patch cleaning that up.


Heiko


> > Anyway, the wrong DT configurations might not be a big deal, as in
> > dw_mci_resume(), it's not the capabilities bit that is checked but the
> > corresponding "pm_flag". This flag is set via calling
> > sdio_set_host_pm_flags(), but as that doesn't happen for an eMMC card
> > we should be fine, right!?
> > 
> > Now, what also do puzzles me, is exactly that piece of code in
> > dw_mci_resume() that is being executed *when* the pm_flag contains
> > MMC_PM_KEEP_POWER:
> > if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
> > 
> >      dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
> >      dw_mci_setup_bus(slot, true);
> > 
> > }
> > 
> > So, in the system resume path, the above do makes sense as you need to
> > restore the registers etc for the dw_mmc controller to enable it to
> > operate the SDIO card. Such as bus width, clocks, etc.
> > 
> > Although, I would expect something similar would be needed in the new
> > runtime resume path as well. And in particular also for eMMC/SD cards,
> > as you need to restore the dw_mmc registers to be able to operate the
> > card again. Don't you?
> 
> yes, we do.
> 
> > So in the end, perhaps you should *always* call dw_mci_set_ios() and
> > dw_mci_setup_bus() in dw_mci_resume() instead of conditionally check
> > for MMC_PM_KEEP_POWER? Or maybe only a subset of those functions?
> 
> Thanks for noticing this.
> 
> Personally, I realize there is no need to check MMC_PM_KEEP_POWER but
> it could be highly related to the cost of S-2-R, I guess. I just checked
> sdhci and saw the similar cases you mentioned at the first glance.
> Maybe I'm wrong but I need more time to investigate this issue later.
> 
> There are still some on-going cleanup work for dw_mmc listed on my TODO
> list, including bugfix, legacy/redundant code etc.. So I will check this
> one either. Maybe Jaehoon could also do some stree test on enxyos
> platforms. :)
> 
> > Kind regards
> > Uffe


^ permalink raw reply

* Re: [RFC v2 1/2] sdhci: Add device tree property broken-highspeed
From: Rob Herring @ 2016-10-18 13:17 UTC (permalink / raw)
  To: Zach Brown
  Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
	adrian.hunter-ral2JQCrhuEAvxtiuMwx3w, mark.rutland-5wv7dgnIgG8,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476297656-7019-2-git-send-email-zach.brown-acOepvfBmUk@public.gmane.org>

On Wed, Oct 12, 2016 at 01:40:55PM -0500, Zach Brown wrote:
> Certain board configurations can make highspeed malfuction due to timing
> issues. In these cases a way is needed to force the controller and card
> into standard speed even if they otherwise appear to be capable of
> highspeed.
> 
> The broken-highspeed property will let the sdhci driver know that
> highspeed will not work.
> 
> Signed-off-by: Zach Brown <zach.brown-acOepvfBmUk@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mmc/mmc.txt | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] mmc: sdhci-iproc: Add brcm,sdhci-iproc compat string in bindings document
From: Rob Herring @ 2016-10-18 13:16 UTC (permalink / raw)
  To: Scott Branden
  Cc: Ulf Hansson, Mark Rutland, Ray Jui, Scott Branden, Adrian Hunter,
	BCM Kernel Feedback, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Anup Patel
In-Reply-To: <1476297352-7812-2-git-send-email-scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On Wed, Oct 12, 2016 at 11:35:51AM -0700, Scott Branden wrote:
> Adds brcm,sdhci-iproc compat string to DT bindings document for
> the iProc SDHCI driver.
> 
> Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> index be56d2b..aa58b94 100644
> --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> +++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> @@ -7,6 +7,7 @@ Required properties:
>  - compatible : Should be one of the following
>  	       "brcm,bcm2835-sdhci"
>  	       "brcm,sdhci-iproc-cygnus"
> +	       "brcm,sdhci-iproc"

Seems kind of generic. SoC specific compatible strings please.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Adrian Hunter @ 2016-10-18 13:09 UTC (permalink / raw)
  To: Ziji Hu, Gregory CLEMENT, Ulf Hansson, linux-mmc
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
	devicetree, Thomas Petazzoni, linux-arm-kernel, Jack(SH) Zhu,
	Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones,
	Shiwu Zhang, Victor Gu, Wei(SOCP) Liu, Wilson Ding, Xueping Liu,
	Hilbert Zhang, Liuliu Zhao, Peng Zhu, Yu Cao
In-Reply-To: <d6e1c702-1dfe-3993-da28-01cf01f24b4a@marvell.com>

On 18/10/16 15:04, Ziji Hu wrote:
> Hi Adrian,
> 
> On 2016/10/17 15:55, Adrian Hunter wrote:
>> On 12/10/16 15:17, Ziji Hu wrote:
>>> Hi Adrian,
>>>
>>> On 2016/10/11 20:39, Adrian Hunter wrote:
> <snip>
>>>>> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
>>>>> +{
>>>>> +	int err;
>>>>> +	u8 *ext_csd = NULL;
>>>>> +
>>>>> +	err = mmc_get_ext_csd(card, &ext_csd);
>>>>> +	kfree(ext_csd);
>>>>> +
>>>>> +	return err;
>>>>> +}
>>>>> +
>>>>> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
>>>>> +{
>>>>> +	struct mmc_command cmd = {0};
>>>>> +	int err;
>>>>> +
>>>>> +	cmd.opcode = SD_IO_RW_DIRECT;
>>>>> +	cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
>>>>> +
>>>>> +	err = mmc_wait_for_cmd(card->host, &cmd, 0);
>>>>> +	if (err)
>>>>> +		return err;
>>>>> +
>>>>> +	if (cmd.resp[0] & R5_ERROR)
>>>>> +		return -EIO;
>>>>> +	if (cmd.resp[0] & R5_FUNCTION_NUMBER)
>>>>> +		return -EINVAL;
>>>>> +	if (cmd.resp[0] & R5_OUT_OF_RANGE)
>>>>> +		return -ERANGE;
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
>>>>> +{
>>>>> +	struct mmc_command cmd = {0};
>>>>> +	int err;
>>>>> +
>>>>> +	cmd.opcode = MMC_SEND_STATUS;
>>>>> +	cmd.arg = card->rca << 16;
>>>>> +	cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>>>>> +
>>>>> +	err = mmc_wait_for_cmd(card->host, &cmd, 0);
>>>>> +	return err;
>>>>> +}
>>>>> +
>>>>> +static int xenon_delay_adj_test(struct mmc_card *card)
>>>>> +{
>>>>> +	WARN_ON(!card);
>>>>> +	WARN_ON(!card->host);
>>>>> +
>>>>> +	if (mmc_card_mmc(card))
>>>>> +		return __xenon_emmc_delay_adj_test(card);
>>>>> +	else if (mmc_card_sd(card))
>>>>> +		return __xenon_sd_delay_adj_test(card);
>>>>> +	else if (mmc_card_sdio(card))
>>>>> +		return __xenon_sdio_delay_adj_test(card);
>>>>> +	else
>>>>> +		return -EINVAL;
>>>>> +}
>>>>
>>>> So you are issuing commands from the ->set_ios() callback.  I would want to
>>>> get Ulf's OK for that before going further.
>>>>
>>> 	Yes, you are correct.
>>> 	In some speed mode, Xenon SDHC has to send a series of transfers to search for a perfect sampling point in PHY delay line.
>>> 	It is like tuning process.
>>>
>>>> One thing: you will need to ensure you don't trigger get HS400 re-tuning
>>>> because it will call back into ->set_ios().
>>>>
>>> 	Could you please make the term "HS400 re-tuning" more detailed?
>>> 	In current MMC driver, "HS400 re-tuning" will go back to HS200, execute HS200 tuning and come back to HS400.
>>> 	I'm sure our Xenon SDHC will not execute it.
>>
>> Currently, re-tuning is automatically enabled whenever tuning is executed,
>> and then re-tuning will be done periodically or after CRC errors.  The
>> function to disable re-tuning mmc_retune_disable() is not exported, however
>> if you have you are determining the sampling point your own way, you could
>> simply not implement ->execute_tuning() and then there would be no tuning
>> and no re-tuning.
>>
> 
> 	It is a little complex in our Xenon SDHC.
> 	For the speed mode which requests tuning, such as SDR104 and HS200, our driver will execute standard tuning as spec requires.
> 	However, for those speed mode in which tuning is not requested in spec, our driver has to issues commands to search for the best sampling point.
> 	
> 	It seems that HS400 re-tuning/tuning is disabled by default. Is it correct?

No, it is enabled by default - there is currently no periodic re-tuning  for
HS400 but CRC errors or runtime suspend / resume will cause re-tuning.

> 
>>>
>>> 	However, in coming eMMC 5.2, there is a real HS400 re-tuning, in which tuning can be directly executed in HS400 mode.
>>> 	Our Xenon SDHC will neither trigger this HS400 re-tuning.
>>> 	But since so far there is no such feature in MMC driver, I cannot give you a 100% guarantee now.
>>>
>>>> And you have the problem that you need to get a reference to the card before
>>>> the card device has been added.  As I wrote in response to the previous
>>>> patch, you should get Ulf's help with that too.
>>>>
>>> 	Sure.
>>> 	I will get card_candidate solved at first.
>>> 	Thank you again for your review and help.
>>>
>>> 	Thank you.
>>>
>>> Best regards,
>>> Hu Ziji
>>>>
>>>
>>
> 


^ permalink raw reply

* Re: [PATCH] mmc: core: Check regulator pointer
From: Shawn Lin @ 2016-10-18 12:29 UTC (permalink / raw)
  To: Maxime Ripard, Ulf Hansson
  Cc: shawn.lin, linux-mmc, linux-kernel@vger.kernel.org
In-Reply-To: <20161018094704.ohubpg4j3jhc7cay@lukather>

在 2016/10/18 17:47, Maxime Ripard 写道:
> Hi,
>
> On Tue, Oct 18, 2016 at 11:03:02AM +0200, Ulf Hansson wrote:
>> On 18 October 2016 at 10:43, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>>> mmc_regulator_get_supply might silently fail if the regulators are not
>>> found, which is the right thing to do since both these regulators are
>>> optional.
>>>
>>> However, the drivers then have no way to know whether or not they should
>>> proceed and call mmc_regulator_set_ocr. And since this function doesn't
>>
>> Host drivers should check "if (!IS_ERR(mmc->supply.vmmc))" before
>> invoking mmc_regulator_set_ocr(). I wasn't aware that some didn't.
>
> Ok, so the sunxi one definitely doesn't:
> http://lxr.free-electrons.com/source/drivers/mmc/host/sunxi-mmc.c#L735
>
>> My point is, that in some cases the regulator is optional, then a host
>> driver need to take other actions to power on/off the card.
>
> What are those actions? Just power up the card through some other
> mean, or is it more tied to the MMC protocol?
>
> Also, I'm wondering if VMMC is actually optional at all. In all cases
> I can think of, it would be represented as a regulator anyway.

If I understand your point correctly, for sunxi's mmc controller, the
only way to power-on/off the vmmc is to call  mmc_regulator_set_ocr?

There are some cases for mmc-core/host to control the power:
1) assign a vmmc-supply for no matter of gpio-based regulator or PMIC
2) use mmc_pwrseq which is often used by sdio fucntion or emmc.
3) use a functional port to control it which is similar to gpio-based
regulator but the host drivers could control this via internal register,
for instance, dw_mmc..

4) And I even see some boards use fixed power which means we cannot
power off vmmc from the hardware part. And also, if I decide to add
alwyas-on and boot-on for the vmmc regulator I was using, and also
there is  no really need for mmc-core/driver to know whether we have
vmmc, right!?



>
> Thanks,
> Maxime
>


-- 
Best Regards
Shawn Lin


^ permalink raw reply

* Re: [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ziji Hu @ 2016-10-18 12:09 UTC (permalink / raw)
  To: Adrian Hunter, Gregory CLEMENT, Ulf Hansson, linux-mmc
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
	devicetree, Thomas Petazzoni, linux-arm-kernel, Jack(SH) Zhu,
	Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones,
	Shiwu Zhang, Victor Gu, Wei(SOCP) Liu, Wilson Ding, Xueping Liu,
	Hilbert Zhang, Liuliu Zhao, Peng Zhu, Yu Cao <yuca>
In-Reply-To: <ec7a2658-5af3-bff8-3cb8-adb72406df5c@intel.com>

Hi Adrian,

On 2016/10/17 16:14, Adrian Hunter wrote:
> On 13/10/16 08:38, Ziji Hu wrote:
>> Hi Adrian,
>>
>> On 2016/10/12 21:07, Adrian Hunter wrote:
>>> On 12/10/16 14:58, Ziji Hu wrote:
>>>> Hi Adrian,
>>>>
>>>> 	Thank you very much for your review.
>>>> 	I will firstly fix the typo.
>>>>
>>>> On 2016/10/11 20:37, Adrian Hunter wrote:
>> <snip>
>>>>>> +
>>>>>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>>>>>> +					     struct mmc_ios *ios)
>>>>>> +{
>>>>>> +	struct sdhci_host *host = mmc_priv(mmc);
>>>>>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>>>>> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>>>>> +
>>>>>> +	/*
>>>>>> +	 * Before SD/SDIO set signal voltage, SD bus clock should be
>>>>>> +	 * disabled. However, sdhci_set_clock will also disable the Internal
>>>>>> +	 * clock in mmc_set_signal_voltage().
>>>>>> +	 * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
>>>>>> +	 * Thus here manually enable internal clock.
>>>>>> +	 *
>>>>>> +	 * After switch completes, it is unnecessary to disable internal clock,
>>>>>> +	 * since keeping internal clock active obeys SD spec.
>>>>>> +	 */
>>>>>> +	enable_xenon_internal_clk(host);
>>>>>> +
>>>>>> +	if (priv->card_candidate) {
>>>>>
>>>>> mmc_power_up() calls __mmc_set_signal_voltage() calls
>>>>> host->ops->start_signal_voltage_switch so priv->card_candidate could be an
>>>>> invalid reference to an old card.
>>>>>
>>>>> So that's not going to work if the card changes - not only for removable
>>>>> cards but even for eMMC if init fails and retries.
>>>>>
>>>> 	As you point out, this piece of code have defects, even though it actually works on Marvell multiple platforms, unless eMMC card is removable.
>>>>
>>>> 	I can add a property to explicitly indicate eMMC type in DTS.
>>>> 	Then card_candidate access can be removed here.
>>>> 	Does it sounds more reasonable to you?
>>>
>>> Sure
>>>
>>>>
>>>>>> +		if (mmc_card_mmc(priv->card_candidate))
>>>>>> +			return xenon_emmc_signal_voltage_switch(mmc, ios);
>>>>>
>>>>> So if all you need to know is whether it is a eMMC, why can't DT tell you?
>>>>>
>>>> 	I can add an eMMC type property in DTS, to remove the card_candidate access here.
>>>>
>>>>>> +	}
>>>>>> +
>>>>>> +	return sdhci_start_signal_voltage_switch(mmc, ios);
>>>>>> +}
>>>>>> +
>>>>>> +/*
>>>>>> + * After determining which slot is used for SDIO,
>>>>>> + * some additional task is required.
>>>>>> + */
>>>>>> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
>>>>>> +{
>>>>>> +	struct sdhci_host *host = mmc_priv(mmc);
>>>>>> +	u32 reg;
>>>>>> +	u8 slot_idx;
>>>>>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>>>>> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>>>>> +
>>>>>> +	/* Link the card for delay adjustment */
>>>>>> +	priv->card_candidate = card;
>>>>>
>>>>> You really need a better way to get the card.  I suggest you take up the
>>>>> issue with Ulf.  One possibility is to have mmc core set host->card = card
>>>>> much earlier.
>>>>>
>>>> 	Could you please tell me if any issue related to card_candidate still exists, after the card_candidate is removed from xenon_start_signal_voltage_switch() in above?
>>>> 	It seems that when init_card is called, the structure card has already been updated and stable in MMC/SD/SDIO initialization sequence.
>>>> 	May I keep it here?
>>>
>>> It works by accident rather than by design.  We can do better.
>>>
>> 	Could you please tell me some details which are satisfied about card_candidate?
>>
>> 	I must admit that card_candidate in xenon_start_signal_voltage_switch() is imperfect.
>> 	But card_candidate in init_card() and later in set_ios() work by design, rather than by accident. We did a lot of tests on several platforms.
>> 	
>> 	The structure mmc_card passed in here is a stable one. Thus in my very own opinion, it is safe and stable to use mmc_card here.
>> 	card_candidate is used only in card initialization. It is not active in later transfers after initialization is done.
>> 	It will always updated with mmc_card in next card initialization.
> 
> Ok, so maybe just add some comments and more explanation of how it works.
> 
> 
	Sure. I will add more detailed comments.
>>
>>>>
>>>>>> +	/* Set tuning functionality of this slot */
>>>>>> +	xenon_slot_tuning_setup(host);
>>>>>> +
>>>>>> +	slot_idx = priv->slot_idx;
>>>>>> +	if (!mmc_card_sdio(card)) {
>>>>>> +		/* Re-enable the Auto-CMD12 cap flag. */
>>>>>> +		host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>>>>>> +		host->flags |= SDHCI_AUTO_CMD12;
>>>>>> +
>>>>>> +		/* Clear SDIO Card Inserted indication */
>>>>>> +		reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>>>>>> +		reg &= ~(1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
>>>>>> +		sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
>>>>>> +
>>>>>> +		if (mmc_card_mmc(card)) {
>>>>>> +			mmc->caps |= MMC_CAP_NONREMOVABLE;
>>>>>> +			if (!(host->quirks2 & SDHCI_QUIRK2_NO_1_8_V))
>>>>>> +				mmc->caps |= MMC_CAP_1_8V_DDR;
>>>>>> +			/*
>>>>>> +			 * Force to clear BUS_TEST to
>>>>>> +			 * skip bus_test_pre and bus_test_post
>>>>>> +			 */
>>>>>> +			mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
>>>>>> +			mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
>>>>>> +				      MMC_CAP2_PACKED_CMD;
>>>>>> +			if (mmc->caps & MMC_CAP_8_BIT_DATA)
>>>>>> +				mmc->caps2 |= MMC_CAP2_HS400_1_8V;
>>>>>> +		}
>>>>>> +	} else {
>>>>>> +		/*
>>>>>> +		 * Delete the Auto-CMD12 cap flag.
>>>>>> +		 * Otherwise, when sending multi-block CMD53,
>>>>>> +		 * Driver will set Transfer Mode Register to enable Auto CMD12.
>>>>>> +		 * However, SDIO device cannot recognize CMD12.
>>>>>> +		 * Thus SDHC will time-out for waiting for CMD12 response.
>>>>>> +		 */
>>>>>> +		host->quirks &= ~SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>>>>>> +		host->flags &= ~SDHCI_AUTO_CMD12;
>>>>>
>>>>> sdhci_set_transfer_mode() won't enable auto-CMD12 for CMD53 anyway, so is
>>>>> this needed?
>>>>>
>>>> 	In Xenon driver, Auto-CMD12 flag is set to enable full support to Auto-CMD feature, both Auto-CMD12 and Auto-CMD23.
>>>> 	As a result, when Xenon SDHC slot can both support SD and SDIO, Auto-CMD12 is disabled when SDIO card is inserted, and renabled when SD is inserted.
>>>>
>>>> 	I recheck the sdhci code to set Auto-CMD bit in Transfer Mode register, in sdhci_set_transfer_mode():
>>>> 	if (mmc_op_multi(cmd->opcode) || data->blocks > 1)
>>>> 	As you can see, as long as it is CMD18/CMD25 OR there are multiple data blocks, Auto-CMD field will be set.
>>>> 	CMD53 doesn't have CMD23. Thus Auto-CMD12 is selected since Auto-CMD12 flag is set.
>>>> 	Thus I have to clear Auto-CMD12 to avoid issuing Auto-CMD12 in SDIO transfer.
>>>
>>>
>>> The code is:
>>>
>>> 	if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
>>> 		mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
>>> 		/*
>>> 		 * If we are sending CMD23, CMD12 never gets sent
>>> 		 * on successful completion (so no Auto-CMD12).
>>> 		 */
>>> 		if (sdhci_auto_cmd12(host, cmd->mrq) &&
>>> 		    (cmd->opcode != SD_IO_RW_EXTENDED))
>>> 			mode |= SDHCI_TRNS_AUTO_CMD12;
>>> 		else if (cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23)) {
>>> 			mode |= SDHCI_TRNS_AUTO_CMD23;
>>> 			sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
>>> 		}
>>> 	}
>>>
>>> You can see the check for SD_IO_RW_EXTENDED which is CMD53.
>>>
>> 	Sorry. I didn't notice CMD53 check was added.
>> 	I introduced this Auto-CMD12 hack since kernel 3.8. It seems that this check is not added in kernel 3.8.
>> 	Thanks for the information. I will remove the Auto-CMD12 hack.
>>
>>>>
>>>> 	I just meet a similar issue in RPMB.
>>>> 	When Auto-CMD12 flag is set, eMMC RPMB access will trigger Auto-CMD12, since CMD25 is in use.
>>>> 	It will cause RPMB access failed.
>>>
>>> Can you explain more about the RPMB issue.  Doesn't it use CMD23, so CMD12
>>> wouldn't be used - auto or manually.
>>>
>> 	RPMB go through the MMC ioctl routine.
>> 	Unlike normal data transfer, MMC ioctl for RPMB explicitly issues CMD23. When CMD25 is issued, there is neither data->sbc nor Auto-CMD23.
>> 	As a result, sdhci driver will automatically enable Auto-CMD12 for RPMB CMD25 if Auto-CMD12 flag is set.
> 
> OK, so SDHCI should also not allow auto-cmd12 if there is no stop command
> i.e. data->stop is null.
> 
	data->stop and cmd->stop are forced to be NULL if Auto-CMD12 is enabled.
	Instead, currently I use cmd->arg to check if it is a RPMB CMD25. If CMD25 argument is NULL, it should be RPMB access.
	But I only verified it on Marvell platform. Please help check it when later I propose the formal patch.
	
>>
>>>>
>>>> 	One possible solution is to drop Auto-CMD12 support and use Auto-CMD23 only, in Xenon driver.
>>>> 	May I know you opinion, please?
>>>
>>> I don't use auto-CMD12 because I don't know if it provides any benefit and
>>> sdhci does not seem to have implemented Auto CMD12 Error Recovery, although
>>> I have never looked at it closely.
>>>
>> 	Actually, Auto-CMD23 is always used on our Xenon. Auto-CMD12 is not used at all.
>> 	But since this driver is a general one for all Marvell products, Auto-CMD12 is also supported in case that Auto-CMD23 is not available.
>>  
>>
> 

^ permalink raw reply

* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-10-18 12:04 UTC (permalink / raw)
  To: Adrian Hunter, Gregory CLEMENT, Ulf Hansson, linux-mmc
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
	devicetree, Thomas Petazzoni, linux-arm-kernel, Jack(SH) Zhu,
	Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones,
	Shiwu Zhang, Victor Gu, Wei(SOCP) Liu, Wilson Ding, Xueping Liu,
	Hilbert Zhang, Liuliu Zhao, Peng Zhu, Yu Cao <yuca>
In-Reply-To: <63e2bacf-6ad9-5e20-2c49-e77eb362ccfc@intel.com>

Hi Adrian,

On 2016/10/17 15:55, Adrian Hunter wrote:
> On 12/10/16 15:17, Ziji Hu wrote:
>> Hi Adrian,
>>
>> On 2016/10/11 20:39, Adrian Hunter wrote:
<snip>
>>>> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
>>>> +{
>>>> +	int err;
>>>> +	u8 *ext_csd = NULL;
>>>> +
>>>> +	err = mmc_get_ext_csd(card, &ext_csd);
>>>> +	kfree(ext_csd);
>>>> +
>>>> +	return err;
>>>> +}
>>>> +
>>>> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
>>>> +{
>>>> +	struct mmc_command cmd = {0};
>>>> +	int err;
>>>> +
>>>> +	cmd.opcode = SD_IO_RW_DIRECT;
>>>> +	cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
>>>> +
>>>> +	err = mmc_wait_for_cmd(card->host, &cmd, 0);
>>>> +	if (err)
>>>> +		return err;
>>>> +
>>>> +	if (cmd.resp[0] & R5_ERROR)
>>>> +		return -EIO;
>>>> +	if (cmd.resp[0] & R5_FUNCTION_NUMBER)
>>>> +		return -EINVAL;
>>>> +	if (cmd.resp[0] & R5_OUT_OF_RANGE)
>>>> +		return -ERANGE;
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
>>>> +{
>>>> +	struct mmc_command cmd = {0};
>>>> +	int err;
>>>> +
>>>> +	cmd.opcode = MMC_SEND_STATUS;
>>>> +	cmd.arg = card->rca << 16;
>>>> +	cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>>>> +
>>>> +	err = mmc_wait_for_cmd(card->host, &cmd, 0);
>>>> +	return err;
>>>> +}
>>>> +
>>>> +static int xenon_delay_adj_test(struct mmc_card *card)
>>>> +{
>>>> +	WARN_ON(!card);
>>>> +	WARN_ON(!card->host);
>>>> +
>>>> +	if (mmc_card_mmc(card))
>>>> +		return __xenon_emmc_delay_adj_test(card);
>>>> +	else if (mmc_card_sd(card))
>>>> +		return __xenon_sd_delay_adj_test(card);
>>>> +	else if (mmc_card_sdio(card))
>>>> +		return __xenon_sdio_delay_adj_test(card);
>>>> +	else
>>>> +		return -EINVAL;
>>>> +}
>>>
>>> So you are issuing commands from the ->set_ios() callback.  I would want to
>>> get Ulf's OK for that before going further.
>>>
>> 	Yes, you are correct.
>> 	In some speed mode, Xenon SDHC has to send a series of transfers to search for a perfect sampling point in PHY delay line.
>> 	It is like tuning process.
>>
>>> One thing: you will need to ensure you don't trigger get HS400 re-tuning
>>> because it will call back into ->set_ios().
>>>
>> 	Could you please make the term "HS400 re-tuning" more detailed?
>> 	In current MMC driver, "HS400 re-tuning" will go back to HS200, execute HS200 tuning and come back to HS400.
>> 	I'm sure our Xenon SDHC will not execute it.
> 
> Currently, re-tuning is automatically enabled whenever tuning is executed,
> and then re-tuning will be done periodically or after CRC errors.  The
> function to disable re-tuning mmc_retune_disable() is not exported, however
> if you have you are determining the sampling point your own way, you could
> simply not implement ->execute_tuning() and then there would be no tuning
> and no re-tuning.
>

	It is a little complex in our Xenon SDHC.
	For the speed mode which requests tuning, such as SDR104 and HS200, our driver will execute standard tuning as spec requires.
	However, for those speed mode in which tuning is not requested in spec, our driver has to issues commands to search for the best sampling point.
	
	It seems that HS400 re-tuning/tuning is disabled by default. Is it correct?

>>
>> 	However, in coming eMMC 5.2, there is a real HS400 re-tuning, in which tuning can be directly executed in HS400 mode.
>> 	Our Xenon SDHC will neither trigger this HS400 re-tuning.
>> 	But since so far there is no such feature in MMC driver, I cannot give you a 100% guarantee now.
>>
>>> And you have the problem that you need to get a reference to the card before
>>> the card device has been added.  As I wrote in response to the previous
>>> patch, you should get Ulf's help with that too.
>>>
>> 	Sure.
>> 	I will get card_candidate solved at first.
>> 	Thank you again for your review and help.
>>
>> 	Thank you.
>>
>> Best regards,
>> Hu Ziji
>>>
>>
> 

^ permalink raw reply

* [RFC PATCH 8/8] mmc: mmc_test: remove BUG_ONs and deploy error handling
From: Shawn Lin @ 2016-10-18 12:04 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, Shawn Lin
In-Reply-To: <1476792192-6265-1-git-send-email-shawn.lin@rock-chips.com>

It is unnecessary to panic the kernel when testing mmc. Instead,
cast a warning for folkz to debug and return the error code to
the caller to indicate the failure of this test should be enough.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/card/mmc_test.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 5a8dc5a..db1a7ac 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -214,7 +214,8 @@ static void mmc_test_prepare_mrq(struct mmc_test_card *test,
 	struct mmc_request *mrq, struct scatterlist *sg, unsigned sg_len,
 	unsigned dev_addr, unsigned blocks, unsigned blksz, int write)
 {
-	BUG_ON(!mrq || !mrq->cmd || !mrq->data || !mrq->stop);
+	if (WARN_ON(!mrq || !mrq->cmd || !mrq->data || !mrq->stop))
+		return;
 
 	if (blocks > 1) {
 		mrq->cmd->opcode = write ?
@@ -694,7 +695,8 @@ static int mmc_test_cleanup(struct mmc_test_card *test)
 static void mmc_test_prepare_broken_mrq(struct mmc_test_card *test,
 	struct mmc_request *mrq, int write)
 {
-	BUG_ON(!mrq || !mrq->cmd || !mrq->data);
+	if (WARN_ON(!mrq || !mrq->cmd || !mrq->data))
+		return;
 
 	if (mrq->data->blocks > 1) {
 		mrq->cmd->opcode = write ?
@@ -714,7 +716,8 @@ static int mmc_test_check_result(struct mmc_test_card *test,
 {
 	int ret;
 
-	BUG_ON(!mrq || !mrq->cmd || !mrq->data);
+	if (WARN_ON(!mrq || !mrq->cmd || !mrq->data))
+		return -EINVAL;
 
 	ret = 0;
 
@@ -755,7 +758,8 @@ static int mmc_test_check_broken_result(struct mmc_test_card *test,
 {
 	int ret;
 
-	BUG_ON(!mrq || !mrq->cmd || !mrq->data);
+	if (WARN_ON(!mrq || !mrq->cmd || !mrq->data))
+		return -EINVAL;
 
 	ret = 0;
 
-- 
2.3.7



^ permalink raw reply related

* [RFC PATCH 7/8] mmc: queue: remove BUG_ON for bounce_sg
From: Shawn Lin @ 2016-10-18 12:04 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, Shawn Lin
In-Reply-To: <1476792192-6265-1-git-send-email-shawn.lin@rock-chips.com>

bounce_sg for mqrq_cur and mqrq_pre are proper
allocated when initializing the queue and will not
be freed before explicitly cleaning the queue. So from
the code itself it should be quite confident to remove
this check. If that BUG_ON take effects, it is mostly
likely the memory is randomly oopsing.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/card/queue.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 8037f73..6c8978a 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -505,8 +505,6 @@ unsigned int mmc_queue_map_sg(struct mmc_queue *mq, struct mmc_queue_req *mqrq)
 			return blk_rq_map_sg(mq->queue, mqrq->req, mqrq->sg);
 	}
 
-	BUG_ON(!mqrq->bounce_sg);
-
 	if (mmc_packed_cmd(cmd_type))
 		sg_len = mmc_queue_packed_map_sg(mq, mqrq->packed,
 						 mqrq->bounce_sg, cmd_type);
-- 
2.3.7



^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox