Re: [PATCH 08/13] mfd: Core support for the WM8350 AudioPlus PMIC

From: Liam Girdwood
Date: Thu Oct 09 2008 - 08:07:28 EST


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


--
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/