RE: [PATCH 4/5] Regulator: Adding OMAP3EVM/TWL4030 specific code in board-omap35x-pmic.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----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

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux