[PATCH v2 0/6] UHS-I support for sh_mobile_sdhi

[PATCH v2 0/6] 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 now
works on the R8A7790 Lager board and another development board.

The pfc block needs to be reconfigured from 3.3V to 1.8V signalling on
the pins wired to the SD card.  This is supported by adding separate
functions for 1.8V signalling in sh-pfc ("sdhi0_1v8" etc.).  I expect
that several SH SoCs have this capability, but I only have the R8A7790
data sheet so I only implemented it for that one.

Changes since v1:
- Reword commit message for "mmc: tmio: Add UHS-I mode support"
- Make sh_mobile_sdhi_start_signal_voltage_switch() succeed if asked
  to switch to 3.3V and the regulator or pinctrl or pinctrl state is
  missing
- Drop change to mmcif clock on Lager
- Correct original author for sdhi clock changes on Lager

Changes since the RFC:
- Replace the 'regulator' devices for signal voltage switching with
  pinctrl functions and states
- Drop 'mmc: sh_mobile_sdhi: Add actual clock rate support' as it's
  redundant
- Use a switch statement in sh_mobile_sdhi_start_signal_voltage_switch()
- Fix subject prefix for the DT changes

Ben.

Ben Hutchings (5):
  mmc: tmio: Add UHS-I mode support
  pinctrl: sh-pfc: Add set_mux operation to struct sh_pfc_function
  pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V
    operation
  mmc: sh_mobile_sdhi: Add UHS-I mode support
  ARM: shmobile: lager: Enable UHS-I SDR-50

Ian Molton (1):
  ARM: shmobile: lager: Set clock rates for SDHI

 arch/arm/boot/dts/r8a7790-lager.dts  | 24 +++++++++++--
 drivers/mmc/host/sh_mobile_sdhi.c    | 60 +++++++++++++++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h          |  3 ++
 drivers/mmc/host/tmio_mmc_pio.c      | 31 ++++++++++++++++
 drivers/pinctrl/sh-pfc/core.c        |  2 +-
 drivers/pinctrl/sh-pfc/core.h        |  1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 70 +++++++++++++++++++++++++++++++++---
 drivers/pinctrl/sh-pfc/pinctrl.c     |  4 +++
 drivers/pinctrl/sh-pfc/sh_pfc.h      | 10 +++++-
 9 files changed, 197 insertions(+), 8 deletions(-)

-- 
2.1.4

[PATCH v2 1/6] 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.

The card_busy implementation is a bit of a guess, but works for me on
an R8A7790 chip.

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 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 4a597f5a53e2..a1a009879946 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -99,6 +99,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,
 };
 
-- 
2.1.4

[PATCH v2 2/6] pinctrl: sh-pfc: Add set_mux operation to struct sh_pfc_function

[Ben Hutchings]

In some cases a change of function requires changes in more than just
the GPSR/IPSR registers.  In particular, changing the SDHI between 3.3V
and 1.8V signalling also requires modifying IOCTRL6.

Add this optional operation so that such special cases can be handled
in each SoC's code.

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

diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c
index 072e7c62cab7..068b1cdeddde 100644
--- a/drivers/pinctrl/sh-pfc/pinctrl.c
+++ b/drivers/pinctrl/sh-pfc/pinctrl.c
@@ -317,6 +317,7 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
 {
 	struct sh_pfc_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 	struct sh_pfc *pfc = pmx->pfc;
+	const struct sh_pfc_function *func = &pfc->info->functions[selector];
 	const struct sh_pfc_pin_group *grp = &pfc->info->groups[group];
 	unsigned long flags;
 	unsigned int i;
@@ -340,6 +341,9 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
 			break;
 	}
 
+	if (func->set_mux)
+		func->set_mux(pfc, func, grp);
+
 done:
 	spin_unlock_irqrestore(&pfc->lock, flags);
 	return ret;
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index c7508d5f6886..0ce4f5c7add8 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -28,6 +28,8 @@ enum {
 #define SH_PFC_PIN_CFG_PULL_DOWN	(1 << 3)
 #define SH_PFC_PIN_CFG_NO_GPIO		(1 << 31)
 
+struct sh_pfc;
+
 struct sh_pfc_pin {
 	u16 pin;
 	u16 enum_id;
@@ -50,17 +52,23 @@ struct sh_pfc_pin_group {
 	unsigned int nr_pins;
 };
 
-#define SH_PFC_FUNCTION(n)				\
+#define SH_PFC_FUNCTION_SPECIAL(n, set_mux_fn)		\
 	{						\
 		.name = #n,				\
 		.groups = n##_groups,			\
 		.nr_groups = ARRAY_SIZE(n##_groups),	\
+		.set_mux = (set_mux_fn),		\
 	}
 
+#define SH_PFC_FUNCTION(n) SH_PFC_FUNCTION_SPECIAL(n, NULL)
+
 struct sh_pfc_function {
 	const char *name;
 	const char * const *groups;
 	unsigned int nr_groups;
+	void (*set_mux)(struct sh_pfc *pfc,
+			const struct sh_pfc_function *func,
+			const struct sh_pfc_pin_group *grp);
 };
 
 struct pinmux_func {
-- 
2.1.4

[PATCH v2 3/6] pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V operation

[Ben Hutchings]

All the SHDIs can operate with either 3.3V or 1.8V signals, depending
on negotiation with the card.

Add separate functions for the 1.8V mode, and implement the set_mux
operation on all SDHI functions to configure the voltage for each
group of pins.

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 drivers/pinctrl/sh-pfc/core.c        |  2 +-
 drivers/pinctrl/sh-pfc/core.h        |  1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 70 +++++++++++++++++++++++++++++++++---
 3 files changed, 68 insertions(+), 5 deletions(-)

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..baba3151687f 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
@@ -4472,6 +4472,8 @@ static const char * const sdhi0_groups[] = {
 	"sdhi0_wp",
 };
 
+#define sdhi0_1v8_groups sdhi0_groups
+
 static const char * const sdhi1_groups[] = {
 	"sdhi1_data1",
 	"sdhi1_data4",
@@ -4480,6 +4482,8 @@ static const char * const sdhi1_groups[] = {
 	"sdhi1_wp",
 };
 
+#define sdhi1_1v8_groups sdhi1_groups
+
 static const char * const sdhi2_groups[] = {
 	"sdhi2_data1",
 	"sdhi2_data4",
@@ -4488,6 +4492,8 @@ static const char * const sdhi2_groups[] = {
 	"sdhi2_wp",
 };
 
+#define sdhi2_1v8_groups sdhi2_groups
+
 static const char * const sdhi3_groups[] = {
 	"sdhi3_data1",
 	"sdhi3_data4",
@@ -4496,6 +4502,8 @@ static const char * const sdhi3_groups[] = {
 	"sdhi3_wp",
 };
 
+#define sdhi3_1v8_groups sdhi3_groups
+
 static const char * const ssi_groups[] = {
 	"ssi0_data",
 	"ssi0129_ctrl",
@@ -4595,6 +4603,56 @@ static const char * const vin3_groups[] = {
 	"vin3_clk",
 };
 
+static void sdhi_set_voltage(struct sh_pfc *pfc,
+			     const struct sh_pfc_function *func,
+			     const struct sh_pfc_pin_group *grp)
+{
+	void __iomem *mapped_reg;
+	u32 data, mask;
+	int index;
+
+	if (WARN_ON(strncmp(func->name, "sdhi", 4)))
+		return;
+	if (WARN_ON(strncmp(func->name, grp->name, 5)))
+		return;
+
+	/* Calculate mask in IOCTRL6 based on the group */
+	index = func->name[4] - '0';
+	if (WARN_ON(index < 0 || index > 3))
+		return;
+	if (!strcmp(grp->name + 5, "_data1"))
+		mask = 0x20;
+	else if (!strcmp(grp->name + 5, "_data4"))
+		mask = 0x3c;
+	else if (!strcmp(grp->name + 5, "_ctrl"))
+		mask = 0xc0;
+	else if (!strcmp(grp->name + 5, "_cd"))
+		mask = 0x02;
+	else if (!strcmp(grp->name + 5, "_wp"))
+		mask = 0x01;
+	else {
+		WARN_ON(1);
+		return;
+	}
+	mask <<= 8 * (3 - index);
+
+	/* Map IOCTRL6 */
+	mapped_reg = sh_pfc_phys_to_virt(pfc, 0xe606008c);
+
+	data = sh_pfc_read_raw_reg(mapped_reg, 32);
+
+	/* Set for 3.3V (high) or 1.8V (low) depending on the function name */
+	if (strcmp(func->name + 5, "_1v8"))
+		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);
+}
+
 static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(audio_clk),
 	SH_PFC_FUNCTION(avb),
@@ -4631,10 +4689,14 @@ static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(scifb0),
 	SH_PFC_FUNCTION(scifb1),
 	SH_PFC_FUNCTION(scifb2),
-	SH_PFC_FUNCTION(sdhi0),
-	SH_PFC_FUNCTION(sdhi1),
-	SH_PFC_FUNCTION(sdhi2),
-	SH_PFC_FUNCTION(sdhi3),
+	SH_PFC_FUNCTION_SPECIAL(sdhi0, sdhi_set_voltage),
+	SH_PFC_FUNCTION_SPECIAL(sdhi0_1v8, sdhi_set_voltage),
+	SH_PFC_FUNCTION_SPECIAL(sdhi1, sdhi_set_voltage),
+	SH_PFC_FUNCTION_SPECIAL(sdhi1_1v8, sdhi_set_voltage),
+	SH_PFC_FUNCTION_SPECIAL(sdhi2, sdhi_set_voltage),
+	SH_PFC_FUNCTION_SPECIAL(sdhi2_1v8, sdhi_set_voltage),
+	SH_PFC_FUNCTION_SPECIAL(sdhi3, sdhi_set_voltage),
+	SH_PFC_FUNCTION_SPECIAL(sdhi3_1v8, sdhi_set_voltage),
 	SH_PFC_FUNCTION(ssi),
 	SH_PFC_FUNCTION(tpu0),
 	SH_PFC_FUNCTION(usb0),
-- 
2.1.4

Re: [PATCH v2 3/6] pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V operation

[Laurent Pinchart]

Hi Ben,

(CC'ing Linus Walleij)

Thank you for the patch.

On Wednesday 10 June 2015 00:23:31 Ben Hutchings wrote:
> All the SHDIs can operate with either 3.3V or 1.8V signals, depending
> on negotiation with the card.
> 
> Add separate functions for the 1.8V mode, and implement the set_mux
> operation on all SDHI functions to configure the voltage for each
> group of pins.

I don't think duplicating all functions for 1.8V and 3.3V is the right way to 
go. Can't we use the pinconf API instead, and in particular the 
PIN_CONFIG_POWER_SOURCE configuration ?

> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> ---
>  drivers/pinctrl/sh-pfc/core.c        |  2 +-
>  drivers/pinctrl/sh-pfc/core.h        |  1 +
>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 70 ++++++++++++++++++++++++++++++---
>  3 files changed, 68 insertions(+), 5 deletions(-)
> 
> 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..baba3151687f
> 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -4472,6 +4472,8 @@ static const char * const sdhi0_groups[] = {
>  	"sdhi0_wp",
>  };
> 
> +#define sdhi0_1v8_groups sdhi0_groups
> +
>  static const char * const sdhi1_groups[] = {
>  	"sdhi1_data1",
>  	"sdhi1_data4",
> @@ -4480,6 +4482,8 @@ static const char * const sdhi1_groups[] = {
>  	"sdhi1_wp",
>  };
> 
> +#define sdhi1_1v8_groups sdhi1_groups
> +
>  static const char * const sdhi2_groups[] = {
>  	"sdhi2_data1",
>  	"sdhi2_data4",
> @@ -4488,6 +4492,8 @@ static const char * const sdhi2_groups[] = {
>  	"sdhi2_wp",
>  };
> 
> +#define sdhi2_1v8_groups sdhi2_groups
> +
>  static const char * const sdhi3_groups[] = {
>  	"sdhi3_data1",
>  	"sdhi3_data4",
> @@ -4496,6 +4502,8 @@ static const char * const sdhi3_groups[] = {
>  	"sdhi3_wp",
>  };
> 
> +#define sdhi3_1v8_groups sdhi3_groups
> +
>  static const char * const ssi_groups[] = {
>  	"ssi0_data",
>  	"ssi0129_ctrl",
> @@ -4595,6 +4603,56 @@ static const char * const vin3_groups[] = {
>  	"vin3_clk",
>  };
> 
> +static void sdhi_set_voltage(struct sh_pfc *pfc,
> +			     const struct sh_pfc_function *func,
> +			     const struct sh_pfc_pin_group *grp)
> +{
> +	void __iomem *mapped_reg;
> +	u32 data, mask;
> +	int index;
> +
> +	if (WARN_ON(strncmp(func->name, "sdhi", 4)))
> +		return;
> +	if (WARN_ON(strncmp(func->name, grp->name, 5)))
> +		return;
> +
> +	/* Calculate mask in IOCTRL6 based on the group */
> +	index = func->name[4] - '0';
> +	if (WARN_ON(index < 0 || index > 3))
> +		return;
> +	if (!strcmp(grp->name + 5, "_data1"))
> +		mask = 0x20;
> +	else if (!strcmp(grp->name + 5, "_data4"))
> +		mask = 0x3c;
> +	else if (!strcmp(grp->name + 5, "_ctrl"))
> +		mask = 0xc0;
> +	else if (!strcmp(grp->name + 5, "_cd"))
> +		mask = 0x02;
> +	else if (!strcmp(grp->name + 5, "_wp"))
> +		mask = 0x01;
> +	else {
> +		WARN_ON(1);
> +		return;
> +	}
> +	mask <<= 8 * (3 - index);
> +
> +	/* Map IOCTRL6 */
> +	mapped_reg = sh_pfc_phys_to_virt(pfc, 0xe606008c);
> +
> +	data = sh_pfc_read_raw_reg(mapped_reg, 32);
> +
> +	/* Set for 3.3V (high) or 1.8V (low) depending on the function name */
> +	if (strcmp(func->name + 5, "_1v8"))
> +		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);
> +}
> +
>  static const struct sh_pfc_function pinmux_functions[] = {
>  	SH_PFC_FUNCTION(audio_clk),
>  	SH_PFC_FUNCTION(avb),
> @@ -4631,10 +4689,14 @@ static const struct sh_pfc_function
> pinmux_functions[] = { SH_PFC_FUNCTION(scifb0),
>  	SH_PFC_FUNCTION(scifb1),
>  	SH_PFC_FUNCTION(scifb2),
> -	SH_PFC_FUNCTION(sdhi0),
> -	SH_PFC_FUNCTION(sdhi1),
> -	SH_PFC_FUNCTION(sdhi2),
> -	SH_PFC_FUNCTION(sdhi3),
> +	SH_PFC_FUNCTION_SPECIAL(sdhi0, sdhi_set_voltage),
> +	SH_PFC_FUNCTION_SPECIAL(sdhi0_1v8, sdhi_set_voltage),
> +	SH_PFC_FUNCTION_SPECIAL(sdhi1, sdhi_set_voltage),
> +	SH_PFC_FUNCTION_SPECIAL(sdhi1_1v8, sdhi_set_voltage),
> +	SH_PFC_FUNCTION_SPECIAL(sdhi2, sdhi_set_voltage),
> +	SH_PFC_FUNCTION_SPECIAL(sdhi2_1v8, sdhi_set_voltage),
> +	SH_PFC_FUNCTION_SPECIAL(sdhi3, sdhi_set_voltage),
> +	SH_PFC_FUNCTION_SPECIAL(sdhi3_1v8, sdhi_set_voltage),
>  	SH_PFC_FUNCTION(ssi),
>  	SH_PFC_FUNCTION(tpu0),
>  	SH_PFC_FUNCTION(usb0),

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/6] pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V operation

[Ben Hutchings]

On Fri, 2015-06-12 at 10:18 +0300, Laurent Pinchart wrote:
> Hi Ben,
> 
> (CC'ing Linus Walleij)
> 
> Thank you for the patch.
> 
> On Wednesday 10 June 2015 00:23:31 Ben Hutchings wrote:
> > All the SHDIs can operate with either 3.3V or 1.8V signals, depending
> > on negotiation with the card.
> > 
> > Add separate functions for the 1.8V mode, and implement the set_mux
> > operation on all SDHI functions to configure the voltage for each
> > group of pins.
> 
> I don't think duplicating all functions for 1.8V and 3.3V is the right way to 
> go. Can't we use the pinconf API instead, and in particular the 
> PIN_CONFIG_POWER_SOURCE configuration ?
[...]

So far as I could see, you can't use the pinconf API with pinmux groups.

Ben.

Re: [PATCH v2 3/6] pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V operation

[Laurent Pinchart]

Hi Ben,

On Friday 12 June 2015 14:23:55 Ben Hutchings wrote:
> On Fri, 2015-06-12 at 10:18 +0300, Laurent Pinchart wrote:
> > Hi Ben,
> > 
> > (CC'ing Linus Walleij)
> > 
> > Thank you for the patch.
> > 
> > On Wednesday 10 June 2015 00:23:31 Ben Hutchings wrote:
> > > All the SHDIs can operate with either 3.3V or 1.8V signals, depending
> > > on negotiation with the card.
> > > 
> > > Add separate functions for the 1.8V mode, and implement the set_mux
> > > operation on all SDHI functions to configure the voltage for each
> > > group of pins.
> > 
> > I don't think duplicating all functions for 1.8V and 3.3V is the right way
> > to go. Can't we use the pinconf API instead, and in particular the
> > PIN_CONFIG_POWER_SOURCE configuration ?
> 
> [...]
> 
> So far as I could see, you can't use the pinconf API with pinmux groups.

Can't you ? Both Documentation/devicetree/bindings/pinctrl/pinctrl-
bindings.txt and Documentation/devicetree/bindings/pinctrl/renesas,pfc-
pinctrl.txt document the usage of pinconf with groups (see for instance 
example 3 in the latter).

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/6] pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V operation

[Ben Hutchings]

On Fri, 2015-06-12 at 22:07 +0300, Laurent Pinchart wrote:
> Hi Ben,
> 
> On Friday 12 June 2015 14:23:55 Ben Hutchings wrote:
> > On Fri, 2015-06-12 at 10:18 +0300, Laurent Pinchart wrote:
> > > Hi Ben,
> > > 
> > > (CC'ing Linus Walleij)
> > > 
> > > Thank you for the patch.
> > > 
> > > On Wednesday 10 June 2015 00:23:31 Ben Hutchings wrote:
> > > > All the SHDIs can operate with either 3.3V or 1.8V signals, depending
> > > > on negotiation with the card.
> > > > 
> > > > Add separate functions for the 1.8V mode, and implement the set_mux
> > > > operation on all SDHI functions to configure the voltage for each
> > > > group of pins.
> > > 
> > > I don't think duplicating all functions for 1.8V and 3.3V is the right way
> > > to go. Can't we use the pinconf API instead, and in particular the
> > > PIN_CONFIG_POWER_SOURCE configuration ?
> > 
> > [...]
> > 
> > So far as I could see, you can't use the pinconf API with pinmux groups.
> 
> Can't you ? Both Documentation/devicetree/bindings/pinctrl/pinctrl-
> bindings.txt and Documentation/devicetree/bindings/pinctrl/renesas,pfc-
> pinctrl.txt document the usage of pinconf with groups (see for instance 
> example 3 in the latter).

Sorry, I should say you can't use them with pinmux *functions*.  You can
either bind a group to a function *or* set the various pinconf
properties for the group, but not both.  Unless I'm very much mistaken.

Ben.

Re: [PATCH v2 3/6] pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V operation

[Laurent Pinchart]

Hi Ben,

On Monday 15 June 2015 01:40:40 Ben Hutchings wrote:
> On Fri, 2015-06-12 at 22:07 +0300, Laurent Pinchart wrote:
> > On Friday 12 June 2015 14:23:55 Ben Hutchings wrote:
> >> On Fri, 2015-06-12 at 10:18 +0300, Laurent Pinchart wrote:
> >>> On Wednesday 10 June 2015 00:23:31 Ben Hutchings wrote:
> >>>> All the SHDIs can operate with either 3.3V or 1.8V signals,
> >>>> depending on negotiation with the card.
> >>>> 
> >>>> Add separate functions for the 1.8V mode, and implement the set_mux
> >>>> operation on all SDHI functions to configure the voltage for each
> >>>> group of pins.
> >>> 
> >>> I don't think duplicating all functions for 1.8V and 3.3V is the right
> >>> way to go. Can't we use the pinconf API instead, and in particular the
> >>> PIN_CONFIG_POWER_SOURCE configuration ?
> >> 
> >> [...]
> >> 
> >> So far as I could see, you can't use the pinconf API with pinmux groups.
> > 
> > Can't you ? Both Documentation/devicetree/bindings/pinctrl/pinctrl-
> > bindings.txt and Documentation/devicetree/bindings/pinctrl/renesas,pfc-
> > pinctrl.txt document the usage of pinconf with groups (see for instance
> > example 3 in the latter).
> 
> Sorry, I should say you can't use them with pinmux *functions*.  You can
> either bind a group to a function *or* set the various pinconf
> properties for the group, but not both.  Unless I'm very much mistaken.

One of us probably is :-)

I'd have to recheck whether this is really a standard behaviour, but the PFC 
driver at least supports setting both functions and configuration as far as I 
can tell. The relevant example from the PFC bindings is

        mmcif_pins: mmcif {
                mux {
                        renesas,groups = "mmc0_data8_0", "mmc0_ctrl_0";
                        renesas,function = "mmc0";
                };
                cfg {
                        renesas,groups = "mmc0_data8_0";
                        renesas,pins = "PORT279";
                        bias-pull-up;
                };
        };

It's a bit more verbose than I'd like, but I think the following should work 
too.

        mmcif_pins: mmcif {
                renesas,groups = "mmc0_data8_0";
                renesas,function = "mmc0";
                bias-pull-up;
        };

(it requires all pins from the groups to use the same configuration though)

I believe the former is the intended way to set both function and config, 
while the latter is a PFC "extension".

On a side note, I have a patch to support the standard "groups", "functions" 
and "pins" properties instead of the Renesas-specific versions, I'll try to 
revive it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/6] pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V operation

[Linus Walleij]

On Mon, Jun 15, 2015 at 4:02 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

> On a side note, I have a patch to support the standard "groups", "functions"
> and "pins" properties instead of the Renesas-specific versions, I'll try to
> revive it.

Yes please :)

Makes things a little less confusing in this confusing boundary between
electronics and code...

Yours,
Linus Walleij

Re: [PATCH v2 3/6] pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V operation

[Laurent Pinchart]

On Tuesday 30 June 2015 08:05:34 Linus Walleij wrote:
> On Mon, Jun 15, 2015 at 4:02 AM, Laurent Pinchart
> 
> <laurent.pinchart@ideasonboard.com> wrote:
> > On a side note, I have a patch to support the standard "groups",
> > "functions" and "pins" properties instead of the Renesas-specific
> > versions, I'll try to revive it.
> 
> Yes please :)

Done :-)

> Makes things a little less confusing in this confusing boundary between
> electronics and code...

-- 
Regards,

Laurent Pinchart

[PATCH v2 4/6] 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.

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

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 354f4f335ed5..2e65b855a7f7 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -30,6 +30,9 @@
 #include <linux/mfd/tmio.h>
 #include <linux/sh_dma.h>
 #include <linux/delay.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/pinctrl-state.h>
+#include <linux/regulator/consumer.h>
 
 #include "tmio_mmc.h"
 
@@ -86,6 +89,8 @@ struct sh_mobile_sdhi {
 	struct clk *clk;
 	struct tmio_mmc_data mmc_data;
 	struct tmio_mmc_dma dma_priv;
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pinctrl_3v3, *pinctrl_1v8;
 };
 
 static void sh_mobile_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
@@ -136,6 +141,49 @@ static void sh_mobile_sdhi_clk_disable(struct platform_device *pdev)
 	clk_disable_unprepare(priv->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);
+	struct pinctrl_state *state;
+	int min_uV, max_uV;
+	int ret;
+
+	switch (signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_330:
+		min_uV = 2700000;
+		max_uV = 3600000;
+		state = priv->pinctrl_3v3;
+		break;
+	case MMC_SIGNAL_VOLTAGE_180:
+		min_uV = 1700000;
+		max_uV = 1950000;
+		state = priv->pinctrl_1v8;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/*
+	 * If anything is missing, assume signal voltage is fixed at
+	 * 3.3V and succeed/fail accordingly.
+	 */
+	if (IS_ERR(host->mmc->supply.vqmmc) ||
+	    IS_ERR(priv->pinctrl) || IS_ERR(state))
+		return signal_voltage == MMC_SIGNAL_VOLTAGE_330 ? 0 : -EINVAL;
+
+	ret = regulator_set_voltage(host->mmc->supply.vqmmc, min_uV, max_uV);
+	if (ret)
+		return ret;
+
+	ret = pinctrl_select_state(priv->pinctrl, state);
+	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;
@@ -228,6 +276,14 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eprobe;
 	}
 
+	priv->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (!IS_ERR(priv->pinctrl)) {
+		priv->pinctrl_3v3 = pinctrl_lookup_state(priv->pinctrl,
+							 PINCTRL_STATE_DEFAULT);
+		priv->pinctrl_1v8 = pinctrl_lookup_state(priv->pinctrl,
+							 "1v8");
+	}
+
 	host = tmio_mmc_host_alloc(pdev);
 	if (!host) {
 		ret = -ENOMEM;
@@ -239,6 +295,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;
-- 
2.1.4

[PATCH v2 5/6] ARM: shmobile: lager: Set clock rates for SDHI

[Ben Hutchings]

From: Ian Molton <ian.molton@codethink.co.uk>

Set the input clocks to the highest supported speeds.

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

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index aaa4f258e279..5f68e53c58ae 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -488,6 +488,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 +501,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>;
-- 
2.1.4

Re: [PATCH v2 5/6] ARM: shmobile: lager: Set clock rates for SDHI

[Kuninori Morimoto]

Hi Ben

Sorry for my small amount of question, but I would like to ask 1 thing

> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index aaa4f258e279..5f68e53c58ae 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -488,6 +488,9 @@
>  	pinctrl-0 = <&sdhi0_pins>;
>  	pinctrl-names = "default";
>  
> +	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
> +	assigned-clock-rates = <156000000>;

Can we use "max-frequency" here ?
tmio/sh_mobile_sdhi have tmio_mmc_clk_update/sh_mobile_sdhi_clk_enable

Re: [PATCH v2 5/6] ARM: shmobile: lager: Set clock rates for SDHI

[Ben Hutchings]

On Thu, 2015-06-11 at 01:20 +0000, Kuninori Morimoto wrote:
> Hi Ben
> 
> Sorry for my small amount of question, but I would like to ask 1 thing
> 
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > index aaa4f258e279..5f68e53c58ae 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -488,6 +488,9 @@
> >  	pinctrl-0 = <&sdhi0_pins>;
> >  	pinctrl-names = "default";
> >  
> > +	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
> > +	assigned-clock-rates = <156000000>;
> 
> Can we use "max-frequency" here ?
> tmio/sh_mobile_sdhi have tmio_mmc_clk_update/sh_mobile_sdhi_clk_enable

The purpose of these properties is to configure the frequency of the
clock supplied to the SDHI block.  sh_mobile_sdhi has no code to do that
but these properties are handled by the clock core.

The max-frequency property tells the MMC core the maximum clock
frequency that the host can run the SD bus at, but anyway tmio_mmc and
sh_mobile_sdhi seem to always override it.

Ben.

[PATCH v2 6/6] ARM: shmobile: lager: Enable UHS-I SDR-50

[Ben Hutchings]

Add the "1v8" pinctrl state and sd-uhs-sdr50 property to SDHI{0,2}.

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

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 5f68e53c58ae..ff5eea8ea765 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -314,11 +314,21 @@
 		renesas,function = "sdhi0";
 	};
 
+	sdhi0_pins_1v8: sd0_1v8 {
+		renesas,groups = "sdhi0_data4", "sdhi0_ctrl";
+		renesas,function = "sdhi0_1v8";
+	};
+
 	sdhi2_pins: sd2 {
 		renesas,groups = "sdhi2_data4", "sdhi2_ctrl";
 		renesas,function = "sdhi2";
 	};
 
+	sdhi2_pins_1v8: sd2_1v8 {
+		renesas,groups = "sdhi2_data4", "sdhi2_ctrl";
+		renesas,function = "sdhi2_1v8";
+	};
+
 	mmc1_pins: mmc1 {
 		renesas,groups = "mmc1_data8", "mmc1_ctrl";
 		renesas,function = "mmc1";
@@ -486,7 +496,8 @@
 
 &sdhi0 {
 	pinctrl-0 = <&sdhi0_pins>;
-	pinctrl-names = "default";
+	pinctrl-1 = <&sdhi0_pins_1v8>;
+	pinctrl-names = "default", "1v8";
 
 	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
 	assigned-clock-rates = <156000000>;
@@ -494,12 +505,14 @@
 	vmmc-supply = <&vcc_sdhi0>;
 	vqmmc-supply = <&vccq_sdhi0>;
 	cd-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
+	sd-uhs-sdr50;
 	status = "okay";
 };
 
 &sdhi2 {
 	pinctrl-0 = <&sdhi2_pins>;
-	pinctrl-names = "default";
+	pinctrl-1 = <&sdhi2_pins_1v8>;
+	pinctrl-names = "default", "1v8";
 
 	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
 	assigned-clock-rates = <97500000>;
@@ -507,6 +520,7 @@
 	vmmc-supply = <&vcc_sdhi2>;
 	vqmmc-supply = <&vccq_sdhi2>;
 	cd-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;
+	sd-uhs-sdr50;
 	status = "okay";
 };
 
-- 
2.1.4

Re: [PATCH v2 0/6] UHS-I support for sh_mobile_sdhi

[Ulf Hansson]

On 10 June 2015 at 01:21, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> 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 now
> works on the R8A7790 Lager board and another development board.
>
> The pfc block needs to be reconfigured from 3.3V to 1.8V signalling on
> the pins wired to the SD card.  This is supported by adding separate
> functions for 1.8V signalling in sh-pfc ("sdhi0_1v8" etc.).  I expect
> that several SH SoCs have this capability, but I only have the R8A7790
> data sheet so I only implemented it for that one.
>
> Changes since v1:
> - Reword commit message for "mmc: tmio: Add UHS-I mode support"
> - Make sh_mobile_sdhi_start_signal_voltage_switch() succeed if asked
>   to switch to 3.3V and the regulator or pinctrl or pinctrl state is
>   missing
> - Drop change to mmcif clock on Lager
> - Correct original author for sdhi clock changes on Lager
>
> Changes since the RFC:
> - Replace the 'regulator' devices for signal voltage switching with
>   pinctrl functions and states
> - Drop 'mmc: sh_mobile_sdhi: Add actual clock rate support' as it's
>   redundant
> - Use a switch statement in sh_mobile_sdhi_start_signal_voltage_switch()
> - Fix subject prefix for the DT changes
>
> Ben.
>
> Ben Hutchings (5):
>   mmc: tmio: Add UHS-I mode support
>   pinctrl: sh-pfc: Add set_mux operation to struct sh_pfc_function
>   pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V
>     operation
>   mmc: sh_mobile_sdhi: Add UHS-I mode support
>   ARM: shmobile: lager: Enable UHS-I SDR-50
>
> Ian Molton (1):
>   ARM: shmobile: lager: Set clock rates for SDHI
>
>  arch/arm/boot/dts/r8a7790-lager.dts  | 24 +++++++++++--
>  drivers/mmc/host/sh_mobile_sdhi.c    | 60 +++++++++++++++++++++++++++++++
>  drivers/mmc/host/tmio_mmc.h          |  3 ++
>  drivers/mmc/host/tmio_mmc_pio.c      | 31 ++++++++++++++++
>  drivers/pinctrl/sh-pfc/core.c        |  2 +-
>  drivers/pinctrl/sh-pfc/core.h        |  1 +
>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 70 +++++++++++++++++++++++++++++++++---
>  drivers/pinctrl/sh-pfc/pinctrl.c     |  4 +++
>  drivers/pinctrl/sh-pfc/sh_pfc.h      | 10 +++++-
>  9 files changed, 197 insertions(+), 8 deletions(-)
>
> --
> 2.1.4
>

Hi Ben,

I have looked at the mmc patches, those looks good to me. Regarding
the pinctrl and ARM patches, I suppose these can be taken through
their respective trees and I can take the mmc patches?

BTW, I noticed that the pinctrl maintainer wasn't in the to-field, you
may need to repost.

Kind regards
Uffe

Re: [PATCH v2 0/6] UHS-I support for sh_mobile_sdhi

[Ben Hutchings]

On Wed, 2015-06-10 at 11:16 +0200, Ulf Hansson wrote:
> On 10 June 2015 at 01:21, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> > 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 now
> > works on the R8A7790 Lager board and another development board.
> >
> > The pfc block needs to be reconfigured from 3.3V to 1.8V signalling on
> > the pins wired to the SD card.  This is supported by adding separate
> > functions for 1.8V signalling in sh-pfc ("sdhi0_1v8" etc.).  I expect
> > that several SH SoCs have this capability, but I only have the R8A7790
> > data sheet so I only implemented it for that one.
> >
> > Changes since v1:
> > - Reword commit message for "mmc: tmio: Add UHS-I mode support"
> > - Make sh_mobile_sdhi_start_signal_voltage_switch() succeed if asked
> >   to switch to 3.3V and the regulator or pinctrl or pinctrl state is
> >   missing
> > - Drop change to mmcif clock on Lager
> > - Correct original author for sdhi clock changes on Lager
> >
> > Changes since the RFC:
> > - Replace the 'regulator' devices for signal voltage switching with
> >   pinctrl functions and states
> > - Drop 'mmc: sh_mobile_sdhi: Add actual clock rate support' as it's
> >   redundant
> > - Use a switch statement in sh_mobile_sdhi_start_signal_voltage_switch()
> > - Fix subject prefix for the DT changes
> >
> > Ben.
> >
> > Ben Hutchings (5):
> >   mmc: tmio: Add UHS-I mode support
> >   pinctrl: sh-pfc: Add set_mux operation to struct sh_pfc_function
> >   pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V
> >     operation
> >   mmc: sh_mobile_sdhi: Add UHS-I mode support
> >   ARM: shmobile: lager: Enable UHS-I SDR-50
> >
> > Ian Molton (1):
> >   ARM: shmobile: lager: Set clock rates for SDHI
> >
> >  arch/arm/boot/dts/r8a7790-lager.dts  | 24 +++++++++++--
> >  drivers/mmc/host/sh_mobile_sdhi.c    | 60 +++++++++++++++++++++++++++++++
> >  drivers/mmc/host/tmio_mmc.h          |  3 ++
> >  drivers/mmc/host/tmio_mmc_pio.c      | 31 ++++++++++++++++
> >  drivers/pinctrl/sh-pfc/core.c        |  2 +-
> >  drivers/pinctrl/sh-pfc/core.h        |  1 +
> >  drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 70 +++++++++++++++++++++++++++++++++---
> >  drivers/pinctrl/sh-pfc/pinctrl.c     |  4 +++
> >  drivers/pinctrl/sh-pfc/sh_pfc.h      | 10 +++++-
> >  9 files changed, 197 insertions(+), 8 deletions(-)
> >
> > --
> > 2.1.4
> >
> 
> Hi Ben,
> 
> I have looked at the mmc patches, those looks good to me. Regarding
> the pinctrl and ARM patches, I suppose these can be taken through
> their respective trees and I can take the mmc patches?

The problem with that is that I think the device tree change will cause
a regression if it's applied without the driver changes.  I would much
prefer if I could get the pinctrl and device tree changes acked by the
respective maintainers to go through the MMC tree.  (I should probably
have said that up front.)

> BTW, I noticed that the pinctrl maintainer wasn't in the to-field, you
> may need to repost.

I have the relevant _list_ (linux-gpio) in cc and thought that would be
sufficient, but maybe not, so cc'ing this message.

Ben.

Re: [PATCH v2 0/6] UHS-I support for sh_mobile_sdhi

[Simon Horman]

On Thu, Jun 11, 2015 at 12:57:57AM +0100, Ben Hutchings wrote:
> On Wed, 2015-06-10 at 11:16 +0200, Ulf Hansson wrote:
> > On 10 June 2015 at 01:21, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> > > 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 now
> > > works on the R8A7790 Lager board and another development board.
> > >
> > > The pfc block needs to be reconfigured from 3.3V to 1.8V signalling on
> > > the pins wired to the SD card.  This is supported by adding separate
> > > functions for 1.8V signalling in sh-pfc ("sdhi0_1v8" etc.).  I expect
> > > that several SH SoCs have this capability, but I only have the R8A7790
> > > data sheet so I only implemented it for that one.
> > >
> > > Changes since v1:
> > > - Reword commit message for "mmc: tmio: Add UHS-I mode support"
> > > - Make sh_mobile_sdhi_start_signal_voltage_switch() succeed if asked
> > >   to switch to 3.3V and the regulator or pinctrl or pinctrl state is
> > >   missing
> > > - Drop change to mmcif clock on Lager
> > > - Correct original author for sdhi clock changes on Lager
> > >
> > > Changes since the RFC:
> > > - Replace the 'regulator' devices for signal voltage switching with
> > >   pinctrl functions and states
> > > - Drop 'mmc: sh_mobile_sdhi: Add actual clock rate support' as it's
> > >   redundant
> > > - Use a switch statement in sh_mobile_sdhi_start_signal_voltage_switch()
> > > - Fix subject prefix for the DT changes
> > >
> > > Ben.
> > >
> > > Ben Hutchings (5):
> > >   mmc: tmio: Add UHS-I mode support
> > >   pinctrl: sh-pfc: Add set_mux operation to struct sh_pfc_function
> > >   pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V
> > >     operation
> > >   mmc: sh_mobile_sdhi: Add UHS-I mode support
> > >   ARM: shmobile: lager: Enable UHS-I SDR-50
> > >
> > > Ian Molton (1):
> > >   ARM: shmobile: lager: Set clock rates for SDHI
> > >
> > >  arch/arm/boot/dts/r8a7790-lager.dts  | 24 +++++++++++--
> > >  drivers/mmc/host/sh_mobile_sdhi.c    | 60 +++++++++++++++++++++++++++++++
> > >  drivers/mmc/host/tmio_mmc.h          |  3 ++
> > >  drivers/mmc/host/tmio_mmc_pio.c      | 31 ++++++++++++++++
> > >  drivers/pinctrl/sh-pfc/core.c        |  2 +-
> > >  drivers/pinctrl/sh-pfc/core.h        |  1 +
> > >  drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 70 +++++++++++++++++++++++++++++++++---
> > >  drivers/pinctrl/sh-pfc/pinctrl.c     |  4 +++
> > >  drivers/pinctrl/sh-pfc/sh_pfc.h      | 10 +++++-
> > >  9 files changed, 197 insertions(+), 8 deletions(-)
> > >
> > > --
> > > 2.1.4
> > >
> > 
> > Hi Ben,
> > 
> > I have looked at the mmc patches, those looks good to me. Regarding
> > the pinctrl and ARM patches, I suppose these can be taken through
> > their respective trees and I can take the mmc patches?
> 
> The problem with that is that I think the device tree change will cause
> a regression if it's applied without the driver changes.  I would much
> prefer if I could get the pinctrl and device tree changes acked by the
> respective maintainers to go through the MMC tree.  (I should probably
> have said that up front.)

Hi Ben,

I may be misunderstanding the above, if so I apologise, but I would
strongly prefer to avoid an arrangement where the kernel and device tree
blob (DTS/DTSI -> DTB) need to be upgrade in lock-step as this tends not to
lead to a good experience for users.  My preference would be to maintain
backwards and forwards compatibility and if appropriate schedule removal of
such compatibility.

> > BTW, I noticed that the pinctrl maintainer wasn't in the to-field, you
> > may need to repost.
> 
> I have the relevant _list_ (linux-gpio) in cc and thought that would be
> sufficient, but maybe not, so cc'ing this message.

Thanks. I have also CCed Laurent who has done extensive work on sh-pfc.

Re: [PATCH v2 0/6] UHS-I support for sh_mobile_sdhi

[Ben Hutchings]

On Thu, 2015-06-11 at 11:49 +0900, Simon Horman wrote:
> On Thu, Jun 11, 2015 at 12:57:57AM +0100, Ben Hutchings wrote:
> > On Wed, 2015-06-10 at 11:16 +0200, Ulf Hansson wrote:
> > > On 10 June 2015 at 01:21, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> > > > 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 now
> > > > works on the R8A7790 Lager board and another development board.
> > > >
> > > > The pfc block needs to be reconfigured from 3.3V to 1.8V signalling on
> > > > the pins wired to the SD card.  This is supported by adding separate
> > > > functions for 1.8V signalling in sh-pfc ("sdhi0_1v8" etc.).  I expect
> > > > that several SH SoCs have this capability, but I only have the R8A7790
> > > > data sheet so I only implemented it for that one.
> > > >
> > > > Changes since v1:
> > > > - Reword commit message for "mmc: tmio: Add UHS-I mode support"
> > > > - Make sh_mobile_sdhi_start_signal_voltage_switch() succeed if asked
> > > >   to switch to 3.3V and the regulator or pinctrl or pinctrl state is
> > > >   missing
> > > > - Drop change to mmcif clock on Lager
> > > > - Correct original author for sdhi clock changes on Lager
> > > >
> > > > Changes since the RFC:
> > > > - Replace the 'regulator' devices for signal voltage switching with
> > > >   pinctrl functions and states
> > > > - Drop 'mmc: sh_mobile_sdhi: Add actual clock rate support' as it's
> > > >   redundant
> > > > - Use a switch statement in sh_mobile_sdhi_start_signal_voltage_switch()
> > > > - Fix subject prefix for the DT changes
> > > >
> > > > Ben.
> > > >
> > > > Ben Hutchings (5):
> > > >   mmc: tmio: Add UHS-I mode support
> > > >   pinctrl: sh-pfc: Add set_mux operation to struct sh_pfc_function
> > > >   pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V
> > > >     operation
> > > >   mmc: sh_mobile_sdhi: Add UHS-I mode support
> > > >   ARM: shmobile: lager: Enable UHS-I SDR-50
> > > >
> > > > Ian Molton (1):
> > > >   ARM: shmobile: lager: Set clock rates for SDHI
> > > >
> > > >  arch/arm/boot/dts/r8a7790-lager.dts  | 24 +++++++++++--
> > > >  drivers/mmc/host/sh_mobile_sdhi.c    | 60 +++++++++++++++++++++++++++++++
> > > >  drivers/mmc/host/tmio_mmc.h          |  3 ++
> > > >  drivers/mmc/host/tmio_mmc_pio.c      | 31 ++++++++++++++++
> > > >  drivers/pinctrl/sh-pfc/core.c        |  2 +-
> > > >  drivers/pinctrl/sh-pfc/core.h        |  1 +
> > > >  drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 70 +++++++++++++++++++++++++++++++++---
> > > >  drivers/pinctrl/sh-pfc/pinctrl.c     |  4 +++
> > > >  drivers/pinctrl/sh-pfc/sh_pfc.h      | 10 +++++-
> > > >  9 files changed, 197 insertions(+), 8 deletions(-)
> > > >
> > > > --
> > > > 2.1.4
> > > >
> > > 
> > > Hi Ben,
> > > 
> > > I have looked at the mmc patches, those looks good to me. Regarding
> > > the pinctrl and ARM patches, I suppose these can be taken through
> > > their respective trees and I can take the mmc patches?
> > 
> > The problem with that is that I think the device tree change will cause
> > a regression if it's applied without the driver changes.  I would much
> > prefer if I could get the pinctrl and device tree changes acked by the
> > respective maintainers to go through the MMC tree.  (I should probably
> > have said that up front.)
> 
> Hi Ben,
> 
> I may be misunderstanding the above, if so I apologise, but I would
> strongly prefer to avoid an arrangement where the kernel and device tree
> blob (DTS/DTSI -> DTB) need to be upgrade in lock-step as this tends not to
> lead to a good experience for users.

I agree.

> My preference would be to maintain
> backwards and forwards compatibility and if appropriate schedule removal of
> such compatibility.
[...]

The problem is that the 'sd-uhs-sdr50' property is interpreted by the
MMC core, so I think it will start trying to use this mode even if the
driver hasn't implemented the necessary operations.  I don't see any way
around that.

The regression is relatively minor as I think the MMC core will fall
back to a lower speed after failing to enable SDR50.  But it will slow
down probing of a card.

Ben.

Re: [PATCH v2 0/6] UHS-I support for sh_mobile_sdhi

[Geert Uytterhoeven]

On Thu, Jun 11, 2015 at 5:02 PM, Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
>> I may be misunderstanding the above, if so I apologise, but I would
>> strongly prefer to avoid an arrangement where the kernel and device tree
>> blob (DTS/DTSI -> DTB) need to be upgrade in lock-step as this tends not to
>> lead to a good experience for users.
>
> I agree.
>
>> My preference would be to maintain
>> backwards and forwards compatibility and if appropriate schedule removal of
>> such compatibility.
> [...]
>
> The problem is that the 'sd-uhs-sdr50' property is interpreted by the
> MMC core, so I think it will start trying to use this mode even if the
> driver hasn't implemented the necessary operations.  I don't see any way
> around that.

Can't the core check for driver capabilities?
I'm not familiar with the MMC core, but e.g. the SPI core only tries modes
that match both SPI master (spi_master.mode_bits) and slave (spi_device.mode).

> The regression is relatively minor as I think the MMC core will fall
> back to a lower speed after failing to enable SDR50.  But it will slow
> down probing of a card.

So it does fall back (after a while). That's good.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 0/6] UHS-I support for sh_mobile_sdhi

[Ben Hutchings]

On Sun, 2015-06-14 at 09:36 +0200, Geert Uytterhoeven wrote:
> On Thu, Jun 11, 2015 at 5:02 PM, Ben Hutchings
> <ben.hutchings@codethink.co.uk> wrote:
> >> I may be misunderstanding the above, if so I apologise, but I would
> >> strongly prefer to avoid an arrangement where the kernel and device tree
> >> blob (DTS/DTSI -> DTB) need to be upgrade in lock-step as this tends not to
> >> lead to a good experience for users.
> >
> > I agree.
> >
> >> My preference would be to maintain
> >> backwards and forwards compatibility and if appropriate schedule removal of
> >> such compatibility.
> > [...]
> >
> > The problem is that the 'sd-uhs-sdr50' property is interpreted by the
> > MMC core, so I think it will start trying to use this mode even if the
> > driver hasn't implemented the necessary operations.  I don't see any way
> > around that.
> 
> Can't the core check for driver capabilities?
> I'm not familiar with the MMC core, but e.g. the SPI core only tries modes
> that match both SPI master (spi_master.mode_bits) and slave (spi_device.mode).

The driver already calls mmc_of_parse(), which initialises its
capabilities based only on the device tree properties.

> > The regression is relatively minor as I think the MMC core will fall
> > back to a lower speed after failing to enable SDR50.  But it will slow
> > down probing of a card.
> 
> So it does fall back (after a while). That's good.

I think I've seen this happen but haven't *specifically* tested yet.

Ben.

Re: [PATCH v2 0/6] UHS-I support for sh_mobile_sdhi

[Ben Hutchings]

On Sun, 2015-06-14 at 09:36 +0200, Geert Uytterhoeven wrote:
> On Thu, Jun 11, 2015 at 5:02 PM, Ben Hutchings
> <ben.hutchings@codethink.co.uk> wrote:
> >> I may be misunderstanding the above, if so I apologise, but I would
> >> strongly prefer to avoid an arrangement where the kernel and device tree
> >> blob (DTS/DTSI -> DTB) need to be upgrade in lock-step as this tends not to
> >> lead to a good experience for users.
> >
> > I agree.
> >
> >> My preference would be to maintain
> >> backwards and forwards compatibility and if appropriate schedule removal of
> >> such compatibility.
> > [...]
> >
> > The problem is that the 'sd-uhs-sdr50' property is interpreted by the
> > MMC core, so I think it will start trying to use this mode even if the
> > driver hasn't implemented the necessary operations.  I don't see any way
> > around that.
> 
> Can't the core check for driver capabilities?
> I'm not familiar with the MMC core, but e.g. the SPI core only tries modes
> that match both SPI master (spi_master.mode_bits) and slave (spi_device.mode).

The driver already calls mmc_of_parse(), which initialises its
capabilities based only on the device tree properties.

> > The regression is relatively minor as I think the MMC core will fall
> > back to a lower speed after failing to enable SDR50.  But it will slow
> > down probing of a card.
> 
> So it does fall back (after a while). That's good.

I think I've seen this happen but haven't *specifically* tested yet.

Ben.