Re: [PATCH 1/2 v2] ARM: shmobile: lager: add gpio/fixed regulator for SDHI

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Morimoto-san,

Thank you for the patch.

On Thursday 14 November 2013 17:55:12 Kuninori Morimoto wrote:
> Fixed regulator is used for SDHI0/2 Vcc.
> We should use da9063 driver for Vccq,
> but, it doesn't have regulator support at this point.
> This patch uses gpio-regulator for it as quick-hack.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v1 -> v2
> 
>  - 2/2 become 1/2
>  - FIXME comment was added
> 
>  arch/arm/mach-shmobile/board-lager.c |   83 ++++++++++++++++++++++++++++++-
>  1 file changed, 82 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-lager.c
> b/arch/arm/mach-shmobile/board-lager.c index fcf06fd..3d1640f 100644
> --- a/arch/arm/mach-shmobile/board-lager.c
> +++ b/arch/arm/mach-shmobile/board-lager.c
> @@ -31,7 +31,9 @@
>  #include <linux/platform_data/rcar-du.h>
>  #include <linux/platform_device.h>
>  #include <linux/phy.h>
> +#include <linux/regulator/driver.h>

I might be mistaken, but is this header really needed ?

>  #include <linux/regulator/fixed.h>
> +#include <linux/regulator/gpio-regulator.h>
>  #include <linux/regulator/machine.h>
>  #include <linux/sh_eth.h>
>  #include <mach/common.h>
> @@ -141,6 +143,71 @@ static struct regulator_consumer_supply
> fixed3v3_power_consumers[] = REGULATOR_SUPPLY("vmmc", "sh_mmcif.1"),
>  };
> 
> +/*
> + * SDHI regulator macro
> + *
> + ** FIXME**
> + * Lager board vqmmc is provided via DA9063 PMIC chip,
> + * and we should use ${LINK}/drivers/mfd/da9063-* driver for it.
> + * but, it doesn't have regulator support at this point.
> + * It uses gpio-regulator for vqmmc as quick-hack.
> + */
> +#define SDHI_REGULATOR(idx, vdd_pin, vccq_pin)				\
> +static struct regulator_consumer_supply vcc_sdhi##idx##_consumer =	\
> +	REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi." #idx);		\
> +									\
> +static struct regulator_init_data vcc_sdhi##idx##_init_data = {		\
> +	.constraints = {						\
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,		\
> +	},								\
> +	.consumer_supplies	= &vcc_sdhi##idx##_consumer,		\
> +	.num_consumer_supplies	= 1,					\
> +};									\
> +									\
> +static struct fixed_voltage_config vcc_sdhi##idx##_info = {		\

You can make this structure const.

> +	.supply_name	= "SDHI" #idx "Vcc",				\
> +	.microvolts	= 3300000,					\
> +	.gpio		= vdd_pin,					\
> +	.enable_high	= 1,						\
> +	.init_data	= &vcc_sdhi##idx##_init_data,			\
> +};									\
> +									\
> +static struct regulator_consumer_supply vccq_sdhi##idx##_consumer=	\
> +	REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi." #idx);		\
> +									\
> +static struct regulator_init_data vccq_sdhi##idx##_init_data = {	\
> +	.constraints = {						\
> +		.input_uV	= 3300000,				\
> +		.min_uV		= 1800000,				\
> +		.max_uV		= 3300000,				\
> +		.valid_ops_mask = REGULATOR_CHANGE_VOLTAGE |		\
> +				  REGULATOR_CHANGE_STATUS,		\
> +	},								\
> +	.consumer_supplies	= &vccq_sdhi##idx##_consumer,		\
> +	.num_consumer_supplies	= 1,					\
> +};									\
> +									\
> +static struct gpio vccq_sdhi##idx##_gpio =				\
> +	{ vccq_pin, GPIOF_OUT_INIT_HIGH, "vccq-sdhi" #idx };		\
> +									\
> +static struct gpio_regulator_state vccq_sdhi##idx##_states[] = {	\
> +	{ .value = 1800000, .gpios = 0 },				\
> +	{ .value = 3300000, .gpios = 1 },				\
> +};									\
> +									\
> +static struct gpio_regulator_config vccq_sdhi##idx##_info = {		\

You can make this structure const.

> +	.supply_name	= "vqmmc",					\
> +	.gpios		= &vccq_sdhi##idx##_gpio,			\
> +	.nr_gpios	= 1,						\
> +	.states		= vccq_sdhi##idx##_states,			\
> +	.nr_states	= ARRAY_SIZE(vccq_sdhi##idx##_states),		\
> +	.type		= REGULATOR_VOLTAGE,				\
> +	.init_data	= &vccq_sdhi##idx##_init_data,			\
> +};
> +
> +SDHI_REGULATOR(0, RCAR_GP_PIN(5, 24), RCAR_GP_PIN(5, 29));
> +SDHI_REGULATOR(2, RCAR_GP_PIN(5, 25), RCAR_GP_PIN(5, 30));
> +
>  /* MMCIF */
>  static const struct sh_mmcif_plat_data mmcif1_pdata __initconst = {
>  	.caps		= MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE,
> @@ -198,6 +265,9 @@ static const struct pinctrl_map lager_pinctrl_map[] = {
> 
>  static void __init lager_add_standard_devices(void)
>  {
> +	int fixed_regulator_idx = 0;
> +	int gpio_regulator_idx = 0;
> +
>  	r8a7790_clock_init();
> 
>  	pinctrl_register_mappings(lager_pinctrl_map,
> @@ -211,7 +281,8 @@ static void __init lager_add_standard_devices(void)
>  	platform_device_register_data(&platform_bus, "gpio-keys", -1,
>  				      &lager_keys_pdata,
>  				      sizeof(lager_keys_pdata));
> -	regulator_register_always_on(0, "fixed-3.3V", fixed3v3_power_consumers,
> +	regulator_register_always_on(fixed_regulator_idx++,
> +				     "fixed-3.3V", fixed3v3_power_consumers,
>  				     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
>  	platform_device_register_resndata(&platform_bus, "sh_mmcif", 1,
>  					  mmcif1_resources, ARRAY_SIZE(mmcif1_resources),
> @@ -223,6 +294,16 @@ static void __init lager_add_standard_devices(void)
>  					  &ether_pdata, sizeof(ether_pdata));
> 
>  	lager_add_du_device();
> +
> +	platform_device_register_data(&platform_bus, "reg-fixed-voltage",
> fixed_regulator_idx++,
> +				      &vcc_sdhi0_info, sizeof(struct fixed_voltage_config));
> +	platform_device_register_data(&platform_bus, "reg-fixed-voltage",
> fixed_regulator_idx++,
> +				      &vcc_sdhi2_info, sizeof(struct fixed_voltage_config));
> +
> +	platform_device_register_data(&platform_bus, "gpio-regulator",
> gpio_regulator_idx++,
> +				      &vccq_sdhi0_info, sizeof(struct
> gpio_regulator_config));
> +	platform_device_register_data(&platform_bus, "gpio-regulator",
> gpio_regulator_idx++,
> +				      &vccq_sdhi2_info, sizeof(struct
> gpio_regulator_config)); }
> 
>  /*
-- 
Regards,

Laurent Pinchart