Re: [PATCH] ARM: EXYNOS4: Configure MAX8997 PMIC for Origen

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Tue, Aug 16, 2011 at 04:38:46PM +0530, Inderpal Singh wrote:
> On 14 August 2011 20:31, Mark Brown <broonie@opensource.wolfsonmicro.com>wrote:
> > On Thu, Aug 11, 2011 at 09:26:05AM +0530, Inderpal Singh wrote:

> > > +static struct regulator_consumer_supply __initdata ldo7_consumer[] = {
> > > +     REGULATOR_SUPPLY("avdd", "soc-audio"), /* Reatek ALC5625*/
> > > +};

> > Ick, no.  The soc-audio device is a virtual device within Linux and is
> > being phased out, any driver adding new soc-audio devices will be
> > rejected.  The CODEC driver should deal with its own power.

> Ok. So does it mean that sound cards will have their own devices? And those
> device names should be listed as consumers?

As I said above "The CODEC driver should deal with its own power."

> > > +static struct i2c_board_info i2c0_devs[] __initdata = {
> > > +[I2C0_MAX8997] = {
> > > +     I2C_BOARD_INFO("max8997", (0xCC >> 1)),
> > > +     .platform_data = &origen_max8997_pdata,
> > > +     },
> > > +};

> > Why are you assigning the array index?  That's very unusual.

> Have seen this kind of initialisation at few places. It looked more
> readable.
> If its not preferred, I will remove it.

You shouldn't need to be looking up individual members in a board info
array in the first place.

> > > +     s3c_i2c0_set_platdata(NULL);
> > > +     i2c0_devs[I2C0_MAX8997].irq = gpio_to_irq(EXYNOS4_GPX0(4));

> > There should be defines to do this statically.

> What advantages do you see in using defines?

It's non-idiomatic to modify the I2C board info at runtime, all the
numbers are known at compile time anyway.