Re: [PATCHv7 5/7] mfd: twl-core: pass driver data from pdata to add_regulator for VDD1 and VDD2

[Tero Kristo <t-kristo@ti.com>]

On Mon, 2011-12-12 at 19:04 +0100, Samuel Ortiz wrote:
> Hi Tero,
> 
> 
> On Mon, Nov 28, 2011 at 04:53:23PM +0200, Tero Kristo wrote:
> > This way, we can add custom flags for VDD1 and VDD2 regulators that
> > get passed all the way to regulator init. This is needed for SMPS
> > regulator support to select used controller mode for these regulators
> > (either voltage processor or default.)
> > 
> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
> > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > ---
> >  drivers/mfd/twl-core.c |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> > index 01ecfee..af93fce 100644
> > --- a/drivers/mfd/twl-core.c
> > +++ b/drivers/mfd/twl-core.c
> > @@ -846,12 +846,14 @@ add_children(struct twl4030_platform_data *pdata, unsigned long features)
> >  			return PTR_ERR(child);
> >  
> >  		child = add_regulator(TWL4030_REG_VDD1, pdata->vdd1,
> > -					features);
> > +					features |
> > +					(u32)pdata->vdd1->driver_data);
> That looks hackish to me. Do you have any guarantee that your driver_data and
> your features bitmaps have zero intersections ?

That is somewhat hackish yes. I changed the implementation a bit for v8
(which you should also have in your inbox now, I sent it out on Friday),
I split the driver_data to a struct that contains a couple of function
pointers and the features flag. It is still using a bitwise OR for
setting the flags inside twl-core.c, but the entity that initializes the
driver_data sets the features always at zero, so twl-core is the only
one who sets these. That version of the patch could be changed from
twl-core point of view to just set the features field instead of OR.

v8 of the patch set actually merges the regulator driver and the
twl-core data passing parts together to avoid merge conflicts, so you
should take a look at the twl-core.c / include/linux/i2c/twl.h part of
patch:

[PATCHv8 4/5] twl4030: add support for external voltage get/set

-Tero