> -----Original Message----- > From: Mike Rapoport [mailto:mike.rapoport@xxxxxxxxx] > Sent: Friday, November 06, 2009 2:46 AM > To: Aggarwal, Anuj > Cc: linux-omap@xxxxxxxxxxxxxxx; broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx; > lrg@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 4/5] Regulator: Adding OMAP3EVM/TWL4030 specific code > in board-omap35x-pmic.c > > On Thu, Nov 5, 2009 at 6:39 PM, Anuj Aggarwal <anuj.aggarwal@xxxxxx> > wrote: > > Adding various regulator-consumers for OMAP3EVM-TWL4030 combination > > in board-omap35x-pmic.c. Also, populating the respective fields > > for omap3evm_twldata structure. > > > > Signed-off-by: Anuj Aggarwal <anuj.aggarwal@xxxxxx> > > --- > > arch/arm/mach-omap2/board-omap35x-pmic.c | 81 > +++++++++++++++++++++++++++++- > > arch/arm/mach-omap2/board-omap3evm.c | 2 +- > > 2 files changed, 80 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm/mach-omap2/board-omap35x-pmic.c b/arch/arm/mach- > omap2/board-omap35x-pmic.c > > index aae07ab..2ef4932 100644 > > --- a/arch/arm/mach-omap2/board-omap35x-pmic.c > > +++ b/arch/arm/mach-omap2/board-omap35x-pmic.c > > @@ -24,10 +24,87 @@ > > * Definitions specific to TWL4030/TPS65950 > > */ > > #if defined(CONFIG_PMIC_TWL4030) > > -static inline void pmic_twl4030_init(void) > > +#if defined(CONFIG_MACH_OMAP3EVM) > > What about beagle, overo and others that use the same regulator for > the same purposes? [Aggarwal, Anuj] This file can be used to accommodate all the common Regulator f/w related code for OMAP3 based platforms. Depending upon the regulators' usage across EVMs, the code can be split in generic and platform specific code. > > > +#include <linux/i2c/twl4030.h> > > + > > +extern struct twl4030_platform_data omap3evm_twldata; > > The *twldata does not have to be global, it can be passed to pmic_init > as a parameter. > > > +/* VDAC */ > > +static struct regulator_consumer_supply vdac_consumers[] = { > > + { > > + .supply = "dac", > > + }, > > +}; > > + > > +static struct regulator_init_data vdac_data = { > > + .constraints = { > > + .name = "VDAC", > > + .min_uV = 1800000, > > + .max_uV = 1800000, > > + .apply_uV = true, > > + .valid_modes_mask = REGULATOR_MODE_NORMAL > > + | REGULATOR_MODE_STANDBY, > > + .valid_ops_mask = REGULATOR_CHANGE_MODE > > + | REGULATOR_CHANGE_STATUS, > > + }, > > + .num_consumer_supplies = ARRAY_SIZE(vdac_consumers), > > + .consumer_supplies = vdac_consumers, > > +}; > > + > > +/* VPLL2 */ > > +static struct regulator_consumer_supply vpll2_consumers[] = { > > + { > > + .supply = "lcd", > > + }, > > + { > > + .supply = "sdi", > > + }, > > +}; > > + > > +static struct regulator_init_data vpll2_data = { > > + .constraints = { > > + .name = "VPLL2", > > + .min_uV = 1800000, > > + .max_uV = 1800000, > > + .apply_uV = true, > > + .valid_modes_mask = REGULATOR_MODE_NORMAL > > + | REGULATOR_MODE_STANDBY, > > + .valid_ops_mask = REGULATOR_CHANGE_MODE > > + | REGULATOR_CHANGE_STATUS, > > + }, > > + .num_consumer_supplies = ARRAY_SIZE(vpll2_consumers), > > + .consumer_supplies = vpll2_consumers, > > +}; > > + > > +/* VMMC1 */ > > +struct regulator_consumer_supply vmmc1_consumers[] = { > > + { > > + .supply = "mmc", > > + }, > > +}; > > + > > +static struct regulator_init_data vmmc1_data = { > > + .constraints = { > > + .name = "VMMC1", > > + .min_uV = 1850000, > > + .max_uV = 3150000, > > + .valid_modes_mask = REGULATOR_MODE_NORMAL > > + | REGULATOR_MODE_STANDBY, > > + .valid_ops_mask = REGULATOR_CHANGE_VOLTAGE > > + | REGULATOR_CHANGE_MODE | > REGULATOR_CHANGE_STATUS, > > + }, > > + .num_consumer_supplies = ARRAY_SIZE(vmmc1_consumers), > > + .consumer_supplies = vmmc1_consumers, > > +}; > > + > > +static void __init pmic_twl4030_init(void) > > { > > - /* TWL4030 specific init code */ > > + /* Initialize the regulator specific fields here */ > > + omap3evm_twldata.vdac = &vdac_data; > > + omap3evm_twldata.vpll2 = &vpll2_data; > > + omap3evm_twldata.vmmc1 = &vmmc1_data; > > } > > +#endif /* CONFIG_MACH_OMAP3EVM */ > > I don't see why would you move board specific code from board specific > file to some "generic" file and add #ifdefs to enable this code only > for that board. Indeed, many OMAP3 boards use TWL/TPS in very similar > way and it does make sence to factor the common code out. But with > your approach each board will have to add its own #ifdef with almost > identical code inside it. [Aggarwal, Anuj] As explained above, same code can be used for OMAP3 based platforms having the same regulator requirements. > > > #else > > static inline void pmic_twl4030_init(void) > > { > > diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach- > omap2/board-omap3evm.c > > index dbdf062..10ac0d2 100644 > > --- a/arch/arm/mach-omap2/board-omap3evm.c > > +++ b/arch/arm/mach-omap2/board-omap3evm.c > > @@ -197,7 +197,7 @@ static struct twl4030_madc_platform_data > omap3evm_madc_data = { > > .irq_line = 1, > > }; > > > > -static struct twl4030_platform_data omap3evm_twldata = { > > +struct twl4030_platform_data omap3evm_twldata = { > > .irq_base = TWL4030_IRQ_BASE, > > .irq_end = TWL4030_IRQ_END, > > > > -- > > 1.6.2.4 > > > > -- > > 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 > > > > > > -- > Sincerely Yours, > Mike. -- 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