Re: [PATCH 12/19] twl4030: mfd_cell is now implicitly available todrivers

[Andres Salomon]

On Thu, 3 Feb 2011 09:31:54 +0000
Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:

> On Wed, Feb 02, 2011 at 11:03:26PM -0800, Andres Salomon wrote:
> > Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote:
> 
> > > Then they are doing it incorrectly. One possible way is to have
> > > parent device carry relevant data in its drvdata and have
> > > children get it from there.
> 
> > I believe some drivers are even using the parent device already.
> > See drivers/leds/leds-mc13783.c, for example, whose parent device
> > drvdata is used to pass around a struct mc13783 to its children.
> > Sounds like a possibility, will need to look into it further.
> 
> That's the current best practice approach.

One of the main reasons to have the cell data in the platform
device rather than its parent device is because the parent device will
have the entire array of cells.  This isn't helpful when you want to
know specifically which cell's .enable hook to call.  One possibility
is to use the pdev->id field.  Currently, mfd-core allows drivers to
override this per-cell; depending upon how drivers make use of this,
I'm tempted to get rid of it from the mfd_cell struct and just always
have it be an index into the mfd_cell array (dictated by
mfd_add_devices).  Of course, wm831x-core/wm831x-dcdc does some pretty
weird stuff with ->id, so I still need to figure out if what it's doing
is really necessary.

I'm also struggling with a way to have cells automatically saved
by mfd-core. One (ugly) option would be to allow an mfd driver to set
drvdata, and mfd-core (in mfd_add_devices) would allocate a struct that
includes a pointer to drvdata as well as to the mfd_cells.  Drvdata
would be updated to point to this new struct instead.  In one scenario,
this requires evil macro redefining; in another, a weird API addition
to mfd_add_devices to pass the size of what's pointed to by the
parent's drvdata.  I don't like this option very much.

Another option is to require structs that mfd drivers assign to
drvdata to include a pointer to cells, but I'd really like a way to
enforce that with the compiler (BUILD_BUG_ON comes to mind).
It also runs into the problem of not knowing the type in drvdata. I'm
imagining something like:

#define mfd_get_cell(pdev, type) (((type *)platform_get_drvdata(pdev))->cell)

Even that would require the weird API of mfd_add_devices needing to be
passed the type of what's pointed to by the parent's drvdata.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/