[PATCH v4 0/8] UHS-I support for sh_mobile_sdhi

[PATCH v4 0/8] 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 implementing the
pinconf power-source parameter.  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 v3:
- Change pinconf power-source argument to millivolts instead of boolean
  for low voltage
- Rename pinctrl voltage operations accordingly
- Use io{read,write}32() instead of sh_pfc_{read,write}_raw_reg()
- Change tmio, sh_mobile_sdhi to adjust input clock frequency to match
  requested bus frequency more closely
- Replace SDHI assigned-clock{,-rate}s properties with max-frequency
- Reorder patch series to put all pinctrl changes first

Changes since v2:
- Implement the pinconf power-source parameter instead of separate
  functions for the low-voltage mode
- Adjust low-voltage mode setter function and device tree accordingly
- Add low-voltage mode getter function for R8A7790

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 (8):
  pinctrl: sh-pfc: Implement pinconf power-source param for voltage
    switching
  pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI
  mmc: tmio, sh_mobile_sdhi: Pass tmio_mmc_host ptr to
    clk_{enable,disable} ops
  mmc: tmio, sh_mobile_sdhi: Add support for variable input clock
    frequency
  mmc: tmio: Add UHS-I mode support
  mmc: sh_mobile_sdhi: Add UHS-I mode support
  ARM: shmobile: r8a7790: Set maximum frequencies for SDHI clocks
  ARM: shmobile: lager: Enable UHS-I SDR-50

 .../bindings/pinctrl/renesas,pfc-pinctrl.txt       |   4 +-
 arch/arm/boot/dts/r8a7790-lager.dts                |  22 +++-
 arch/arm/boot/dts/r8a7790.dtsi                     |   4 +
 drivers/mmc/host/sh_mobile_sdhi.c                  | 126 +++++++++++++++++++--
 drivers/mmc/host/tmio_mmc.h                        |   9 +-
 drivers/mmc/host/tmio_mmc_pio.c                    |  56 ++++++---
 drivers/pinctrl/sh-pfc/core.c                      |   2 +-
 drivers/pinctrl/sh-pfc/core.h                      |   1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c               |  71 +++++++++++-
 drivers/pinctrl/sh-pfc/pinctrl.c                   |  44 ++++++-
 drivers/pinctrl/sh-pfc/sh_pfc.h                    |   5 +
 11 files changed, 313 insertions(+), 31 deletions(-)

-- 
2.1.4

[PATCH v4 1/8] pinctrl: sh-pfc: Implement pinconf power-source param for voltage switching

[Ben Hutchings]

The pfc in the R8A7790 (and probably others in the R-Car gen 2 family)
supports switching SDHI signals between 3.3V and 1.8V nominal voltage,
and the SD driver should do that when switching to and from UHS modes.

Add a flag for pins that have configurable I/O voltage and SoC
operations to get and set the nominal voltage.  Implement the pinconf
power-source parameter using these operations.

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 .../bindings/pinctrl/renesas,pfc-pinctrl.txt       |  4 +-
 drivers/pinctrl/sh-pfc/pinctrl.c                   | 44 +++++++++++++++++++++-
 drivers/pinctrl/sh-pfc/sh_pfc.h                    |  5 +++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,pfc-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,pfc-pinctrl.txt
index bfe72ec055e3..57487481ce8c 100644
--- a/Documentation/devicetree/bindings/pinctrl/renesas,pfc-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,pfc-pinctrl.txt
@@ -69,7 +69,9 @@ Pin Configuration Node Properties:
 
 The pin configuration parameters use the generic pinconf bindings defined in
 pinctrl-bindings.txt in this directory. The supported parameters are
-bias-disable, bias-pull-up and bias-pull-down.
+bias-disable, bias-pull-up, bias-pull-down and power-source. For pins that
+have a configurable I/O voltage, the power-source value should be the
+nominal I/O voltage in millivolts.
 

 GPIO
diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c
index 072e7c62cab7..72ae7d0d8b75 100644
--- a/drivers/pinctrl/sh-pfc/pinctrl.c
+++ b/drivers/pinctrl/sh-pfc/pinctrl.c
@@ -465,6 +465,9 @@ static bool sh_pfc_pinconf_validate(struct sh_pfc *pfc, unsigned int _pin,
 	case PIN_CONFIG_BIAS_PULL_DOWN:
 		return pin->configs & SH_PFC_PIN_CFG_PULL_DOWN;
 
+	case PIN_CONFIG_POWER_SOURCE:
+		return pin->configs & SH_PFC_PIN_CFG_IO_VOLTAGE;
+
 	default:
 		return false;
 	}
@@ -477,7 +480,6 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin,
 	struct sh_pfc *pfc = pmx->pfc;
 	enum pin_config_param param = pinconf_to_config_param(*config);
 	unsigned long flags;
-	unsigned int bias;
 
 	if (!sh_pfc_pinconf_validate(pfc, _pin, param))
 		return -ENOTSUPP;
@@ -485,7 +487,9 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin,
 	switch (param) {
 	case PIN_CONFIG_BIAS_DISABLE:
 	case PIN_CONFIG_BIAS_PULL_UP:
-	case PIN_CONFIG_BIAS_PULL_DOWN:
+	case PIN_CONFIG_BIAS_PULL_DOWN: {
+		unsigned int bias;
+
 		if (!pfc->info->ops || !pfc->info->ops->get_bias)
 			return -ENOTSUPP;
 
@@ -498,6 +502,24 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin,
 
 		*config = 0;
 		break;
+	}
+
+	case PIN_CONFIG_POWER_SOURCE: {
+		int ret;
+
+		if (!pfc->info->ops || !pfc->info->ops->get_io_voltage)
+			return -ENOTSUPP;
+
+		spin_lock_irqsave(&pfc->lock, flags);
+		ret = pfc->info->ops->get_io_voltage(pfc, _pin);
+		spin_unlock_irqrestore(&pfc->lock, flags);
+
+		if (ret < 0)
+			return ret;
+
+		*config = ret;
+		break;
+	}
 
 	default:
 		return -ENOTSUPP;
@@ -534,6 +556,24 @@ static int sh_pfc_pinconf_set(struct pinctrl_dev *pctldev, unsigned _pin,
 
 			break;
 
+		case PIN_CONFIG_POWER_SOURCE: {
+			unsigned int arg =
+				pinconf_to_config_argument(configs[i]);
+			int ret;
+
+			if (!pfc->info->ops || !pfc->info->ops->set_io_voltage)
+				return -ENOTSUPP;
+
+			spin_lock_irqsave(&pfc->lock, flags);
+			ret = pfc->info->ops->set_io_voltage(pfc, _pin, arg);
+			spin_unlock_irqrestore(&pfc->lock, flags);
+
+			if (ret)
+				return ret;
+
+			break;
+		}
+
 		default:
 			return -ENOTSUPP;
 		}
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index c7508d5f6886..734f7a92229c 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -12,6 +12,7 @@
 #define __SH_PFC_H
 
 #include <linux/bug.h>
+#include <linux/pinctrl/pinconf-generic.h>
 #include <linux/stringify.h>
 
 enum {
@@ -26,6 +27,7 @@ enum {
 #define SH_PFC_PIN_CFG_OUTPUT		(1 << 1)
 #define SH_PFC_PIN_CFG_PULL_UP		(1 << 2)
 #define SH_PFC_PIN_CFG_PULL_DOWN	(1 << 3)
+#define SH_PFC_PIN_CFG_IO_VOLTAGE	(1 << 4)
 #define SH_PFC_PIN_CFG_NO_GPIO		(1 << 31)
 
 struct sh_pfc_pin {
@@ -121,6 +123,9 @@ struct sh_pfc_soc_operations {
 	unsigned int (*get_bias)(struct sh_pfc *pfc, unsigned int pin);
 	void (*set_bias)(struct sh_pfc *pfc, unsigned int pin,
 			 unsigned int bias);
+	int (*get_io_voltage)(struct sh_pfc *pfc, unsigned int pin);
+	int (*set_io_voltage)(struct sh_pfc *pfc, unsigned int pin,
+			      u16 voltage_mV);
 };
 
 struct sh_pfc_soc_info {
-- 
2.1.4

Re: [PATCH v4 1/8] pinctrl: sh-pfc: Implement pinconf power-source param for voltage switching

[Laurent Pinchart]

Hi Ben,

Thank you for the patch.

On Tuesday 30 June 2015 17:53:59 Ben Hutchings wrote:
> The pfc in the R8A7790 (and probably others in the R-Car gen 2 family)
> supports switching SDHI signals between 3.3V and 1.8V nominal voltage,
> and the SD driver should do that when switching to and from UHS modes.
> 
> Add a flag for pins that have configurable I/O voltage and SoC
> operations to get and set the nominal voltage.  Implement the pinconf
> power-source parameter using these operations.
> 
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  .../bindings/pinctrl/renesas,pfc-pinctrl.txt       |  4 +-
>  drivers/pinctrl/sh-pfc/pinctrl.c                   | 44 ++++++++++++++++++-
>  drivers/pinctrl/sh-pfc/sh_pfc.h                    |  5 +++
>  3 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/pinctrl/renesas,pfc-pinctrl.txt
> b/Documentation/devicetree/bindings/pinctrl/renesas,pfc-pinctrl.txt index
> bfe72ec055e3..57487481ce8c 100644
> --- a/Documentation/devicetree/bindings/pinctrl/renesas,pfc-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,pfc-pinctrl.txt
> @@ -69,7 +69,9 @@ Pin Configuration Node Properties:
> 
>  The pin configuration parameters use the generic pinconf bindings defined
> in pinctrl-bindings.txt in this directory. The supported parameters are
> -bias-disable, bias-pull-up and bias-pull-down.
> +bias-disable, bias-pull-up, bias-pull-down and power-source. For pins that
> +have a configurable I/O voltage, the power-source value should be the
> +nominal I/O voltage in millivolts.
> 
> 
>  GPIO
> diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c
> b/drivers/pinctrl/sh-pfc/pinctrl.c index 072e7c62cab7..72ae7d0d8b75 100644
> --- a/drivers/pinctrl/sh-pfc/pinctrl.c
> +++ b/drivers/pinctrl/sh-pfc/pinctrl.c
> @@ -465,6 +465,9 @@ static bool sh_pfc_pinconf_validate(struct sh_pfc *pfc,
> unsigned int _pin, case PIN_CONFIG_BIAS_PULL_DOWN:
>  		return pin->configs & SH_PFC_PIN_CFG_PULL_DOWN;
> 
> +	case PIN_CONFIG_POWER_SOURCE:
> +		return pin->configs & SH_PFC_PIN_CFG_IO_VOLTAGE;
> +
>  	default:
>  		return false;
>  	}
> @@ -477,7 +480,6 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev
> *pctldev, unsigned _pin, struct sh_pfc *pfc = pmx->pfc;
>  	enum pin_config_param param = pinconf_to_config_param(*config);
>  	unsigned long flags;
> -	unsigned int bias;
> 
>  	if (!sh_pfc_pinconf_validate(pfc, _pin, param))
>  		return -ENOTSUPP;
> @@ -485,7 +487,9 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev
> *pctldev, unsigned _pin, switch (param) {
>  	case PIN_CONFIG_BIAS_DISABLE:
>  	case PIN_CONFIG_BIAS_PULL_UP:
> -	case PIN_CONFIG_BIAS_PULL_DOWN:
> +	case PIN_CONFIG_BIAS_PULL_DOWN: {
> +		unsigned int bias;
> +
>  		if (!pfc->info->ops || !pfc->info->ops->get_bias)
>  			return -ENOTSUPP;
> 
> @@ -498,6 +502,24 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev
> *pctldev, unsigned _pin,
> 
>  		*config = 0;
>  		break;
> +	}
> +
> +	case PIN_CONFIG_POWER_SOURCE: {
> +		int ret;
> +
> +		if (!pfc->info->ops || !pfc->info->ops->get_io_voltage)
> +			return -ENOTSUPP;
> +
> +		spin_lock_irqsave(&pfc->lock, flags);
> +		ret = pfc->info->ops->get_io_voltage(pfc, _pin);
> +		spin_unlock_irqrestore(&pfc->lock, flags);
> +
> +		if (ret < 0)
> +			return ret;
> +
> +		*config = ret;
> +		break;
> +	}
> 
>  	default:
>  		return -ENOTSUPP;
> @@ -534,6 +556,24 @@ static int sh_pfc_pinconf_set(struct pinctrl_dev
> *pctldev, unsigned _pin,
> 
>  			break;
> 
> +		case PIN_CONFIG_POWER_SOURCE: {
> +			unsigned int arg =
> +				pinconf_to_config_argument(configs[i]);
> +			int ret;
> +
> +			if (!pfc->info->ops || !pfc->info->ops->set_io_voltage)
> +				return -ENOTSUPP;
> +
> +			spin_lock_irqsave(&pfc->lock, flags);
> +			ret = pfc->info->ops->set_io_voltage(pfc, _pin, arg);
> +			spin_unlock_irqrestore(&pfc->lock, flags);
> +
> +			if (ret)
> +				return ret;
> +
> +			break;
> +		}
> +
>  		default:
>  			return -ENOTSUPP;
>  		}
> diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h
> b/drivers/pinctrl/sh-pfc/sh_pfc.h index c7508d5f6886..734f7a92229c 100644
> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> @@ -12,6 +12,7 @@
>  #define __SH_PFC_H
> 
>  #include <linux/bug.h>
> +#include <linux/pinctrl/pinconf-generic.h>
>  #include <linux/stringify.h>
> 
>  enum {
> @@ -26,6 +27,7 @@ enum {
>  #define SH_PFC_PIN_CFG_OUTPUT		(1 << 1)
>  #define SH_PFC_PIN_CFG_PULL_UP		(1 << 2)
>  #define SH_PFC_PIN_CFG_PULL_DOWN	(1 << 3)
> +#define SH_PFC_PIN_CFG_IO_VOLTAGE	(1 << 4)
>  #define SH_PFC_PIN_CFG_NO_GPIO		(1 << 31)
> 
>  struct sh_pfc_pin {
> @@ -121,6 +123,9 @@ struct sh_pfc_soc_operations {
>  	unsigned int (*get_bias)(struct sh_pfc *pfc, unsigned int pin);
>  	void (*set_bias)(struct sh_pfc *pfc, unsigned int pin,
>  			 unsigned int bias);
> +	int (*get_io_voltage)(struct sh_pfc *pfc, unsigned int pin);
> +	int (*set_io_voltage)(struct sh_pfc *pfc, unsigned int pin,
> +			      u16 voltage_mV);
>  };
> 
>  struct sh_pfc_soc_info {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 1/8] pinctrl: sh-pfc: Implement pinconf power-source param for voltage switching

[Linus Walleij]

On Tue, Jun 30, 2015 at 6:53 PM, Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:

> The pfc in the R8A7790 (and probably others in the R-Car gen 2 family)
> supports switching SDHI signals between 3.3V and 1.8V nominal voltage,
> and the SD driver should do that when switching to and from UHS modes.
>
> Add a flag for pins that have configurable I/O voltage and SoC
> operations to get and set the nominal voltage.  Implement the pinconf
> power-source parameter using these operations.
>
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>

Patch applied with Laurent's ACK.

Yours,
Linus Walleij

[PATCH v4 2/8] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI

[Ben Hutchings]

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

Implement the {get,set}_io_voltage operations and set the related
capability flag for the associated 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 | 71 +++++++++++++++++++++++++++++++++++-
 3 files changed, 72 insertions(+), 2 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..ec6657de6a46 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
@@ -21,6 +21,7 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/platform_data/gpio-rcar.h>
 
@@ -1739,10 +1740,20 @@ static const u16 pinmux_data[] = {
 #define PIN_NUMBER(r, c) (((r) - 'A') * 31 + (c) + 200)
 #define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
 
+#define PIN_IO_VOLTAGE(bank, _pin, _name, sfx)		\
+	[RCAR_GP_PIN(bank, _pin)].configs = SH_PFC_PIN_CFG_IO_VOLTAGE
+
 static const struct sh_pfc_pin pinmux_pins[] = {
 	PINMUX_GPIO_GP_ALL(),
 
-	/* Pins not associated with a GPIO port */
+	/*
+	 * All pins assigned to GPIO bank 3 can be used for SD interfaces
+	 * in which case they support both 3.3V and 1.8V signalling.
+	 */
+	PORT_GP_32(3, PIN_IO_VOLTAGE, unused),
+
+	/* Pins not associated with a GPIO port, placed after all the GPIOs */
+	[RCAR_GP_PIN(5, 31) + 1] =
 	SH_PFC_PIN_NAMED(ROW_GROUP_A('F'), 15, AF15),
 	SH_PFC_PIN_NAMED(ROW_GROUP_A('G'), 15, AG15),
 	SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
@@ -4595,6 +4606,58 @@ static const char * const vin3_groups[] = {
 	"vin3_clk",
 };
 
+static int sdhi_get_io_voltage(struct sh_pfc *pfc, unsigned int pin)
+{
+	void __iomem *mapped_reg;
+	u32 data, mask;
+
+	if (WARN(pin < RCAR_GP_PIN(3, 0) || pin > RCAR_GP_PIN(3, 31),
+		 "sdhi_get_low_voltage: invalid pin %#x", pin))
+		return -EIO;
+
+	/* Map IOCTRL6 */
+	mapped_reg = sh_pfc_phys_to_virt(pfc, 0xe606008c);
+
+	/* Bits in IOCTRL6 are numbered in opposite order to pins */
+	mask = 0x80000000 >> (pin & 0x1f);
+
+	data = ioread32(mapped_reg);
+
+	return (data & mask) ? 3300 : 1800;
+}
+
+static int
+sdhi_set_io_voltage(struct sh_pfc *pfc, unsigned int pin, u16 voltage_mV)
+{
+	void __iomem *mapped_reg;
+	u32 data, mask;
+
+	if (WARN(pin < RCAR_GP_PIN(3, 0) || pin > RCAR_GP_PIN(3, 31),
+		 "invalid pin %#x", pin))
+		return -EIO;
+
+	if (voltage_mV != 1800 && voltage_mV != 3300)
+		return -EINVAL;
+
+	/* Map IOCTRL6 */
+	mapped_reg = sh_pfc_phys_to_virt(pfc, 0xe606008c);
+
+	/* Bits in IOCTRL6 are numbered in opposite order to pins */
+	mask = 0x80000000 >> (pin & 0x1f);
+
+	data = ioread32(mapped_reg);
+
+	if (voltage_mV == 3300)
+		data |= mask;
+	else
+		data &= ~mask;
+
+	iowrite32(~data, sh_pfc_phys_to_virt(pfc, pfc->info->unlock_reg));
+	iowrite32(data, mapped_reg);
+
+	return 0;
+}
+
 static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(audio_clk),
 	SH_PFC_FUNCTION(avb),
@@ -5586,8 +5649,14 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 	{ },
 };
 
+static const struct sh_pfc_soc_operations pinmux_ops = {
+	.get_io_voltage = sdhi_get_io_voltage,
+	.set_io_voltage = sdhi_set_io_voltage,
+};
+
 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 },
-- 
2.1.4

Re: [PATCH v4 2/8] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI

[Laurent Pinchart]

Hi Ben,

Thank you for the patch.

On Tuesday 30 June 2015 17:54:09 Ben Hutchings wrote:
> All the SHDIs can operate with either 3.3V or 1.8V signals, depending
> on negotiation with the card.
> 
> Implement the {get,set}_io_voltage operations and set the related
> capability flag for the associated 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 | 71 ++++++++++++++++++++++++++++++++-
>  3 files changed, 72 insertions(+), 2 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..ec6657de6a46
> 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -21,6 +21,7 @@
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
> USA */
> 
> +#include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/platform_data/gpio-rcar.h>
> 
> @@ -1739,10 +1740,20 @@ static const u16 pinmux_data[] = {
>  #define PIN_NUMBER(r, c) (((r) - 'A') * 31 + (c) + 200)
>  #define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> 
> +#define PIN_IO_VOLTAGE(bank, _pin, _name, sfx)		\
> +	[RCAR_GP_PIN(bank, _pin)].configs = SH_PFC_PIN_CFG_IO_VOLTAGE
> +
>  static const struct sh_pfc_pin pinmux_pins[] = {
>  	PINMUX_GPIO_GP_ALL(),
> 
> -	/* Pins not associated with a GPIO port */
> +	/*
> +	 * All pins assigned to GPIO bank 3 can be used for SD interfaces
> +	 * in which case they support both 3.3V and 1.8V signalling.
> +	 */
> +	PORT_GP_32(3, PIN_IO_VOLTAGE, unused),
> +
> +	/* Pins not associated with a GPIO port, placed after all the GPIOs */
> +	[RCAR_GP_PIN(5, 31) + 1] =

I'm sorry but I still don't like this. gcc outputs a warning when overriding 
an initializer if you enable -Wextra, which leads me to believe this construct 
is dubious. It should at least be tested with LLVM.

I'd much prefer replacing PINMUX_GPIO_GP_ALL() with a version that will 
initialize the pins correctly right away.

Another option which might be better would be to add a new operation to 
retrieve the pin config bitmask instead of storing it in sh_pfc_pin. Most SoCs 
don't have per-pin config bitmasks, so we could save a bit of memory by doing 
so. r8a73a4 and r8a7790 could easily compute the bitmask in C code, while 
sh73a0 and r8a7740 would have a private table. The drawback would be a 
slightly increased execution time.

>  	SH_PFC_PIN_NAMED(ROW_GROUP_A('F'), 15, AF15),
>  	SH_PFC_PIN_NAMED(ROW_GROUP_A('G'), 15, AG15),
>  	SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
> @@ -4595,6 +4606,58 @@ static const char * const vin3_groups[] = {
>  	"vin3_clk",
>  };
> 
> +static int sdhi_get_io_voltage(struct sh_pfc *pfc, unsigned int pin)
> +{
> +	void __iomem *mapped_reg;
> +	u32 data, mask;
> +
> +	if (WARN(pin < RCAR_GP_PIN(3, 0) || pin > RCAR_GP_PIN(3, 31),
> +		 "sdhi_get_low_voltage: invalid pin %#x", pin))

The function name is wrong. You could use __func__, but as WARN() will output 
a backtrace I think you can just drop it. And given that this shouldn't happen 
at all given that only port 3 pins have the IO_VOLTAGE flag set, I'd just drop 
the warning completely. Same for shdi_set_io_voltage().

> +		return -EIO;
> +
> +	/* Map IOCTRL6 */
> +	mapped_reg = sh_pfc_phys_to_virt(pfc, 0xe606008c);
> +
> +	/* Bits in IOCTRL6 are numbered in opposite order to pins */
> +	mask = 0x80000000 >> (pin & 0x1f);
> +
> +	data = ioread32(mapped_reg);
> +
> +	return (data & mask) ? 3300 : 1800;
> +}
> +
> +static int
> +sdhi_set_io_voltage(struct sh_pfc *pfc, unsigned int pin, u16 voltage_mV)
> +{
> +	void __iomem *mapped_reg;
> +	u32 data, mask;
> +
> +	if (WARN(pin < RCAR_GP_PIN(3, 0) || pin > RCAR_GP_PIN(3, 31),
> +		 "invalid pin %#x", pin))
> +		return -EIO;
> +
> +	if (voltage_mV != 1800 && voltage_mV != 3300)
> +		return -EINVAL;
> +
> +	/* Map IOCTRL6 */
> +	mapped_reg = sh_pfc_phys_to_virt(pfc, 0xe606008c);
> +
> +	/* Bits in IOCTRL6 are numbered in opposite order to pins */
> +	mask = 0x80000000 >> (pin & 0x1f);
> +
> +	data = ioread32(mapped_reg);
> +
> +	if (voltage_mV == 3300)
> +		data |= mask;
> +	else
> +		data &= ~mask;
> +
> +	iowrite32(~data, sh_pfc_phys_to_virt(pfc, pfc->info->unlock_reg));
> +	iowrite32(data, mapped_reg);
> +
> +	return 0;
> +}
> +
>  static const struct sh_pfc_function pinmux_functions[] = {
>  	SH_PFC_FUNCTION(audio_clk),
>  	SH_PFC_FUNCTION(avb),
> @@ -5586,8 +5649,14 @@ static const struct pinmux_cfg_reg
> pinmux_config_regs[] = { { },
>  };
> 
> +static const struct sh_pfc_soc_operations pinmux_ops = {
> +	.get_io_voltage = sdhi_get_io_voltage,
> +	.set_io_voltage = sdhi_set_io_voltage,
> +};
> +
>  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 },

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/8] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI

[Ben Hutchings]

On Wed, 2015-07-01 at 10:37 +0300, Laurent Pinchart wrote:
[...]
> > +#define PIN_IO_VOLTAGE(bank, _pin, _name, sfx)		\
> > +	[RCAR_GP_PIN(bank, _pin)].configs = SH_PFC_PIN_CFG_IO_VOLTAGE
> > +
> >  static const struct sh_pfc_pin pinmux_pins[] = {
> >  	PINMUX_GPIO_GP_ALL(),
> > 
> > -	/* Pins not associated with a GPIO port */
> > +	/*
> > +	 * All pins assigned to GPIO bank 3 can be used for SD interfaces
> > +	 * in which case they support both 3.3V and 1.8V signalling.
> > +	 */
> > +	PORT_GP_32(3, PIN_IO_VOLTAGE, unused),
> > +
> > +	/* Pins not associated with a GPIO port, placed after all the GPIOs */
> > +	[RCAR_GP_PIN(5, 31) + 1] =
> 
> I'm sorry but I still don't like this. gcc outputs a warning when overriding 
> an initializer if you enable -Wextra, which leads me to believe this construct 
> is dubious. It should at least be tested with LLVM.
> 
> I'd much prefer replacing PINMUX_GPIO_GP_ALL() with a version that will 
> initialize the pins correctly right away.
[...]

Something like this?

diff --git a/drivers/pinctrl/sh-pfc/pfc-emev2.c b/drivers/pinctrl/sh-pfc/pfc-emev2.c
index 849c6943ed30..ad1a8281a91b 100644
--- a/drivers/pinctrl/sh-pfc/pfc-emev2.c
+++ b/drivers/pinctrl/sh-pfc/pfc-emev2.c
@@ -228,7 +228,7 @@ enum {
 
 /* Expand to a list of sh_pfc_pin entries (named PORT#).
  * NOTE: No config are recorded since the driver do not handle pinconf. */
-#define __PIN_CFG(pn, pfx, sfx)  SH_PFC_PIN_CFG(pfx, 0)
+#define __PIN_CFG(pn, pfx, sfx)  SH_PFC_PORT_CFG(pfx, 0)
 #define PINMUX_EMEV_GPIO_ALL()	  CPU_ALL_PORT(__PIN_CFG, , unused)
 
 static const struct sh_pfc_pin pinmux_pins[] = {
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c b/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
index 280a56f97786..ca1538371563 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
@@ -1272,8 +1272,8 @@ static const u16 pinmux_data[] = {
 #define __IO	(SH_PFC_PIN_CFG_INPUT | SH_PFC_PIN_CFG_OUTPUT)
 #define __PUD	(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
 
-#define R8A73A4_PIN_IO_PU_PD(pin)       SH_PFC_PIN_CFG(pin, __IO | __PUD)
-#define R8A73A4_PIN_O(pin)              SH_PFC_PIN_CFG(pin, __O)
+#define R8A73A4_PIN_IO_PU_PD(pin)       SH_PFC_PORT_CFG(pin, __IO | __PUD)
+#define R8A73A4_PIN_O(pin)              SH_PFC_PORT_CFG(pin, __O)
 
 static const struct sh_pfc_pin pinmux_pins[] = {
 	R8A73A4_PIN_IO_PU_PD(0), R8A73A4_PIN_IO_PU_PD(1),
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7740.c b/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
index b486e9d20cc2..92939cbd7ad0 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
@@ -1535,15 +1535,15 @@ static const u16 pinmux_data[] = {
 #define __PU		(SH_PFC_PIN_CFG_PULL_UP)
 #define __PUD		(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
 
-#define R8A7740_PIN_I_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PD)
-#define R8A7740_PIN_I_PU(pin)		SH_PFC_PIN_CFG(pin, __I | __PU)
-#define R8A7740_PIN_I_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __I | __PUD)
-#define R8A7740_PIN_IO(pin)		SH_PFC_PIN_CFG(pin, __IO)
-#define R8A7740_PIN_IO_PD(pin)		SH_PFC_PIN_CFG(pin, __IO | __PD)
-#define R8A7740_PIN_IO_PU(pin)		SH_PFC_PIN_CFG(pin, __IO | __PU)
-#define R8A7740_PIN_IO_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __IO | __PUD)
-#define R8A7740_PIN_O(pin)		SH_PFC_PIN_CFG(pin, __O)
-#define R8A7740_PIN_O_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __O | __PUD)
+#define R8A7740_PIN_I_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PD)
+#define R8A7740_PIN_I_PU(pin)		SH_PFC_PORT_CFG(pin, __I | __PU)
+#define R8A7740_PIN_I_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __I | __PUD)
+#define R8A7740_PIN_IO(pin)		SH_PFC_PORT_CFG(pin, __IO)
+#define R8A7740_PIN_IO_PD(pin)		SH_PFC_PORT_CFG(pin, __IO | __PD)
+#define R8A7740_PIN_IO_PU(pin)		SH_PFC_PORT_CFG(pin, __IO | __PU)
+#define R8A7740_PIN_IO_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __IO | __PUD)
+#define R8A7740_PIN_O(pin)		SH_PFC_PORT_CFG(pin, __O)
+#define R8A7740_PIN_O_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __O | __PUD)
 
 static const struct sh_pfc_pin pinmux_pins[] = {
 	/* Table 56-1 (I/O and Pull U/D) */
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
index 1137bbc810cd..1e59e1775374 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
@@ -1740,20 +1740,23 @@ static const u16 pinmux_data[] = {
 #define PIN_NUMBER(r, c) (((r) - 'A') * 31 + (c) + 200)
 #define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
 
-#define PIN_IO_VOLTAGE(bank, _pin, _name, sfx)		\
-	[RCAR_GP_PIN(bank, _pin)].configs = SH_PFC_PIN_CFG_IO_VOLTAGE
+/*
+ * All pins assigned to GPIO bank 3 can be used for SD interfaces in
+ * which case they support both 3.3V and 1.8V signalling.
+ */
+#define GP_GPIO_IO_VOLTAGE(bank, _pin, _name, sfx)			\
+	[RCAR_GP_PIN(bank, _pin)] =					\
+		SH_PFC_GPIO_CFG(bank, _pin, _name, SH_PFC_PIN_CFG_IO_VOLTAGE)
 
 static const struct sh_pfc_pin pinmux_pins[] = {
-	PINMUX_GPIO_GP_ALL(),
-
-	/*
-	 * All pins assigned to GPIO bank 3 can be used for SD interfaces
-	 * in which case they support both 3.3V and 1.8V signalling.
-	 */
-	PORT_GP_32(3, PIN_IO_VOLTAGE, unused),
+	PORT_GP_32(0, _GP_GPIO, unused),
+	PORT_GP_32(1, _GP_GPIO, unused),
+	PORT_GP_32(2, _GP_GPIO, unused),
+	PORT_GP_32(3, GP_GPIO_IO_VOLTAGE, unused),
+	PORT_GP_32(4, _GP_GPIO, unused),
+	PORT_GP_32(5, _GP_GPIO, unused),
 
 	/* Pins not associated with a GPIO port, placed after all the GPIOs */
-	[RCAR_GP_PIN(5, 31) + 1] =
 	SH_PFC_PIN_NAMED(ROW_GROUP_A('F'), 15, AF15),
 	SH_PFC_PIN_NAMED(ROW_GROUP_A('G'), 15, AG15),
 	SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
diff --git a/drivers/pinctrl/sh-pfc/pfc-sh73a0.c b/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
index d2efbfb776ac..2c798550cd8c 100644
--- a/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
+++ b/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
@@ -1166,14 +1166,14 @@ static const u16 pinmux_data[] = {
 #define __PU		(SH_PFC_PIN_CFG_PULL_UP)
 #define __PUD		(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
 
-#define SH73A0_PIN_I_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PD)
-#define SH73A0_PIN_I_PU(pin)		SH_PFC_PIN_CFG(pin, __I | __PU)
-#define SH73A0_PIN_I_PU_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PUD)
-#define SH73A0_PIN_IO(pin)		SH_PFC_PIN_CFG(pin, __IO)
-#define SH73A0_PIN_IO_PD(pin)		SH_PFC_PIN_CFG(pin, __IO | __PD)
-#define SH73A0_PIN_IO_PU(pin)		SH_PFC_PIN_CFG(pin, __IO | __PU)
-#define SH73A0_PIN_IO_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __IO | __PUD)
-#define SH73A0_PIN_O(pin)		SH_PFC_PIN_CFG(pin, __O)
+#define SH73A0_PIN_I_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PD)
+#define SH73A0_PIN_I_PU(pin)		SH_PFC_PORT_CFG(pin, __I | __PU)
+#define SH73A0_PIN_I_PU_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PUD)
+#define SH73A0_PIN_IO(pin)		SH_PFC_PORT_CFG(pin, __IO)
+#define SH73A0_PIN_IO_PD(pin)		SH_PFC_PORT_CFG(pin, __IO | __PD)
+#define SH73A0_PIN_IO_PU(pin)		SH_PFC_PORT_CFG(pin, __IO | __PU)
+#define SH73A0_PIN_IO_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __IO | __PUD)
+#define SH73A0_PIN_O(pin)		SH_PFC_PORT_CFG(pin, __O)
 
 /* Pin numbers for pins without a corresponding GPIO port number are computed
  * from the row and column numbers with a 1000 offset to avoid collisions with
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index 734f7a92229c..3eca740bba02 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -272,14 +272,18 @@ struct sh_pfc_soc_info {
 		.enum_id = _pin##_DATA,					\
 	}
 
-/* SH_PFC_PIN_CFG - Expand to a sh_pfc_pin entry (named PORT#) with config */
-#define SH_PFC_PIN_CFG(_pin, cfgs)					\
+/* SH_PFC_{PORT,GPIO}_CFG - Expand to a sh_pfc_pin entry with config */
+#define _SH_PFC_PIN_CFG(_pin, _name, cfgs)				\
 	{								\
 		.pin = _pin,						\
-		.name = __stringify(PORT##_pin),			\
-		.enum_id = PORT##_pin##_DATA,				\
+		.name = __stringify(_name),				\
+		.enum_id = _name##_DATA,				\
 		.configs = cfgs,					\
 	}
+#define SH_PFC_PORT_CFG(_pin, cfgs)				\
+	_SH_PFC_PIN_CFG(PORT##_pin, PORT##_pin, cfgs)
+#define SH_PFC_GPIO_CFG(bank, _pin, _name, cfgs)		\
+	_SH_PFC_PIN_CFG((bank * 32) + _pin, _name, cfgs)
 
 /* SH_PFC_PIN_NAMED - Expand to a sh_pfc_pin entry with the given name */
 #define SH_PFC_PIN_NAMED(row, col, _name)				\
--- END ---

If you're happy with that, I'll re-send the series (hopefully for the
last time!) with the r8a7790 changes squashed into "pinctrl: sh-pfc:
r8a7790: Implement voltage switching for SDHI" and the SH_PFC_PIN_CFG
macro change as a new patch before it.

Ben.

Re: [PATCH v4 2/8] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI

[Ben Hutchings]

On Fri, 2015-07-03 at 00:21 +0100, Ben Hutchings wrote:
> On Wed, 2015-07-01 at 10:37 +0300, Laurent Pinchart wrote:
> [...]
> > > +#define PIN_IO_VOLTAGE(bank, _pin, _name, sfx)		\
> > > +	[RCAR_GP_PIN(bank, _pin)].configs = SH_PFC_PIN_CFG_IO_VOLTAGE
> > > +
> > >  static const struct sh_pfc_pin pinmux_pins[] = {
> > >  	PINMUX_GPIO_GP_ALL(),
> > > 
> > > -	/* Pins not associated with a GPIO port */
> > > +	/*
> > > +	 * All pins assigned to GPIO bank 3 can be used for SD interfaces
> > > +	 * in which case they support both 3.3V and 1.8V signalling.
> > > +	 */
> > > +	PORT_GP_32(3, PIN_IO_VOLTAGE, unused),
> > > +
> > > +	/* Pins not associated with a GPIO port, placed after all the GPIOs */
> > > +	[RCAR_GP_PIN(5, 31) + 1] =
> > 
> > I'm sorry but I still don't like this. gcc outputs a warning when overriding 
> > an initializer if you enable -Wextra, which leads me to believe this construct 
> > is dubious. It should at least be tested with LLVM.
> > 
> > I'd much prefer replacing PINMUX_GPIO_GP_ALL() with a version that will 
> > initialize the pins correctly right away.
> [...]
> 
> Something like this?

Laurent, please can you answer whether this is the right way to go.

Ben.

> diff --git a/drivers/pinctrl/sh-pfc/pfc-emev2.c b/drivers/pinctrl/sh-pfc/pfc-emev2.c
> index 849c6943ed30..ad1a8281a91b 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-emev2.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-emev2.c
> @@ -228,7 +228,7 @@ enum {
>  
>  /* Expand to a list of sh_pfc_pin entries (named PORT#).
>   * NOTE: No config are recorded since the driver do not handle pinconf. */
> -#define __PIN_CFG(pn, pfx, sfx)  SH_PFC_PIN_CFG(pfx, 0)
> +#define __PIN_CFG(pn, pfx, sfx)  SH_PFC_PORT_CFG(pfx, 0)
>  #define PINMUX_EMEV_GPIO_ALL()	  CPU_ALL_PORT(__PIN_CFG, , unused)
>  
>  static const struct sh_pfc_pin pinmux_pins[] = {
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c b/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
> index 280a56f97786..ca1538371563 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
> @@ -1272,8 +1272,8 @@ static const u16 pinmux_data[] = {
>  #define __IO	(SH_PFC_PIN_CFG_INPUT | SH_PFC_PIN_CFG_OUTPUT)
>  #define __PUD	(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
>  
> -#define R8A73A4_PIN_IO_PU_PD(pin)       SH_PFC_PIN_CFG(pin, __IO | __PUD)
> -#define R8A73A4_PIN_O(pin)              SH_PFC_PIN_CFG(pin, __O)
> +#define R8A73A4_PIN_IO_PU_PD(pin)       SH_PFC_PORT_CFG(pin, __IO | __PUD)
> +#define R8A73A4_PIN_O(pin)              SH_PFC_PORT_CFG(pin, __O)
>  
>  static const struct sh_pfc_pin pinmux_pins[] = {
>  	R8A73A4_PIN_IO_PU_PD(0), R8A73A4_PIN_IO_PU_PD(1),
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7740.c b/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
> index b486e9d20cc2..92939cbd7ad0 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
> @@ -1535,15 +1535,15 @@ static const u16 pinmux_data[] = {
>  #define __PU		(SH_PFC_PIN_CFG_PULL_UP)
>  #define __PUD		(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
>  
> -#define R8A7740_PIN_I_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PD)
> -#define R8A7740_PIN_I_PU(pin)		SH_PFC_PIN_CFG(pin, __I | __PU)
> -#define R8A7740_PIN_I_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __I | __PUD)
> -#define R8A7740_PIN_IO(pin)		SH_PFC_PIN_CFG(pin, __IO)
> -#define R8A7740_PIN_IO_PD(pin)		SH_PFC_PIN_CFG(pin, __IO | __PD)
> -#define R8A7740_PIN_IO_PU(pin)		SH_PFC_PIN_CFG(pin, __IO | __PU)
> -#define R8A7740_PIN_IO_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __IO | __PUD)
> -#define R8A7740_PIN_O(pin)		SH_PFC_PIN_CFG(pin, __O)
> -#define R8A7740_PIN_O_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __O | __PUD)
> +#define R8A7740_PIN_I_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PD)
> +#define R8A7740_PIN_I_PU(pin)		SH_PFC_PORT_CFG(pin, __I | __PU)
> +#define R8A7740_PIN_I_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __I | __PUD)
> +#define R8A7740_PIN_IO(pin)		SH_PFC_PORT_CFG(pin, __IO)
> +#define R8A7740_PIN_IO_PD(pin)		SH_PFC_PORT_CFG(pin, __IO | __PD)
> +#define R8A7740_PIN_IO_PU(pin)		SH_PFC_PORT_CFG(pin, __IO | __PU)
> +#define R8A7740_PIN_IO_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __IO | __PUD)
> +#define R8A7740_PIN_O(pin)		SH_PFC_PORT_CFG(pin, __O)
> +#define R8A7740_PIN_O_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __O | __PUD)
>  
>  static const struct sh_pfc_pin pinmux_pins[] = {
>  	/* Table 56-1 (I/O and Pull U/D) */
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> index 1137bbc810cd..1e59e1775374 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -1740,20 +1740,23 @@ static const u16 pinmux_data[] = {
>  #define PIN_NUMBER(r, c) (((r) - 'A') * 31 + (c) + 200)
>  #define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
>  
> -#define PIN_IO_VOLTAGE(bank, _pin, _name, sfx)		\
> -	[RCAR_GP_PIN(bank, _pin)].configs = SH_PFC_PIN_CFG_IO_VOLTAGE
> +/*
> + * All pins assigned to GPIO bank 3 can be used for SD interfaces in
> + * which case they support both 3.3V and 1.8V signalling.
> + */
> +#define GP_GPIO_IO_VOLTAGE(bank, _pin, _name, sfx)			\
> +	[RCAR_GP_PIN(bank, _pin)] =					\
> +		SH_PFC_GPIO_CFG(bank, _pin, _name, SH_PFC_PIN_CFG_IO_VOLTAGE)
>  
>  static const struct sh_pfc_pin pinmux_pins[] = {
> -	PINMUX_GPIO_GP_ALL(),
> -
> -	/*
> -	 * All pins assigned to GPIO bank 3 can be used for SD interfaces
> -	 * in which case they support both 3.3V and 1.8V signalling.
> -	 */
> -	PORT_GP_32(3, PIN_IO_VOLTAGE, unused),
> +	PORT_GP_32(0, _GP_GPIO, unused),
> +	PORT_GP_32(1, _GP_GPIO, unused),
> +	PORT_GP_32(2, _GP_GPIO, unused),
> +	PORT_GP_32(3, GP_GPIO_IO_VOLTAGE, unused),
> +	PORT_GP_32(4, _GP_GPIO, unused),
> +	PORT_GP_32(5, _GP_GPIO, unused),
>  
>  	/* Pins not associated with a GPIO port, placed after all the GPIOs */
> -	[RCAR_GP_PIN(5, 31) + 1] =
>  	SH_PFC_PIN_NAMED(ROW_GROUP_A('F'), 15, AF15),
>  	SH_PFC_PIN_NAMED(ROW_GROUP_A('G'), 15, AG15),
>  	SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
> diff --git a/drivers/pinctrl/sh-pfc/pfc-sh73a0.c b/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
> index d2efbfb776ac..2c798550cd8c 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
> @@ -1166,14 +1166,14 @@ static const u16 pinmux_data[] = {
>  #define __PU		(SH_PFC_PIN_CFG_PULL_UP)
>  #define __PUD		(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
>  
> -#define SH73A0_PIN_I_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PD)
> -#define SH73A0_PIN_I_PU(pin)		SH_PFC_PIN_CFG(pin, __I | __PU)
> -#define SH73A0_PIN_I_PU_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PUD)
> -#define SH73A0_PIN_IO(pin)		SH_PFC_PIN_CFG(pin, __IO)
> -#define SH73A0_PIN_IO_PD(pin)		SH_PFC_PIN_CFG(pin, __IO | __PD)
> -#define SH73A0_PIN_IO_PU(pin)		SH_PFC_PIN_CFG(pin, __IO | __PU)
> -#define SH73A0_PIN_IO_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __IO | __PUD)
> -#define SH73A0_PIN_O(pin)		SH_PFC_PIN_CFG(pin, __O)
> +#define SH73A0_PIN_I_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PD)
> +#define SH73A0_PIN_I_PU(pin)		SH_PFC_PORT_CFG(pin, __I | __PU)
> +#define SH73A0_PIN_I_PU_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PUD)
> +#define SH73A0_PIN_IO(pin)		SH_PFC_PORT_CFG(pin, __IO)
> +#define SH73A0_PIN_IO_PD(pin)		SH_PFC_PORT_CFG(pin, __IO | __PD)
> +#define SH73A0_PIN_IO_PU(pin)		SH_PFC_PORT_CFG(pin, __IO | __PU)
> +#define SH73A0_PIN_IO_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __IO | __PUD)
> +#define SH73A0_PIN_O(pin)		SH_PFC_PORT_CFG(pin, __O)
>  
>  /* Pin numbers for pins without a corresponding GPIO port number are computed
>   * from the row and column numbers with a 1000 offset to avoid collisions with
> diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
> index 734f7a92229c..3eca740bba02 100644
> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> @@ -272,14 +272,18 @@ struct sh_pfc_soc_info {
>  		.enum_id = _pin##_DATA,					\
>  	}
>  
> -/* SH_PFC_PIN_CFG - Expand to a sh_pfc_pin entry (named PORT#) with config */
> -#define SH_PFC_PIN_CFG(_pin, cfgs)					\
> +/* SH_PFC_{PORT,GPIO}_CFG - Expand to a sh_pfc_pin entry with config */
> +#define _SH_PFC_PIN_CFG(_pin, _name, cfgs)				\
>  	{								\
>  		.pin = _pin,						\
> -		.name = __stringify(PORT##_pin),			\
> -		.enum_id = PORT##_pin##_DATA,				\
> +		.name = __stringify(_name),				\
> +		.enum_id = _name##_DATA,				\
>  		.configs = cfgs,					\
>  	}
> +#define SH_PFC_PORT_CFG(_pin, cfgs)				\
> +	_SH_PFC_PIN_CFG(PORT##_pin, PORT##_pin, cfgs)
> +#define SH_PFC_GPIO_CFG(bank, _pin, _name, cfgs)		\
> +	_SH_PFC_PIN_CFG((bank * 32) + _pin, _name, cfgs)
>  
>  /* SH_PFC_PIN_NAMED - Expand to a sh_pfc_pin entry with the given name */
>  #define SH_PFC_PIN_NAMED(row, col, _name)				\
> --- END ---
> 
> If you're happy with that, I'll re-send the series (hopefully for the
> last time!) with the r8a7790 changes squashed into "pinctrl: sh-pfc:
> r8a7790: Implement voltage switching for SDHI" and the SH_PFC_PIN_CFG
> macro change as a new patch before it.
> 
> Ben.
> 

Re: [PATCH v4 2/8] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI

[Kuninori Morimoto]

Hi Laurent

Sorry for replying to old patch.
But, it seems Ben is waiting your answer about this patch
(This patch-set is not yet upstreamed)

> On Fri, 2015-07-03 at 00:21 +0100, Ben Hutchings wrote:
> > On Wed, 2015-07-01 at 10:37 +0300, Laurent Pinchart wrote:
> > [...]
> > > > +#define PIN_IO_VOLTAGE(bank, _pin, _name, sfx)		\
> > > > +	[RCAR_GP_PIN(bank, _pin)].configs = SH_PFC_PIN_CFG_IO_VOLTAGE
> > > > +
> > > >  static const struct sh_pfc_pin pinmux_pins[] = {
> > > >  	PINMUX_GPIO_GP_ALL(),
> > > > 
> > > > -	/* Pins not associated with a GPIO port */
> > > > +	/*
> > > > +	 * All pins assigned to GPIO bank 3 can be used for SD interfaces
> > > > +	 * in which case they support both 3.3V and 1.8V signalling.
> > > > +	 */
> > > > +	PORT_GP_32(3, PIN_IO_VOLTAGE, unused),
> > > > +
> > > > +	/* Pins not associated with a GPIO port, placed after all the GPIOs */
> > > > +	[RCAR_GP_PIN(5, 31) + 1] =
> > > 
> > > I'm sorry but I still don't like this. gcc outputs a warning when overriding 
> > > an initializer if you enable -Wextra, which leads me to believe this construct 
> > > is dubious. It should at least be tested with LLVM.
> > > 
> > > I'd much prefer replacing PINMUX_GPIO_GP_ALL() with a version that will 
> > > initialize the pins correctly right away.
> > [...]
> > 
> > Something like this?
> 
> Laurent, please can you answer whether this is the right way to go.
> 
> Ben.
> 
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-emev2.c b/drivers/pinctrl/sh-pfc/pfc-emev2.c
> > index 849c6943ed30..ad1a8281a91b 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-emev2.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-emev2.c
> > @@ -228,7 +228,7 @@ enum {
> >  
> >  /* Expand to a list of sh_pfc_pin entries (named PORT#).
> >   * NOTE: No config are recorded since the driver do not handle pinconf. */
> > -#define __PIN_CFG(pn, pfx, sfx)  SH_PFC_PIN_CFG(pfx, 0)
> > +#define __PIN_CFG(pn, pfx, sfx)  SH_PFC_PORT_CFG(pfx, 0)
> >  #define PINMUX_EMEV_GPIO_ALL()	  CPU_ALL_PORT(__PIN_CFG, , unused)
> >  
> >  static const struct sh_pfc_pin pinmux_pins[] = {
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c b/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
> > index 280a56f97786..ca1538371563 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
> > @@ -1272,8 +1272,8 @@ static const u16 pinmux_data[] = {
> >  #define __IO	(SH_PFC_PIN_CFG_INPUT | SH_PFC_PIN_CFG_OUTPUT)
> >  #define __PUD	(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
> >  
> > -#define R8A73A4_PIN_IO_PU_PD(pin)       SH_PFC_PIN_CFG(pin, __IO | __PUD)
> > -#define R8A73A4_PIN_O(pin)              SH_PFC_PIN_CFG(pin, __O)
> > +#define R8A73A4_PIN_IO_PU_PD(pin)       SH_PFC_PORT_CFG(pin, __IO | __PUD)
> > +#define R8A73A4_PIN_O(pin)              SH_PFC_PORT_CFG(pin, __O)
> >  
> >  static const struct sh_pfc_pin pinmux_pins[] = {
> >  	R8A73A4_PIN_IO_PU_PD(0), R8A73A4_PIN_IO_PU_PD(1),
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7740.c b/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
> > index b486e9d20cc2..92939cbd7ad0 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
> > @@ -1535,15 +1535,15 @@ static const u16 pinmux_data[] = {
> >  #define __PU		(SH_PFC_PIN_CFG_PULL_UP)
> >  #define __PUD		(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
> >  
> > -#define R8A7740_PIN_I_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PD)
> > -#define R8A7740_PIN_I_PU(pin)		SH_PFC_PIN_CFG(pin, __I | __PU)
> > -#define R8A7740_PIN_I_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __I | __PUD)
> > -#define R8A7740_PIN_IO(pin)		SH_PFC_PIN_CFG(pin, __IO)
> > -#define R8A7740_PIN_IO_PD(pin)		SH_PFC_PIN_CFG(pin, __IO | __PD)
> > -#define R8A7740_PIN_IO_PU(pin)		SH_PFC_PIN_CFG(pin, __IO | __PU)
> > -#define R8A7740_PIN_IO_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __IO | __PUD)
> > -#define R8A7740_PIN_O(pin)		SH_PFC_PIN_CFG(pin, __O)
> > -#define R8A7740_PIN_O_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __O | __PUD)
> > +#define R8A7740_PIN_I_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PD)
> > +#define R8A7740_PIN_I_PU(pin)		SH_PFC_PORT_CFG(pin, __I | __PU)
> > +#define R8A7740_PIN_I_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __I | __PUD)
> > +#define R8A7740_PIN_IO(pin)		SH_PFC_PORT_CFG(pin, __IO)
> > +#define R8A7740_PIN_IO_PD(pin)		SH_PFC_PORT_CFG(pin, __IO | __PD)
> > +#define R8A7740_PIN_IO_PU(pin)		SH_PFC_PORT_CFG(pin, __IO | __PU)
> > +#define R8A7740_PIN_IO_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __IO | __PUD)
> > +#define R8A7740_PIN_O(pin)		SH_PFC_PORT_CFG(pin, __O)
> > +#define R8A7740_PIN_O_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __O | __PUD)
> >  
> >  static const struct sh_pfc_pin pinmux_pins[] = {
> >  	/* Table 56-1 (I/O and Pull U/D) */
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > index 1137bbc810cd..1e59e1775374 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > @@ -1740,20 +1740,23 @@ static const u16 pinmux_data[] = {
> >  #define PIN_NUMBER(r, c) (((r) - 'A') * 31 + (c) + 200)
> >  #define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> >  
> > -#define PIN_IO_VOLTAGE(bank, _pin, _name, sfx)		\
> > -	[RCAR_GP_PIN(bank, _pin)].configs = SH_PFC_PIN_CFG_IO_VOLTAGE
> > +/*
> > + * All pins assigned to GPIO bank 3 can be used for SD interfaces in
> > + * which case they support both 3.3V and 1.8V signalling.
> > + */
> > +#define GP_GPIO_IO_VOLTAGE(bank, _pin, _name, sfx)			\
> > +	[RCAR_GP_PIN(bank, _pin)] =					\
> > +		SH_PFC_GPIO_CFG(bank, _pin, _name, SH_PFC_PIN_CFG_IO_VOLTAGE)
> >  
> >  static const struct sh_pfc_pin pinmux_pins[] = {
> > -	PINMUX_GPIO_GP_ALL(),
> > -
> > -	/*
> > -	 * All pins assigned to GPIO bank 3 can be used for SD interfaces
> > -	 * in which case they support both 3.3V and 1.8V signalling.
> > -	 */
> > -	PORT_GP_32(3, PIN_IO_VOLTAGE, unused),
> > +	PORT_GP_32(0, _GP_GPIO, unused),
> > +	PORT_GP_32(1, _GP_GPIO, unused),
> > +	PORT_GP_32(2, _GP_GPIO, unused),
> > +	PORT_GP_32(3, GP_GPIO_IO_VOLTAGE, unused),
> > +	PORT_GP_32(4, _GP_GPIO, unused),
> > +	PORT_GP_32(5, _GP_GPIO, unused),
> >  
> >  	/* Pins not associated with a GPIO port, placed after all the GPIOs */
> > -	[RCAR_GP_PIN(5, 31) + 1] =
> >  	SH_PFC_PIN_NAMED(ROW_GROUP_A('F'), 15, AF15),
> >  	SH_PFC_PIN_NAMED(ROW_GROUP_A('G'), 15, AG15),
> >  	SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-sh73a0.c b/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
> > index d2efbfb776ac..2c798550cd8c 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
> > @@ -1166,14 +1166,14 @@ static const u16 pinmux_data[] = {
> >  #define __PU		(SH_PFC_PIN_CFG_PULL_UP)
> >  #define __PUD		(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
> >  
> > -#define SH73A0_PIN_I_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PD)
> > -#define SH73A0_PIN_I_PU(pin)		SH_PFC_PIN_CFG(pin, __I | __PU)
> > -#define SH73A0_PIN_I_PU_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PUD)
> > -#define SH73A0_PIN_IO(pin)		SH_PFC_PIN_CFG(pin, __IO)
> > -#define SH73A0_PIN_IO_PD(pin)		SH_PFC_PIN_CFG(pin, __IO | __PD)
> > -#define SH73A0_PIN_IO_PU(pin)		SH_PFC_PIN_CFG(pin, __IO | __PU)
> > -#define SH73A0_PIN_IO_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __IO | __PUD)
> > -#define SH73A0_PIN_O(pin)		SH_PFC_PIN_CFG(pin, __O)
> > +#define SH73A0_PIN_I_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PD)
> > +#define SH73A0_PIN_I_PU(pin)		SH_PFC_PORT_CFG(pin, __I | __PU)
> > +#define SH73A0_PIN_I_PU_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PUD)
> > +#define SH73A0_PIN_IO(pin)		SH_PFC_PORT_CFG(pin, __IO)
> > +#define SH73A0_PIN_IO_PD(pin)		SH_PFC_PORT_CFG(pin, __IO | __PD)
> > +#define SH73A0_PIN_IO_PU(pin)		SH_PFC_PORT_CFG(pin, __IO | __PU)
> > +#define SH73A0_PIN_IO_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __IO | __PUD)
> > +#define SH73A0_PIN_O(pin)		SH_PFC_PORT_CFG(pin, __O)
> >  
> >  /* Pin numbers for pins without a corresponding GPIO port number are computed
> >   * from the row and column numbers with a 1000 offset to avoid collisions with
> > diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
> > index 734f7a92229c..3eca740bba02 100644
> > --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> > +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> > @@ -272,14 +272,18 @@ struct sh_pfc_soc_info {
> >  		.enum_id = _pin##_DATA,					\
> >  	}
> >  
> > -/* SH_PFC_PIN_CFG - Expand to a sh_pfc_pin entry (named PORT#) with config */
> > -#define SH_PFC_PIN_CFG(_pin, cfgs)					\
> > +/* SH_PFC_{PORT,GPIO}_CFG - Expand to a sh_pfc_pin entry with config */
> > +#define _SH_PFC_PIN_CFG(_pin, _name, cfgs)				\
> >  	{								\
> >  		.pin = _pin,						\
> > -		.name = __stringify(PORT##_pin),			\
> > -		.enum_id = PORT##_pin##_DATA,				\
> > +		.name = __stringify(_name),				\
> > +		.enum_id = _name##_DATA,				\
> >  		.configs = cfgs,					\
> >  	}
> > +#define SH_PFC_PORT_CFG(_pin, cfgs)				\
> > +	_SH_PFC_PIN_CFG(PORT##_pin, PORT##_pin, cfgs)
> > +#define SH_PFC_GPIO_CFG(bank, _pin, _name, cfgs)		\
> > +	_SH_PFC_PIN_CFG((bank * 32) + _pin, _name, cfgs)
> >  
> >  /* SH_PFC_PIN_NAMED - Expand to a sh_pfc_pin entry with the given name */
> >  #define SH_PFC_PIN_NAMED(row, col, _name)				\
> > --- END ---
> > 
> > If you're happy with that, I'll re-send the series (hopefully for the
> > last time!) with the r8a7790 changes squashed into "pinctrl: sh-pfc:
> > r8a7790: Implement voltage switching for SDHI" and the SH_PFC_PIN_CFG
> > macro change as a new patch before it.
> > 
> > Ben.
> > 
> 
> 


Best regards
---
Kuninori Morimoto

Re: [PATCH v4 2/8] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI

[Laurent Pinchart]

Hi Ben,

Just realized this mail thread fell through the cracks, sorry :-/

On Friday 03 July 2015 00:21:50 Ben Hutchings wrote:
> On Wed, 2015-07-01 at 10:37 +0300, Laurent Pinchart wrote:
> [...]
> 
> > > +#define PIN_IO_VOLTAGE(bank, _pin, _name, sfx)		\
> > > +	[RCAR_GP_PIN(bank, _pin)].configs = SH_PFC_PIN_CFG_IO_VOLTAGE
> > > +
> > > 
> > >  static const struct sh_pfc_pin pinmux_pins[] = {
> > >  
> > >  	PINMUX_GPIO_GP_ALL(),
> > > 
> > > -	/* Pins not associated with a GPIO port */
> > > +	/*
> > > +	 * All pins assigned to GPIO bank 3 can be used for SD interfaces
> > > +	 * in which case they support both 3.3V and 1.8V signalling.
> > > +	 */
> > > +	PORT_GP_32(3, PIN_IO_VOLTAGE, unused),
> > > +
> > > +	/* Pins not associated with a GPIO port, placed after all the GPIOs 
*/
> > > +	[RCAR_GP_PIN(5, 31) + 1] =
> > 
> > I'm sorry but I still don't like this. gcc outputs a warning when
> > overriding an initializer if you enable -Wextra, which leads me to
> > believe this construct is dubious. It should at least be tested with
> > LLVM.
> > 
> > I'd much prefer replacing PINMUX_GPIO_GP_ALL() with a version that will
> > initialize the pins correctly right away.
> 
> [...]
> 
> Something like this?
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-emev2.c
> b/drivers/pinctrl/sh-pfc/pfc-emev2.c index 849c6943ed30..ad1a8281a91b
> 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-emev2.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-emev2.c
> @@ -228,7 +228,7 @@ enum {
> 
>  /* Expand to a list of sh_pfc_pin entries (named PORT#).
>   * NOTE: No config are recorded since the driver do not handle pinconf. */
> -#define __PIN_CFG(pn, pfx, sfx)  SH_PFC_PIN_CFG(pfx, 0)
> +#define __PIN_CFG(pn, pfx, sfx)  SH_PFC_PORT_CFG(pfx, 0)
>  #define PINMUX_EMEV_GPIO_ALL()	  CPU_ALL_PORT(__PIN_CFG, , unused)
> 
>  static const struct sh_pfc_pin pinmux_pins[] = {
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c index 280a56f97786..ca1538371563
> 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
> @@ -1272,8 +1272,8 @@ static const u16 pinmux_data[] = {
>  #define __IO	(SH_PFC_PIN_CFG_INPUT | SH_PFC_PIN_CFG_OUTPUT)
>  #define __PUD	(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
> 
> -#define R8A73A4_PIN_IO_PU_PD(pin)       SH_PFC_PIN_CFG(pin, __IO | __PUD)
> -#define R8A73A4_PIN_O(pin)              SH_PFC_PIN_CFG(pin, __O)
> +#define R8A73A4_PIN_IO_PU_PD(pin)       SH_PFC_PORT_CFG(pin, __IO | __PUD)
> +#define R8A73A4_PIN_O(pin)              SH_PFC_PORT_CFG(pin, __O)
> 
>  static const struct sh_pfc_pin pinmux_pins[] = {
>  	R8A73A4_PIN_IO_PU_PD(0), R8A73A4_PIN_IO_PU_PD(1),
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a7740.c index b486e9d20cc2..92939cbd7ad0
> 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7740.c
> @@ -1535,15 +1535,15 @@ static const u16 pinmux_data[] = {
>  #define __PU		(SH_PFC_PIN_CFG_PULL_UP)
>  #define __PUD		(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
> 
> -#define R8A7740_PIN_I_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PD)
> -#define R8A7740_PIN_I_PU(pin)		SH_PFC_PIN_CFG(pin, __I | __PU)
> -#define R8A7740_PIN_I_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __I | __PUD)
> -#define R8A7740_PIN_IO(pin)		SH_PFC_PIN_CFG(pin, __IO)
> -#define R8A7740_PIN_IO_PD(pin)		SH_PFC_PIN_CFG(pin, __IO | __PD)
> -#define R8A7740_PIN_IO_PU(pin)		SH_PFC_PIN_CFG(pin, __IO | __PU)
> -#define R8A7740_PIN_IO_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __IO | __PUD)
> -#define R8A7740_PIN_O(pin)		SH_PFC_PIN_CFG(pin, __O)
> -#define R8A7740_PIN_O_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __O | __PUD)
> +#define R8A7740_PIN_I_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PD)
> +#define R8A7740_PIN_I_PU(pin)		SH_PFC_PORT_CFG(pin, __I | __PU)
> +#define R8A7740_PIN_I_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __I | __PUD)
> +#define R8A7740_PIN_IO(pin)		SH_PFC_PORT_CFG(pin, __IO)
> +#define R8A7740_PIN_IO_PD(pin)		SH_PFC_PORT_CFG(pin, __IO | __PD)
> +#define R8A7740_PIN_IO_PU(pin)		SH_PFC_PORT_CFG(pin, __IO | __PU)
> +#define R8A7740_PIN_IO_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __IO | __PUD)
> +#define R8A7740_PIN_O(pin)		SH_PFC_PORT_CFG(pin, __O)
> +#define R8A7740_PIN_O_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __O | __PUD)
> 
>  static const struct sh_pfc_pin pinmux_pins[] = {
>  	/* Table 56-1 (I/O and Pull U/D) */
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 1137bbc810cd..1e59e1775374
> 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -1740,20 +1740,23 @@ static const u16 pinmux_data[] = {
>  #define PIN_NUMBER(r, c) (((r) - 'A') * 31 + (c) + 200)
>  #define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> 
> -#define PIN_IO_VOLTAGE(bank, _pin, _name, sfx)		\
> -	[RCAR_GP_PIN(bank, _pin)].configs = SH_PFC_PIN_CFG_IO_VOLTAGE
> +/*
> + * All pins assigned to GPIO bank 3 can be used for SD interfaces in
> + * which case they support both 3.3V and 1.8V signalling.
> + */
> +#define GP_GPIO_IO_VOLTAGE(bank, _pin, _name, sfx)			\
> +	[RCAR_GP_PIN(bank, _pin)] =					\
> +		SH_PFC_GPIO_CFG(bank, _pin, _name, SH_PFC_PIN_CFG_IO_VOLTAGE)
> 
>  static const struct sh_pfc_pin pinmux_pins[] = {
> -	PINMUX_GPIO_GP_ALL(),
> -
> -	/*
> -	 * All pins assigned to GPIO bank 3 can be used for SD interfaces
> -	 * in which case they support both 3.3V and 1.8V signalling.
> -	 */
> -	PORT_GP_32(3, PIN_IO_VOLTAGE, unused),
> +	PORT_GP_32(0, _GP_GPIO, unused),
> +	PORT_GP_32(1, _GP_GPIO, unused),
> +	PORT_GP_32(2, _GP_GPIO, unused),
> +	PORT_GP_32(3, GP_GPIO_IO_VOLTAGE, unused),
> +	PORT_GP_32(4, _GP_GPIO, unused),
> +	PORT_GP_32(5, _GP_GPIO, unused),
> 
>  	/* Pins not associated with a GPIO port, placed after all the GPIOs */
> -	[RCAR_GP_PIN(5, 31) + 1] =
>  	SH_PFC_PIN_NAMED(ROW_GROUP_A('F'), 15, AF15),
>  	SH_PFC_PIN_NAMED(ROW_GROUP_A('G'), 15, AG15),
>  	SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
> diff --git a/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
> b/drivers/pinctrl/sh-pfc/pfc-sh73a0.c index d2efbfb776ac..2c798550cd8c
> 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-sh73a0.c
> @@ -1166,14 +1166,14 @@ static const u16 pinmux_data[] = {
>  #define __PU		(SH_PFC_PIN_CFG_PULL_UP)
>  #define __PUD		(SH_PFC_PIN_CFG_PULL_DOWN | SH_PFC_PIN_CFG_PULL_UP)
> 
> -#define SH73A0_PIN_I_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PD)
> -#define SH73A0_PIN_I_PU(pin)		SH_PFC_PIN_CFG(pin, __I | __PU)
> -#define SH73A0_PIN_I_PU_PD(pin)		SH_PFC_PIN_CFG(pin, __I | __PUD)
> -#define SH73A0_PIN_IO(pin)		SH_PFC_PIN_CFG(pin, __IO)
> -#define SH73A0_PIN_IO_PD(pin)		SH_PFC_PIN_CFG(pin, __IO | __PD)
> -#define SH73A0_PIN_IO_PU(pin)		SH_PFC_PIN_CFG(pin, __IO | __PU)
> -#define SH73A0_PIN_IO_PU_PD(pin)	SH_PFC_PIN_CFG(pin, __IO | __PUD)
> -#define SH73A0_PIN_O(pin)		SH_PFC_PIN_CFG(pin, __O)
> +#define SH73A0_PIN_I_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PD)
> +#define SH73A0_PIN_I_PU(pin)		SH_PFC_PORT_CFG(pin, __I | __PU)
> +#define SH73A0_PIN_I_PU_PD(pin)		SH_PFC_PORT_CFG(pin, __I | __PUD)
> +#define SH73A0_PIN_IO(pin)		SH_PFC_PORT_CFG(pin, __IO)
> +#define SH73A0_PIN_IO_PD(pin)		SH_PFC_PORT_CFG(pin, __IO | __PD)
> +#define SH73A0_PIN_IO_PU(pin)		SH_PFC_PORT_CFG(pin, __IO | __PU)
> +#define SH73A0_PIN_IO_PU_PD(pin)	SH_PFC_PORT_CFG(pin, __IO | __PUD)
> +#define SH73A0_PIN_O(pin)		SH_PFC_PORT_CFG(pin, __O)
> 
>  /* Pin numbers for pins without a corresponding GPIO port number are
> computed * from the row and column numbers with a 1000 offset to avoid
> collisions with diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h
> b/drivers/pinctrl/sh-pfc/sh_pfc.h index 734f7a92229c..3eca740bba02 100644
> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> @@ -272,14 +272,18 @@ struct sh_pfc_soc_info {
>  		.enum_id = _pin##_DATA,					\
>  	}
> 
> -/* SH_PFC_PIN_CFG - Expand to a sh_pfc_pin entry (named PORT#) with config
> */
> -#define SH_PFC_PIN_CFG(_pin, cfgs)					\
> +/* SH_PFC_{PORT,GPIO}_CFG - Expand to a sh_pfc_pin entry with config */
> +#define _SH_PFC_PIN_CFG(_pin, _name, cfgs)				\
>  	{								\
>  		.pin = _pin,						\
> -		.name = __stringify(PORT##_pin),			\
> -		.enum_id = PORT##_pin##_DATA,				\
> +		.name = __stringify(_name),				\
> +		.enum_id = _name##_DATA,				\
>  		.configs = cfgs,					\
>  	}
> +#define SH_PFC_PORT_CFG(_pin, cfgs)				\
> +	_SH_PFC_PIN_CFG(PORT##_pin, PORT##_pin, cfgs)
> +#define SH_PFC_GPIO_CFG(bank, _pin, _name, cfgs)		\
> +	_SH_PFC_PIN_CFG((bank * 32) + _pin, _name, cfgs)
> 
>  /* SH_PFC_PIN_NAMED - Expand to a sh_pfc_pin entry with the given name */
>  #define SH_PFC_PIN_NAMED(row, col, _name)				\
> --- END ---
> 
> If you're happy with that, I'll re-send the series (hopefully for the
> last time!) with the r8a7790 changes squashed into "pinctrl: sh-pfc:
> r8a7790: Implement voltage switching for SDHI" and the SH_PFC_PIN_CFG
> macro change as a new patch before it.

That looks good to me. I'd split it in two patches though, one that reworks 
the existing macros (the drivers/pinctrl/sh-pfc/sh_pfc.h changes) and one that 
implements voltage switching for SDHI on r8a7790.

I would also avoid renaming SH_PFC_PIN_CFG to SH_PFC_PORT_CFG to avoid 
modifying unrelated files, you can name the low-level macro __SH_PFC_PIN_CFG 
and add

#define SH_PFC_PIN_CFG(_pin, cfgs)				\
	__SH_PFC_PIN_CFG(PORT##_pin, PORT##_pin, cfgs)

Additionally, your new SH_PFC_GPIO_CFG macro seems identical to _GP_GPIO now. 
It would make sense to merge the two.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/8] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2616 bytes --]


> > b/drivers/pinctrl/sh-pfc/sh_pfc.h index 734f7a92229c..3eca740bba02 100644
> > --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> > +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> > @@ -272,14 +272,18 @@ struct sh_pfc_soc_info {
> >  		.enum_id = _pin##_DATA,					\
> >  	}
> > 
> > -/* SH_PFC_PIN_CFG - Expand to a sh_pfc_pin entry (named PORT#) with config
> > */
> > -#define SH_PFC_PIN_CFG(_pin, cfgs)					\
> > +/* SH_PFC_{PORT,GPIO}_CFG - Expand to a sh_pfc_pin entry with config */
> > +#define _SH_PFC_PIN_CFG(_pin, _name, cfgs)				\
> >  	{								\
> >  		.pin = _pin,						\
> > -		.name = __stringify(PORT##_pin),			\
> > -		.enum_id = PORT##_pin##_DATA,				\
> > +		.name = __stringify(_name),				\
> > +		.enum_id = _name##_DATA,				\
> >  		.configs = cfgs,					\
> >  	}
> > +#define SH_PFC_PORT_CFG(_pin, cfgs)				\
> > +	_SH_PFC_PIN_CFG(PORT##_pin, PORT##_pin, cfgs)
> > +#define SH_PFC_GPIO_CFG(bank, _pin, _name, cfgs)		\
> > +	_SH_PFC_PIN_CFG((bank * 32) + _pin, _name, cfgs)
> > 
> >  /* SH_PFC_PIN_NAMED - Expand to a sh_pfc_pin entry with the given name */
> >  #define SH_PFC_PIN_NAMED(row, col, _name)				\
> > --- END ---
> > 
> > If you're happy with that, I'll re-send the series (hopefully for the
> > last time!) with the r8a7790 changes squashed into "pinctrl: sh-pfc:
> > r8a7790: Implement voltage switching for SDHI" and the SH_PFC_PIN_CFG
> > macro change as a new patch before it.
> 
> That looks good to me. I'd split it in two patches though, one that reworks 
> the existing macros (the drivers/pinctrl/sh-pfc/sh_pfc.h changes) and one that 
> implements voltage switching for SDHI on r8a7790.
> 
> I would also avoid renaming SH_PFC_PIN_CFG to SH_PFC_PORT_CFG to avoid 
> modifying unrelated files, you can name the low-level macro __SH_PFC_PIN_CFG 
> and add
> 
> #define SH_PFC_PIN_CFG(_pin, cfgs)				\
> 	__SH_PFC_PIN_CFG(PORT##_pin, PORT##_pin, cfgs)
> 
> Additionally, your new SH_PFC_GPIO_CFG macro seems identical to _GP_GPIO now. 
> It would make sense to merge the two.

It is identical. And since we meanwhile also have PORT_GP_CFG_32, I
wonder if we can't simply do this in CPU_ALL_PORT?

@@ -30,7 +31,7 @@
 	PORT_GP_32(0, fn, sfx),						\
 	PORT_GP_30(1, fn, sfx),						\
 	PORT_GP_30(2, fn, sfx),						\
-	PORT_GP_32(3, fn, sfx),						\
+	PORT_GP_CFG_32(3, fn, sfx, SH_PFC_PIN_CFG_IO_VOLTAGE),		\
 	PORT_GP_32(4, fn, sfx),						\
 	PORT_GP_32(5, fn, sfx)
 
And skip all the macro refactoring?

I'll test this once I have access to my Lager again, but though I'll let
you know already...

Thanks,

   Wolfram

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 3/8] mmc: tmio, sh_mobile_sdhi: Pass tmio_mmc_host ptr to clk_{enable,disable} ops

[Ben Hutchings]

Change the clk_enable operation to take a pointer to the struct
tmio_mmc_host and have it set f_max.  For consistency, also change the
clk_disable operation to take a pointer to struct tmio_mmc_host.

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 drivers/mmc/host/sh_mobile_sdhi.c | 12 +++++-------
 drivers/mmc/host/tmio_mmc.h       |  4 ++--
 drivers/mmc/host/tmio_mmc_pio.c   |  4 ++--
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 354f4f335ed5..f6e8c98819ad 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -111,16 +111,15 @@ static void sh_mobile_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
 	sd_ctrl_write16(host, EXT_ACC, val);
 }
 
-static int sh_mobile_sdhi_clk_enable(struct platform_device *pdev, unsigned int *f)
+static int sh_mobile_sdhi_clk_enable(struct tmio_mmc_host *host)
 {
-	struct mmc_host *mmc = platform_get_drvdata(pdev);
-	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct mmc_host *mmc = host->mmc;
 	struct sh_mobile_sdhi *priv = host_to_priv(host);
 	int ret = clk_prepare_enable(priv->clk);
 	if (ret < 0)
 		return ret;
 
-	*f = clk_get_rate(priv->clk);
+	mmc->f_max = clk_get_rate(priv->clk);
 
 	/* enable 16bit data access on SDBUF as default */
 	sh_mobile_sdhi_sdbuf_width(host, 16);
@@ -128,11 +127,10 @@ static int sh_mobile_sdhi_clk_enable(struct platform_device *pdev, unsigned int
 	return 0;
 }
 
-static void sh_mobile_sdhi_clk_disable(struct platform_device *pdev)
+static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
 {
-	struct mmc_host *mmc = platform_get_drvdata(pdev);
-	struct tmio_mmc_host *host = mmc_priv(mmc);
 	struct sh_mobile_sdhi *priv = host_to_priv(host);
+
 	clk_disable_unprepare(priv->clk);
 }
 
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 4a597f5a53e2..68fd8d791358 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -95,8 +95,8 @@ struct tmio_mmc_host {
 	bool			sdio_irq_enabled;
 
 	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
-	int (*clk_enable)(struct platform_device *pdev, unsigned int *f);
-	void (*clk_disable)(struct platform_device *pdev);
+	int (*clk_enable)(struct tmio_mmc_host *host);
+	void (*clk_disable)(struct tmio_mmc_host *host);
 	int (*multi_io_quirk)(struct mmc_card *card,
 			      unsigned int direction, int blk_size);
 };
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index e3dcf31a8bd6..8fdb8a6209e6 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -840,7 +840,7 @@ static int tmio_mmc_clk_update(struct tmio_mmc_host *host)
 	if (!host->clk_enable)
 		return -ENOTSUPP;
 
-	ret = host->clk_enable(host->pdev, &mmc->f_max);
+	ret = host->clk_enable(host);
 	if (!ret)
 		mmc->f_min = mmc->f_max / 512;
 
@@ -1246,7 +1246,7 @@ int tmio_mmc_host_runtime_suspend(struct device *dev)
 		tmio_mmc_clk_stop(host);
 
 	if (host->clk_disable)
-		host->clk_disable(host->pdev);
+		host->clk_disable(host);
 
 	return 0;
 }
-- 
2.1.4

[PATCH v4 4/8] mmc: tmio, sh_mobile_sdhi: Add support for variable input clock frequency

[Ben Hutchings]

Currently tmio_mmc assumes that the input clock frequency is fixed and
only its own clock divider can be changed.  This is not true in the
case of sh_mobile_sdhi; we can use the clock API to change it.

In tmio_mmc:
- Delegate setting of f_min from tmio to the clk_enable operation (if
  implemented), as it can be smaller than f_max / 512
- Add an optional clk_update operation called from tmio_mmc_set_clock()
  that updates the input clock frequency
- Rename tmio_mmc_clk_update() to tmio_mmc_clk_enable(), to avoid
  confusion with the clk_update operation

In sh_mobile_sdhi:
- Make the setting of f_max conditional; it should be set through the
  max-frequency property in the device tree in future
- Set f_min based on the input clock's minimum frequency
- Implement the clk_update operation, selecting the best input clock
  frequency for the bus frequency that's wanted

sh_mobile_sdhi_clk_update() is loosely based on Kuninori Morimoto's work
in sh_mmcif.

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 drivers/mmc/host/sh_mobile_sdhi.c | 56 +++++++++++++++++++++++++++++++++++++--
 drivers/mmc/host/tmio_mmc.h       |  2 ++
 drivers/mmc/host/tmio_mmc_pio.c   | 23 +++++++---------
 3 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index f6e8c98819ad..6ed36b984e6e 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -119,7 +119,20 @@ static int sh_mobile_sdhi_clk_enable(struct tmio_mmc_host *host)
 	if (ret < 0)
 		return ret;
 
-	mmc->f_max = clk_get_rate(priv->clk);
+	/*
+	 * The clock driver may not know what maximum frequency
+	 * actually works, so it should be set with the max-frequency
+	 * property which will already have been read to f_max.  If it
+	 * was missing, assume the current frequency is the maximum.
+	 */
+	if (!mmc->f_max)
+		mmc->f_max = clk_get_rate(priv->clk);
+
+	/*
+	 * Minimum frequency is the minimum input clock frequency
+	 * divided by our maximum divider.
+	 */
+	mmc->f_min = max(clk_round_rate(priv->clk, 1) / 512, 1L);
 
 	/* enable 16bit data access on SDBUF as default */
 	sh_mobile_sdhi_sdbuf_width(host, 16);
@@ -127,6 +140,44 @@ static int sh_mobile_sdhi_clk_enable(struct tmio_mmc_host *host)
 	return 0;
 }
 
+static unsigned int sh_mobile_sdhi_clk_update(struct tmio_mmc_host *host,
+					      unsigned int new_clock)
+{
+	struct sh_mobile_sdhi *priv = host_to_priv(host);
+	unsigned int freq, best_freq, diff_min, diff;
+	int i;
+
+	diff_min = ~0;
+	best_freq = 0;
+
+	/*
+	 * We want the bus clock to be as close as possible to, but no
+	 * greater than, new_clock.  As we can divide by 1 << i for
+	 * any i in [0, 9] we want the input clock to be as close as
+	 * possible, but no greater than, new_clock << i.
+	 */
+	for (i = min(9, ilog2(UINT_MAX / new_clock)); i >= 0; i--) {
+		freq = clk_round_rate(priv->clk, new_clock << i);
+		if (freq > (new_clock << i)) {
+			/* Too fast; look for a slightly slower option */
+			freq = clk_round_rate(priv->clk,
+					      (new_clock << i) / 4 * 3);
+			if (freq > (new_clock << i))
+				continue;
+		}
+
+		diff = new_clock - (freq >> i);
+		if (diff <= diff_min) {
+			best_freq = freq;
+			diff_min = diff;
+		}
+	}
+
+	clk_set_rate(priv->clk, best_freq);
+
+	return best_freq;
+}
+
 static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
 {
 	struct sh_mobile_sdhi *priv = host_to_priv(host);
@@ -235,6 +286,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	host->dma		= dma_priv;
 	host->write16_hook	= sh_mobile_sdhi_write16_hook;
 	host->clk_enable	= sh_mobile_sdhi_clk_enable;
+	host->clk_update	= sh_mobile_sdhi_clk_update;
 	host->clk_disable	= sh_mobile_sdhi_clk_disable;
 	host->multi_io_quirk	= sh_mobile_sdhi_multi_io_quirk;
 	/* SD control register space size is 0x100, 0x200 for bus_shift=1 */
@@ -342,7 +394,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		}
 	}
 
-	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
+	dev_info(&pdev->dev, "%s base at 0x%08lx max clock rate %u MHz\n",
 		 mmc_hostname(host->mmc), (unsigned long)
 		 (platform_get_resource(pdev, IORESOURCE_MEM, 0)->start),
 		 host->mmc->f_max / 1000000);
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 68fd8d791358..b44b58902906 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -96,6 +96,8 @@ struct tmio_mmc_host {
 
 	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
 	int (*clk_enable)(struct tmio_mmc_host *host);
+	unsigned int (*clk_update)(struct tmio_mmc_host *host,
+				   unsigned int new_clock);
 	void (*clk_disable)(struct tmio_mmc_host *host);
 	int (*multi_io_quirk)(struct mmc_card *card,
 			      unsigned int direction, int blk_size);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 8fdb8a6209e6..4869dd9ffa9f 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -156,8 +156,12 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host,
 	u32 clk = 0, clock;
 
 	if (new_clock) {
-		for (clock = host->mmc->f_min, clk = 0x80000080;
-			new_clock >= (clock<<1); clk >>= 1)
+		if (host->clk_update)
+			clock = host->clk_update(host, new_clock) / 512;
+		else
+			clock = host->mmc->f_min;
+
+		for (clk = 0x80000080; new_clock >= (clock << 1); clk >>= 1)
 			clock <<= 1;
 
 		/* 1/1 clock is option */
@@ -832,19 +836,12 @@ fail:
 	pm_runtime_put_autosuspend(mmc_dev(mmc));
 }
 
-static int tmio_mmc_clk_update(struct tmio_mmc_host *host)
+static int tmio_mmc_clk_enable(struct tmio_mmc_host *host)
 {
-	struct mmc_host *mmc = host->mmc;
-	int ret;
-
 	if (!host->clk_enable)
 		return -ENOTSUPP;
 
-	ret = host->clk_enable(host);
-	if (!ret)
-		mmc->f_min = mmc->f_max / 512;
-
-	return ret;
+	return host->clk_enable(host);
 }
 
 static void tmio_mmc_power_on(struct tmio_mmc_host *host, unsigned short vdd)
@@ -1130,7 +1127,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 				  mmc->caps & MMC_CAP_NONREMOVABLE ||
 				  mmc->slot.cd_irq >= 0);
 
-	if (tmio_mmc_clk_update(_host) < 0) {
+	if (tmio_mmc_clk_enable(_host) < 0) {
 		mmc->f_max = pdata->hclk;
 		mmc->f_min = mmc->f_max / 512;
 	}
@@ -1258,7 +1255,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 
 	tmio_mmc_reset(host);
-	tmio_mmc_clk_update(host);
+	tmio_mmc_clk_enable(host);
 
 	if (host->clk_cache) {
 		tmio_mmc_set_clock(host, host->clk_cache);
-- 
2.1.4

[PATCH v4 5/8] 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 b44b58902906..aabd36955e73 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -101,6 +101,9 @@ struct tmio_mmc_host {
 	void (*clk_disable)(struct tmio_mmc_host *host);
 	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 4869dd9ffa9f..192bf3dda964 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1008,12 +1008,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 v4 6/8] 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 6ed36b984e6e..76899b90946e 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)
@@ -185,6 +190,49 @@ static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
 	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;
@@ -277,6 +325,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;
@@ -289,6 +345,10 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	host->clk_update	= sh_mobile_sdhi_clk_update;
 	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 v4 7/8] ARM: shmobile: r8a7790: Set maximum frequencies for SDHI clocks

[Ben Hutchings]

Taken from the datasheet.

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

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 4bb2f4c17321..618b38938b24 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -490,6 +490,7 @@
 		reg = <0 0xee100000 0 0x328>;
 		interrupts = <0 165 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
+		max-frequency = <156000000>;
 		dmas = <&dmac1 0xcd>, <&dmac1 0xce>;
 		dma-names = "tx", "rx";
 		status = "disabled";
@@ -500,6 +501,7 @@
 		reg = <0 0xee120000 0 0x328>;
 		interrupts = <0 166 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp3_clks R8A7790_CLK_SDHI1>;
+		max-frequency = <156000000>;
 		dmas = <&dmac1 0xc9>, <&dmac1 0xca>;
 		dma-names = "tx", "rx";
 		status = "disabled";
@@ -510,6 +512,7 @@
 		reg = <0 0xee140000 0 0x100>;
 		interrupts = <0 167 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
+		max-frequency = <97500000>;
 		dmas = <&dmac1 0xc1>, <&dmac1 0xc2>;
 		dma-names = "tx", "rx";
 		status = "disabled";
@@ -520,6 +523,7 @@
 		reg = <0 0xee160000 0 0x100>;
 		interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp3_clks R8A7790_CLK_SDHI3>;
+		max-frequency = <97500000>;
 		dmas = <&dmac1 0xd3>, <&dmac1 0xd4>;
 		dma-names = "tx", "rx";
 		status = "disabled";
-- 
2.1.4

Re: [PATCH v4 7/8] ARM: shmobile: r8a7790: Set maximum frequencies for SDHI clocks

[Simon Horman]

Morimoto-san,

could you find some time to review this patch?

On Tue, Jun 30, 2015 at 05:57:16PM +0100, Ben Hutchings wrote:
> Taken from the datasheet.
> 
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> ---
>  arch/arm/boot/dts/r8a7790.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index 4bb2f4c17321..618b38938b24 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -490,6 +490,7 @@
>  		reg = <0 0xee100000 0 0x328>;
>  		interrupts = <0 165 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
> +		max-frequency = <156000000>;
>  		dmas = <&dmac1 0xcd>, <&dmac1 0xce>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> @@ -500,6 +501,7 @@
>  		reg = <0 0xee120000 0 0x328>;
>  		interrupts = <0 166 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI1>;
> +		max-frequency = <156000000>;
>  		dmas = <&dmac1 0xc9>, <&dmac1 0xca>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> @@ -510,6 +512,7 @@
>  		reg = <0 0xee140000 0 0x100>;
>  		interrupts = <0 167 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
> +		max-frequency = <97500000>;
>  		dmas = <&dmac1 0xc1>, <&dmac1 0xc2>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> @@ -520,6 +523,7 @@
>  		reg = <0 0xee160000 0 0x100>;
>  		interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI3>;
> +		max-frequency = <97500000>;
>  		dmas = <&dmac1 0xd3>, <&dmac1 0xd4>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> -- 
> 2.1.4
> 
> 
> 
> 

Re: [PATCH v4 7/8] ARM: shmobile: r8a7790: Set maximum frequencies for SDHI clocks

[Kuninori Morimoto]

Hi Simon

> Morimoto-san,
> 
> could you find some time to review this patch?

Sorry, I had forgot about this, I sent Acked-by mail.
But, I guess Ben will send v5 patch

> 
> On Tue, Jun 30, 2015 at 05:57:16PM +0100, Ben Hutchings wrote:
> > Taken from the datasheet.
> > 
> > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> > ---
> >  arch/arm/boot/dts/r8a7790.dtsi | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> > index 4bb2f4c17321..618b38938b24 100644
> > --- a/arch/arm/boot/dts/r8a7790.dtsi
> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
> > @@ -490,6 +490,7 @@
> >  		reg = <0 0xee100000 0 0x328>;
> >  		interrupts = <0 165 IRQ_TYPE_LEVEL_HIGH>;
> >  		clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
> > +		max-frequency = <156000000>;
> >  		dmas = <&dmac1 0xcd>, <&dmac1 0xce>;
> >  		dma-names = "tx", "rx";
> >  		status = "disabled";
> > @@ -500,6 +501,7 @@
> >  		reg = <0 0xee120000 0 0x328>;
> >  		interrupts = <0 166 IRQ_TYPE_LEVEL_HIGH>;
> >  		clocks = <&mstp3_clks R8A7790_CLK_SDHI1>;
> > +		max-frequency = <156000000>;
> >  		dmas = <&dmac1 0xc9>, <&dmac1 0xca>;
> >  		dma-names = "tx", "rx";
> >  		status = "disabled";
> > @@ -510,6 +512,7 @@
> >  		reg = <0 0xee140000 0 0x100>;
> >  		interrupts = <0 167 IRQ_TYPE_LEVEL_HIGH>;
> >  		clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
> > +		max-frequency = <97500000>;
> >  		dmas = <&dmac1 0xc1>, <&dmac1 0xc2>;
> >  		dma-names = "tx", "rx";
> >  		status = "disabled";
> > @@ -520,6 +523,7 @@
> >  		reg = <0 0xee160000 0 0x100>;
> >  		interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>;
> >  		clocks = <&mstp3_clks R8A7790_CLK_SDHI3>;
> > +		max-frequency = <97500000>;
> >  		dmas = <&dmac1 0xd3>, <&dmac1 0xd4>;
> >  		dma-names = "tx", "rx";
> >  		status = "disabled";
> > -- 
> > 2.1.4
> > 
> > 
> > 
> > 

Re: [PATCH v4 7/8] ARM: shmobile: r8a7790: Set maximum frequencies for SDHI clocks

[Simon Horman]

Hi Morimoto-san,

On Tue, Jul 07, 2015 at 01:19:55AM +0000, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > Morimoto-san,
> > 
> > could you find some time to review this patch?
> 
> Sorry, I had forgot about this, I sent Acked-by mail.
> But, I guess Ben will send v5 patch

Thanks.

Ben, I'm happy to take this patch now or after you repost it.
Let me know which is best for you.

Re: [PATCH v4 7/8] ARM: shmobile: r8a7790: Set maximum frequencies for SDHI clocks

[Kuninori Morimoto]

Hi

> Taken from the datasheet.
> 
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> ---

Maybe Ben needs to update v5 patch ?

But, for this patch

Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

>  arch/arm/boot/dts/r8a7790.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index 4bb2f4c17321..618b38938b24 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -490,6 +490,7 @@
>  		reg = <0 0xee100000 0 0x328>;
>  		interrupts = <0 165 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
> +		max-frequency = <156000000>;
>  		dmas = <&dmac1 0xcd>, <&dmac1 0xce>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> @@ -500,6 +501,7 @@
>  		reg = <0 0xee120000 0 0x328>;
>  		interrupts = <0 166 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI1>;
> +		max-frequency = <156000000>;
>  		dmas = <&dmac1 0xc9>, <&dmac1 0xca>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> @@ -510,6 +512,7 @@
>  		reg = <0 0xee140000 0 0x100>;
>  		interrupts = <0 167 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
> +		max-frequency = <97500000>;
>  		dmas = <&dmac1 0xc1>, <&dmac1 0xc2>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> @@ -520,6 +523,7 @@
>  		reg = <0 0xee160000 0 0x100>;
>  		interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&mstp3_clks R8A7790_CLK_SDHI3>;
> +		max-frequency = <97500000>;
>  		dmas = <&dmac1 0xd3>, <&dmac1 0xd4>;
>  		dma-names = "tx", "rx";
>  		status = "disabled";
> -- 
> 2.1.4
> 
> 
> 
> 

[PATCH v4 8/8] 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 | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index aaa4f258e279..c14c416b6704 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -312,11 +312,25 @@
 	sdhi0_pins: sd0 {
 		renesas,groups = "sdhi0_data4", "sdhi0_ctrl";
 		renesas,function = "sdhi0";
+		power-source = <3300>;
+	};
+
+	sdhi0_pins_1v8: sd0_1v8 {
+		renesas,groups = "sdhi0_data4", "sdhi0_ctrl";
+		renesas,function = "sdhi0";
+		power-source = <1800>;
 	};
 
 	sdhi2_pins: sd2 {
 		renesas,groups = "sdhi2_data4", "sdhi2_ctrl";
 		renesas,function = "sdhi2";
+		power-source = <3300>;
+	};
+
+	sdhi2_pins_1v8: sd2_1v8 {
+		renesas,groups = "sdhi2_data4", "sdhi2_ctrl";
+		renesas,function = "sdhi2";
+		power-source = <1800>;
 	};
 
 	mmc1_pins: mmc1 {
@@ -486,21 +500,25 @@
 
 &sdhi0 {
 	pinctrl-0 = <&sdhi0_pins>;
-	pinctrl-names = "default";
+	pinctrl-1 = <&sdhi0_pins_1v8>;
+	pinctrl-names = "default", "1v8";
 
 	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";
 
 	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 v4 0/8] UHS-I support for sh_mobile_sdhi

[Kuninori Morimoto]

Hi Ben

Thank you for your hard work

> 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 implementing the
> pinconf power-source parameter.  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.

For tmio/sh_mobile_sdhi

Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Best regards
---
Kuninori Morimoto