[PATCH 01/11] ASoC: ak5386: switch to using gpiod API
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);
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;
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) {
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);
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",
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 */
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); }
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>,
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;
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>;
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;
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
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.
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.
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.
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.
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.
participants (3)
-
Dmitry Torokhov -
Krzysztof Kozlowski -
Mark Brown