Re: [PATCH 2/2] mfd: wm8994: Add some OF properties

From: Sylwester Nawrocki
Date: Thu Apr 11 2013 - 13:07:00 EST


On 04/11/2013 06:37 PM, Mark Brown wrote:
> Add properties for some of the more important bits of platform data and
> fill out the binding document.
>
> Not all of the current platform data is suitable for the sort of fixed
> configuration that is done using DT, some of it should have runtime
> mechanisms added instead and some is unlikely to ever be used in practical
> systems.
>
> Signed-off-by: Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
Tested-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>

Couple cosmetics remarks below, just in case you happen to resend the patch
for some other reason.

> ---
> Documentation/devicetree/bindings/sound/wm8994.txt | 58 ++++++++++++++-
> drivers/mfd/wm8994-core.c | 75 +++++++++++++++++++-
> 2 files changed, 130 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/wm8994.txt b/Documentation/devicetree/bindings/sound/wm8994.txt
> index 7a7eb1e..bb731ab 100644
> --- a/Documentation/devicetree/bindings/sound/wm8994.txt
> +++ b/Documentation/devicetree/bindings/sound/wm8994.txt
> @@ -5,14 +5,70 @@ on the board).
>
> Required properties:
>
> - - compatible : "wlf,wm1811", "wlf,wm8994", "wlf,wm8958"
> + - compatible : One of "wlf,wm1811", "wlf,wm8994" or "wlf,wm8958"

Missing dot.

> +Optional properties:

> +
> + - wlf,ldoena-always-driven : If present LDOENA is always driven

Ditto.

> Example:
>
> codec: wm8994@1a {
> compatible = "wlf,wm8994";
> reg = <0x1a>;
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> +
> + lineout1-se;
^^^^^^^^^^^^^

Could be tab instead of spaces.

> +#ifdef CONFIG_OF
> +static int wm8994_set_pdata_from_of(struct wm8994 *wm8994)
> +{
> + struct device_node *np = wm8994->dev->of_node;
> + struct wm8994_pdata *pdata = &wm8994->pdata;
> + int i;
> +
> + if (!np)
> + return 0;
> +
> + if (of_property_read_u32_array(np, "wlf,gpio-cfg", pdata->gpio_defaults,
> + ARRAY_SIZE(pdata->gpio_defaults)) >= 0) {
> + for (i = 0; i < ARRAY_SIZE(pdata->gpio_defaults); i++) {
> + if (wm8994->pdata.gpio_defaults[i] == 0)

Could be simplified to: if (pdata->gpio_defaults[i] == 0)

Thanks for the updated series.

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