[alsa-devel] [PATCH v2 1/2] ASoC: wm8985: add device tree binding for WM8985

Add device tree binding for the WM8985 codec driver.
Signed-off-by: Petr Kulhavy petr@barix.com Acked-by: Rob Herring robh@kernel.org --- v1: initial v2: no change
Documentation/devicetree/bindings/sound/wm8985.txt | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/wm8985.txt
diff --git a/Documentation/devicetree/bindings/sound/wm8985.txt b/Documentation/devicetree/bindings/sound/wm8985.txt new file mode 100644 index 000000000000..788d64766257 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/wm8985.txt @@ -0,0 +1,36 @@ +Wolfson Microelectronics WM8985 and WM8758 audio codecs + +These devices support both I2C and SPI (configured with pin strapping +on the board). + +Required properties: + + - compatible : "wlf,wm8985" or "wlf,wm8758" + + - reg : the I2C address of the device for I2C, the chip select + number for SPI. + +Pins on the device for linking into audio routes: + + * LIN : left microphone pre-amp negative input + * LIP : left microphone pre-amp positive input + * RIN : right microphone pre-amp negative input + * RIP : right microphone pre-amp positive input + * L2 : left line input / secondary pre-amp positive input + * R2 : right line input / secondary pre-amp positive input + * AUXL : left auxiliary input (WM8985 only) + * AUXR : right auxiliary input (WM8985 only) + + * HPL : left headphone / line output (the LOUT1 pin) + * HPR : right headphone / line output (the ROUT1 pin) + * SPKL : left headphone / line output (the LOUT2 pin) + * SPKR : right headphone / line output (the ROUT2 pin) + * Mic Bias : microphone bias + + +Example: + +wm8758: audio-codec@1a { + compatible = "wlf,wm8758"; + reg = <0x1a>; +};

Add device-tree support to the WM8985 driver.
Signed-off-by: Petr Kulhavy petr@barix.com --- v1: initial v2: add missing of_match_ptr() use chip type enum instead of chip structure
sound/soc/codecs/wm8985.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/wm8985.c b/sound/soc/codecs/wm8985.c index 436c7e2c5a0b..0b2dd2bc6fff 100644 --- a/sound/soc/codecs/wm8985.c +++ b/sound/soc/codecs/wm8985.c @@ -32,6 +32,7 @@ #include <sound/soc.h> #include <sound/initval.h> #include <sound/tlv.h> +#include <linux/of_device.h>
#include "wm8985.h"
@@ -1174,10 +1175,21 @@ static const struct regmap_config wm8985_regmap = { .num_reg_defaults = ARRAY_SIZE(wm8985_reg_defaults), };
+static enum wm8985_type wm8985_data = WM8985; +static enum wm8985_type wm8758_data = WM8758; + +static const struct of_device_id wm8985_of_match[] = { + { .compatible = "wlf,wm8985", .data = &wm8985_data}, + { .compatible = "wlf,wm8758", .data = &wm8758_data}, + { } +}; +MODULE_DEVICE_TABLE(of, wm8985_of_match); + #if defined(CONFIG_SPI_MASTER) static int wm8985_spi_probe(struct spi_device *spi) { struct wm8985_priv *wm8985; + const struct of_device_id *of_id; int ret;
wm8985 = devm_kzalloc(&spi->dev, sizeof *wm8985, GFP_KERNEL); @@ -1186,7 +1198,11 @@ static int wm8985_spi_probe(struct spi_device *spi)
spi_set_drvdata(spi, wm8985);
- wm8985->dev_type = WM8985; + of_id = of_match_device(wm8985_of_match, &spi->dev); + if (of_id) + wm8985->dev_type = *((enum wm8985_type *)of_id->data); + else + wm8985->dev_type = WM8985;
wm8985->regmap = devm_regmap_init_spi(spi, &wm8985_regmap); if (IS_ERR(wm8985->regmap)) { @@ -1210,6 +1226,7 @@ static int wm8985_spi_remove(struct spi_device *spi) static struct spi_driver wm8985_spi_driver = { .driver = { .name = "wm8985", + .of_match_table = of_match_ptr(wm8985_of_match), }, .probe = wm8985_spi_probe, .remove = wm8985_spi_remove @@ -1221,6 +1238,7 @@ static int wm8985_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { struct wm8985_priv *wm8985; + const struct of_device_id *of_id; int ret;
wm8985 = devm_kzalloc(&i2c->dev, sizeof *wm8985, GFP_KERNEL); @@ -1229,7 +1247,11 @@ static int wm8985_i2c_probe(struct i2c_client *i2c,
i2c_set_clientdata(i2c, wm8985);
- wm8985->dev_type = id->driver_data; + of_id = of_match_device(wm8985_of_match, &i2c->dev); + if (of_id) + wm8985->dev_type = *((enum wm8985_type *)of_id->data); + else + wm8985->dev_type = id->driver_data;
wm8985->regmap = devm_regmap_init_i2c(i2c, &wm8985_regmap); if (IS_ERR(wm8985->regmap)) { @@ -1260,6 +1282,7 @@ MODULE_DEVICE_TABLE(i2c, wm8985_i2c_id); static struct i2c_driver wm8985_i2c_driver = { .driver = { .name = "wm8985", + .of_match_table = of_match_ptr(wm8985_of_match), }, .probe = wm8985_i2c_probe, .remove = wm8985_i2c_remove,

On Tue, May 17, 2016 at 02:49:49PM +0200, Petr Kulhavy wrote:
Add device-tree support to the WM8985 driver.
Signed-off-by: Petr Kulhavy petr@barix.com
v1: initial v2: add missing of_match_ptr() use chip type enum instead of chip structure
sound/soc/codecs/wm8985.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/wm8985.c b/sound/soc/codecs/wm8985.c index 436c7e2c5a0b..0b2dd2bc6fff 100644 --- a/sound/soc/codecs/wm8985.c +++ b/sound/soc/codecs/wm8985.c @@ -32,6 +32,7 @@ #include <sound/soc.h> #include <sound/initval.h> #include <sound/tlv.h> +#include <linux/of_device.h>
#include "wm8985.h"
@@ -1174,10 +1175,21 @@ static const struct regmap_config wm8985_regmap = { .num_reg_defaults = ARRAY_SIZE(wm8985_reg_defaults), };
+static enum wm8985_type wm8985_data = WM8985; +static enum wm8985_type wm8758_data = WM8758;
+static const struct of_device_id wm8985_of_match[] = {
- { .compatible = "wlf,wm8985", .data = &wm8985_data},
- { .compatible = "wlf,wm8758", .data = &wm8758_data},
You can probably just use (void *)WM8985 instead here.
Thanks, Charles

On 20 May 2016 at 18:21, Charles Keepax ckeepax@opensource.wolfsonmicro.com wrote:
On Tue, May 17, 2016 at 02:49:49PM +0200, Petr Kulhavy wrote:
+static enum wm8985_type wm8985_data = WM8985; +static enum wm8985_type wm8758_data = WM8758;
+static const struct of_device_id wm8985_of_match[] = {
{ .compatible = "wlf,wm8985", .data = &wm8985_data},
{ .compatible = "wlf,wm8758", .data = &wm8758_data},
You can probably just use (void *)WM8985 instead here.
I know this pretty safe with integers but can you do this with enums? Doesn't it lead to undefined behaviour?
Regards Petr

On Mon, May 23, 2016 at 09:23:05AM +0200, Petr Kulhavy wrote:
On 20 May 2016 at 18:21, Charles Keepax ckeepax@opensource.wolfsonmicro.com wrote:
On Tue, May 17, 2016 at 02:49:49PM +0200, Petr Kulhavy wrote:
+static enum wm8985_type wm8985_data = WM8985; +static enum wm8985_type wm8758_data = WM8758;
+static const struct of_device_id wm8985_of_match[] = {
{ .compatible = "wlf,wm8985", .data = &wm8985_data},
{ .compatible = "wlf,wm8758", .data = &wm8758_data},
You can probably just use (void *)WM8985 instead here.
I know this pretty safe with integers but can you do this with enums? Doesn't it lead to undefined behaviour?
Hmm... its not uncommon that it is done in the kernel, but that is rarely an indicator of if something is actually technically valid or not. Casting from an integer to a pointer is technically implementation defined, but we clearly rely on gcc's implementation in the kernel. However, without actually reading the spec in more detail I am unclear on if an enum counts as an integer here, my suspicion is that it would since it generally always implicitly casts to an int.
I guess the correct fix is that .data should probably be a kernel_ulong_t in of_device_id. But if no one else objects I am happy for you to leave your current code as is.
Thanks, Charles

On 23 May 2016 at 10:31, Charles Keepax ckeepax@opensource.wolfsonmicro.com wrote:
On Mon, May 23, 2016 at 09:23:05AM +0200, Petr Kulhavy wrote:
On 20 May 2016 at 18:21, Charles Keepax ckeepax@opensource.wolfsonmicro.com wrote:
On Tue, May 17, 2016 at 02:49:49PM +0200, Petr Kulhavy wrote:
+static enum wm8985_type wm8985_data = WM8985; +static enum wm8985_type wm8758_data = WM8758;
+static const struct of_device_id wm8985_of_match[] = {
{ .compatible = "wlf,wm8985", .data = &wm8985_data},
{ .compatible = "wlf,wm8758", .data = &wm8758_data},
You can probably just use (void *)WM8985 instead here.
I know this pretty safe with integers but can you do this with enums? Doesn't it lead to undefined behaviour?
Hmm... its not uncommon that it is done in the kernel, but that is rarely an indicator of if something is actually technically valid or not. Casting from an integer to a pointer is technically implementation defined, but we clearly rely on gcc's implementation in the kernel. However, without actually reading the spec in more detail I am unclear on if an enum counts as an integer here, my suspicion is that it would since it generally always implicitly casts to an int.
I guess the correct fix is that .data should probably be a kernel_ulong_t in of_device_id. But if no one else objects I am happy for you to leave your current code as is.
If I understand this Stack Overflow answer right: http://stackoverflow.com/a/2331327 The enum should be casted to an integer of size <= size of a pointer for the architecture. So this should be more or less ok? .data = (void*)(int)WM8985
Thanks Petr

On Tue, May 17, 2016 at 02:49:48PM +0200, Petr Kulhavy wrote:
Add device tree binding for the WM8985 codec driver.
Signed-off-by: Petr Kulhavy petr@barix.com Acked-by: Rob Herring robh@kernel.org
v1: initial v2: no change
Documentation/devicetree/bindings/sound/wm8985.txt | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/wm8985.txt
diff --git a/Documentation/devicetree/bindings/sound/wm8985.txt b/Documentation/devicetree/bindings/sound/wm8985.txt new file mode 100644 index 000000000000..788d64766257 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/wm8985.txt @@ -0,0 +1,36 @@ +Wolfson Microelectronics WM8985 and WM8758 audio codecs
+These devices support both I2C and SPI (configured with pin strapping +on the board).
+Required properties:
- compatible : "wlf,wm8985" or "wlf,wm8758"
- reg : the I2C address of the device for I2C, the chip select
number for SPI.
+Pins on the device for linking into audio routes:
- LIN : left microphone pre-amp negative input
- LIP : left microphone pre-amp positive input
- RIN : right microphone pre-amp negative input
- RIP : right microphone pre-amp positive input
- L2 : left line input / secondary pre-amp positive input
- R2 : right line input / secondary pre-amp positive input
- AUXL : left auxiliary input (WM8985 only)
- AUXR : right auxiliary input (WM8985 only)
- HPL : left headphone / line output (the LOUT1 pin)
- HPR : right headphone / line output (the ROUT1 pin)
- SPKL : left headphone / line output (the LOUT2 pin)
- SPKR : right headphone / line output (the ROUT2 pin)
- Mic Bias : microphone bias
+Example:
+wm8758: audio-codec@1a {
- compatible = "wlf,wm8758";
- reg = <0x1a>;
+};
You should probably mention the regulators as well.
Thanks, Charles

On 20 May 2016 at 18:14, Charles Keepax ckeepax@opensource.wolfsonmicro.com wrote:
You should probably mention the regulators as well.
Thanks, I will add them.
Petr
participants (2)
-
Charles Keepax
-
Petr Kulhavy