Re: [PATCH 1/2] mfd: Core support for the WM8400 AudioPlus HiFiCODEC and PMU

[Mark Brown]

On Fri, Sep 12, 2008 at 12:54:19PM +0200, Samuel Ortiz wrote:

> > +config MFD_WM8400
> > +	tristate "Support Wolfson Microelectronics WM8400"
> > +	select MFD_CORE
> It seems you're not using the mfd-core API currently.
> Maybe we could get rid of this select for now ?

Done, will be in the next version I push out.

> > --- /dev/null
> > +++ b/include/linux/mfd/wm8400-private.h
> Just a remark here: many chunks from this file are really wm8400 private and
> could live in drivers/mfd/wm8400.h.

Which chunks in particular?  Note that I've not currently included the
audio driver - it will need much more interaction with the core than the
regulator support does and it doesn't feel right to have it peer into
drivers/mfd for things it needs.  While I have split much of that into a
separate audio header file for the next submission there are several
areas which cross over multiple functions.

> I would also prefer to have one single wm8400.h file in include/linux/mfd/,
> containing both those definitions and the ones you're adding with the
> regulator patch.

The goal in splitting out wm8400.h from wm8400-private.h was to ensure
that the (fairly small) interface intended for use by clients of the
driver was clearly separated from the interfaces intended for use within
the WM8400 drivers themselves.  I'd like to keep this separation since
it makes it clearer to users which interfaces they should be using and
having to include a separate file makes this very easy to spot on
review.
--
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/