lkml.org 
[lkml]   [2008]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 08/13] mfd: Core support for the WM8350 AudioPlus PMIC
    From
    Date
    On Mon, 2008-10-06 at 13:38 +0100, Mark Brown wrote:

    > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
    > index 0541dfb..c32d676 100644
    > --- a/drivers/mfd/Kconfig
    > +++ b/drivers/mfd/Kconfig
    > @@ -86,6 +86,53 @@ config MFD_WM8400
    > the device, additional drivers must be enabled in order to use
    > the functionality of the device.
    >
    > +config MFD_WM8350
    > + tristate
    > +
    > +config MFD_WM8350_CONFIG_MODE_0
    > + bool "Support WM8350 in configuration mode 0"
    > + depends on MFD_WM8350
    > + default y
    > + help
    > + The WM8350 offers four configuration modes with different
    > + initial register states. This option enables support for the
    > + WM8350 in mode 0.
    > +
    > + If unsure say Y

    I would make the WM8350 mode configuration selectable by the target
    machines Kconfig rather than let the user choose. Wrong choices would
    probably mean broken hardware.

    >
    > menu "Multimedia Capabilities Port drivers"
    > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
    > index 172439d..b0adca0 100644
    > --- a/drivers/mfd/Makefile
    > +++ b/drivers/mfd/Makefile
    > @@ -13,6 +13,8 @@ obj-$(CONFIG_MFD_TC6387XB) += tc6387xb.o
    > obj-$(CONFIG_MFD_TC6393XB) += tc6393xb.o
    >
    > obj-$(CONFIG_MFD_WM8400) += wm8400-core.o
    > +wm8350-objs := wm8350-core.o wm8350-regmap.o
    > +obj-$(CONFIG_MFD_WM8350) += wm8350.o
    >
    > obj-$(CONFIG_MFD_CORE) += mfd-core.o
    >
    > diff --git a/drivers/mfd/wm8350-core.c b/drivers/mfd/wm8350-core.c
    > new file mode 100644
    > index 0000000..c7552c0
    > --- /dev/null
    > +++ b/drivers/mfd/wm8350-core.c

    > +
    > +/*
    > + * Cache is always host endian.
    > + */
    > +static int wm8350_create_cache(struct wm8350 *wm8350, int mode)
    > +{
    > + int i, ret = 0;
    > + u16 value;
    > + const u16 *reg_map;
    > +
    > + switch (mode) {
    > +#ifdef CONFIG_MFD_WM8350_CONFIG_MODE_0
    > + case 0:
    > + reg_map = wm8350_mode0_defaults;
    > + break;
    > +#endif
    > +#ifdef CONFIG_MFD_WM8350_CONFIG_MODE_1
    > + case 1:
    > + reg_map = wm8350_mode1_defaults;
    > + break;
    > +#endif
    > +#ifdef CONFIG_MFD_WM8350_CONFIG_MODE_2
    > + case 2:
    > + reg_map = wm8350_mode2_defaults;
    > + break;
    > +#endif
    > +#ifdef CONFIG_MFD_WM8350_CONFIG_MODE_3
    > + case 3:
    > + reg_map = wm8350_mode3_defaults;
    > + break;
    > +#endif

    Shouldn't this be #elif for each mode ?

    Ditto for the default register values. I would also #error if no mode
    was selected just to make it's correctly set by machine/board authors.

    Liam




    \
     
     \ /
      Last update: 2008-10-09 14:09    [W:0.036 / U:60.752 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site