[PATCH 17/18] omap: rx51: Add supplies for the tlv320aic3x codec driver

[Eduardo Valentin]

On Thu, May 06, 2010 at 08:57:16AM +0200, Valentin Eduardo (Nokia-D/Helsinki) wrote:
> Hello Jarkko,
> 
> Just minor comments on this one, sorry for being late for that.
> 
> On Wed, May 05, 2010 at 09:33:23PM +0200, ext Tony Lindgren wrote:
> > From: Jarkko Nikula <jhnikula at gmail.com>
> > 
> > Upcoming change to tlv320aic3x codec driver require four supplies.
> > Implement this by connecting analogic supplies to TWL4030 VMMC2 and digital
> > supplies to TWL4030 VIO.
> > 
> > Signed-off-by: Jarkko Nikula <jhnikula at gmail.com>
> > Cc: Eduardo Valentin <eduardo.valentin at nokia.com>
> > Signed-off-by: Tony Lindgren <tony at atomide.com>
> > ---
> >  arch/arm/mach-omap2/board-rx51-peripherals.c |   60 +++++++++++++++++++++++---
> >  1 files changed, 53 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
> > index 3addfe6..8179d55 100644
> > --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
> > +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
> > @@ -277,7 +277,7 @@ static struct regulator_consumer_supply rx51_vmmc1_supply = {
> >  	.dev_name = "mmci-omap-hs.0",
> >  };
> >  
> > -static struct regulator_consumer_supply rx51_vmmc2_supply = {
> > +static struct regulator_consumer_supply rx51_vaux3_supply = {
> >  	.supply   = "vmmc",
> >  	.dev_name = "mmci-omap-hs.1",
> >  };
> > @@ -287,6 +287,35 @@ static struct regulator_consumer_supply rx51_vsim_supply = {
> >  	.dev_name = "mmci-omap-hs.1",
> >  };
> >  
> > +static struct regulator_consumer_supply rx51_vmmc2_supplies[] = {
> > +	/* tlv320aic3x analog supplies */
> > +	{
> > +		.supply		= "AVDD",
> > +		.dev_name	= "2-0018",
> > +	},
> > +	{
> > +		.supply		= "DRVDD",
> > +		.dev_name	= "2-0018",
> > +	},
> > +	/* Keep vmmc as last item. It is not iterated for newer boards */
> > +	{
> > +		.supply		= "vmmc",
> > +		.dev_name	= "mmci-omap-hs.1",
> > +	},
> > +};
> > +
> > +static struct regulator_consumer_supply rx51_vio_supplies[] = {
> > +	/* tlv320aic3x digital supplies */
> > +	{
> > +		.supply		= "IOVDD",
> > +		.dev_name	= "2-0018"
> > +	},
> > +	{
> > +		.supply		= "DVDD",
> > +		.dev_name	= "2-0018"
> > +	},
> > +};
> 
> 
> This isn't mandatory, but I find the code more readable if you use the REGULATOR_SUPPLY macro,
> which kinda suitable for cases like yours, where you are passing the pair supply&dev_name.
> > +
> >  static struct regulator_init_data rx51_vaux1 = {
> >  	.constraints = {
> >  		.name			= "V28",
> > @@ -338,7 +367,7 @@ static struct regulator_init_data rx51_vaux3_mmc = {
> >  					| REGULATOR_CHANGE_STATUS,
> >  	},
> >  	.num_consumer_supplies	= 1,
> > -	.consumer_supplies	= &rx51_vmmc2_supply,
> > +	.consumer_supplies	= &rx51_vaux3_supply,
> >  };
> >  
> >  static struct regulator_init_data rx51_vaux4 = {
> > @@ -380,8 +409,8 @@ static struct regulator_init_data rx51_vmmc2 = {
> >  					| REGULATOR_CHANGE_MODE
> >  					| REGULATOR_CHANGE_STATUS,
> >  	},
> > -	.num_consumer_supplies	= 1,
> > -	.consumer_supplies	= &rx51_vmmc2_supply,
> > +	.num_consumer_supplies	= ARRAY_SIZE(rx51_vmmc2_supplies),
> > +	.consumer_supplies	= rx51_vmmc2_supplies,
> >  };
> >  
> >  static struct regulator_init_data rx51_vsim = {
> > @@ -411,6 +440,20 @@ static struct regulator_init_data rx51_vdac = {
> >  	},
> >  };
> >  
> > +static struct regulator_init_data rx51_vio = {
> > +	.constraints = {
> > +		.min_uV			= 1800000,
> > +		.max_uV			= 1800000,
> > +		.valid_modes_mask	= REGULATOR_MODE_NORMAL
> > +					| REGULATOR_MODE_STANDBY,
> > +		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
> 
> I'm not sure if we would ever change voltage level in VIO in rx51 case.
> It could enter sleep mode. But even there it wouldn't change voltage level.
> Except, of cource, during off mode transition. But then, the regfw wouldn't care.

Actually, one correction here, even during off mode transition I believe we need to
keep it, otherwise some wake up source would be screwed.
> 
> 
> > +					| REGULATOR_CHANGE_MODE
> > +					| REGULATOR_CHANGE_STATUS,
> > +	},
> > +	.num_consumer_supplies	= ARRAY_SIZE(rx51_vio_supplies),
> > +	.consumer_supplies	= rx51_vio_supplies,
> > +};
> > +
> >  static int rx51_twlgpio_setup(struct device *dev, unsigned gpio, unsigned n)
> >  {
> >  	/* FIXME this gpio setup is just a placeholder for now */
> > @@ -618,6 +661,7 @@ static struct twl4030_platform_data rx51_twldata __initdata = {
> >  	.vmmc1			= &rx51_vmmc1,
> >  	.vsim			= &rx51_vsim,
> >  	.vdac			= &rx51_vdac,
> > +	.vio			= &rx51_vio,
> >  };
> >  
> >  static struct i2c_board_info __initdata rx51_peripherals_i2c_board_info_1[] = {
> > @@ -638,12 +682,14 @@ static struct i2c_board_info __initdata rx51_peripherals_i2c_board_info_2[] = {
> >  static int __init rx51_i2c_init(void)
> >  {
> >  	if ((system_rev >= SYSTEM_REV_S_USES_VAUX3 && system_rev < 0x100) ||
> > -	    system_rev >= SYSTEM_REV_B_USES_VAUX3)
> > +	    system_rev >= SYSTEM_REV_B_USES_VAUX3) {
> >  		rx51_twldata.vaux3 = &rx51_vaux3_mmc;
> > -	else {
> > +		/* Only older boards use VMMC2 for internal MMC */
> > +		rx51_vmmc2.num_consumer_supplies--;
> > +	} else {
> >  		rx51_twldata.vaux3 = &rx51_vaux3_cam;
> > -		rx51_twldata.vmmc2 = &rx51_vmmc2;
> >  	}
> > +	rx51_twldata.vmmc2 = &rx51_vmmc2;
> >  	omap_register_i2c_bus(1, 2200, rx51_peripherals_i2c_board_info_1,
> >  			      ARRAY_SIZE(rx51_peripherals_i2c_board_info_1));
> >  	omap_register_i2c_bus(2, 100, rx51_peripherals_i2c_board_info_2,
> >