[RFC PATCH 0/7] UHS-I support for sh_mobile_sdhi

[RFC PATCH 0/7] UHS-I support for sh_mobile_sdhi

[Ben Hutchings]

This series adds support for UHS-I in sh_mobile_sdhi, partly implemented
in tmio_mmc.  This does not yet include tuning for SDR-104, but SDR-50
works.

The pfc block needs to be configured for 1.8V or 3.3V signalling on the
SDHI, separately from the external voltage regulator.  I've treated this
as an additional regulator that sh_mobile_sdhi has to adjust, but I'm
not entirely happy with this or the way it's represented in the DT.  I
would appreciate advice on how to improve that.

Ben.

Ben Dooks (1):
  ARM: shmobile: r8a7790-lager.dts: Set sdhi and mmcif clock rates

Ben Hutchings (4):
  mmc: tmio: Add UHS-I mode support
  pinctrl: sh-pfc: r8a7790: Add regulators for SD voltage switch
  ARM: shmobile: r8a7790: Add nodes for pfc SD voltage regulators
  mmc: sh_mobile_sdhi: Add UHS-I mode support

Shinobu Uehara (1):
  mmc: sh_mobile_sdhi: Add actual clock rate support

William Towle (1):
  ARM: shmobile: r8a7790-lager.dts: Assert UHS-I SDR-50 capability

 arch/arm/boot/dts/r8a7790-lager.dts  |   21 ++++
 arch/arm/boot/dts/r8a7790.dtsi       |   21 ++++
 drivers/mmc/host/sh_mobile_sdhi.c    |   62 +++++++++++
 drivers/mmc/host/tmio_mmc.h          |    3 +
 drivers/mmc/host/tmio_mmc_pio.c      |   31 ++++++
 drivers/pinctrl/sh-pfc/Kconfig       |    1 +
 drivers/pinctrl/sh-pfc/core.c        |    2 +-
 drivers/pinctrl/sh-pfc/core.h        |    1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |  189 ++++++++++++++++++++++++++++++++++
 9 files changed, 330 insertions(+), 1 deletion(-)

-- 
1.7.10.4

[RFC PATCH 2/7] mmc: sh_mobile_sdhi: Add actual clock rate support

[Ben Hutchings]

From: Shinobu Uehara <shinobu.uehara.xc@renesas.com>

It is necessary to stop the SD clock before using
the actual clock in case of SDHI controller.

Signed-off-by: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 drivers/mmc/host/sh_mobile_sdhi.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 6906a905cd54..92a58c6007fe 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -136,6 +136,18 @@ static void sh_mobile_sdhi_clk_disable(struct platform_device *pdev)
 	clk_disable_unprepare(priv->clk);
 }
 
+static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int clk)
+{
+	struct mmc_host *mmc = dev_get_drvdata(&pdev->dev);
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+
+	if (clk == true) {
+		sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
+				sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+		sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x00ff);
+	}
+}
+
 static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
 {
 	int timeout = 1000;
@@ -234,6 +246,8 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eprobe;
 	}
 
+	host->set_clk_div	= sh_mobile_sdhi_set_clk_div;
+
 	host->dma		= dma_priv;
 	host->write16_hook	= sh_mobile_sdhi_write16_hook;
 	host->clk_enable	= sh_mobile_sdhi_clk_enable;
-- 
1.7.10.4

Re: [RFC PATCH 2/7] mmc: sh_mobile_sdhi: Add actual clock rate support

[Sergei Shtylyov]

On 04/30/2015 03:31 PM, Ben Hutchings wrote:

> From: Shinobu Uehara <shinobu.uehara.xc@renesas.com>

> It is necessary to stop the SD clock before using
> the actual clock in case of SDHI controller.

> Signed-off-by: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> ---
>   drivers/mmc/host/sh_mobile_sdhi.c |   14 ++++++++++++++
>   1 file changed, 14 insertions(+)

> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 6906a905cd54..92a58c6007fe 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -136,6 +136,18 @@ static void sh_mobile_sdhi_clk_disable(struct platform_device *pdev)
>   	clk_disable_unprepare(priv->clk);
>   }
>
> +static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int clk)
> +{
> +	struct mmc_host *mmc = dev_get_drvdata(&pdev->dev);
> +	struct tmio_mmc_host *host = mmc_priv(mmc);
> +
> +	if (clk == true) {

    'clk' is *int*. Perhaps, should be just *if* (clk)?

> +		sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
> +				sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> +		sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x00ff);
> +	}
> +}
> +
>   static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
>   {
>   	int timeout = 1000;
[...]

WBR, Sergei

Re: [RFC PATCH 2/7] mmc: sh_mobile_sdhi: Add actual clock rate support

[Ben Hutchings]

On Thu, 2015-04-30 at 19:34 +0300, Sergei Shtylyov wrote:
> On 04/30/2015 03:31 PM, Ben Hutchings wrote:
> 
> > From: Shinobu Uehara <shinobu.uehara.xc-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> 
> > It is necessary to stop the SD clock before using
> > the actual clock in case of SDHI controller.
> 
> > Signed-off-by: Shinobu Uehara <shinobu.uehara.xc-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> > Signed-off-by: Ben Hutchings <ben.hutchings-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
> > ---
> >   drivers/mmc/host/sh_mobile_sdhi.c |   14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> 
> > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> > index 6906a905cd54..92a58c6007fe 100644
> > --- a/drivers/mmc/host/sh_mobile_sdhi.c
> > +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> > @@ -136,6 +136,18 @@ static void sh_mobile_sdhi_clk_disable(struct platform_device *pdev)
> >   	clk_disable_unprepare(priv->clk);
> >   }
> >
> > +static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int clk)
> > +{
> > +	struct mmc_host *mmc = dev_get_drvdata(&pdev->dev);
> > +	struct tmio_mmc_host *host = mmc_priv(mmc);
> > +
> > +	if (clk == true) {
> 
>     'clk' is *int*. Perhaps, should be just *if* (clk)?

clk is either 0 or 1 and should maybe be bool.  But you're right,
regardless of its type, it doesn't make sense to compare with true.

Ben.

> > +		sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
> > +				sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> > +		sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x00ff);
> > +	}
> > +}
> > +
> >   static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
> >   {
> >   	int timeout = 1000;
> [...]
> 
> WBR, Sergei
> 


--
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

Re: [RFC PATCH 2/7] mmc: sh_mobile_sdhi: Add actual clock rate support

[Ben Hutchings]

On Wed, 2015-05-06 at 02:05 +0100, Ben Hutchings wrote:
> On Thu, 2015-04-30 at 19:34 +0300, Sergei Shtylyov wrote:
> > On 04/30/2015 03:31 PM, Ben Hutchings wrote:
> > 
> > > From: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
> > 
> > > It is necessary to stop the SD clock before using
> > > the actual clock in case of SDHI controller.
> > 
> > > Signed-off-by: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
> > > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> > > ---
> > >   drivers/mmc/host/sh_mobile_sdhi.c |   14 ++++++++++++++
> > >   1 file changed, 14 insertions(+)
> > 
> > > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> > > index 6906a905cd54..92a58c6007fe 100644
> > > --- a/drivers/mmc/host/sh_mobile_sdhi.c
> > > +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> > > @@ -136,6 +136,18 @@ static void sh_mobile_sdhi_clk_disable(struct platform_device *pdev)
> > >   	clk_disable_unprepare(priv->clk);
> > >   }
> > >
> > > +static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int clk)
> > > +{
> > > +	struct mmc_host *mmc = dev_get_drvdata(&pdev->dev);
> > > +	struct tmio_mmc_host *host = mmc_priv(mmc);
> > > +
> > > +	if (clk == true) {
> > 
> >     'clk' is *int*. Perhaps, should be just *if* (clk)?
> 
> clk is either 0 or 1 and should maybe be bool.  But you're right,
> regardless of its type, it doesn't make sense to compare with true.
[...]

Anyway, I think this is redundant with:

commit b30d6958a014812158d9729a3e0b6d448f6e8968
Author: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date:   Sun Aug 24 20:02:16 2014 -0700

    mmc: tmio: remove SCLKEN bit setting from tmio_mmc_set_clock()

commit da29fe2bf573f0ae56fdc2e790387cb73fc8c6f8
Author: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
Date:   Sun Aug 24 20:03:00 2014 -0700

    mmc: tmio: add actual clock support as option

so I will probably just drop it.

Ben.

[RFC PATCH 3/7] pinctrl: sh-pfc: r8a7790: Add regulators for SD voltage switch

[Ben Hutchings]

Model the choice of 1.8V or 3.3V signalling for each SD interface as a
regulator.

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 drivers/pinctrl/sh-pfc/Kconfig       |    1 +
 drivers/pinctrl/sh-pfc/core.c        |    2 +-
 drivers/pinctrl/sh-pfc/core.h        |    1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |  189 ++++++++++++++++++++++++++++++++++
 4 files changed, 192 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/Kconfig b/drivers/pinctrl/sh-pfc/Kconfig
index 8c4b3d391823..4b1895a6ac69 100644
--- a/drivers/pinctrl/sh-pfc/Kconfig
+++ b/drivers/pinctrl/sh-pfc/Kconfig
@@ -49,6 +49,7 @@ config PINCTRL_PFC_R8A7790
 	def_bool y
 	depends on ARCH_R8A7790
 	select PINCTRL_SH_PFC
+	select REGULATOR if OF
 
 config PINCTRL_PFC_R8A7791
 	def_bool y
diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index 7b2c9495c383..7d51f96afc9a 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -92,7 +92,7 @@ static int sh_pfc_map_resources(struct sh_pfc *pfc,
 	return 0;
 }
 
-static void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 reg)
+void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 reg)
 {
 	struct sh_pfc_window *window;
 	phys_addr_t address = reg;
diff --git a/drivers/pinctrl/sh-pfc/core.h b/drivers/pinctrl/sh-pfc/core.h
index 6dc8a6fc2746..af355629c5d2 100644
--- a/drivers/pinctrl/sh-pfc/core.h
+++ b/drivers/pinctrl/sh-pfc/core.h
@@ -57,6 +57,7 @@ int sh_pfc_unregister_gpiochip(struct sh_pfc *pfc);
 int sh_pfc_register_pinctrl(struct sh_pfc *pfc);
 int sh_pfc_unregister_pinctrl(struct sh_pfc *pfc);
 
+void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 address);
 u32 sh_pfc_read_raw_reg(void __iomem *mapped_reg, unsigned int reg_width);
 void sh_pfc_write_raw_reg(void __iomem *mapped_reg, unsigned int reg_width,
 			  u32 data);
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
index 22a5470889f5..0c9d2c018a10 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
@@ -23,6 +23,13 @@
 
 #include <linux/kernel.h>
 #include <linux/platform_data/gpio-rcar.h>
+#ifdef CONFIG_OF
+#include <linux/of.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/string.h>
+#endif
 
 #include "core.h"
 #include "sh_pfc.h"
@@ -5586,8 +5593,190 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 	{ },
 };
 
+#ifdef CONFIG_OF
+
+struct r8a7790_sd_regulator_data {
+	struct regulator_desc desc;
+	char name[10];
+	struct regulator_dev *dev;
+	int state;
+};
+
+#define SD_REGULATOR_NAME "regulator-r8a7790-sd"
+
+#define SD_LOW_VOLTAGE	1800000
+#define SD_STD_VOLTAGE	3300000
+
+static int r8a7790_sd_regulator_set_voltage(struct regulator_dev *dev,
+					    int min_uV, int max_uV,
+					    unsigned int *selector)
+{
+	struct r8a7790_sd_regulator_data *drvdata = rdev_get_drvdata(dev);
+	struct sh_pfc *pfc = dev_get_drvdata(dev->dev.parent);
+	void __iomem *mapped_reg;
+	u32 data, mask;
+	int state;
+
+	if (min_uV <= SD_LOW_VOLTAGE && max_uV >= SD_LOW_VOLTAGE)
+		state = 0;
+	else if (min_uV <= SD_STD_VOLTAGE && max_uV >= SD_STD_VOLTAGE)
+		state = 1;
+	else
+		return -EINVAL;
+
+	/* Map IOCTRL6 */
+	mapped_reg = sh_pfc_phys_to_virt(pfc, 0xe606008c);
+
+	spin_lock(&pfc->lock);
+
+	data = sh_pfc_read_raw_reg(mapped_reg, 32);
+
+	/* Set I/O voltage for the 8 pins for this SD interface */
+	mask = 0xff << (24 - drvdata->desc.id * 8);
+	if (state)
+		data |= mask;
+	else
+		data &= ~mask;
+
+	sh_pfc_write_raw_reg(
+		sh_pfc_phys_to_virt(pfc, pfc->info->unlock_reg), 32,
+		~data);
+	sh_pfc_write_raw_reg(mapped_reg, 32, data);
+
+	spin_unlock(&pfc->lock);
+
+	drvdata->state = state;
+	if (selector)
+		*selector = state;
+
+	return 0;
+}
+
+static int r8a7790_sd_regulator_list_voltage(struct regulator_dev *dev,
+				       unsigned int selector)
+{
+	switch (selector) {
+	case 0:
+		return SD_LOW_VOLTAGE;
+	case 1:
+		return SD_STD_VOLTAGE;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int r8a7790_sd_regulator_get_voltage(struct regulator_dev *dev)
+{
+	struct r8a7790_sd_regulator_data *drvdata = rdev_get_drvdata(dev);
+
+	return r8a7790_sd_regulator_list_voltage(dev, drvdata->state);
+}
+
+static const struct regulator_ops r8a7790_sd_regulator_ops = {
+	.set_voltage = r8a7790_sd_regulator_set_voltage,
+	.get_voltage = r8a7790_sd_regulator_get_voltage,
+	.list_voltage = r8a7790_sd_regulator_list_voltage,
+};
+
+static const struct regulator_init_data r8a7790_sd_regulator_init = {
+	.constraints = {
+		.valid_ops_mask = REGULATOR_CHANGE_VOLTAGE,
+		.min_uV = SD_LOW_VOLTAGE,
+		.max_uV = SD_STD_VOLTAGE,
+	},
+};
+
+static int r8a7790_sd_regulator_probe(struct sh_pfc *pfc, int index)
+{
+	char child_name[20];
+	struct device_node *np;
+	struct r8a7790_sd_regulator_data *drvdata;
+	struct regulator_config cfg = { };
+	void __iomem *mapped_reg;
+	int ret;
+
+	snprintf(child_name, sizeof(child_name), "sd-regulator@%d", index);
+	np = NULL;
+	while ((np = of_get_next_available_child(pfc->dev->of_node, np))) {
+		if (!strcmp(kbasename(np->full_name), child_name))
+			break;
+	}
+	if (!np) {
+		dev_dbg(pfc->dev, "no %s child node found\n", child_name);
+		return -ENODEV;
+	}
+
+	drvdata = devm_kzalloc(pfc->dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	drvdata->desc.owner = THIS_MODULE;
+	/* XXX drvdata->desc.enable_time = ???; */
+	drvdata->desc.id = index;
+	drvdata->desc.type = REGULATOR_VOLTAGE;
+	drvdata->desc.ops = &r8a7790_sd_regulator_ops;
+	drvdata->desc.n_voltages = 2;
+
+	snprintf(drvdata->name, sizeof(drvdata->name), "sd%d-vccq", index);
+	drvdata->desc.name = drvdata->name;
+
+	/* Read initial state from IOCTRL6 */
+	mapped_reg = sh_pfc_phys_to_virt(pfc, 0xe606008c);
+	switch ((sh_pfc_read_raw_reg(mapped_reg, 32) >> (24 - index * 8)) &
+		0xff) {
+	case 0:		/* low = 1.8V */
+		drvdata->state = 0;
+		break;
+	case 0xff:	/* standard = 3.3V */
+		drvdata->state = 1;
+		break;
+	default:	/* mixed?! */
+		drvdata->state = -1;
+		break;
+	}
+
+	cfg.dev = pfc->dev;
+	cfg.of_node = np;
+	cfg.driver_data = drvdata;
+	cfg.init_data = &r8a7790_sd_regulator_init;
+
+	drvdata->dev = devm_regulator_register(pfc->dev, &drvdata->desc, &cfg);
+	if (IS_ERR(drvdata->dev)) {
+		ret = PTR_ERR(drvdata->dev);
+		dev_err(pfc->dev, "Failed to register regulator: %d\n", ret);
+	}
+
+out:
+	of_node_put(np);
+	return ret;
+}
+
+static int r8a7790_pinmux_soc_init(struct sh_pfc *pfc)
+{
+	int i, ret;
+
+	for (i = 0; i < 4; ++i) {
+		ret = r8a7790_sd_regulator_probe(pfc, i);
+		if (ret && ret != -ENODEV)
+			return ret;
+	}
+
+	return 0;
+}
+
+#endif /* CONFIG_OF */
+
+static const struct sh_pfc_soc_operations pinmux_ops = {
+#ifdef CONFIG_OF
+	.init = r8a7790_pinmux_soc_init,
+#endif
+};
+
 const struct sh_pfc_soc_info r8a7790_pinmux_info = {
 	.name = "r8a77900_pfc",
+	.ops = &pinmux_ops,
+
 	.unlock_reg = 0xe6060000, /* PMMR */
 
 	.function = { PINMUX_FUNCTION_BEGIN, PINMUX_FUNCTION_END },
-- 
1.7.10.4

Re: [RFC PATCH 3/7] pinctrl: sh-pfc: r8a7790: Add regulators for SD voltage switch

[Ulf Hansson]

On 30 April 2015 at 14:31, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> Model the choice of 1.8V or 3.3V signalling for each SD interface as a
> regulator.
>
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>

You need also to send this to the pinctrl maintainer and the corresponding list.

Kind regards
Uffe

> ---
>  drivers/pinctrl/sh-pfc/Kconfig       |    1 +
>  drivers/pinctrl/sh-pfc/core.c        |    2 +-
>  drivers/pinctrl/sh-pfc/core.h        |    1 +
>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c |  189 ++++++++++++++++++++++++++++++++++
>  4 files changed, 192 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/Kconfig b/drivers/pinctrl/sh-pfc/Kconfig
> index 8c4b3d391823..4b1895a6ac69 100644
> --- a/drivers/pinctrl/sh-pfc/Kconfig
> +++ b/drivers/pinctrl/sh-pfc/Kconfig
> @@ -49,6 +49,7 @@ config PINCTRL_PFC_R8A7790
>         def_bool y
>         depends on ARCH_R8A7790
>         select PINCTRL_SH_PFC
> +       select REGULATOR if OF
>
>  config PINCTRL_PFC_R8A7791
>         def_bool y
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index 7b2c9495c383..7d51f96afc9a 100644
> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -92,7 +92,7 @@ static int sh_pfc_map_resources(struct sh_pfc *pfc,
>         return 0;
>  }
>
> -static void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 reg)
> +void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 reg)
>  {
>         struct sh_pfc_window *window;
>         phys_addr_t address = reg;
> diff --git a/drivers/pinctrl/sh-pfc/core.h b/drivers/pinctrl/sh-pfc/core.h
> index 6dc8a6fc2746..af355629c5d2 100644
> --- a/drivers/pinctrl/sh-pfc/core.h
> +++ b/drivers/pinctrl/sh-pfc/core.h
> @@ -57,6 +57,7 @@ int sh_pfc_unregister_gpiochip(struct sh_pfc *pfc);
>  int sh_pfc_register_pinctrl(struct sh_pfc *pfc);
>  int sh_pfc_unregister_pinctrl(struct sh_pfc *pfc);
>
> +void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 address);
>  u32 sh_pfc_read_raw_reg(void __iomem *mapped_reg, unsigned int reg_width);
>  void sh_pfc_write_raw_reg(void __iomem *mapped_reg, unsigned int reg_width,
>                           u32 data);
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> index 22a5470889f5..0c9d2c018a10 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -23,6 +23,13 @@
>
>  #include <linux/kernel.h>
>  #include <linux/platform_data/gpio-rcar.h>
> +#ifdef CONFIG_OF
> +#include <linux/of.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/of_regulator.h>
> +#include <linux/string.h>
> +#endif
>
>  #include "core.h"
>  #include "sh_pfc.h"
> @@ -5586,8 +5593,190 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
>         { },
>  };
>
> +#ifdef CONFIG_OF
> +
> +struct r8a7790_sd_regulator_data {
> +       struct regulator_desc desc;
> +       char name[10];
> +       struct regulator_dev *dev;
> +       int state;
> +};
> +
> +#define SD_REGULATOR_NAME "regulator-r8a7790-sd"
> +
> +#define SD_LOW_VOLTAGE 1800000
> +#define SD_STD_VOLTAGE 3300000
> +
> +static int r8a7790_sd_regulator_set_voltage(struct regulator_dev *dev,
> +                                           int min_uV, int max_uV,
> +                                           unsigned int *selector)
> +{
> +       struct r8a7790_sd_regulator_data *drvdata = rdev_get_drvdata(dev);
> +       struct sh_pfc *pfc = dev_get_drvdata(dev->dev.parent);
> +       void __iomem *mapped_reg;
> +       u32 data, mask;
> +       int state;
> +
> +       if (min_uV <= SD_LOW_VOLTAGE && max_uV >= SD_LOW_VOLTAGE)
> +               state = 0;
> +       else if (min_uV <= SD_STD_VOLTAGE && max_uV >= SD_STD_VOLTAGE)
> +               state = 1;
> +       else
> +               return -EINVAL;
> +
> +       /* Map IOCTRL6 */
> +       mapped_reg = sh_pfc_phys_to_virt(pfc, 0xe606008c);
> +
> +       spin_lock(&pfc->lock);
> +
> +       data = sh_pfc_read_raw_reg(mapped_reg, 32);
> +
> +       /* Set I/O voltage for the 8 pins for this SD interface */
> +       mask = 0xff << (24 - drvdata->desc.id * 8);
> +       if (state)
> +               data |= mask;
> +       else
> +               data &= ~mask;
> +
> +       sh_pfc_write_raw_reg(
> +               sh_pfc_phys_to_virt(pfc, pfc->info->unlock_reg), 32,
> +               ~data);
> +       sh_pfc_write_raw_reg(mapped_reg, 32, data);
> +
> +       spin_unlock(&pfc->lock);
> +
> +       drvdata->state = state;
> +       if (selector)
> +               *selector = state;
> +
> +       return 0;
> +}
> +
> +static int r8a7790_sd_regulator_list_voltage(struct regulator_dev *dev,
> +                                      unsigned int selector)
> +{
> +       switch (selector) {
> +       case 0:
> +               return SD_LOW_VOLTAGE;
> +       case 1:
> +               return SD_STD_VOLTAGE;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static int r8a7790_sd_regulator_get_voltage(struct regulator_dev *dev)
> +{
> +       struct r8a7790_sd_regulator_data *drvdata = rdev_get_drvdata(dev);
> +
> +       return r8a7790_sd_regulator_list_voltage(dev, drvdata->state);
> +}
> +
> +static const struct regulator_ops r8a7790_sd_regulator_ops = {
> +       .set_voltage = r8a7790_sd_regulator_set_voltage,
> +       .get_voltage = r8a7790_sd_regulator_get_voltage,
> +       .list_voltage = r8a7790_sd_regulator_list_voltage,
> +};
> +
> +static const struct regulator_init_data r8a7790_sd_regulator_init = {
> +       .constraints = {
> +               .valid_ops_mask = REGULATOR_CHANGE_VOLTAGE,
> +               .min_uV = SD_LOW_VOLTAGE,
> +               .max_uV = SD_STD_VOLTAGE,
> +       },
> +};
> +
> +static int r8a7790_sd_regulator_probe(struct sh_pfc *pfc, int index)
> +{
> +       char child_name[20];
> +       struct device_node *np;
> +       struct r8a7790_sd_regulator_data *drvdata;
> +       struct regulator_config cfg = { };
> +       void __iomem *mapped_reg;
> +       int ret;
> +
> +       snprintf(child_name, sizeof(child_name), "sd-regulator@%d", index);
> +       np = NULL;
> +       while ((np = of_get_next_available_child(pfc->dev->of_node, np))) {
> +               if (!strcmp(kbasename(np->full_name), child_name))
> +                       break;
> +       }
> +       if (!np) {
> +               dev_dbg(pfc->dev, "no %s child node found\n", child_name);
> +               return -ENODEV;
> +       }
> +
> +       drvdata = devm_kzalloc(pfc->dev, sizeof(*drvdata), GFP_KERNEL);
> +       if (!drvdata) {
> +               ret = -ENOMEM;
> +               goto out;
> +       }
> +       drvdata->desc.owner = THIS_MODULE;
> +       /* XXX drvdata->desc.enable_time = ???; */
> +       drvdata->desc.id = index;
> +       drvdata->desc.type = REGULATOR_VOLTAGE;
> +       drvdata->desc.ops = &r8a7790_sd_regulator_ops;
> +       drvdata->desc.n_voltages = 2;
> +
> +       snprintf(drvdata->name, sizeof(drvdata->name), "sd%d-vccq", index);
> +       drvdata->desc.name = drvdata->name;
> +
> +       /* Read initial state from IOCTRL6 */
> +       mapped_reg = sh_pfc_phys_to_virt(pfc, 0xe606008c);
> +       switch ((sh_pfc_read_raw_reg(mapped_reg, 32) >> (24 - index * 8)) &
> +               0xff) {
> +       case 0:         /* low = 1.8V */
> +               drvdata->state = 0;
> +               break;
> +       case 0xff:      /* standard = 3.3V */
> +               drvdata->state = 1;
> +               break;
> +       default:        /* mixed?! */
> +               drvdata->state = -1;
> +               break;
> +       }
> +
> +       cfg.dev = pfc->dev;
> +       cfg.of_node = np;
> +       cfg.driver_data = drvdata;
> +       cfg.init_data = &r8a7790_sd_regulator_init;
> +
> +       drvdata->dev = devm_regulator_register(pfc->dev, &drvdata->desc, &cfg);
> +       if (IS_ERR(drvdata->dev)) {
> +               ret = PTR_ERR(drvdata->dev);
> +               dev_err(pfc->dev, "Failed to register regulator: %d\n", ret);
> +       }
> +
> +out:
> +       of_node_put(np);
> +       return ret;
> +}
> +
> +static int r8a7790_pinmux_soc_init(struct sh_pfc *pfc)
> +{
> +       int i, ret;
> +
> +       for (i = 0; i < 4; ++i) {
> +               ret = r8a7790_sd_regulator_probe(pfc, i);
> +               if (ret && ret != -ENODEV)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +#endif /* CONFIG_OF */
> +
> +static const struct sh_pfc_soc_operations pinmux_ops = {
> +#ifdef CONFIG_OF
> +       .init = r8a7790_pinmux_soc_init,
> +#endif
> +};
> +
>  const struct sh_pfc_soc_info r8a7790_pinmux_info = {
>         .name = "r8a77900_pfc",
> +       .ops = &pinmux_ops,
> +
>         .unlock_reg = 0xe6060000, /* PMMR */
>
>         .function = { PINMUX_FUNCTION_BEGIN, PINMUX_FUNCTION_END },
> --
> 1.7.10.4
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/7] pinctrl: sh-pfc: r8a7790: Add regulators for SD voltage switch

[Ben Hutchings]

On Tue, 2015-05-05 at 09:52 +0200, Ulf Hansson wrote:
> On 30 April 2015 at 14:31, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> > Model the choice of 1.8V or 3.3V signalling for each SD interface as a
> > regulator.
> >
> > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> 
> You need also to send this to the pinctrl maintainer and the corresponding list.
[...]

OK, will do next time.

Ben.

[RFC PATCH 4/7] ARM: shmobile: r8a7790: Add nodes for pfc SD voltage regulators

[Ben Hutchings]

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 arch/arm/boot/dts/r8a7790.dtsi |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 4bb2f4c17321..23e826153a9d 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -483,6 +483,23 @@
 	pfc: pfc@e6060000 {
 		compatible = "renesas,pfc-r8a7790";
 		reg = <0 0xe6060000 0 0x250>;
+
+		vccq_ref_sdhi0: sd-regulator@0 {
+			compatible = "renesas,pfc-r8a7790-sd-regulator";
+			status = "disabled";
+		};
+		vccq_ref_sdhi1: sd-regulator@1 {
+			compatible = "renesas,pfc-r8a7790-sd-regulator";
+			status = "disabled";
+		};
+		vccq_ref_sdhi2: sd-regulator@2 {
+			compatible = "renesas,pfc-r8a7790-sd-regulator";
+			status = "disabled";
+		};
+		vccq_ref_sdhi3: sd-regulator@3 {
+			compatible = "renesas,pfc-r8a7790-sd-regulator";
+			status = "disabled";
+		};
 	};
 
 	sdhi0: sd@ee100000 {
@@ -490,6 +507,7 @@
 		reg = <0 0xee100000 0 0x328>;
 		interrupts = <0 165 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
+		vqmmc-ref-supply = <&vccq_ref_sdhi0>;
 		dmas = <&dmac1 0xcd>, <&dmac1 0xce>;
 		dma-names = "tx", "rx";
 		status = "disabled";
@@ -500,6 +518,7 @@
 		reg = <0 0xee120000 0 0x328>;
 		interrupts = <0 166 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp3_clks R8A7790_CLK_SDHI1>;
+		vqmmc-ref-supply = <&vccq_ref_sdhi1>;
 		dmas = <&dmac1 0xc9>, <&dmac1 0xca>;
 		dma-names = "tx", "rx";
 		status = "disabled";
@@ -510,6 +529,7 @@
 		reg = <0 0xee140000 0 0x100>;
 		interrupts = <0 167 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
+		vqmmc-ref-supply = <&vccq_ref_sdhi2>;
 		dmas = <&dmac1 0xc1>, <&dmac1 0xc2>;
 		dma-names = "tx", "rx";
 		status = "disabled";
@@ -520,6 +540,7 @@
 		reg = <0 0xee160000 0 0x100>;
 		interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp3_clks R8A7790_CLK_SDHI3>;
+		vqmmc-ref-supply = <&vccq_ref_sdhi3>;
 		dmas = <&dmac1 0xd3>, <&dmac1 0xd4>;
 		dma-names = "tx", "rx";
 		status = "disabled";
-- 
1.7.10.4

Re: [RFC PATCH 4/7] ARM: shmobile: r8a7790: Add nodes for pfc SD voltage regulators

[Simon Horman]

Hi Ben,

thanks for your patch-set.

On Thu, Apr 30, 2015 at 01:31:54PM +0100, Ben Hutchings wrote:
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> ---
>  arch/arm/boot/dts/r8a7790.dtsi |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index 4bb2f4c17321..23e826153a9d 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -483,6 +483,23 @@
>  	pfc: pfc@e6060000 {
>  		compatible = "renesas,pfc-r8a7790";
>  		reg = <0 0xe6060000 0 0x250>;
> +
> +		vccq_ref_sdhi0: sd-regulator@0 {
> +			compatible = "renesas,pfc-r8a7790-sd-regulator";

I'm a little confused. What is "renesas,pfc-r8a7790-sd-regulator"?
It suspect that it should at least be documented under
Documentation/devicetree/bindings/

> +			status = "disabled";
> +		};
> +		vccq_ref_sdhi1: sd-regulator@1 {
> +			compatible = "renesas,pfc-r8a7790-sd-regulator";
> +			status = "disabled";
> +		};
> +		vccq_ref_sdhi2: sd-regulator@2 {
> +			compatible = "renesas,pfc-r8a7790-sd-regulator";
> +			status = "disabled";
> +		};
> +		vccq_ref_sdhi3: sd-regulator@3 {
> +			compatible = "renesas,pfc-r8a7790-sd-regulator";
> +			status = "disabled";
> +		};
>  	};
>  
>  	sdhi0: sd@ee100000 {
> @@ -490,6 +507,7 @@
>  		reg = <0 0xee100000 0 0x328>;
>  		interrupts = <0 165 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
> +		vqmmc-ref-supply = <&vccq_ref_sdhi0>;
>  		dmas = <&dmac1 0xcd>, <&dmac1 0xce>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> @@ -500,6 +518,7 @@
>  		reg = <0 0xee120000 0 0x328>;
>  		interrupts = <0 166 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI1>;
> +		vqmmc-ref-supply = <&vccq_ref_sdhi1>;
>  		dmas = <&dmac1 0xc9>, <&dmac1 0xca>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> @@ -510,6 +529,7 @@
>  		reg = <0 0xee140000 0 0x100>;
>  		interrupts = <0 167 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
> +		vqmmc-ref-supply = <&vccq_ref_sdhi2>;
>  		dmas = <&dmac1 0xc1>, <&dmac1 0xc2>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> @@ -520,6 +540,7 @@
>  		reg = <0 0xee160000 0 0x100>;
>  		interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI3>;
> +		vqmmc-ref-supply = <&vccq_ref_sdhi3>;
>  		dmas = <&dmac1 0xd3>, <&dmac1 0xd4>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> -- 
> 1.7.10.4
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC PATCH 4/7] ARM: shmobile: r8a7790: Add nodes for pfc SD voltage regulators

[Ben Hutchings]

On Fri, 2015-05-01 at 09:57 +0900, Simon Horman wrote:
> Hi Ben,
> 
> thanks for your patch-set.
> 
> On Thu, Apr 30, 2015 at 01:31:54PM +0100, Ben Hutchings wrote:
> > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> > ---
> >  arch/arm/boot/dts/r8a7790.dtsi |   21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> > index 4bb2f4c17321..23e826153a9d 100644
> > --- a/arch/arm/boot/dts/r8a7790.dtsi
> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
> > @@ -483,6 +483,23 @@
> >  	pfc: pfc@e6060000 {
> >  		compatible = "renesas,pfc-r8a7790";
> >  		reg = <0 0xe6060000 0 0x250>;
> > +
> > +		vccq_ref_sdhi0: sd-regulator@0 {
> > +			compatible = "renesas,pfc-r8a7790-sd-regulator";
> 
> I'm a little confused. What is "renesas,pfc-r8a7790-sd-regulator"?
> It suspect that it should at least be documented under
> Documentation/devicetree/bindings/
[...]

It will be if it's agreed that this is the way to represent the voltage
switch in the pfc.  The sh-pfc driver is changed in patch 3/7 to look
for sd-regulator@{0..3} nodes in an r8a7790 DT.  The nodes carry only
their address and status.  The compatible value is currently ignored.

Ben.

Re: [RFC PATCH 4/7] ARM: shmobile: r8a7790: Add nodes for pfc SD voltage regulators

[Simon Horman]

On Wed, May 06, 2015 at 02:18:02AM +0100, Ben Hutchings wrote:
> On Fri, 2015-05-01 at 09:57 +0900, Simon Horman wrote:
> > Hi Ben,
> > 
> > thanks for your patch-set.
> > 
> > On Thu, Apr 30, 2015 at 01:31:54PM +0100, Ben Hutchings wrote:
> > > Signed-off-by: Ben Hutchings <ben.hutchings-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
> > > ---
> > >  arch/arm/boot/dts/r8a7790.dtsi |   21 +++++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> > > index 4bb2f4c17321..23e826153a9d 100644
> > > --- a/arch/arm/boot/dts/r8a7790.dtsi
> > > +++ b/arch/arm/boot/dts/r8a7790.dtsi
> > > @@ -483,6 +483,23 @@
> > >  	pfc: pfc@e6060000 {
> > >  		compatible = "renesas,pfc-r8a7790";
> > >  		reg = <0 0xe6060000 0 0x250>;
> > > +
> > > +		vccq_ref_sdhi0: sd-regulator@0 {
> > > +			compatible = "renesas,pfc-r8a7790-sd-regulator";
> > 
> > I'm a little confused. What is "renesas,pfc-r8a7790-sd-regulator"?
> > It suspect that it should at least be documented under
> > Documentation/devicetree/bindings/
> [...]
> 
> It will be if it's agreed that this is the way to represent the voltage
> switch in the pfc.  The sh-pfc driver is changed in patch 3/7 to look
> for sd-regulator@{0..3} nodes in an r8a7790 DT.  The nodes carry only
> their address and status.  The compatible value is currently ignored.

Thanks, that makes sense.

As a rule I generally don't apply DT patches that use bindings
that haven't already been accepted. So if this does turn out to
be the way to go then please document it.
--
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

[RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ben Hutchings]

Implement voltage switch, supporting modes up to SDR-50.

Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.

This uses two voltage regulators, one external and one on the pfc.

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 drivers/mmc/host/sh_mobile_sdhi.c |   48 +++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 92a58c6007fe..c8538a256e22 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -30,6 +30,7 @@
 #include <linux/mfd/tmio.h>
 #include <linux/sh_dma.h>
 #include <linux/delay.h>
+#include <linux/regulator/consumer.h>
 
 #include "tmio_mmc.h"
 
@@ -84,6 +85,7 @@ MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
 
 struct sh_mobile_sdhi {
 	struct clk *clk;
+	struct regulator *vqmmc_ref;
 	struct tmio_mmc_data mmc_data;
 	struct tmio_mmc_dma dma_priv;
 };
@@ -148,6 +150,41 @@ static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int clk)
 	}
 }
 
+static int sh_mobile_sdhi_start_signal_voltage_switch(
+	struct tmio_mmc_host *host, unsigned char signal_voltage)
+{
+	struct sh_mobile_sdhi *priv = host_to_priv(host);
+	int min_uV, max_uV;
+	int ret;
+
+	if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
+		min_uV = 2700000;
+		max_uV = 3600000;
+	} else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
+		min_uV = 1700000;
+		max_uV = 1950000;
+	} else {
+		return -EINVAL;
+	}
+
+	if (!IS_ERR(host->mmc->supply.vqmmc)) {
+		ret = regulator_set_voltage(host->mmc->supply.vqmmc,
+					    min_uV, max_uV);
+		if (ret)
+			return ret;
+	}
+
+	if (!IS_ERR(priv->vqmmc_ref)) {
+		ret = regulator_set_voltage(priv->vqmmc_ref,
+					    min_uV, max_uV);
+		if (ret)
+			return ret;
+	}
+
+	usleep_range(5000, 5500);
+	return 0;
+}
+
 static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
 {
 	int timeout = 1000;
@@ -240,6 +277,13 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eprobe;
 	}
 
+	priv->vqmmc_ref = devm_regulator_get_optional(&pdev->dev, "vqmmc-ref");
+	if (IS_ERR(priv->vqmmc_ref)) {
+		if (PTR_ERR(priv->vqmmc_ref) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		dev_info(&pdev->dev, "No vqmmc reference regulator found\n");
+	}
+
 	host = tmio_mmc_host_alloc(pdev);
 	if (!host) {
 		ret = -ENOMEM;
@@ -253,6 +297,10 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	host->clk_enable	= sh_mobile_sdhi_clk_enable;
 	host->clk_disable	= sh_mobile_sdhi_clk_disable;
 	host->multi_io_quirk	= sh_mobile_sdhi_multi_io_quirk;
+
+	host->start_signal_voltage_switch
+				= sh_mobile_sdhi_start_signal_voltage_switch;
+
 	/* SD control register space size is 0x100, 0x200 for bus_shift=1 */
 	if (resource_size(res) > 0x100)
 		host->bus_shift = 1;
-- 
1.7.10.4

Re: [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Sergei Shtylyov]

Hello.

On 04/30/2015 03:32 PM, Ben Hutchings wrote:

> Implement voltage switch, supporting modes up to SDR-50.

> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.

> This uses two voltage regulators, one external and one on the pfc.

> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> ---
>   drivers/mmc/host/sh_mobile_sdhi.c |   48 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 48 insertions(+)

> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 92a58c6007fe..c8538a256e22 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
[...]
> @@ -148,6 +150,41 @@ static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int clk)
>   	}
>   }
>
> +static int sh_mobile_sdhi_start_signal_voltage_switch(
> +	struct tmio_mmc_host *host, unsigned char signal_voltage)
> +{
> +	struct sh_mobile_sdhi *priv = host_to_priv(host);
> +	int min_uV, max_uV;
> +	int ret;
> +
> +	if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> +		min_uV = 2700000;
> +		max_uV = 3600000;
> +	} else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
> +		min_uV = 1700000;
> +		max_uV = 1950000;
> +	} else {
> +		return -EINVAL;
> +	}

    The above is asking to be a *switch* statement.

[...]

WBR, Sergei

Re: [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ben Hutchings]

On Thu, 2015-04-30 at 19:04 +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 04/30/2015 03:32 PM, Ben Hutchings wrote:
> 
> > Implement voltage switch, supporting modes up to SDR-50.
> 
> > Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
> 
> > This uses two voltage regulators, one external and one on the pfc.
> 
> > Signed-off-by: Ben Hutchings <ben.hutchings-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
> > ---
> >   drivers/mmc/host/sh_mobile_sdhi.c |   48 +++++++++++++++++++++++++++++++++++++
> >   1 file changed, 48 insertions(+)
> 
> > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> > index 92a58c6007fe..c8538a256e22 100644
> > --- a/drivers/mmc/host/sh_mobile_sdhi.c
> > +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> [...]
> > @@ -148,6 +150,41 @@ static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int clk)
> >   	}
> >   }
> >
> > +static int sh_mobile_sdhi_start_signal_voltage_switch(
> > +	struct tmio_mmc_host *host, unsigned char signal_voltage)
> > +{
> > +	struct sh_mobile_sdhi *priv = host_to_priv(host);
> > +	int min_uV, max_uV;
> > +	int ret;
> > +
> > +	if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> > +		min_uV = 2700000;
> > +		max_uV = 3600000;
> > +	} else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
> > +		min_uV = 1700000;
> > +		max_uV = 1950000;
> > +	} else {
> > +		return -EINVAL;
> > +	}
> 
>     The above is asking to be a *switch* statement.

I suppose so, yes.

Ben.


--
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

Re: [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ulf Hansson]

On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> Implement voltage switch, supporting modes up to SDR-50.
>
> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
>
> This uses two voltage regulators, one external and one on the pfc.

Why two? If there is a parent child relation ship, that should be
handled through the regulator tree, right!? Please elaborate.

Kind regards
Uffe

>
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c |   48 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 92a58c6007fe..c8538a256e22 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -30,6 +30,7 @@
>  #include <linux/mfd/tmio.h>
>  #include <linux/sh_dma.h>
>  #include <linux/delay.h>
> +#include <linux/regulator/consumer.h>
>
>  #include "tmio_mmc.h"
>
> @@ -84,6 +85,7 @@ MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
>
>  struct sh_mobile_sdhi {
>         struct clk *clk;
> +       struct regulator *vqmmc_ref;
>         struct tmio_mmc_data mmc_data;
>         struct tmio_mmc_dma dma_priv;
>  };
> @@ -148,6 +150,41 @@ static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int clk)
>         }
>  }
>
> +static int sh_mobile_sdhi_start_signal_voltage_switch(
> +       struct tmio_mmc_host *host, unsigned char signal_voltage)
> +{
> +       struct sh_mobile_sdhi *priv = host_to_priv(host);
> +       int min_uV, max_uV;
> +       int ret;
> +
> +       if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> +               min_uV = 2700000;
> +               max_uV = 3600000;
> +       } else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
> +               min_uV = 1700000;
> +               max_uV = 1950000;
> +       } else {
> +               return -EINVAL;
> +       }
> +
> +       if (!IS_ERR(host->mmc->supply.vqmmc)) {
> +               ret = regulator_set_voltage(host->mmc->supply.vqmmc,
> +                                           min_uV, max_uV);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       if (!IS_ERR(priv->vqmmc_ref)) {
> +               ret = regulator_set_voltage(priv->vqmmc_ref,
> +                                           min_uV, max_uV);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       usleep_range(5000, 5500);
> +       return 0;
> +}
> +
>  static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
>  {
>         int timeout = 1000;
> @@ -240,6 +277,13 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>                 goto eprobe;
>         }
>
> +       priv->vqmmc_ref = devm_regulator_get_optional(&pdev->dev, "vqmmc-ref");
> +       if (IS_ERR(priv->vqmmc_ref)) {
> +               if (PTR_ERR(priv->vqmmc_ref) == -EPROBE_DEFER)
> +                       return -EPROBE_DEFER;
> +               dev_info(&pdev->dev, "No vqmmc reference regulator found\n");
> +       }
> +
>         host = tmio_mmc_host_alloc(pdev);
>         if (!host) {
>                 ret = -ENOMEM;
> @@ -253,6 +297,10 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>         host->clk_enable        = sh_mobile_sdhi_clk_enable;
>         host->clk_disable       = sh_mobile_sdhi_clk_disable;
>         host->multi_io_quirk    = sh_mobile_sdhi_multi_io_quirk;
> +
> +       host->start_signal_voltage_switch
> +                               = sh_mobile_sdhi_start_signal_voltage_switch;
> +
>         /* SD control register space size is 0x100, 0x200 for bus_shift=1 */
>         if (resource_size(res) > 0x100)
>                 host->bus_shift = 1;
> --
> 1.7.10.4
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Linux-kernel] [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ben Dooks]

On 05/05/15 10:56, Ulf Hansson wrote:
> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
>> Implement voltage switch, supporting modes up to SDR-50.
>>
>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
>>
>> This uses two voltage regulators, one external and one on the pfc.
> 
> Why two? If there is a parent child relation ship, that should be
> handled through the regulator tree, right!? Please elaborate.

The card main power is separate from the IO line voltages.

To get to the high-speed, card power is left at 3.3V and the IO
voltage is changed to 1.8V.

In the systems we have the power gate is separate from the controls
for the IO but not integrated into the MMC controller itself.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [Linux-kernel] [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ulf Hansson]

On 5 May 2015 at 10:35, Ben Dooks <ben.dooks-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org> wrote:
> On 05/05/15 10:56, Ulf Hansson wrote:
>> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org> wrote:
>>> Implement voltage switch, supporting modes up to SDR-50.
>>>
>>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
>>>
>>> This uses two voltage regulators, one external and one on the pfc.
>>
>> Why two? If there is a parent child relation ship, that should be
>> handled through the regulator tree, right!? Please elaborate.
>
> The card main power is separate from the IO line voltages.
>
> To get to the high-speed, card power is left at 3.3V and the IO
> voltage is changed to 1.8V.
>
> In the systems we have the power gate is separate from the controls
> for the IO but not integrated into the MMC controller itself.

Okay, that's what I was expecting and hoping for :-)

Then you need to rework $subject patch according to below.

1) Use mmc_regulator_get_supply() to fetch both the I/O voltage
regulator (vqmmc) and the main power regulator (vmmc).Your "vqmmc_ref"
regulator should thus be renamed to "vmmc".
2) The vmmc regulator should not be handled from the
->start_signal_voltage_switch() callback, since it's only vqmmc
voltage levels that should be changed from there.
3) The voltage levels changes for vmmc shall be handled via the
->set_ios() callback.

I suggest you go and have a look in drivers/mmc/host/mmci.c, that
should provide you with a good inspiration.

Kind regards
Uffe
--
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

Re: [Linux-kernel] [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ben Hutchings]

On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote:
> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> > On 05/05/15 10:56, Ulf Hansson wrote:
> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> >>> Implement voltage switch, supporting modes up to SDR-50.
> >>>
> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
> >>>
> >>> This uses two voltage regulators, one external and one on the pfc.
> >>
> >> Why two? If there is a parent child relation ship, that should be
> >> handled through the regulator tree, right!? Please elaborate.
> >
> > The card main power is separate from the IO line voltages.
> >
> > To get to the high-speed, card power is left at 3.3V and the IO
> > voltage is changed to 1.8V.
> >
> > In the systems we have the power gate is separate from the controls
> > for the IO but not integrated into the MMC controller itself.

In this case, there are *three* regulators:

1. External regulator for card power (VDD pin): "vmmc"
2. External regulator for pull-up voltage for the data pins: "vqmmc"
3. Internal regulator for input(?) level on the data pins:
   "vqmmc_ref" (I'm open to suggestions of a better name)

> Okay, that's what I was expecting and hoping for :-)
> 
> Then you need to rework $subject patch according to below.
> 
> 1) Use mmc_regulator_get_supply() to fetch both the I/O voltage
> regulator (vqmmc) and the main power regulator (vmmc).

We already do that.

> Your "vqmmc_ref" regulator should thus be renamed to "vmmc".

No, we have one of those already.

> 2) The vmmc regulator should not be handled from the
> ->start_signal_voltage_switch() callback, since it's only vqmmc
> voltage levels that should be changed from there.

It isn't.

> 3) The voltage levels changes for vmmc shall be handled via the
> ->set_ios() callback.

We don't support UHS-II so we never change the voltage of this
regulator.

Ben.

> I suggest you go and have a look in drivers/mmc/host/mmci.c, that
> should provide you with a good inspiration.
> 
> Kind regards
> Uffe

Re: [Linux-kernel] [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ulf Hansson]

On 6 May 2015 at 03:38, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote:
>> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> > On 05/05/15 10:56, Ulf Hansson wrote:
>> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
>> >>> Implement voltage switch, supporting modes up to SDR-50.
>> >>>
>> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
>> >>>
>> >>> This uses two voltage regulators, one external and one on the pfc.
>> >>
>> >> Why two? If there is a parent child relation ship, that should be
>> >> handled through the regulator tree, right!? Please elaborate.
>> >
>> > The card main power is separate from the IO line voltages.
>> >
>> > To get to the high-speed, card power is left at 3.3V and the IO
>> > voltage is changed to 1.8V.
>> >
>> > In the systems we have the power gate is separate from the controls
>> > for the IO but not integrated into the MMC controller itself.
>
> In this case, there are *three* regulators:
>
> 1. External regulator for card power (VDD pin): "vmmc"
> 2. External regulator for pull-up voltage for the data pins: "vqmmc"

Is this really a regulator and not just about changing a pinctrl setting?

The reason why I wonder, is because there are several other mmc host
driver's the use a specific pinctrl state for this.

> 3. Internal regulator for input(?) level on the data pins:
>    "vqmmc_ref" (I'm open to suggestions of a better name)
>
>> Okay, that's what I was expecting and hoping for :-)
>>
>> Then you need to rework $subject patch according to below.
>>
>> 1) Use mmc_regulator_get_supply() to fetch both the I/O voltage
>> regulator (vqmmc) and the main power regulator (vmmc).
>
> We already do that.

Okay, great!

>
>> Your "vqmmc_ref" regulator should thus be renamed to "vmmc".
>
> No, we have one of those already.
>
>> 2) The vmmc regulator should not be handled from the
>> ->start_signal_voltage_switch() callback, since it's only vqmmc
>> voltage levels that should be changed from there.
>
> It isn't.
>
>> 3) The voltage levels changes for vmmc shall be handled via the
>> ->set_ios() callback.
>
> We don't support UHS-II so we never change the voltage of this
> regulator.

That's not related to UHS-II. Voltage level negotiation for vmmc is
done even for legacy mode cards.

Kind regards
Uffe

Re: [Linux-kernel] [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ben Hutchings]

On Wed, 2015-05-06 at 10:44 +0200, Ulf Hansson wrote:
> On 6 May 2015 at 03:38, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> > On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote:
> >> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> >> > On 05/05/15 10:56, Ulf Hansson wrote:
> >> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> >> >>> Implement voltage switch, supporting modes up to SDR-50.
> >> >>>
> >> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
> >> >>>
> >> >>> This uses two voltage regulators, one external and one on the pfc.
> >> >>
> >> >> Why two? If there is a parent child relation ship, that should be
> >> >> handled through the regulator tree, right!? Please elaborate.
> >> >
> >> > The card main power is separate from the IO line voltages.
> >> >
> >> > To get to the high-speed, card power is left at 3.3V and the IO
> >> > voltage is changed to 1.8V.
> >> >
> >> > In the systems we have the power gate is separate from the controls
> >> > for the IO but not integrated into the MMC controller itself.
> >
> > In this case, there are *three* regulators:
> >
> > 1. External regulator for card power (VDD pin): "vmmc"
> > 2. External regulator for pull-up voltage for the data pins: "vqmmc"
> 
> Is this really a regulator and not just about changing a pinctrl setting?

Looking at the Lager board, the pull-up voltage appears to be controlled
by the external PMIC (DA9063), which is signalled using GPIOs (I don't
know why, when it's also connected to I2C).

> The reason why I wonder, is because there are several other mmc host
> driver's the use a specific pinctrl state for this.
> 
> > 3. Internal regulator for input(?) level on the data pins:
> >    "vqmmc_ref" (I'm open to suggestions of a better name)

This one is implemented in the pinctrl (pfc) block.

[...]
> >> 3) The voltage levels changes for vmmc shall be handled via the
> >> ->set_ios() callback.
> >
> > We don't support UHS-II so we never change the voltage of this
> > regulator.
> 
> That's not related to UHS-II. Voltage level negotiation for vmmc is
> done even for legacy mode cards.

I'm looking at "SD Specifications, Part 1, Physical Layer Simplified
Specifications, Version 4.10" and it seems clear from that, that only
UHS-II cards have a variable supply voltage.  Maybe that changed in a
later version?  Anyway, I don't have any board that can change the
supply voltage.

Ben.

Re: [Linux-kernel] [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ulf Hansson]

On 6 May 2015 at 15:49, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> On Wed, 2015-05-06 at 10:44 +0200, Ulf Hansson wrote:
>> On 6 May 2015 at 03:38, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
>> > On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote:
>> >> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> >> > On 05/05/15 10:56, Ulf Hansson wrote:
>> >> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
>> >> >>> Implement voltage switch, supporting modes up to SDR-50.
>> >> >>>
>> >> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
>> >> >>>
>> >> >>> This uses two voltage regulators, one external and one on the pfc.
>> >> >>
>> >> >> Why two? If there is a parent child relation ship, that should be
>> >> >> handled through the regulator tree, right!? Please elaborate.
>> >> >
>> >> > The card main power is separate from the IO line voltages.
>> >> >
>> >> > To get to the high-speed, card power is left at 3.3V and the IO
>> >> > voltage is changed to 1.8V.
>> >> >
>> >> > In the systems we have the power gate is separate from the controls
>> >> > for the IO but not integrated into the MMC controller itself.
>> >
>> > In this case, there are *three* regulators:
>> >
>> > 1. External regulator for card power (VDD pin): "vmmc"
>> > 2. External regulator for pull-up voltage for the data pins: "vqmmc"
>>
>> Is this really a regulator and not just about changing a pinctrl setting?
>
> Looking at the Lager board, the pull-up voltage appears to be controlled
> by the external PMIC (DA9063), which is signalled using GPIOs (I don't
> know why, when it's also connected to I2C).

Okay, thus we should have a regulator for it.

>
>> The reason why I wonder, is because there are several other mmc host
>> driver's the use a specific pinctrl state for this.
>>
>> > 3. Internal regulator for input(?) level on the data pins:
>> >    "vqmmc_ref" (I'm open to suggestions of a better name)
>
> This one is implemented in the pinctrl (pfc) block.

I understand this as we should have a specific UHS pinctrl state,
instead of a regulator. Right?

[...]

Kind regards
Uffe

Re: [Linux-kernel] [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ben Hutchings]

On Mon, 2015-05-11 at 10:54 +0200, Ulf Hansson wrote:
> On 6 May 2015 at 15:49, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> > On Wed, 2015-05-06 at 10:44 +0200, Ulf Hansson wrote:
> >> On 6 May 2015 at 03:38, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> >> > On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote:
> >> >> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> >> >> > On 05/05/15 10:56, Ulf Hansson wrote:
> >> >> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> >> >> >>> Implement voltage switch, supporting modes up to SDR-50.
> >> >> >>>
> >> >> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
> >> >> >>>
> >> >> >>> This uses two voltage regulators, one external and one on the pfc.
> >> >> >>
> >> >> >> Why two? If there is a parent child relation ship, that should be
> >> >> >> handled through the regulator tree, right!? Please elaborate.
> >> >> >
> >> >> > The card main power is separate from the IO line voltages.
> >> >> >
> >> >> > To get to the high-speed, card power is left at 3.3V and the IO
> >> >> > voltage is changed to 1.8V.
> >> >> >
> >> >> > In the systems we have the power gate is separate from the controls
> >> >> > for the IO but not integrated into the MMC controller itself.
> >> >
> >> > In this case, there are *three* regulators:
> >> >
> >> > 1. External regulator for card power (VDD pin): "vmmc"
> >> > 2. External regulator for pull-up voltage for the data pins: "vqmmc"
> >>
> >> Is this really a regulator and not just about changing a pinctrl setting?
> >
> > Looking at the Lager board, the pull-up voltage appears to be controlled
> > by the external PMIC (DA9063), which is signalled using GPIOs (I don't
> > know why, when it's also connected to I2C).
> 
> Okay, thus we should have a regulator for it.
> 
> >
> >> The reason why I wonder, is because there are several other mmc host
> >> driver's the use a specific pinctrl state for this.
> >>
> >> > 3. Internal regulator for input(?) level on the data pins:
> >> >    "vqmmc_ref" (I'm open to suggestions of a better name)
> >
> > This one is implemented in the pinctrl (pfc) block.
> 
> I understand this as we should have a specific UHS pinctrl state,
> instead of a regulator. Right?

That was another option I considered, but I didn't see anything in the
pinctrl API that would allow changing the pin configuration dynamically.
(And sh_mobile_sdhi doesn't explicitly do anything with pinctrl at the
moment.)

Now that you mention pinctrl states, I can see that might be a good way
to handle this.  Since the states are identified by strings, does that
mean it's OK to add a driver-specific state such as "sh-pfc-1.8v" and
not add it to the common pinctrl headers?

Ben.

Re: [Linux-kernel] [RFC PATCH 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ulf Hansson]

On 11 May 2015 at 16:01, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> On Mon, 2015-05-11 at 10:54 +0200, Ulf Hansson wrote:
>> On 6 May 2015 at 15:49, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
>> > On Wed, 2015-05-06 at 10:44 +0200, Ulf Hansson wrote:
>> >> On 6 May 2015 at 03:38, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
>> >> > On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote:
>> >> >> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> >> >> > On 05/05/15 10:56, Ulf Hansson wrote:
>> >> >> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
>> >> >> >>> Implement voltage switch, supporting modes up to SDR-50.
>> >> >> >>>
>> >> >> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
>> >> >> >>>
>> >> >> >>> This uses two voltage regulators, one external and one on the pfc.
>> >> >> >>
>> >> >> >> Why two? If there is a parent child relation ship, that should be
>> >> >> >> handled through the regulator tree, right!? Please elaborate.
>> >> >> >
>> >> >> > The card main power is separate from the IO line voltages.
>> >> >> >
>> >> >> > To get to the high-speed, card power is left at 3.3V and the IO
>> >> >> > voltage is changed to 1.8V.
>> >> >> >
>> >> >> > In the systems we have the power gate is separate from the controls
>> >> >> > for the IO but not integrated into the MMC controller itself.
>> >> >
>> >> > In this case, there are *three* regulators:
>> >> >
>> >> > 1. External regulator for card power (VDD pin): "vmmc"
>> >> > 2. External regulator for pull-up voltage for the data pins: "vqmmc"
>> >>
>> >> Is this really a regulator and not just about changing a pinctrl setting?
>> >
>> > Looking at the Lager board, the pull-up voltage appears to be controlled
>> > by the external PMIC (DA9063), which is signalled using GPIOs (I don't
>> > know why, when it's also connected to I2C).
>>
>> Okay, thus we should have a regulator for it.
>>
>> >
>> >> The reason why I wonder, is because there are several other mmc host
>> >> driver's the use a specific pinctrl state for this.
>> >>
>> >> > 3. Internal regulator for input(?) level on the data pins:
>> >> >    "vqmmc_ref" (I'm open to suggestions of a better name)
>> >
>> > This one is implemented in the pinctrl (pfc) block.
>>
>> I understand this as we should have a specific UHS pinctrl state,
>> instead of a regulator. Right?
>
> That was another option I considered, but I didn't see anything in the
> pinctrl API that would allow changing the pin configuration dynamically.
> (And sh_mobile_sdhi doesn't explicitly do anything with pinctrl at the
> moment.)
>
> Now that you mention pinctrl states, I can see that might be a good way
> to handle this.  Since the states are identified by strings, does that
> mean it's OK to add a driver-specific state such as "sh-pfc-1.8v" and
> not add it to the common pinctrl headers?

Yes, we already have other drivers doing it like that.

Depending on what SoC this driver is used for, maybe you want this
state to be optional though.

Kind regards
Uffe

[RFC PATCH 6/7] ARM: shmobile: r8a7790-lager.dts: Set sdhi and mmcif clock rates

[Ben Hutchings]

From: Ben Dooks <ben.dooks@codethink.co.uk>

[bwh: Fold in fix from Ian Molton]
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 arch/arm/boot/dts/r8a7790-lager.dts |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index aaa4f258e279..343ec0ccc8df 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -413,6 +413,11 @@
 	vmmc-supply = <&fixedregulator3v3>;
 	bus-width = <8>;
 	non-removable;
+
+	assigned-clocks = <&mstp3_clks R8A7790_CLK_MMCIF1>;
+	assigned-clock-rates = <97500000>;
+	max-frequency = <50000000>;
+
 	status = "okay";
 };
 
@@ -488,6 +493,9 @@
 	pinctrl-0 = <&sdhi0_pins>;
 	pinctrl-names = "default";
 
+	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
+	assigned-clock-rates = <156000000>;
+
 	vmmc-supply = <&vcc_sdhi0>;
 	vqmmc-supply = <&vccq_sdhi0>;
 	cd-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
@@ -498,6 +506,9 @@
 	pinctrl-0 = <&sdhi2_pins>;
 	pinctrl-names = "default";
 
+	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
+	assigned-clock-rates = <97500000>;
+
 	vmmc-supply = <&vcc_sdhi2>;
 	vqmmc-supply = <&vccq_sdhi2>;
 	cd-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;
-- 
1.7.10.4

Re: [RFC PATCH 6/7] ARM: shmobile: r8a7790-lager.dts: Set sdhi and mmcif clock rates

[Sergei Shtylyov]

On 04/30/2015 03:32 PM, Ben Hutchings wrote:

> From: Ben Dooks <ben.dooks@codethink.co.uk>

> [bwh: Fold in fix from Ian Molton]
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>

    Not signed off by Ben Dooks?

[...]

WBR, Sergei

Re: [RFC PATCH 6/7] ARM: shmobile: r8a7790-lager.dts: Set sdhi and mmcif clock rates

[Ben Hutchings]

On Thu, 2015-04-30 at 19:06 +0300, Sergei Shtylyov wrote:
> On 04/30/2015 03:32 PM, Ben Hutchings wrote:
> 
> > From: Ben Dooks <ben.dooks@codethink.co.uk>
> 
> > [bwh: Fold in fix from Ian Molton]
> > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> 
>     Not signed off by Ben Dooks?

I'm quite capable of inferring that a colleague intended their work to
be released under GPL, thanks.

Ben.

Re: [RFC PATCH 6/7] ARM: shmobile: r8a7790-lager.dts: Set sdhi and mmcif clock rates

[Simon Horman]

Hi Ben,

Please use the following prefix for r8a7790-lager.dts patches:

ARM: shmobile: lager:

On Thu, Apr 30, 2015 at 01:32:46PM +0100, Ben Hutchings wrote:
> From: Ben Dooks <ben.dooks-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
> 
> [bwh: Fold in fix from Ian Molton]
> Signed-off-by: Ben Hutchings <ben.hutchings-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index aaa4f258e279..343ec0ccc8df 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -413,6 +413,11 @@
>  	vmmc-supply = <&fixedregulator3v3>;
>  	bus-width = <8>;
>  	non-removable;
> +
> +	assigned-clocks = <&mstp3_clks R8A7790_CLK_MMCIF1>;
> +	assigned-clock-rates = <97500000>;
> +	max-frequency = <50000000>;
> +
>  	status = "okay";
>  };
>  
> @@ -488,6 +493,9 @@
>  	pinctrl-0 = <&sdhi0_pins>;
>  	pinctrl-names = "default";
>  
> +	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
> +	assigned-clock-rates = <156000000>;
> +
>  	vmmc-supply = <&vcc_sdhi0>;
>  	vqmmc-supply = <&vccq_sdhi0>;
>  	cd-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
> @@ -498,6 +506,9 @@
>  	pinctrl-0 = <&sdhi2_pins>;
>  	pinctrl-names = "default";
>  
> +	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
> +	assigned-clock-rates = <97500000>;
> +
>  	vmmc-supply = <&vcc_sdhi2>;
>  	vqmmc-supply = <&vccq_sdhi2>;
>  	cd-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;
> -- 
> 1.7.10.4
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.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

Re: [RFC PATCH 6/7] ARM: shmobile: r8a7790-lager.dts: Set sdhi and mmcif clock rates

[Ben Hutchings]

On Fri, 2015-05-01 at 09:59 +0900, Simon Horman wrote:
> Hi Ben,
> 
> Please use the following prefix for r8a7790-lager.dts patches:
> 
> ARM: shmobile: lager:
[...]

OK, I'll do that in future.  I thought I'd seen the long name used more
often in the log, but I was mistaken.

Ben.


--
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

Re: [RFC PATCH 6/7] ARM: shmobile: r8a7790-lager.dts: Set sdhi and mmcif clock rates

[Simon Horman]

On Wed, May 06, 2015 at 02:46:57AM +0100, Ben Hutchings wrote:
> On Fri, 2015-05-01 at 09:59 +0900, Simon Horman wrote:
> > Hi Ben,
> > 
> > Please use the following prefix for r8a7790-lager.dts patches:
> > 
> > ARM: shmobile: lager:
> [...]
> 
> OK, I'll do that in future.  I thought I'd seen the long name used more
> often in the log, but I was mistaken.

Possibly, I don't always make sure the prefixes are as consistent as I should.

[RFC PATCH 1/7] mmc: tmio: Add UHS-I mode support

[Ben Hutchings]

Based on work by Shinobu Uehara and Ben Dooks.  This adds the
voltage switch operation needed for all UHS-I modes, but not
the tuning needed for SDR-104 which will come later.

XXX The card_busy implementation is a bit of a guess.

Signed-off-by: Ben Hutchings <ben.hutchings-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
---
 drivers/mmc/host/tmio_mmc.h     |    3 +++
 drivers/mmc/host/tmio_mmc_pio.c |   31 +++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index fc3805ed69d1..a60380b94105 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -103,6 +103,9 @@ struct tmio_mmc_host {
 	void (*clk_disable)(struct platform_device *pdev);
 	int (*multi_io_quirk)(struct mmc_card *card,
 			      unsigned int direction, int blk_size);
+
+	int (*start_signal_voltage_switch)(struct tmio_mmc_host *host,
+					   unsigned char signal_voltage);
 };
 
 struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index e3dcf31a8bd6..3c1e0e53c0b1 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1011,12 +1011,43 @@ static int tmio_multi_io_quirk(struct mmc_card *card,
 	return blk_size;
 }
 
+static int tmio_mmc_start_signal_voltage_switch(struct mmc_host *mmc,
+	struct mmc_ios *ios)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+
+	if (!host->start_signal_voltage_switch)
+		return 0;
+
+	return host->start_signal_voltage_switch(host, ios->signal_voltage);
+}
+
+static int tmio_mmc_card_busy(struct mmc_host *mmc)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	u32 status;
+
+	pm_runtime_get_sync(mmc_dev(mmc));
+	status = sd_ctrl_read32(host, CTL_STATUS);
+	pm_runtime_mark_last_busy(mmc_dev(mmc));
+	pm_runtime_put_autosuspend(mmc_dev(mmc));
+
+	/*
+	 * Card signals busy by pulling down all of DAT[3:0].  This status
+	 * flag appears to reflect DAT3.
+	 */
+	return !(status & TMIO_STAT_SIGSTATE_A);
+}
+
 static const struct mmc_host_ops tmio_mmc_ops = {
 	.request	= tmio_mmc_request,
 	.set_ios	= tmio_mmc_set_ios,
 	.get_ro         = tmio_mmc_get_ro,
 	.get_cd		= mmc_gpio_get_cd,
 	.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
+	.start_signal_voltage_switch
+			= tmio_mmc_start_signal_voltage_switch,
+	.card_busy	= tmio_mmc_card_busy,
 	.multi_io_quirk	= tmio_multi_io_quirk,
 };
 
-- 
1.7.10.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

Re: [RFC PATCH 1/7] mmc: tmio: Add UHS-I mode support

[Kuninori Morimoto]

Hi Ben

Thank you for your patch

> Based on work by Shinobu Uehara and Ben Dooks.  This adds the
> voltage switch operation needed for all UHS-I modes, but not
> the tuning needed for SDR-104 which will come later.
> 
> XXX The card_busy implementation is a bit of a guess.
> 
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>

If original patch was created by Shinobu/Ben Dooks,
and you updated it, I prefer to add these guys on Signed-off-by line.

>  static const struct mmc_host_ops tmio_mmc_ops = {
>  	.request	= tmio_mmc_request,
>  	.set_ios	= tmio_mmc_set_ios,
>  	.get_ro         = tmio_mmc_get_ro,
>  	.get_cd		= mmc_gpio_get_cd,
>  	.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
> +	.start_signal_voltage_switch
> +			= tmio_mmc_start_signal_voltage_switch,
> +	.card_busy	= tmio_mmc_card_busy,
>  	.multi_io_quirk	= tmio_multi_io_quirk,
>  };

I prefer to separate this patch for these 2 new callbacks if possible.

Re: [RFC PATCH 1/7] mmc: tmio: Add UHS-I mode support

[Ben Hutchings]

On Mon, 2015-05-11 at 03:38 +0000, Kuninori Morimoto wrote:
> Hi Ben
> 
> Thank you for your patch
> 
> > Based on work by Shinobu Uehara and Ben Dooks.  This adds the
> > voltage switch operation needed for all UHS-I modes, but not
> > the tuning needed for SDR-104 which will come later.
> > 
> > XXX The card_busy implementation is a bit of a guess.
> > 
> > Signed-off-by: Ben Hutchings <ben.hutchings-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
> 
> If original patch was created by Shinobu/Ben Dooks,
> and you updated it, I prefer to add these guys on Signed-off-by line.
> 
> >  static const struct mmc_host_ops tmio_mmc_ops = {
> >  	.request	= tmio_mmc_request,
> >  	.set_ios	= tmio_mmc_set_ios,
> >  	.get_ro         = tmio_mmc_get_ro,
> >  	.get_cd		= mmc_gpio_get_cd,
> >  	.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
> > +	.start_signal_voltage_switch
> > +			= tmio_mmc_start_signal_voltage_switch,
> > +	.card_busy	= tmio_mmc_card_busy,
> >  	.multi_io_quirk	= tmio_multi_io_quirk,
> >  };
> 
> I prefer to separate this patch for these 2 new callbacks if possible.

They are both used only by mmc_set_signal_voltage(), which warns if only
start_signal_voltage_switch is implemented.  So it doesn't make sense to
add them separately.

Ben.


--
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

Re: [RFC PATCH 1/7] mmc: tmio: Add UHS-I mode support

[Ben Hutchings]

On Mon, 2015-05-11 at 03:38 +0000, Kuninori Morimoto wrote:
> Hi Ben
> 
> Thank you for your patch
> 
> > Based on work by Shinobu Uehara and Ben Dooks.  This adds the
> > voltage switch operation needed for all UHS-I modes, but not
> > the tuning needed for SDR-104 which will come later.
> > 
> > XXX The card_busy implementation is a bit of a guess.
> > 
> > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> 
> If original patch was created by Shinobu/Ben Dooks,
> and you updated it, I prefer to add these guys on Signed-off-by line.
[...]

It's not correct to keep someone else's Signed-off-by when making
substantial changes.

Ben.

[RFC PATCH 7/7] ARM: shmobile: r8a7790-lager.dts: Assert UHS-I SDR-50 capability

[Ben Hutchings]

From: William Towle <william.towle-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>

[bwh: Also enable the 'regulators' on the pfc that we need]
Signed-off-by: Ben Hutchings <ben.hutchings-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
---
 arch/arm/boot/dts/r8a7790-lager.dts |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 343ec0ccc8df..8bb4507f3112 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -384,6 +384,14 @@
 		renesas,groups = "audio_clk_a";
 		renesas,function = "audio_clk";
 	};
+
+	sd-regulator@0 {
+		status = "ok";
+	};
+
+	sd-regulator@2 {
+		status = "ok";
+	};
 };
 
 &ether {
@@ -499,6 +507,7 @@
 	vmmc-supply = <&vcc_sdhi0>;
 	vqmmc-supply = <&vccq_sdhi0>;
 	cd-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
+	sd-uhs-sdr50;
 	status = "okay";
 };
 
@@ -512,6 +521,7 @@
 	vmmc-supply = <&vcc_sdhi2>;
 	vqmmc-supply = <&vccq_sdhi2>;
 	cd-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;
+	sd-uhs-sdr50;
 	status = "okay";
 };
 
-- 
1.7.10.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

Re: [RFC PATCH 7/7] ARM: shmobile: r8a7790-lager.dts: Assert UHS-I SDR-50 capability

[Sergei Shtylyov]

On 04/30/2015 03:33 PM, Ben Hutchings wrote:

> From: William Towle <william.towle@codethink.co.uk>

> [bwh: Also enable the 'regulators' on the pfc that we need]
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>

    Not signed off by William?

[...]

WBR, Sergei