All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ASoC: rt5677: Switch to use descriptor-based gpiod API
@ 2015-06-22 18:12 Ben Zhang
  2015-06-22 18:13 ` [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API Ben Zhang
  2015-07-07 13:57 ` Applied "ASoC: rt5677: Switch to use descriptor-based gpiod " Mark Brown
  0 siblings, 2 replies; 9+ messages in thread
From: Ben Zhang @ 2015-06-22 18:12 UTC (permalink / raw)
  To: Mark Brown
  Cc: Oder Chiou, alsa-devel, Liam Girdwood, Ben Zhang, Anatol Pomozov,
	Bard Liao, Dylan Reid

This patch makes the driver use the new descriptor-based gpiod API
so that gpio assignment info can be provided by Device Tree, ACPI
or board files.

Signed-off-by: Ben Zhang <benzh@chromium.org>
---
 sound/soc/codecs/rt5677.c | 85 +++++++++++++++++------------------------------
 sound/soc/codecs/rt5677.h |  5 +--
 2 files changed, 33 insertions(+), 57 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index aba00fd8..c166217 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -15,13 +15,11 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/pm.h>
-#include <linux/of_gpio.h>
 #include <linux/regmap.h>
 #include <linux/i2c.h>
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 #include <linux/firmware.h>
-#include <linux/gpio.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -4761,10 +4759,10 @@ static int rt5677_remove(struct snd_soc_codec *codec)
 	struct rt5677_priv *rt5677 = snd_soc_codec_get_drvdata(codec);
 
 	regmap_write(rt5677->regmap, RT5677_RESET, 0x10ec);
-	if (gpio_is_valid(rt5677->pow_ldo2))
-		gpio_set_value_cansleep(rt5677->pow_ldo2, 0);
-	if (gpio_is_valid(rt5677->reset_pin))
-		gpio_set_value_cansleep(rt5677->reset_pin, 0);
+	if (rt5677->pow_ldo2)
+		gpiod_set_value_cansleep(rt5677->pow_ldo2, 0);
+	if (rt5677->reset_pin)
+		gpiod_set_value_cansleep(rt5677->reset_pin, 0);
 
 	return 0;
 }
@@ -4778,10 +4776,10 @@ static int rt5677_suspend(struct snd_soc_codec *codec)
 		regcache_cache_only(rt5677->regmap, true);
 		regcache_mark_dirty(rt5677->regmap);
 
-		if (gpio_is_valid(rt5677->pow_ldo2))
-			gpio_set_value_cansleep(rt5677->pow_ldo2, 0);
-		if (gpio_is_valid(rt5677->reset_pin))
-			gpio_set_value_cansleep(rt5677->reset_pin, 0);
+		if (rt5677->pow_ldo2)
+			gpiod_set_value_cansleep(rt5677->pow_ldo2, 0);
+		if (rt5677->reset_pin)
+			gpiod_set_value_cansleep(rt5677->reset_pin, 0);
 	}
 
 	return 0;
@@ -4792,12 +4790,11 @@ static int rt5677_resume(struct snd_soc_codec *codec)
 	struct rt5677_priv *rt5677 = snd_soc_codec_get_drvdata(codec);
 
 	if (!rt5677->dsp_vad_en) {
-		if (gpio_is_valid(rt5677->pow_ldo2))
-			gpio_set_value_cansleep(rt5677->pow_ldo2, 1);
-		if (gpio_is_valid(rt5677->reset_pin))
-			gpio_set_value_cansleep(rt5677->reset_pin, 1);
-		if (gpio_is_valid(rt5677->pow_ldo2) ||
-		    gpio_is_valid(rt5677->reset_pin))
+		if (rt5677->pow_ldo2)
+			gpiod_set_value_cansleep(rt5677->pow_ldo2, 1);
+		if (rt5677->reset_pin)
+			gpiod_set_value_cansleep(rt5677->reset_pin, 1);
+		if (rt5677->pow_ldo2 || rt5677->reset_pin)
 			msleep(10);
 
 		regcache_cache_only(rt5677->regmap, false);
@@ -5034,24 +5031,6 @@ static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np)
 	rt5677->pdata.lout3_diff = of_property_read_bool(np,
 					"realtek,lout3-differential");
 
-	rt5677->pow_ldo2 = of_get_named_gpio(np,
-					"realtek,pow-ldo2-gpio", 0);
-	rt5677->reset_pin = of_get_named_gpio(np,
-					"realtek,reset-gpio", 0);
-
-	/*
-	 * POW_LDO2 is optional (it may be statically tied on the board).
-	 * -ENOENT means that the property doesn't exist, i.e. there is no
-	 * GPIO, so is not an error. Any other error code means the property
-	 * exists, but could not be parsed.
-	 */
-	if (!gpio_is_valid(rt5677->pow_ldo2) &&
-			(rt5677->pow_ldo2 != -ENOENT))
-		return rt5677->pow_ldo2;
-	if (!gpio_is_valid(rt5677->reset_pin) &&
-			(rt5677->reset_pin != -ENOENT))
-		return rt5677->reset_pin;
-
 	of_property_read_u8_array(np, "realtek,gpio-config",
 		rt5677->pdata.gpio_config, RT5677_GPIO_NUM);
 
@@ -5155,30 +5134,26 @@ static int rt5677_i2c_probe(struct i2c_client *i2c,
 		rt5677->reset_pin = -EINVAL;
 	}
 
-	if (gpio_is_valid(rt5677->pow_ldo2)) {
-		ret = devm_gpio_request_one(&i2c->dev, rt5677->pow_ldo2,
-					    GPIOF_OUT_INIT_HIGH,
-					    "RT5677 POW_LDO2");
-		if (ret < 0) {
-			dev_err(&i2c->dev, "Failed to request POW_LDO2 %d: %d\n",
-				rt5677->pow_ldo2, ret);
-			return ret;
-		}
+	/* pow-ldo2 and reset are optional. The codec pins may be statically
+	 * connected on the board without gpios. If the gpio device property
+	 * isn't specified, devm_gpiod_get_optional returns NULL.
+	 */
+	rt5677->pow_ldo2 = devm_gpiod_get_optional(&i2c->dev,
+			"realtek,pow-ldo2", GPIOD_OUT_HIGH);
+	if (IS_ERR(rt5677->pow_ldo2)) {
+		ret = PTR_ERR(rt5677->pow_ldo2);
+		dev_err(&i2c->dev, "Failed to request POW_LDO2: %d\n", ret);
+		rt5677->pow_ldo2 = 0;
 	}
-
-	if (gpio_is_valid(rt5677->reset_pin)) {
-		ret = devm_gpio_request_one(&i2c->dev, rt5677->reset_pin,
-					    GPIOF_OUT_INIT_HIGH,
-					    "RT5677 RESET");
-		if (ret < 0) {
-			dev_err(&i2c->dev, "Failed to request RESET %d: %d\n",
-				rt5677->reset_pin, ret);
-			return ret;
-		}
+	rt5677->reset_pin = devm_gpiod_get_optional(&i2c->dev,
+			"realtek,reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(rt5677->reset_pin)) {
+		ret = PTR_ERR(rt5677->reset_pin);
+		dev_err(&i2c->dev, "Failed to request RESET: %d\n", ret);
+		rt5677->reset_pin = 0;
 	}
 
-	if (gpio_is_valid(rt5677->pow_ldo2) ||
-	    gpio_is_valid(rt5677->reset_pin)) {
+	if (rt5677->pow_ldo2 || rt5677->reset_pin) {
 		/* Wait a while until I2C bus becomes available. The datasheet
 		 * does not specify the exact we should wait but startup
 		 * sequence mentiones at least a few milliseconds.
diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
index 7eca38a..d46855a 100644
--- a/sound/soc/codecs/rt5677.h
+++ b/sound/soc/codecs/rt5677.h
@@ -14,6 +14,7 @@
 
 #include <sound/rt5677.h>
 #include <linux/gpio/driver.h>
+#include <linux/gpio/consumer.h>
 
 /* Info */
 #define RT5677_RESET				0x00
@@ -1775,8 +1776,8 @@ struct rt5677_priv {
 	int pll_src;
 	int pll_in;
 	int pll_out;
-	int pow_ldo2; /* POW_LDO2 pin */
-	int reset_pin; /* RESET pin */
+	struct gpio_desc *pow_ldo2; /* POW_LDO2 pin */
+	struct gpio_desc *reset_pin; /* RESET pin */
 	enum rt5677_type type;
 #ifdef CONFIG_GPIOLIB
 	struct gpio_chip gpio_chip;
-- 
2.4.3.573.g4eafbef

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API
  2015-06-22 18:12 [PATCH 1/2] ASoC: rt5677: Switch to use descriptor-based gpiod API Ben Zhang
@ 2015-06-22 18:13 ` Ben Zhang
  2015-06-22 22:18   ` Pierre-Louis Bossart
  2015-07-07 13:57   ` Applied "ASoC: rt5677: Switch to use unified device property API" to the asoc tree Mark Brown
  2015-07-07 13:57 ` Applied "ASoC: rt5677: Switch to use descriptor-based gpiod " Mark Brown
  1 sibling, 2 replies; 9+ messages in thread
From: Ben Zhang @ 2015-06-22 18:13 UTC (permalink / raw)
  To: Mark Brown
  Cc: Oder Chiou, alsa-devel, Liam Girdwood, Ben Zhang, Anatol Pomozov,
	Bard Liao, Dylan Reid

This patch makes the driver use the unified device property API
so that platform data can be provided by Device Tree, ACPI
or board files.

Signed-off-by: Ben Zhang <benzh@chromium.org>
---
 sound/soc/codecs/rt5677.c | 57 +++++++++++++++++++++--------------------------
 1 file changed, 25 insertions(+), 32 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index c166217..69e45d8 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -20,6 +20,7 @@
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 #include <linux/firmware.h>
+#include <linux/property.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -5018,27 +5019,29 @@ static const struct i2c_device_id rt5677_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rt5677_i2c_id);
 
-static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np)
+static void rt5677_read_device_properties(struct rt5677_priv *rt5677,
+		struct device *dev)
 {
-	rt5677->pdata.in1_diff = of_property_read_bool(np,
-					"realtek,in1-differential");
-	rt5677->pdata.in2_diff = of_property_read_bool(np,
-					"realtek,in2-differential");
-	rt5677->pdata.lout1_diff = of_property_read_bool(np,
-					"realtek,lout1-differential");
-	rt5677->pdata.lout2_diff = of_property_read_bool(np,
-					"realtek,lout2-differential");
-	rt5677->pdata.lout3_diff = of_property_read_bool(np,
-					"realtek,lout3-differential");
-
-	of_property_read_u8_array(np, "realtek,gpio-config",
-		rt5677->pdata.gpio_config, RT5677_GPIO_NUM);
-
-	of_property_read_u32(np, "realtek,jd1-gpio", &rt5677->pdata.jd1_gpio);
-	of_property_read_u32(np, "realtek,jd2-gpio", &rt5677->pdata.jd2_gpio);
-	of_property_read_u32(np, "realtek,jd3-gpio", &rt5677->pdata.jd3_gpio);
-
-	return 0;
+	rt5677->pdata.in1_diff = device_property_read_bool(dev,
+			"realtek,in1-differential");
+	rt5677->pdata.in2_diff = device_property_read_bool(dev,
+			"realtek,in2-differential");
+	rt5677->pdata.lout1_diff = device_property_read_bool(dev,
+			"realtek,lout1-differential");
+	rt5677->pdata.lout2_diff = device_property_read_bool(dev,
+			"realtek,lout2-differential");
+	rt5677->pdata.lout3_diff = device_property_read_bool(dev,
+			"realtek,lout3-differential");
+
+	device_property_read_u8_array(dev, "realtek,gpio-config",
+			rt5677->pdata.gpio_config, RT5677_GPIO_NUM);
+
+	device_property_read_u32(dev, "realtek,jd1-gpio",
+			&rt5677->pdata.jd1_gpio);
+	device_property_read_u32(dev, "realtek,jd2-gpio",
+			&rt5677->pdata.jd2_gpio);
+	device_property_read_u32(dev, "realtek,jd3-gpio",
+			&rt5677->pdata.jd3_gpio);
 }
 
 static struct regmap_irq rt5677_irqs[] = {
@@ -5121,18 +5124,8 @@ static int rt5677_i2c_probe(struct i2c_client *i2c,
 
 	if (pdata)
 		rt5677->pdata = *pdata;
-
-	if (i2c->dev.of_node) {
-		ret = rt5677_parse_dt(rt5677, i2c->dev.of_node);
-		if (ret) {
-			dev_err(&i2c->dev, "Failed to parse device tree: %d\n",
-				ret);
-			return ret;
-		}
-	} else {
-		rt5677->pow_ldo2 = -EINVAL;
-		rt5677->reset_pin = -EINVAL;
-	}
+	else
+		rt5677_read_device_properties(rt5677, &i2c->dev);
 
 	/* pow-ldo2 and reset are optional. The codec pins may be statically
 	 * connected on the board without gpios. If the gpio device property
-- 
2.4.3.573.g4eafbef

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API
  2015-06-22 18:13 ` [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API Ben Zhang
@ 2015-06-22 22:18   ` Pierre-Louis Bossart
  2015-06-22 22:50     ` Mark Brown
  2015-07-07 13:57   ` Applied "ASoC: rt5677: Switch to use unified device property API" to the asoc tree Mark Brown
  1 sibling, 1 reply; 9+ messages in thread
From: Pierre-Louis Bossart @ 2015-06-22 22:18 UTC (permalink / raw)
  To: Ben Zhang, Mark Brown
  Cc: Oder Chiou, alsa-devel, Liam Girdwood, Anatol Pomozov, Bard Liao,
	Dylan Reid

On 6/22/15 8:13 PM, Ben Zhang wrote:
> This patch makes the driver use the unified device property API
> so that platform data can be provided by Device Tree, ACPI
> or board files.
>
> Signed-off-by: Ben Zhang <benzh@chromium.org>
> ---
>   sound/soc/codecs/rt5677.c | 57 +++++++++++++++++++++--------------------------
>   1 file changed, 25 insertions(+), 32 deletions(-)
>
> diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
> index c166217..69e45d8 100644
> --- a/sound/soc/codecs/rt5677.c
> +++ b/sound/soc/codecs/rt5677.c
> @@ -20,6 +20,7 @@
>   #include <linux/platform_device.h>
>   #include <linux/spi/spi.h>
>   #include <linux/firmware.h>
> +#include <linux/property.h>
>   #include <sound/core.h>
>   #include <sound/pcm.h>
>   #include <sound/pcm_params.h>
> @@ -5018,27 +5019,29 @@ static const struct i2c_device_id rt5677_i2c_id[] = {
>   };
>   MODULE_DEVICE_TABLE(i2c, rt5677_i2c_id);
>
> -static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np)
> +static void rt5677_read_device_properties(struct rt5677_priv *rt5677,
> +		struct device *dev)
>   {
> -	rt5677->pdata.in1_diff = of_property_read_bool(np,
> -					"realtek,in1-differential");
> -	rt5677->pdata.in2_diff = of_property_read_bool(np,
> -					"realtek,in2-differential");
> -	rt5677->pdata.lout1_diff = of_property_read_bool(np,
> -					"realtek,lout1-differential");
> -	rt5677->pdata.lout2_diff = of_property_read_bool(np,
> -					"realtek,lout2-differential");
> -	rt5677->pdata.lout3_diff = of_property_read_bool(np,
> -					"realtek,lout3-differential");
> -
> -	of_property_read_u8_array(np, "realtek,gpio-config",
> -		rt5677->pdata.gpio_config, RT5677_GPIO_NUM);
> -
> -	of_property_read_u32(np, "realtek,jd1-gpio", &rt5677->pdata.jd1_gpio);
> -	of_property_read_u32(np, "realtek,jd2-gpio", &rt5677->pdata.jd2_gpio);
> -	of_property_read_u32(np, "realtek,jd3-gpio", &rt5677->pdata.jd3_gpio);
> -
> -	return 0;
> +	rt5677->pdata.in1_diff = device_property_read_bool(dev,
> +			"realtek,in1-differential");

Shouldn't it be device_property_present() ?
thanks for starting this transition, this will be very useful for 
ACPI-based solutions.
-Pierre

> +	rt5677->pdata.in2_diff = device_property_read_bool(dev,
> +			"realtek,in2-differential");
> +	rt5677->pdata.lout1_diff = device_property_read_bool(dev,
> +			"realtek,lout1-differential");
> +	rt5677->pdata.lout2_diff = device_property_read_bool(dev,
> +			"realtek,lout2-differential");
> +	rt5677->pdata.lout3_diff = device_property_read_bool(dev,
> +			"realtek,lout3-differential");
> +
> +	device_property_read_u8_array(dev, "realtek,gpio-config",
> +			rt5677->pdata.gpio_config, RT5677_GPIO_NUM);
> +
> +	device_property_read_u32(dev, "realtek,jd1-gpio",
> +			&rt5677->pdata.jd1_gpio);
> +	device_property_read_u32(dev, "realtek,jd2-gpio",
> +			&rt5677->pdata.jd2_gpio);
> +	device_property_read_u32(dev, "realtek,jd3-gpio",
> +			&rt5677->pdata.jd3_gpio);
>   }
>
>   static struct regmap_irq rt5677_irqs[] = {
> @@ -5121,18 +5124,8 @@ static int rt5677_i2c_probe(struct i2c_client *i2c,
>
>   	if (pdata)
>   		rt5677->pdata = *pdata;
> -
> -	if (i2c->dev.of_node) {
> -		ret = rt5677_parse_dt(rt5677, i2c->dev.of_node);
> -		if (ret) {
> -			dev_err(&i2c->dev, "Failed to parse device tree: %d\n",
> -				ret);
> -			return ret;
> -		}
> -	} else {
> -		rt5677->pow_ldo2 = -EINVAL;
> -		rt5677->reset_pin = -EINVAL;
> -	}
> +	else
> +		rt5677_read_device_properties(rt5677, &i2c->dev);
>
>   	/* pow-ldo2 and reset are optional. The codec pins may be statically
>   	 * connected on the board without gpios. If the gpio device property
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API
  2015-06-22 22:18   ` Pierre-Louis Bossart
@ 2015-06-22 22:50     ` Mark Brown
  2015-06-22 23:07       ` Ben Zhang
  2015-06-23  0:05       ` Darren Hart
  0 siblings, 2 replies; 9+ messages in thread
From: Mark Brown @ 2015-06-22 22:50 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Oder Chiou, alsa-devel, Darren Hart, Liam Girdwood, Ben Zhang,
	Anatol Pomozov, Grant Likely, Bard Liao, Dylan Reid


[-- Attachment #1.1: Type: text/plain, Size: 604 bytes --]

On Tue, Jun 23, 2015 at 12:18:54AM +0200, Pierre-Louis Bossart wrote:
> On 6/22/15 8:13 PM, Ben Zhang wrote:

> >+	rt5677->pdata.in1_diff = device_property_read_bool(dev,
> >+			"realtek,in1-differential");

> Shouldn't it be device_property_present() ?

At least on the DT side they should be equivalent - I don't know if ACPI
works differently?

> thanks for starting this transition, this will be very useful for ACPI-based
> solutions.

Have the Intel audio people been speaking to the UEFI forum about using
the new registration process for these properties?  More from interest
than anything else.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API
  2015-06-22 22:50     ` Mark Brown
@ 2015-06-22 23:07       ` Ben Zhang
  2015-06-23  0:05       ` Darren Hart
  1 sibling, 0 replies; 9+ messages in thread
From: Ben Zhang @ 2015-06-22 23:07 UTC (permalink / raw)
  To: Mark Brown
  Cc: Oder Chiou, alsa-devel, Darren Hart, Pierre-Louis Bossart,
	Liam Girdwood, Anatol Pomozov, Grant Likely, Bard Liao,
	Dylan Reid

On Mon, Jun 22, 2015 at 3:50 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jun 23, 2015 at 12:18:54AM +0200, Pierre-Louis Bossart wrote:
>> On 6/22/15 8:13 PM, Ben Zhang wrote:
>
>> >+    rt5677->pdata.in1_diff = device_property_read_bool(dev,
>> >+                    "realtek,in1-differential");
>
>> Shouldn't it be device_property_present() ?
>
> At least on the DT side they should be equivalent - I don't know if ACPI
> works differently?

I think they are equivalent on the ACPI side as well. device_property_read_bool
is defined as a wrapper of device_property_present in include/linux/property.h.
I tested that the function returns true iff an entry for the property
exists in the _DSD.

For example in1_diff is true when:
Name (_DSD, Package () {
    ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
    Package () {
        Package () { "realtek,in1-differential", 1 },
        ....
    }
})

>> thanks for starting this transition, this will be very useful for ACPI-based
>> solutions.
>
> Have the Intel audio people been speaking to the UEFI forum about using
> the new registration process for these properties?  More from interest
> than anything else.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API
  2015-06-22 22:50     ` Mark Brown
  2015-06-22 23:07       ` Ben Zhang
@ 2015-06-23  0:05       ` Darren Hart
  2015-06-23  9:46         ` Mark Brown
  1 sibling, 1 reply; 9+ messages in thread
From: Darren Hart @ 2015-06-23  0:05 UTC (permalink / raw)
  To: Mark Brown, Pierre-Louis Bossart
  Cc: Oder Chiou, alsa-devel, Liam Girdwood, Ben Zhang, Anatol Pomozov,
	Grant Likely, Bard Liao, Dylan Reid

On 6/22/15, 3:50 PM, "Mark Brown" <broonie@kernel.org> wrote:

>On Tue, Jun 23, 2015 at 12:18:54AM +0200, Pierre-Louis Bossart wrote:
>> On 6/22/15 8:13 PM, Ben Zhang wrote:
>
>> >+	rt5677->pdata.in1_diff = device_property_read_bool(dev,
>> >+			"realtek,in1-differential");
>
>> Shouldn't it be device_property_present() ?
>
>At least on the DT side they should be equivalent - I don't know if ACPI
>works differently?
>
>> thanks for starting this transition, this will be very useful for
>>ACPI-based
>> solutions.
>
>Have the Intel audio people been speaking to the UEFI forum about using
>the new registration process for these properties?  More from interest
>than anything else.

Yes (although "audio people" is a rather broad term at Intel :-)


-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API
  2015-06-23  0:05       ` Darren Hart
@ 2015-06-23  9:46         ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2015-06-23  9:46 UTC (permalink / raw)
  To: Darren Hart
  Cc: Oder Chiou, alsa-devel, Pierre-Louis Bossart, Liam Girdwood,
	Ben Zhang, Anatol Pomozov, Grant Likely, Bard Liao, Dylan Reid


[-- Attachment #1.1: Type: text/plain, Size: 563 bytes --]

On Mon, Jun 22, 2015 at 05:05:37PM -0700, Darren Hart wrote:
> On 6/22/15, 3:50 PM, "Mark Brown" <broonie@kernel.org> wrote:

> >Have the Intel audio people been speaking to the UEFI forum about using
> >the new registration process for these properties?  More from interest
> >than anything else.

> Yes (although "audio people" is a rather broad term at Intel :-)

It is, as is ACPI people, and these are among the issues here.  Is this
stuff going to get used as a trial run of the new process?  It'd be good
to know if we should be pushing people towards it.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Applied "ASoC: rt5677: Switch to use unified device property API" to the asoc tree
  2015-06-22 18:13 ` [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API Ben Zhang
  2015-06-22 22:18   ` Pierre-Louis Bossart
@ 2015-07-07 13:57   ` Mark Brown
  1 sibling, 0 replies; 9+ messages in thread
From: Mark Brown @ 2015-07-07 13:57 UTC (permalink / raw)
  To: Ben Zhang, Mark Brown; +Cc: alsa-devel

The patch

   ASoC: rt5677: Switch to use unified device property API

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 9bfde72157036f4eaa44f3e8982217ce1b3e14b6 Mon Sep 17 00:00:00 2001
From: Ben Zhang <benzh@chromium.org>
Date: Mon, 22 Jun 2015 11:13:00 -0700
Subject: [PATCH] ASoC: rt5677: Switch to use unified device property API

This patch makes the driver use the unified device property API
so that platform data can be provided by Device Tree, ACPI
or board files.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5677.c | 57 +++++++++++++++++++++--------------------------
 1 file changed, 25 insertions(+), 32 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 2322430..13b871f 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -20,6 +20,7 @@
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 #include <linux/firmware.h>
+#include <linux/property.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -5021,27 +5022,29 @@ static const struct i2c_device_id rt5677_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rt5677_i2c_id);
 
-static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np)
+static void rt5677_read_device_properties(struct rt5677_priv *rt5677,
+		struct device *dev)
 {
-	rt5677->pdata.in1_diff = of_property_read_bool(np,
-					"realtek,in1-differential");
-	rt5677->pdata.in2_diff = of_property_read_bool(np,
-					"realtek,in2-differential");
-	rt5677->pdata.lout1_diff = of_property_read_bool(np,
-					"realtek,lout1-differential");
-	rt5677->pdata.lout2_diff = of_property_read_bool(np,
-					"realtek,lout2-differential");
-	rt5677->pdata.lout3_diff = of_property_read_bool(np,
-					"realtek,lout3-differential");
-
-	of_property_read_u8_array(np, "realtek,gpio-config",
-		rt5677->pdata.gpio_config, RT5677_GPIO_NUM);
-
-	of_property_read_u32(np, "realtek,jd1-gpio", &rt5677->pdata.jd1_gpio);
-	of_property_read_u32(np, "realtek,jd2-gpio", &rt5677->pdata.jd2_gpio);
-	of_property_read_u32(np, "realtek,jd3-gpio", &rt5677->pdata.jd3_gpio);
-
-	return 0;
+	rt5677->pdata.in1_diff = device_property_read_bool(dev,
+			"realtek,in1-differential");
+	rt5677->pdata.in2_diff = device_property_read_bool(dev,
+			"realtek,in2-differential");
+	rt5677->pdata.lout1_diff = device_property_read_bool(dev,
+			"realtek,lout1-differential");
+	rt5677->pdata.lout2_diff = device_property_read_bool(dev,
+			"realtek,lout2-differential");
+	rt5677->pdata.lout3_diff = device_property_read_bool(dev,
+			"realtek,lout3-differential");
+
+	device_property_read_u8_array(dev, "realtek,gpio-config",
+			rt5677->pdata.gpio_config, RT5677_GPIO_NUM);
+
+	device_property_read_u32(dev, "realtek,jd1-gpio",
+			&rt5677->pdata.jd1_gpio);
+	device_property_read_u32(dev, "realtek,jd2-gpio",
+			&rt5677->pdata.jd2_gpio);
+	device_property_read_u32(dev, "realtek,jd3-gpio",
+			&rt5677->pdata.jd3_gpio);
 }
 
 static struct regmap_irq rt5677_irqs[] = {
@@ -5124,18 +5127,8 @@ static int rt5677_i2c_probe(struct i2c_client *i2c,
 
 	if (pdata)
 		rt5677->pdata = *pdata;
-
-	if (i2c->dev.of_node) {
-		ret = rt5677_parse_dt(rt5677, i2c->dev.of_node);
-		if (ret) {
-			dev_err(&i2c->dev, "Failed to parse device tree: %d\n",
-				ret);
-			return ret;
-		}
-	} else {
-		rt5677->pow_ldo2 = -EINVAL;
-		rt5677->reset_pin = -EINVAL;
-	}
+	else
+		rt5677_read_device_properties(rt5677, &i2c->dev);
 
 	/* pow-ldo2 and reset are optional. The codec pins may be statically
 	 * connected on the board without gpios. If the gpio device property
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Applied "ASoC: rt5677: Switch to use descriptor-based gpiod API" to the asoc tree
  2015-06-22 18:12 [PATCH 1/2] ASoC: rt5677: Switch to use descriptor-based gpiod API Ben Zhang
  2015-06-22 18:13 ` [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API Ben Zhang
@ 2015-07-07 13:57 ` Mark Brown
  1 sibling, 0 replies; 9+ messages in thread
From: Mark Brown @ 2015-07-07 13:57 UTC (permalink / raw)
  To: Ben Zhang, Mark Brown; +Cc: alsa-devel

The patch

   ASoC: rt5677: Switch to use descriptor-based gpiod API

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From efd901ee4bc8312e3bbf5561fdab8e3765e26334 Mon Sep 17 00:00:00 2001
From: Ben Zhang <benzh@chromium.org>
Date: Mon, 22 Jun 2015 11:12:59 -0700
Subject: [PATCH] ASoC: rt5677: Switch to use descriptor-based gpiod API

This patch makes the driver use the new descriptor-based gpiod API
so that gpio assignment info can be provided by Device Tree, ACPI
or board files.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5677.c | 85 +++++++++++++++++------------------------------
 sound/soc/codecs/rt5677.h |  5 +--
 2 files changed, 33 insertions(+), 57 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 9048ba7..2322430 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -15,13 +15,11 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/pm.h>
-#include <linux/of_gpio.h>
 #include <linux/regmap.h>
 #include <linux/i2c.h>
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 #include <linux/firmware.h>
-#include <linux/gpio.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -4764,10 +4762,10 @@ static int rt5677_remove(struct snd_soc_codec *codec)
 	struct rt5677_priv *rt5677 = snd_soc_codec_get_drvdata(codec);
 
 	regmap_write(rt5677->regmap, RT5677_RESET, 0x10ec);
-	if (gpio_is_valid(rt5677->pow_ldo2))
-		gpio_set_value_cansleep(rt5677->pow_ldo2, 0);
-	if (gpio_is_valid(rt5677->reset_pin))
-		gpio_set_value_cansleep(rt5677->reset_pin, 0);
+	if (rt5677->pow_ldo2)
+		gpiod_set_value_cansleep(rt5677->pow_ldo2, 0);
+	if (rt5677->reset_pin)
+		gpiod_set_value_cansleep(rt5677->reset_pin, 0);
 
 	return 0;
 }
@@ -4781,10 +4779,10 @@ static int rt5677_suspend(struct snd_soc_codec *codec)
 		regcache_cache_only(rt5677->regmap, true);
 		regcache_mark_dirty(rt5677->regmap);
 
-		if (gpio_is_valid(rt5677->pow_ldo2))
-			gpio_set_value_cansleep(rt5677->pow_ldo2, 0);
-		if (gpio_is_valid(rt5677->reset_pin))
-			gpio_set_value_cansleep(rt5677->reset_pin, 0);
+		if (rt5677->pow_ldo2)
+			gpiod_set_value_cansleep(rt5677->pow_ldo2, 0);
+		if (rt5677->reset_pin)
+			gpiod_set_value_cansleep(rt5677->reset_pin, 0);
 	}
 
 	return 0;
@@ -4795,12 +4793,11 @@ static int rt5677_resume(struct snd_soc_codec *codec)
 	struct rt5677_priv *rt5677 = snd_soc_codec_get_drvdata(codec);
 
 	if (!rt5677->dsp_vad_en) {
-		if (gpio_is_valid(rt5677->pow_ldo2))
-			gpio_set_value_cansleep(rt5677->pow_ldo2, 1);
-		if (gpio_is_valid(rt5677->reset_pin))
-			gpio_set_value_cansleep(rt5677->reset_pin, 1);
-		if (gpio_is_valid(rt5677->pow_ldo2) ||
-		    gpio_is_valid(rt5677->reset_pin))
+		if (rt5677->pow_ldo2)
+			gpiod_set_value_cansleep(rt5677->pow_ldo2, 1);
+		if (rt5677->reset_pin)
+			gpiod_set_value_cansleep(rt5677->reset_pin, 1);
+		if (rt5677->pow_ldo2 || rt5677->reset_pin)
 			msleep(10);
 
 		regcache_cache_only(rt5677->regmap, false);
@@ -5037,24 +5034,6 @@ static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np)
 	rt5677->pdata.lout3_diff = of_property_read_bool(np,
 					"realtek,lout3-differential");
 
-	rt5677->pow_ldo2 = of_get_named_gpio(np,
-					"realtek,pow-ldo2-gpio", 0);
-	rt5677->reset_pin = of_get_named_gpio(np,
-					"realtek,reset-gpio", 0);
-
-	/*
-	 * POW_LDO2 is optional (it may be statically tied on the board).
-	 * -ENOENT means that the property doesn't exist, i.e. there is no
-	 * GPIO, so is not an error. Any other error code means the property
-	 * exists, but could not be parsed.
-	 */
-	if (!gpio_is_valid(rt5677->pow_ldo2) &&
-			(rt5677->pow_ldo2 != -ENOENT))
-		return rt5677->pow_ldo2;
-	if (!gpio_is_valid(rt5677->reset_pin) &&
-			(rt5677->reset_pin != -ENOENT))
-		return rt5677->reset_pin;
-
 	of_property_read_u8_array(np, "realtek,gpio-config",
 		rt5677->pdata.gpio_config, RT5677_GPIO_NUM);
 
@@ -5158,30 +5137,26 @@ static int rt5677_i2c_probe(struct i2c_client *i2c,
 		rt5677->reset_pin = -EINVAL;
 	}
 
-	if (gpio_is_valid(rt5677->pow_ldo2)) {
-		ret = devm_gpio_request_one(&i2c->dev, rt5677->pow_ldo2,
-					    GPIOF_OUT_INIT_HIGH,
-					    "RT5677 POW_LDO2");
-		if (ret < 0) {
-			dev_err(&i2c->dev, "Failed to request POW_LDO2 %d: %d\n",
-				rt5677->pow_ldo2, ret);
-			return ret;
-		}
+	/* pow-ldo2 and reset are optional. The codec pins may be statically
+	 * connected on the board without gpios. If the gpio device property
+	 * isn't specified, devm_gpiod_get_optional returns NULL.
+	 */
+	rt5677->pow_ldo2 = devm_gpiod_get_optional(&i2c->dev,
+			"realtek,pow-ldo2", GPIOD_OUT_HIGH);
+	if (IS_ERR(rt5677->pow_ldo2)) {
+		ret = PTR_ERR(rt5677->pow_ldo2);
+		dev_err(&i2c->dev, "Failed to request POW_LDO2: %d\n", ret);
+		rt5677->pow_ldo2 = 0;
 	}
-
-	if (gpio_is_valid(rt5677->reset_pin)) {
-		ret = devm_gpio_request_one(&i2c->dev, rt5677->reset_pin,
-					    GPIOF_OUT_INIT_HIGH,
-					    "RT5677 RESET");
-		if (ret < 0) {
-			dev_err(&i2c->dev, "Failed to request RESET %d: %d\n",
-				rt5677->reset_pin, ret);
-			return ret;
-		}
+	rt5677->reset_pin = devm_gpiod_get_optional(&i2c->dev,
+			"realtek,reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(rt5677->reset_pin)) {
+		ret = PTR_ERR(rt5677->reset_pin);
+		dev_err(&i2c->dev, "Failed to request RESET: %d\n", ret);
+		rt5677->reset_pin = 0;
 	}
 
-	if (gpio_is_valid(rt5677->pow_ldo2) ||
-	    gpio_is_valid(rt5677->reset_pin)) {
+	if (rt5677->pow_ldo2 || rt5677->reset_pin) {
 		/* Wait a while until I2C bus becomes available. The datasheet
 		 * does not specify the exact we should wait but startup
 		 * sequence mentiones at least a few milliseconds.
diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
index 7eca38a..d46855a 100644
--- a/sound/soc/codecs/rt5677.h
+++ b/sound/soc/codecs/rt5677.h
@@ -14,6 +14,7 @@
 
 #include <sound/rt5677.h>
 #include <linux/gpio/driver.h>
+#include <linux/gpio/consumer.h>
 
 /* Info */
 #define RT5677_RESET				0x00
@@ -1775,8 +1776,8 @@ struct rt5677_priv {
 	int pll_src;
 	int pll_in;
 	int pll_out;
-	int pow_ldo2; /* POW_LDO2 pin */
-	int reset_pin; /* RESET pin */
+	struct gpio_desc *pow_ldo2; /* POW_LDO2 pin */
+	struct gpio_desc *reset_pin; /* RESET pin */
 	enum rt5677_type type;
 #ifdef CONFIG_GPIOLIB
 	struct gpio_chip gpio_chip;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-07-07 13:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-22 18:12 [PATCH 1/2] ASoC: rt5677: Switch to use descriptor-based gpiod API Ben Zhang
2015-06-22 18:13 ` [PATCH 2/2] ASoC: rt5677: Switch to use unified device property API Ben Zhang
2015-06-22 22:18   ` Pierre-Louis Bossart
2015-06-22 22:50     ` Mark Brown
2015-06-22 23:07       ` Ben Zhang
2015-06-23  0:05       ` Darren Hart
2015-06-23  9:46         ` Mark Brown
2015-07-07 13:57   ` Applied "ASoC: rt5677: Switch to use unified device property API" to the asoc tree Mark Brown
2015-07-07 13:57 ` Applied "ASoC: rt5677: Switch to use descriptor-based gpiod " Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.