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@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > Cc: Eduardo Valentin <eduardo.valentin@xxxxxxxxx>
> > Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx>
> > ---
> > 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,
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
