From: broonie@opensource.wolfsonmicro.com (Mark Brown)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 04/17] ASoC: Add device tree binding for WM8903
Date: Wed, 23 Nov 2011 10:20:28 +0000 [thread overview]
Message-ID: <20111123102027.GB4332@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <1322011285-4002-5-git-send-email-swarren@nvidia.com>
On Tue, Nov 22, 2011 at 06:21:12PM -0700, Stephen Warren wrote:
> + - irq-active-low : Indicates whether the IRQ output should be active low
> + (property present) or active high (property absent).
I think we ought to be coming up with a standard binding for this stuff
rather than having every device defining it's own way of plugging the
interrupt lines together - many devices have lots of flexibility with
how their interrupt line signals for compatibility reasons.
> + - gpio-cfg : A list of GPIO pin mux register values. The list must be 5
> + entries long. If absent, no configuration of these registers is
> + performed.
What's a "GPIO pin mux"?
> + /* 0x8000 = Not configured */
> + gpio-cfg = < 0x8000 0x8000 0 0x8000 0x8000 >;
The 0x8000 isn't documented in the binding and this doesn't seem
terribly idiomatic for device tree.
> -static void wm8903_init_gpio(struct snd_soc_codec *codec)
> +static void wm8903_init_gpio(struct snd_soc_codec *codec,
> + struct wm8903_platform_data *pdata)
Why?
> static int wm8903_probe(struct snd_soc_codec *codec)
> {
> struct wm8903_platform_data *pdata = dev_get_platdata(codec->dev);
> + struct wm8903_platform_data lpdata;
Why and what is "lpdata" supposed to mean?
> + if (!pdata && codec->dev->of_node) {
> + lpdata.irq_active_low = 0;
> + lpdata.micdet_cfg = 0;
This should be set by default.
> + lpdata.micdet_delay = 100;
> + lpdata.gpio_base = -1;
This means that the defaults are in different different places if we
have platform data and if we have device tree. We should restructure
the code so that we only have defaults in one place.
> + if (of_property_read_u32(np, "micdet-cfg", &val32) >= 0)
> + lpdata.micdet_cfg = val32;
I'd rather have defaults as an else case I think.
> +#if defined(CONFIG_OF)
ifdef.
next prev parent reply other threads:[~2011-11-23 10:20 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-23 1:21 [PATCH 00/17] ASoC: Add Tegra DT, cleanup, and related Stephen Warren
2011-11-23 1:21 ` [PATCH 01/17] arm/tegra: board-dt: audio: Enable clocks, fix AUXDATA Stephen Warren
2011-11-23 10:38 ` Mark Brown
2011-11-23 17:44 ` Stephen Warren
2011-11-23 22:01 ` Olof Johansson
2011-11-23 1:21 ` [PATCH 02/17] arm/dt: Tegra: Clean up I2S and DAS nodes Stephen Warren
2011-11-23 1:21 ` [PATCH 03/17] arm/dt: Tegra: Enable audio on WM8903 boards, disable others Stephen Warren
2011-11-23 16:50 ` Stephen Warren
2011-11-23 1:21 ` [PATCH 04/17] ASoC: Add device tree binding for WM8903 Stephen Warren
2011-11-23 10:20 ` Mark Brown [this message]
2011-11-23 1:21 ` [PATCH 05/17] ASoC: Tegra: Move DAS configuration into machine drivers Stephen Warren
2011-11-23 10:24 ` Mark Brown
2011-11-23 1:21 ` [PATCH 06/17] ASoC: Tegra PCM: Use module_platform_driver Stephen Warren
2011-11-23 10:24 ` Mark Brown
2011-11-23 1:21 ` [PATCH 07/17] ASoC: Tegra DAS: Use devm_ APIs and module_platform_driver Stephen Warren
2011-11-23 6:58 ` Thierry Reding
2011-11-23 10:23 ` Mark Brown
2011-11-23 17:29 ` [alsa-devel] " Stephen Warren
2011-11-23 20:40 ` Thierry Reding
2011-11-23 10:25 ` Mark Brown
2011-11-23 1:21 ` [PATCH 08/17] ASoC: Tegra I2S: " Stephen Warren
2011-11-23 7:00 ` Thierry Reding
2011-11-23 10:25 ` Mark Brown
2011-11-23 1:21 ` [PATCH 09/17] ASoC: Tegra I2S: Remove dependency on pdev->id Stephen Warren
2011-11-23 11:03 ` Mark Brown
2011-11-23 17:54 ` Stephen Warren
2011-11-23 18:03 ` Mark Brown
2011-11-23 1:21 ` [PATCH 10/17] ASoC: Tegra DAS: Add device tree binding Stephen Warren
2011-11-23 11:07 ` Mark Brown
2011-11-23 1:21 ` [PATCH 11/17] ASoC: Tegra I2S: " Stephen Warren
2011-11-23 7:04 ` Thierry Reding
2011-11-23 10:48 ` Mark Brown
2011-11-23 10:57 ` Thierry Reding
2011-11-23 11:27 ` Mark Brown
2011-11-23 1:21 ` [PATCH 12/17] ASoC: Tegra+WM8903 machine: Use devm_ APIs and module_platform_driver Stephen Warren
2011-11-23 7:05 ` Thierry Reding
2011-11-23 11:08 ` Mark Brown
2011-11-23 1:21 ` [PATCH 13/17] ASoC: Tegra TrimSlice " Stephen Warren
2011-11-23 7:06 ` Thierry Reding
2011-11-23 11:11 ` Mark Brown
2011-11-23 1:21 ` [PATCH 14/17] ASoC: Implement "auto nc pins" feature Stephen Warren
2011-11-23 11:15 ` Mark Brown
2011-11-23 1:21 ` [PATCH 15/17] ASoC: Tegra+WM903 machine: Use new auto_nc_codec_pins feature Stephen Warren
2011-11-23 1:21 ` [PATCH 16/17] ASoC: TrimSlice " Stephen Warren
2011-11-23 1:21 ` [PATCH 17/17] ASoC: Tegra+WM8903 machine: Add device tree binding Stephen Warren
2011-11-23 7:15 ` Thierry Reding
2011-11-23 11:26 ` Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111123102027.GB4332@opensource.wolfsonmicro.com \
--to=broonie@opensource.wolfsonmicro.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).