[PATCH 01/11] ASoC: ak5386: switch to using gpiod API

[PATCH 01/11] ASoC: ak5386: switch to using gpiod API

[Dmitry Torokhov]

Switch the driver from legacy gpio API that is deprecated to the newer
gpiod API that respects line polarities described in ACPI/DT.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 sound/soc/codecs/ak5386.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/sound/soc/codecs/ak5386.c b/sound/soc/codecs/ak5386.c
index 0c5e00679c7d..0562890b5dc7 100644
--- a/sound/soc/codecs/ak5386.c
+++ b/sound/soc/codecs/ak5386.c
@@ -6,11 +6,12 @@
  * (c) 2013 Daniel Mack <zonque@gmail.com>
  */
 
+
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/of.h>
-#include <linux/of_gpio.h>
-#include <linux/of_device.h>
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
 #include <sound/pcm.h>
@@ -21,7 +22,7 @@ static const char * const supply_names[] = {
 };
 
 struct ak5386_priv {
-	int reset_gpio;
+	struct gpio_desc *reset_gpio;
 	struct regulator_bulk_data supplies[ARRAY_SIZE(supply_names)];
 };
 
@@ -111,8 +112,8 @@ static int ak5386_hw_params(struct snd_pcm_substream *substream,
 	 * the AK5386 in power-down mode (PDN pin = “L”).
 	 */
 
-	if (gpio_is_valid(priv->reset_gpio))
-		gpio_set_value(priv->reset_gpio, 1);
+	if (priv->reset_gpio)
+		gpiod_set_value(priv->reset_gpio, 0);
 
 	return 0;
 }
@@ -123,8 +124,8 @@ static int ak5386_hw_free(struct snd_pcm_substream *substream,
 	struct snd_soc_component *component = dai->component;
 	struct ak5386_priv *priv = snd_soc_component_get_drvdata(component);
 
-	if (gpio_is_valid(priv->reset_gpio))
-		gpio_set_value(priv->reset_gpio, 0);
+	if (priv->reset_gpio)
+		gpiod_set_value(priv->reset_gpio, 1);
 
 	return 0;
 }
@@ -168,7 +169,6 @@ static int ak5386_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	priv->reset_gpio = -EINVAL;
 	dev_set_drvdata(dev, priv);
 
 	for (i = 0; i < ARRAY_SIZE(supply_names); i++)
@@ -179,15 +179,13 @@ static int ak5386_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	if (of_match_device(of_match_ptr(ak5386_dt_ids), dev))
-		priv->reset_gpio = of_get_named_gpio(dev->of_node,
-						      "reset-gpio", 0);
+	priv->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						   GPIOD_OUT_HIGH);
+	ret = PTR_ERR_OR_ZERO(priv->reset_gpio);
+	if (ret)
+		return ret;
 
-	if (gpio_is_valid(priv->reset_gpio))
-		if (devm_gpio_request_one(dev, priv->reset_gpio,
-					  GPIOF_OUT_INIT_LOW,
-					  "AK5386 Reset"))
-			priv->reset_gpio = -EINVAL;
+	gpiod_set_consumer_name(priv->reset_gpio, "AK5386 Reset");
 
 	return devm_snd_soc_register_component(dev, &soc_component_ak5386,
 				      &ak5386_dai, 1);
-- 
2.38.1.431.g37b22c650d-goog

[PATCH 02/11] ASoC: max98373: switch to using gpiod API

[Dmitry Torokhov]

Switch the driver from legacy gpio API that is deprecated to the newer
gpiod API that respects line polarities described in ACPI/DT.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 sound/soc/codecs/max98373-i2c.c | 39 +++++++++++++++++++--------------
 sound/soc/codecs/max98373.c     | 18 ---------------
 sound/soc/codecs/max98373.h     |  1 -
 3 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/sound/soc/codecs/max98373-i2c.c b/sound/soc/codecs/max98373-i2c.c
index 3e04c7f0cce4..969cdca83bc1 100644
--- a/sound/soc/codecs/max98373-i2c.c
+++ b/sound/soc/codecs/max98373-i2c.c
@@ -3,12 +3,12 @@
 
 #include <linux/acpi.h>
 #include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/of.h>
-#include <linux/of_gpio.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
@@ -520,14 +520,13 @@ static int max98373_i2c_probe(struct i2c_client *i2c)
 	int ret = 0;
 	int reg = 0;
 	int i;
-	struct max98373_priv *max98373 = NULL;
+	struct max98373_priv *max98373;
+	struct gpio_desc *reset_gpio;
 
 	max98373 = devm_kzalloc(&i2c->dev, sizeof(*max98373), GFP_KERNEL);
+	if (!max98373)
+		return -ENOMEM;
 
-	if (!max98373) {
-		ret = -ENOMEM;
-		return ret;
-	}
 	i2c_set_clientdata(i2c, max98373);
 
 	/* update interleave mode info */
@@ -557,17 +556,23 @@ static int max98373_i2c_probe(struct i2c_client *i2c)
 	max98373_slot_config(&i2c->dev, max98373);
 
 	/* Power on device */
-	if (gpio_is_valid(max98373->reset_gpio)) {
-		ret = devm_gpio_request(&i2c->dev, max98373->reset_gpio,
-					"MAX98373_RESET");
-		if (ret) {
-			dev_err(&i2c->dev, "%s: Failed to request gpio %d\n",
-				__func__, max98373->reset_gpio);
-			return -EINVAL;
-		}
-		gpio_direction_output(max98373->reset_gpio, 0);
+	/* Acquire and assert reset line */
+	reset_gpio = devm_gpiod_get_optional(&i2c->dev, "maxim,reset",
+					     GPIOD_OUT_HIGH);
+	ret = PTR_ERR_OR_ZERO(reset_gpio);
+	if (ret) {
+		dev_err(&i2c->dev, "%s: Failed to request reset gpio: %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	gpiod_set_consumer_name(reset_gpio, "MAX98373_RESET");
+
+	if (reset_gpio) {
+		/* Keep line asserted to reset device */
 		msleep(50);
-		gpio_direction_output(max98373->reset_gpio, 1);
+		/* Deassert reset line */
+		gpiod_set_value_cansleep(reset_gpio, 0);
 		msleep(20);
 	}
 
diff --git a/sound/soc/codecs/max98373.c b/sound/soc/codecs/max98373.c
index f90a6a7ba83b..401546c1084e 100644
--- a/sound/soc/codecs/max98373.c
+++ b/sound/soc/codecs/max98373.c
@@ -12,9 +12,6 @@
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
-#include <linux/gpio.h>
-#include <linux/of.h>
-#include <linux/of_gpio.h>
 #include <sound/tlv.h>
 #include "max98373.h"
 
@@ -478,21 +475,6 @@ void max98373_slot_config(struct device *dev,
 		max98373->i_slot = value & 0xF;
 	else
 		max98373->i_slot = 1;
-	if (dev->of_node) {
-		max98373->reset_gpio = of_get_named_gpio(dev->of_node,
-						"maxim,reset-gpio", 0);
-		if (!gpio_is_valid(max98373->reset_gpio)) {
-			dev_err(dev, "Looking up %s property in node %s failed %d\n",
-				"maxim,reset-gpio", dev->of_node->full_name,
-				max98373->reset_gpio);
-		} else {
-			dev_dbg(dev, "maxim,reset-gpio=%d",
-				max98373->reset_gpio);
-		}
-	} else {
-		/* this makes reset_gpio as invalid */
-		max98373->reset_gpio = -1;
-	}
 
 	if (!device_property_read_u32(dev, "maxim,spkfb-slot-no", &value))
 		max98373->spkfb_slot = value & 0xF;
diff --git a/sound/soc/codecs/max98373.h b/sound/soc/codecs/max98373.h
index e1810b3b1620..7b259789e1be 100644
--- a/sound/soc/codecs/max98373.h
+++ b/sound/soc/codecs/max98373.h
@@ -213,7 +213,6 @@ struct max98373_cache {
 
 struct max98373_priv {
 	struct regmap *regmap;
-	int reset_gpio;
 	unsigned int v_slot;
 	unsigned int i_slot;
 	unsigned int spkfb_slot;
-- 
2.38.1.431.g37b22c650d-goog

[PATCH 03/11] ASoC: tas5086: switch to using gpiod API

[Dmitry Torokhov]

Switch the driver from legacy gpio API that is deprecated to the newer
gpiod API that respects line polarities described in ACPI/DT.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 sound/soc/codecs/tas5086.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/sound/soc/codecs/tas5086.c b/sound/soc/codecs/tas5086.c
index 22143cc5afa7..66b22b800e01 100644
--- a/sound/soc/codecs/tas5086.c
+++ b/sound/soc/codecs/tas5086.c
@@ -24,14 +24,14 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -246,7 +246,7 @@ struct tas5086_private {
 	/* Current sample rate for de-emphasis control */
 	int		rate;
 	/* GPIO driving Reset pin, if any */
-	int		gpio_nreset;
+	struct gpio_desc *reset_gpio;
 	struct		regulator_bulk_data supplies[ARRAY_SIZE(supply_names)];
 };
 
@@ -462,11 +462,11 @@ static int tas5086_mute_stream(struct snd_soc_dai *dai, int mute, int stream)
 
 static void tas5086_reset(struct tas5086_private *priv)
 {
-	if (gpio_is_valid(priv->gpio_nreset)) {
+	if (priv->reset_gpio) {
 		/* Reset codec - minimum assertion time is 400ns */
-		gpio_direction_output(priv->gpio_nreset, 0);
+		gpiod_set_value_cansleep(priv->reset_gpio, 1);
 		udelay(1);
-		gpio_set_value(priv->gpio_nreset, 1);
+		gpiod_set_value_cansleep(priv->reset_gpio, 0);
 
 		/* Codec needs ~15ms to wake up */
 		msleep(15);
@@ -867,9 +867,10 @@ static void tas5086_remove(struct snd_soc_component *component)
 {
 	struct tas5086_private *priv = snd_soc_component_get_drvdata(component);
 
-	if (gpio_is_valid(priv->gpio_nreset))
+	if (priv->reset_gpio) {
 		/* Set codec to the reset state */
-		gpio_set_value(priv->gpio_nreset, 0);
+		gpiod_set_value_cansleep(priv->reset_gpio, 1);
+	}
 
 	regulator_bulk_disable(ARRAY_SIZE(priv->supplies), priv->supplies);
 };
@@ -914,7 +915,6 @@ static int tas5086_i2c_probe(struct i2c_client *i2c)
 {
 	struct tas5086_private *priv;
 	struct device *dev = &i2c->dev;
-	int gpio_nreset = -EINVAL;
 	int i, ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -940,16 +940,12 @@ static int tas5086_i2c_probe(struct i2c_client *i2c)
 
 	i2c_set_clientdata(i2c, priv);
 
-	if (of_match_device(of_match_ptr(tas5086_dt_ids), dev)) {
-		struct device_node *of_node = dev->of_node;
-		gpio_nreset = of_get_named_gpio(of_node, "reset-gpio", 0);
-	}
-
-	if (gpio_is_valid(gpio_nreset))
-		if (devm_gpio_request(dev, gpio_nreset, "TAS5086 Reset"))
-			gpio_nreset = -EINVAL;
+	priv->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	ret = PTR_ERR_OR_ZERO(priv->reset_gpio);
+	if (ret)
+		return ret;
 
-	priv->gpio_nreset = gpio_nreset;
+	gpiod_set_consumer_name(priv->reset_gpio, "TAS5086 Reset");
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(priv->supplies), priv->supplies);
 	if (ret < 0) {
-- 
2.38.1.431.g37b22c650d-goog

[PATCH 04/11] ASoC: tpa6130a2: remove support for platform data

[Dmitry Torokhov]

There are no users of tpa6130a2_platform_data in the mainline kernel,
remove it.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 include/sound/tpa6130a2-plat.h | 17 -----------------
 sound/soc/codecs/tpa6130a2.c   | 18 ++++--------------
 2 files changed, 4 insertions(+), 31 deletions(-)
 delete mode 100644 include/sound/tpa6130a2-plat.h

diff --git a/include/sound/tpa6130a2-plat.h b/include/sound/tpa6130a2-plat.h
deleted file mode 100644
index a60930e36e93..000000000000
--- a/include/sound/tpa6130a2-plat.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * TPA6130A2 driver platform header
- *
- * Copyright (C) Nokia Corporation
- *
- * Author: Peter Ujfalusi <peter.ujfalusi@ti.com>
- */
-
-#ifndef TPA6130A2_PLAT_H
-#define TPA6130A2_PLAT_H
-
-struct tpa6130a2_platform_data {
-	int power_gpio;
-};
-
-#endif
diff --git a/sound/soc/codecs/tpa6130a2.c b/sound/soc/codecs/tpa6130a2.c
index 8c00db32996b..5f00bfc32917 100644
--- a/sound/soc/codecs/tpa6130a2.c
+++ b/sound/soc/codecs/tpa6130a2.c
@@ -14,7 +14,6 @@
 #include <linux/gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
-#include <sound/tpa6130a2-plat.h>
 #include <sound/soc.h>
 #include <sound/tlv.h>
 #include <linux/of.h>
@@ -218,16 +217,15 @@ MODULE_DEVICE_TABLE(i2c, tpa6130a2_id);
 
 static int tpa6130a2_probe(struct i2c_client *client)
 {
-	struct device *dev;
+	struct device *dev = &client->dev;
 	struct tpa6130a2_data *data;
-	struct tpa6130a2_platform_data *pdata = client->dev.platform_data;
-	struct device_node *np = client->dev.of_node;
 	const struct i2c_device_id *id;
 	const char *regulator;
 	unsigned int version;
 	int ret;
 
-	dev = &client->dev;
+	if (!dev->of_node)
+		return -ENODEV;
 
 	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -239,15 +237,7 @@ static int tpa6130a2_probe(struct i2c_client *client)
 	if (IS_ERR(data->regmap))
 		return PTR_ERR(data->regmap);
 
-	if (pdata) {
-		data->power_gpio = pdata->power_gpio;
-	} else if (np) {
-		data->power_gpio = of_get_named_gpio(np, "power-gpio", 0);
-	} else {
-		dev_err(dev, "Platform data not set\n");
-		dump_stack();
-		return -ENODEV;
-	}
+	data->power_gpio = of_get_named_gpio(dev->of_node, "power-gpio", 0);
 
 	i2c_set_clientdata(client, data);
 
-- 
2.38.1.431.g37b22c650d-goog

[PATCH 05/11] ASoC: tpa6130a2: switch to using gpiod API

[Dmitry Torokhov]

Switch the driver from legacy gpio API that is deprecated to the newer
gpiod API that respects line polarities described in ACPI/DT.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 sound/soc/codecs/tpa6130a2.c | 42 +++++++++++++++---------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/sound/soc/codecs/tpa6130a2.c b/sound/soc/codecs/tpa6130a2.c
index 5f00bfc32917..696a27b472aa 100644
--- a/sound/soc/codecs/tpa6130a2.c
+++ b/sound/soc/codecs/tpa6130a2.c
@@ -9,15 +9,15 @@
 
 #include <linux/module.h>
 #include <linux/errno.h>
+#include <linux/err.h>
 #include <linux/device.h>
 #include <linux/i2c.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <sound/soc.h>
 #include <sound/tlv.h>
 #include <linux/of.h>
-#include <linux/of_gpio.h>
 #include <linux/regmap.h>
 
 #include "tpa6130a2.h"
@@ -32,7 +32,7 @@ struct tpa6130a2_data {
 	struct device *dev;
 	struct regmap *regmap;
 	struct regulator *supply;
-	int power_gpio;
+	struct gpio_desc *power_gpio;
 	enum tpa_model id;
 };
 
@@ -48,8 +48,8 @@ static int tpa6130a2_power(struct tpa6130a2_data *data, bool enable)
 			return ret;
 		}
 		/* Power on */
-		if (data->power_gpio >= 0)
-			gpio_set_value(data->power_gpio, 1);
+		if (data->power_gpio)
+			gpiod_set_value(data->power_gpio, 1);
 
 		/* Sync registers */
 		regcache_cache_only(data->regmap, false);
@@ -58,8 +58,8 @@ static int tpa6130a2_power(struct tpa6130a2_data *data, bool enable)
 			dev_err(data->dev,
 				"Failed to sync registers: %d\n", ret);
 			regcache_cache_only(data->regmap, true);
-			if (data->power_gpio >= 0)
-				gpio_set_value(data->power_gpio, 0);
+			if (data->power_gpio)
+				gpiod_set_value(data->power_gpio, 0);
 			ret2 = regulator_disable(data->supply);
 			if (ret2 != 0)
 				dev_err(data->dev,
@@ -75,8 +75,8 @@ static int tpa6130a2_power(struct tpa6130a2_data *data, bool enable)
 		regcache_cache_only(data->regmap, true);
 
 		/* Power off */
-		if (data->power_gpio >= 0)
-			gpio_set_value(data->power_gpio, 0);
+		if (data->power_gpio)
+			gpiod_set_value(data->power_gpio, 0);
 
 		ret = regulator_disable(data->supply);
 		if (ret != 0) {
@@ -224,37 +224,29 @@ static int tpa6130a2_probe(struct i2c_client *client)
 	unsigned int version;
 	int ret;
 
-	if (!dev->of_node)
-		return -ENODEV;
-
 	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
+	i2c_set_clientdata(client, data);
 	data->dev = dev;
 
 	data->regmap = devm_regmap_init_i2c(client, &tpa6130a2_regmap_config);
 	if (IS_ERR(data->regmap))
 		return PTR_ERR(data->regmap);
 
-	data->power_gpio = of_get_named_gpio(dev->of_node, "power-gpio", 0);
+	data->power_gpio = devm_gpiod_get_optional(dev, "power", GPIOD_OUT_LOW);
+	ret = PTR_ERR_OR_ZERO(data->power_gpio);
+	if (ret) {
+		dev_err(dev, "Failed to request power GPIO: %d\n", ret);
+		return ret;
+	}
 
-	i2c_set_clientdata(client, data);
+	gpiod_set_consumer_name(data->power_gpio, "tpa6130a2 enable");
 
 	id = i2c_match_id(tpa6130a2_id, client);
 	data->id = id->driver_data;
 
-	if (data->power_gpio >= 0) {
-		ret = devm_gpio_request(dev, data->power_gpio,
-					"tpa6130a2 enable");
-		if (ret < 0) {
-			dev_err(dev, "Failed to request power GPIO (%d)\n",
-				data->power_gpio);
-			return ret;
-		}
-		gpio_direction_output(data->power_gpio, 0);
-	}
-
 	switch (data->id) {
 	default:
 		dev_warn(dev, "Unknown TPA model (%d). Assuming 6130A2\n",
-- 
2.38.1.431.g37b22c650d-goog

[PATCH 06/11] ASoC: tlv320aic32x4: remove support for platform data

[Dmitry Torokhov]

There are no users of aic32x4_pdata in the mainline kernel, remove it.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 include/sound/tlv320aic32x4.h    | 52 --------------------------------
 sound/soc/codecs/tlv320aic32x4.c | 14 +++------
 sound/soc/codecs/tlv320aic32x4.h | 27 +++++++++++++++++
 3 files changed, 32 insertions(+), 61 deletions(-)
 delete mode 100644 include/sound/tlv320aic32x4.h

diff --git a/include/sound/tlv320aic32x4.h b/include/sound/tlv320aic32x4.h
deleted file mode 100644
index 0abf74d7edbd..000000000000
--- a/include/sound/tlv320aic32x4.h
+++ /dev/null
@@ -1,52 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * tlv320aic32x4.h  --  TLV320AIC32X4 Soc Audio driver platform data
- *
- * Copyright 2011 Vista Silicon S.L.
- *
- * Author: Javier Martin <javier.martin@vista-silicon.com>
- */
-
-#ifndef _AIC32X4_PDATA_H
-#define _AIC32X4_PDATA_H
-
-#define AIC32X4_PWR_MICBIAS_2075_LDOIN		0x00000001
-#define AIC32X4_PWR_AVDD_DVDD_WEAK_DISABLE	0x00000002
-#define AIC32X4_PWR_AIC32X4_LDO_ENABLE		0x00000004
-#define AIC32X4_PWR_CMMODE_LDOIN_RANGE_18_36	0x00000008
-#define AIC32X4_PWR_CMMODE_HP_LDOIN_POWERED	0x00000010
-
-#define AIC32X4_MICPGA_ROUTE_LMIC_IN2R_10K	0x00000001
-#define AIC32X4_MICPGA_ROUTE_RMIC_IN1L_10K	0x00000002
-
-/* GPIO API */
-#define AIC32X4_MFPX_DEFAULT_VALUE	0xff
-
-#define AIC32X4_MFP1_DIN_DISABLED	0
-#define AIC32X4_MFP1_DIN_ENABLED	0x2
-#define AIC32X4_MFP1_GPIO_IN		0x4
-
-#define AIC32X4_MFP2_GPIO_OUT_LOW	0x0
-#define AIC32X4_MFP2_GPIO_OUT_HIGH	0x1
-
-#define AIC32X4_MFP_GPIO_ENABLED	0x4
-
-#define AIC32X4_MFP5_GPIO_DISABLED	0x0
-#define AIC32X4_MFP5_GPIO_INPUT		0x8
-#define AIC32X4_MFP5_GPIO_OUTPUT	0xc
-#define AIC32X4_MFP5_GPIO_OUT_LOW	0x0
-#define AIC32X4_MFP5_GPIO_OUT_HIGH	0x1
-
-struct aic32x4_setup_data {
-	unsigned int gpio_func[5];
-};
-
-struct aic32x4_pdata {
-	struct aic32x4_setup_data *setup;
-	u32 power_cfg;
-	u32 micpga_routing;
-	bool swapdacs;
-	int rstn_gpio;
-};
-
-#endif
diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c
index ffe1828a4b7e..2dd0fe255ee6 100644
--- a/sound/soc/codecs/tlv320aic32x4.c
+++ b/sound/soc/codecs/tlv320aic32x4.c
@@ -22,7 +22,6 @@
 #include <linux/of_clk.h>
 #include <linux/regulator/consumer.h>
 
-#include <sound/tlv320aic32x4.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -33,6 +32,10 @@
 
 #include "tlv320aic32x4.h"
 
+struct aic32x4_setup_data {
+	unsigned int gpio_func[5];
+};
+
 struct aic32x4_priv {
 	struct regmap *regmap;
 	u32 power_cfg;
@@ -1336,7 +1339,6 @@ static int aic32x4_setup_regulators(struct device *dev,
 int aic32x4_probe(struct device *dev, struct regmap *regmap)
 {
 	struct aic32x4_priv *aic32x4;
-	struct aic32x4_pdata *pdata = dev->platform_data;
 	struct device_node *np = dev->of_node;
 	int ret;
 
@@ -1353,13 +1355,7 @@ int aic32x4_probe(struct device *dev, struct regmap *regmap)
 
 	dev_set_drvdata(dev, aic32x4);
 
-	if (pdata) {
-		aic32x4->power_cfg = pdata->power_cfg;
-		aic32x4->swapdacs = pdata->swapdacs;
-		aic32x4->micpga_routing = pdata->micpga_routing;
-		aic32x4->rstn_gpio = pdata->rstn_gpio;
-		aic32x4->mclk_name = "mclk";
-	} else if (np) {
+	if (np) {
 		ret = aic32x4_parse_dt(aic32x4, np);
 		if (ret) {
 			dev_err(dev, "Failed to parse DT node\n");
diff --git a/sound/soc/codecs/tlv320aic32x4.h b/sound/soc/codecs/tlv320aic32x4.h
index 4de5bd9e8cc5..f0724b6b17ee 100644
--- a/sound/soc/codecs/tlv320aic32x4.h
+++ b/sound/soc/codecs/tlv320aic32x4.h
@@ -232,4 +232,31 @@ int aic32x4_register_clocks(struct device *dev, const char *mclk_name);
 #define AIC32X4_MAX_CODEC_CLKIN_FREQ    110000000
 #define AIC32X4_MAX_PLL_CLKIN		20000000
 
+#define AIC32X4_PWR_MICBIAS_2075_LDOIN		0x00000001
+#define AIC32X4_PWR_AVDD_DVDD_WEAK_DISABLE	0x00000002
+#define AIC32X4_PWR_AIC32X4_LDO_ENABLE		0x00000004
+#define AIC32X4_PWR_CMMODE_LDOIN_RANGE_18_36	0x00000008
+#define AIC32X4_PWR_CMMODE_HP_LDOIN_POWERED	0x00000010
+
+#define AIC32X4_MICPGA_ROUTE_LMIC_IN2R_10K	0x00000001
+#define AIC32X4_MICPGA_ROUTE_RMIC_IN1L_10K	0x00000002
+
+/* GPIO API */
+#define AIC32X4_MFPX_DEFAULT_VALUE	0xff
+
+#define AIC32X4_MFP1_DIN_DISABLED	0
+#define AIC32X4_MFP1_DIN_ENABLED	0x2
+#define AIC32X4_MFP1_GPIO_IN		0x4
+
+#define AIC32X4_MFP2_GPIO_OUT_LOW	0x0
+#define AIC32X4_MFP2_GPIO_OUT_HIGH	0x1
+
+#define AIC32X4_MFP_GPIO_ENABLED	0x4
+
+#define AIC32X4_MFP5_GPIO_DISABLED	0x0
+#define AIC32X4_MFP5_GPIO_INPUT		0x8
+#define AIC32X4_MFP5_GPIO_OUTPUT	0xc
+#define AIC32X4_MFP5_GPIO_OUT_LOW	0x0
+#define AIC32X4_MFP5_GPIO_OUT_HIGH	0x1
+
 #endif				/* _TLV320AIC32X4_H */
-- 
2.38.1.431.g37b22c650d-goog

[PATCH 07/11] ASoC: tlv320aic32x4: switch to using gpiod API

[Dmitry Torokhov]

Switch the driver from legacy gpio API that is deprecated to the newer
gpiod API that respects line polarities described in ACPI/DT.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 sound/soc/codecs/tlv320aic32x4.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c
index 2dd0fe255ee6..36a3b3eb4d56 100644
--- a/sound/soc/codecs/tlv320aic32x4.c
+++ b/sound/soc/codecs/tlv320aic32x4.c
@@ -13,9 +13,9 @@
 #include <linux/moduleparam.h>
 #include <linux/init.h>
 #include <linux/delay.h>
+#include <linux/err.h>
 #include <linux/pm.h>
-#include <linux/gpio.h>
-#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/cdev.h>
 #include <linux/slab.h>
 #include <linux/clk.h>
@@ -41,7 +41,7 @@ struct aic32x4_priv {
 	u32 power_cfg;
 	u32 micpga_routing;
 	bool swapdacs;
-	int rstn_gpio;
+	struct gpio_desc *reset_gpio;
 	const char *mclk_name;
 
 	struct regulator *supply_ldo;
@@ -1230,7 +1230,6 @@ static int aic32x4_parse_dt(struct aic32x4_priv *aic32x4,
 
 	aic32x4->swapdacs = false;
 	aic32x4->micpga_routing = 0;
-	aic32x4->rstn_gpio = of_get_named_gpio(np, "reset-gpios", 0);
 
 	if (of_property_read_u32_array(np, "aic32x4-gpio-func",
 				aic32x4_setup->gpio_func, 5) >= 0)
@@ -1365,16 +1364,16 @@ int aic32x4_probe(struct device *dev, struct regmap *regmap)
 		aic32x4->power_cfg = 0;
 		aic32x4->swapdacs = false;
 		aic32x4->micpga_routing = 0;
-		aic32x4->rstn_gpio = -1;
 		aic32x4->mclk_name = "mclk";
 	}
 
-	if (gpio_is_valid(aic32x4->rstn_gpio)) {
-		ret = devm_gpio_request_one(dev, aic32x4->rstn_gpio,
-				GPIOF_OUT_INIT_LOW, "tlv320aic32x4 rstn");
-		if (ret != 0)
-			return ret;
-	}
+	aic32x4->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						      GPIOD_OUT_HIGH);
+	ret = PTR_ERR_OR_ZERO(aic32x4->reset_gpio);
+	if (ret)
+		return ret;
+
+	gpiod_set_consumer_name(aic32x4->reset_gpio, "tlv320aic32x4 rstn");
 
 	ret = aic32x4_setup_regulators(dev, aic32x4);
 	if (ret) {
@@ -1382,9 +1381,9 @@ int aic32x4_probe(struct device *dev, struct regmap *regmap)
 		return ret;
 	}
 
-	if (gpio_is_valid(aic32x4->rstn_gpio)) {
+	if (aic32x4->reset_gpio) {
 		ndelay(10);
-		gpio_set_value_cansleep(aic32x4->rstn_gpio, 1);
+		gpiod_set_value_cansleep(aic32x4->reset_gpio, 0);
 		mdelay(1);
 	}
 
-- 
2.38.1.431.g37b22c650d-goog

[PATCH 08/11] ASoC: dt-bindings: wcd9335: fix reset line polarity in example

[Dmitry Torokhov]

When resetting the block, the reset line is being driven low and then
high, which means that the line in DTS should be annotated as "active
low".

Fixes: 1877c9fda1b7 ("ASoC: dt-bindings: add dt bindings for wcd9335 audio codec")
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

This was sent out previously as part of other series. Collected
Krzysztof's ack.

 Documentation/devicetree/bindings/sound/qcom,wcd9335.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd9335.txt b/Documentation/devicetree/bindings/sound/qcom,wcd9335.txt
index 5d6ea66a863f..1f75feec3dec 100644
--- a/Documentation/devicetree/bindings/sound/qcom,wcd9335.txt
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd9335.txt
@@ -109,7 +109,7 @@ audio-codec@1{
 	reg  = <1 0>;
 	interrupts = <&msmgpio 54 IRQ_TYPE_LEVEL_HIGH>;
 	interrupt-names = "intr2"
-	reset-gpios = <&msmgpio 64 0>;
+	reset-gpios = <&msmgpio 64 GPIO_ACTIVE_LOW>;
 	slim-ifc-dev  = <&wc9335_ifd>;
 	clock-names = "mclk", "native";
 	clocks = <&rpmcc RPM_SMD_DIV_CLK1>,
-- 
2.38.1.431.g37b22c650d-goog

[PATCH 09/11] ASoC: wcd9335: switch to using gpiod API

[Dmitry Torokhov]

Switch the driver from legacy gpio API that is deprecated to the newer
gpiod API that respects line polarities described in ACPI/DT.

Also the old code did not actually request the reset line, but was
toggling it directly; this has been fixed.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 sound/soc/codecs/wcd9335.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c
index d2548fdf9ae5..27cbec3e6763 100644
--- a/sound/soc/codecs/wcd9335.c
+++ b/sound/soc/codecs/wcd9335.c
@@ -11,12 +11,13 @@
 #include <linux/regulator/consumer.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/slimbus.h>
 #include <sound/soc.h>
 #include <sound/pcm_params.h>
 #include <sound/soc-dapm.h>
-#include <linux/of_gpio.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <sound/tlv.h>
@@ -329,7 +330,7 @@ struct wcd9335_codec {
 	int comp_enabled[COMPANDER_MAX];
 
 	int intr1;
-	int reset_gpio;
+	struct gpio_desc *reset_gpio;
 	struct regulator_bulk_data supplies[WCD9335_MAX_SUPPLY];
 
 	unsigned int rx_port_value[WCD9335_RX_MAX];
@@ -5032,25 +5033,27 @@ static const struct regmap_irq_chip wcd9335_regmap_irq1_chip = {
 static int wcd9335_parse_dt(struct wcd9335_codec *wcd)
 {
 	struct device *dev = wcd->dev;
-	struct device_node *np = dev->of_node;
 	int ret;
 
-	wcd->reset_gpio = of_get_named_gpio(np,	"reset-gpios", 0);
-	if (wcd->reset_gpio < 0) {
-		dev_err(dev, "Reset GPIO missing from DT\n");
-		return wcd->reset_gpio;
+	wcd->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	ret = PTR_ERR_OR_ZERO(wcd->reset_gpio);
+	if (ret) {
+		dev_err(dev, "failed to request reset GPIO: %d\n", ret);
+		return ret;
 	}
 
 	wcd->mclk = devm_clk_get(dev, "mclk");
-	if (IS_ERR(wcd->mclk)) {
-		dev_err(dev, "mclk not found\n");
-		return PTR_ERR(wcd->mclk);
+	ret = PTR_ERR_OR_ZERO(wcd->mclk);
+	if (ret) {
+		dev_err(dev, "mclk not found: %d\n", ret);
+		return ret;
 	}
 
 	wcd->native_clk = devm_clk_get(dev, "slimbus");
-	if (IS_ERR(wcd->native_clk)) {
-		dev_err(dev, "slimbus clock not found\n");
-		return PTR_ERR(wcd->native_clk);
+	ret = PTR_ERR_OR_ZERO(wcd->native_clk);
+	if (ret) {
+		dev_err(dev, "slimbus clock not found: %d\n", ret);
+		return ret;
 	}
 
 	wcd->supplies[0].supply = "vdd-buck";
@@ -5088,9 +5091,9 @@ static int wcd9335_power_on_reset(struct wcd9335_codec *wcd)
 	 */
 	usleep_range(600, 650);
 
-	gpio_direction_output(wcd->reset_gpio, 0);
+	gpiod_set_value_cansleep(wcd->reset_gpio, 1);
 	msleep(20);
-	gpio_set_value(wcd->reset_gpio, 1);
+	gpiod_set_value_cansleep(wcd->reset_gpio, 0);
 	msleep(20);
 
 	return 0;
-- 
2.38.1.431.g37b22c650d-goog

[PATCH 10/11] ASoC: dt-bindings: wcd938x: fix codec reset line polarity in example

[Dmitry Torokhov]

The driver for the codec, when resetting the chip, first drives the line
low, and then high. This means that the line is active low. Change the
annotation in the example DTS accordingly.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

This was sent out previously as part of other series. Collected
Krzysztof's ack and added missing include per Rob's feedback.

 Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
index 51547190f709..3cb542d559c6 100644
--- a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml
@@ -110,9 +110,10 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
     codec {
         compatible = "qcom,wcd9380-codec";
-        reset-gpios = <&tlmm 32 0>;
+        reset-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
         #sound-dai-cells = <1>;
         qcom,tx-device = <&wcd938x_tx>;
         qcom,rx-device = <&wcd938x_rx>;
-- 
2.38.1.431.g37b22c650d-goog

[PATCH 11/11] ASoC: wcd938x: switch to using gpiod API

[Dmitry Torokhov]

Switch the driver from legacy gpio API that is deprecated to the newer
gpiod API that respects line polarities described in ACPI/DT.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 sound/soc/codecs/wcd938x.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
index aca06a4026f3..b85bc750c7e0 100644
--- a/sound/soc/codecs/wcd938x.c
+++ b/sound/soc/codecs/wcd938x.c
@@ -6,12 +6,14 @@
 #include <linux/platform_device.h>
 #include <linux/device.h>
 #include <linux/delay.h>
+#include <linux/err.h>
 #include <linux/gpio/consumer.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/pm_runtime.h>
 #include <linux/component.h>
 #include <sound/tlv.h>
-#include <linux/of_gpio.h>
 #include <linux/of.h>
 #include <sound/jack.h>
 #include <sound/pcm.h>
@@ -194,7 +196,7 @@ struct wcd938x_priv {
 	int flyback_cur_det_disable;
 	int ear_rx_path;
 	int variant;
-	int reset_gpio;
+	struct gpio_desc *reset_gpio;
 	struct gpio_desc *us_euro_gpio;
 	u32 micb1_mv;
 	u32 micb2_mv;
@@ -4234,18 +4236,19 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
 	struct wcd_mbhc_config *cfg = &wcd938x->mbhc_cfg;
 	int ret;
 
-	wcd938x->reset_gpio = of_get_named_gpio(dev->of_node, "reset-gpios", 0);
-	if (wcd938x->reset_gpio < 0) {
-		dev_err(dev, "Failed to get reset gpio: err = %d\n",
-			wcd938x->reset_gpio);
-		return wcd938x->reset_gpio;
+	wcd938x->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_ASIS);
+	ret = PTR_ERR_OR_ZERO(wcd938x->reset_gpio);
+	if (ret) {
+		dev_err(dev, "Failed to get reset gpio: err = %d\n", ret);
+		return ret;
 	}
 
 	wcd938x->us_euro_gpio = devm_gpiod_get_optional(dev, "us-euro",
 						GPIOD_OUT_LOW);
-	if (IS_ERR(wcd938x->us_euro_gpio)) {
+	ret = PTR_ERR_OR_ZERO(wcd938x->us_euro_gpio);
+	if (ret) {
 		dev_err(dev, "us-euro swap Control GPIO not found\n");
-		return PTR_ERR(wcd938x->us_euro_gpio);
+		return ret;
 	}
 
 	cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
@@ -4285,11 +4288,11 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device
 
 static int wcd938x_reset(struct wcd938x_priv *wcd938x)
 {
-	gpio_direction_output(wcd938x->reset_gpio, 0);
-	/* 20us sleep required after pulling the reset gpio to LOW */
+	gpiod_set_value_cansleep(wcd938x->reset_gpio, 1);
+	/* 20us sleep required after asserting the reset gpio */
 	usleep_range(20, 30);
-	gpio_set_value(wcd938x->reset_gpio, 1);
-	/* 20us sleep required after pulling the reset gpio to HIGH */
+	gpiod_set_value_cansleep(wcd938x->reset_gpio, 0);
+	/* 20us sleep required after releasing the reset gpio */
 	usleep_range(20, 30);
 
 	return 0;
-- 
2.38.1.431.g37b22c650d-goog

Re: [PATCH 01/11] ASoC: ak5386: switch to using gpiod API

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

On Tue, Nov 15, 2022 at 09:38:07PM -0800, Dmitry Torokhov wrote:

> Switch the driver from legacy gpio API that is deprecated to the newer
> gpiod API that respects line polarities described in ACPI/DT.

> -	if (gpio_is_valid(priv->reset_gpio))
> -		gpio_set_value(priv->reset_gpio, 1);
> +	if (priv->reset_gpio)
> +		gpiod_set_value(priv->reset_gpio, 0);

How are we ensuring that people have described signals as active
low/high in existing DTs, and are we positive that the signal is
described as active low for all devices?  In particular if the
signal is described as a reset signal then it's active high even
if we want it low while the device is actually in use.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 01/11] ASoC: ak5386: switch to using gpiod API

[Dmitry Torokhov]

On Wed, Nov 16, 2022 at 10:36:27AM +0000, Mark Brown wrote:
> On Tue, Nov 15, 2022 at 09:38:07PM -0800, Dmitry Torokhov wrote:
> 
> > Switch the driver from legacy gpio API that is deprecated to the newer
> > gpiod API that respects line polarities described in ACPI/DT.
> 
> > -	if (gpio_is_valid(priv->reset_gpio))
> > -		gpio_set_value(priv->reset_gpio, 1);
> > +	if (priv->reset_gpio)
> > +		gpiod_set_value(priv->reset_gpio, 0);
> 
> How are we ensuring that people have described signals as active
> low/high in existing DTs, and are we positive that the signal is
> described as active low for all devices?  In particular if the
> signal is described as a reset signal then it's active high even
> if we want it low while the device is actually in use.

I have been going through in-kernel DTSes and adjusting ones that are
incorrect. For external ones I think we should take a pragmatic approach
and say that if driver has last non-mechanical update in 2014 and there
are no users submitted to mainline since then (as this one), then it is
highly unlikely that devices currently using this component/codec will
be updated to the 6.2+ kernel even if they are still in service. And if
this does happen the breakage will be immediately obvious as we'll keep
the codec in reset state.

But if you really want to I can add quirk(s) to gpiolib forcing this
line to be treated as active-low regardless of what specified in DTS.
This kind of negates benefit of going to gpiod though.

Please let me know.

-- 
Dmitry

Re: [PATCH 01/11] ASoC: ak5386: switch to using gpiod API

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

On Wed, Nov 16, 2022 at 11:07:48AM -0800, Dmitry Torokhov wrote:
> On Wed, Nov 16, 2022 at 10:36:27AM +0000, Mark Brown wrote:

> > How are we ensuring that people have described signals as active
> > low/high in existing DTs, and are we positive that the signal is
> > described as active low for all devices?  In particular if the
> > signal is described as a reset signal then it's active high even
> > if we want it low while the device is actually in use.

> I have been going through in-kernel DTSes and adjusting ones that are
> incorrect. For external ones I think we should take a pragmatic approach
> and say that if driver has last non-mechanical update in 2014 and there
> are no users submitted to mainline since then (as this one), then it is
> highly unlikely that devices currently using this component/codec will
> be updated to the 6.2+ kernel even if they are still in service. And if
> this does happen the breakage will be immediately obvious as we'll keep
> the codec in reset state.

> But if you really want to I can add quirk(s) to gpiolib forcing this
> line to be treated as active-low regardless of what specified in DTS.
> This kind of negates benefit of going to gpiod though.

That doesn't address the bit about checking that the device
describes the signal as active low in hardware - it's assuming
that the signal is described by the device as an active low
reset and not for example as a shutdown signal.

TBH I'm not thrilled about just randomly breaking ABI
compatibility for neatness reasons, it's really not helping
people take device tree ABI compatibility seriously.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 01/11] ASoC: ak5386: switch to using gpiod API

[Dmitry Torokhov]

On Thu, Nov 17, 2022 at 11:34:06AM +0000, Mark Brown wrote:
> On Wed, Nov 16, 2022 at 11:07:48AM -0800, Dmitry Torokhov wrote:
> > On Wed, Nov 16, 2022 at 10:36:27AM +0000, Mark Brown wrote:
> 
> > > How are we ensuring that people have described signals as active
> > > low/high in existing DTs, and are we positive that the signal is
> > > described as active low for all devices?  In particular if the
> > > signal is described as a reset signal then it's active high even
> > > if we want it low while the device is actually in use.
> 
> > I have been going through in-kernel DTSes and adjusting ones that are
> > incorrect. For external ones I think we should take a pragmatic approach
> > and say that if driver has last non-mechanical update in 2014 and there
> > are no users submitted to mainline since then (as this one), then it is
> > highly unlikely that devices currently using this component/codec will
> > be updated to the 6.2+ kernel even if they are still in service. And if
> > this does happen the breakage will be immediately obvious as we'll keep
> > the codec in reset state.
> 
> > But if you really want to I can add quirk(s) to gpiolib forcing this
> > line to be treated as active-low regardless of what specified in DTS.
> > This kind of negates benefit of going to gpiod though.
> 
> That doesn't address the bit about checking that the device
> describes the signal as active low in hardware - it's assuming
> that the signal is described by the device as an active low
> reset and not for example as a shutdown signal.

Huh? If we add a quirk to gpiolib to treat the signal as active low
(i.e. preserve current driver behavior - I am talking about this
particular peripheral here, not treating everything as active low of
course).

> 
> TBH I'm not thrilled about just randomly breaking ABI
> compatibility for neatness reasons, it's really not helping
> people take device tree ABI compatibility seriously.

Yes, I freely admit I do not take device tree ABI compatibility
seriously. IMO, with the exception of a few peripherals, it is a
solution in search of a problem, and we declared stability of it too
early, before we came up with reasonable rules for how resources should
be described. I strongly believe that in vast majority of cases devices
with out-of-tree DTs will not be updated to upstream kernels as this
requires significant engineering effort and vendors usually not
interested in doing that.

Thanks.

-- 
Dmitry

Re: [PATCH 01/11] ASoC: ak5386: switch to using gpiod API

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]

On Thu, Nov 17, 2022 at 10:31:49PM -0800, Dmitry Torokhov wrote:
> On Thu, Nov 17, 2022 at 11:34:06AM +0000, Mark Brown wrote:

> > That doesn't address the bit about checking that the device
> > describes the signal as active low in hardware - it's assuming
> > that the signal is described by the device as an active low
> > reset and not for example as a shutdown signal.

> Huh? If we add a quirk to gpiolib to treat the signal as active low
> (i.e. preserve current driver behavior - I am talking about this
> particular peripheral here, not treating everything as active low of
> course).

My comments were more generic ones about the whole series since
all the patches seemed to be doing the same thing with flipping
the polarity - some of the GPIOs were labelled as things like
reset which is active high if it's not nRESET or something even
though we want to pull it low while using the device.

> > TBH I'm not thrilled about just randomly breaking ABI
> > compatibility for neatness reasons, it's really not helping
> > people take device tree ABI compatibility seriously.

> Yes, I freely admit I do not take device tree ABI compatibility
> seriously. IMO, with the exception of a few peripherals, it is a
> solution in search of a problem, and we declared stability of it too
> early, before we came up with reasonable rules for how resources should
> be described. I strongly believe that in vast majority of cases devices
> with out-of-tree DTs will not be updated to upstream kernels as this
> requires significant engineering effort and vendors usually not
> interested in doing that.

There are practical systems which ship DTs as part of the
firmware, and frankly things like this do contribute to the
issue.  The systems that just ship their DTs are obviously a lot
less visible, but that's the whole goal here.  It's most common
with more server type systems using EDK2 for the firmware, ACPI
isn't always a good fit for them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 11/11] ASoC: wcd938x: switch to using gpiod API

[Krzysztof Kozlowski]

On 16/11/2022 06:38, Dmitry Torokhov wrote:
> Switch the driver from legacy gpio API that is deprecated to the newer
> gpiod API that respects line polarities described in ACPI/DT.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  sound/soc/codecs/wcd938x.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 

I almost sent same patch. DTS was already aligned with this, thus:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Tested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof