[PATCH 0/2] ASoC: codec: Convert to GPIO descriptors for tlv320aic32x4

[PATCH 0/2] ASoC: codec: Convert to GPIO descriptors for tlv320aic32x4

[Peng Fan]

This patchset is a pick up of patch 1,2 from [1]. And I also collect
Linus's R-b for patch 2. After this patchset, there is only one user of
of_gpio.h left in sound driver(pxa2xx-ac97).

of_gpio.h is deprecated, update the driver to use GPIO descriptors.

Patch 1 is to drop legacy platform data which in-tree no users are using it
Patch 2 is to convert to GPIO descriptors

Checking the DTS that use the device, all are using GPIOD_ACTIVE_LOW
polarity for reset-gpios, so all should work as expected with this patch.

[1] https://lore.kernel.org/all/20250408-asoc-gpio-v1-0-c0db9d3fd6e9@nxp.com/

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (2):
      ASoC: codec: tlv320aic32x4: Drop aic32x4_pdata usage
      ASoC: codec: tlv320aic32x4: Convert to GPIO descriptors

 include/sound/tlv320aic32x4.h    |  9 -------
 sound/soc/codecs/tlv320aic32x4.c | 53 +++++++++++++++++-----------------------
 2 files changed, 23 insertions(+), 39 deletions(-)
---
base-commit: 26ffb3d6f02cd0935fb9fa3db897767beee1cb2a
change-id: 20250706-asoc-gpio-1-bd0762d29351

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH 1/2] ASoC: codec: tlv320aic32x4: Drop aic32x4_pdata usage

[Peng Fan]

There is no machine is using aic32x4_pdata as platform_data, so
remove the dead code.

Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 include/sound/tlv320aic32x4.h    | 9 ---------
 sound/soc/codecs/tlv320aic32x4.c | 9 +--------
 2 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/include/sound/tlv320aic32x4.h b/include/sound/tlv320aic32x4.h
index 0abf74d7edbd69484c45ad6a1c39b3f67d61bd63..b779d671a99576deadc6e647edff9b1b3a5d33c2 100644
--- a/include/sound/tlv320aic32x4.h
+++ b/include/sound/tlv320aic32x4.h
@@ -40,13 +40,4 @@
 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 54ea4bc58c276d9ab39a15d312287dfb300dbab9..7dbcf7f7130b04a27f58f20beb83eb3676c79c3d 100644
--- a/sound/soc/codecs/tlv320aic32x4.c
+++ b/sound/soc/codecs/tlv320aic32x4.c
@@ -1346,7 +1346,6 @@ int aic32x4_probe(struct device *dev, struct regmap *regmap,
 		  enum aic32x4_type type)
 {
 	struct aic32x4_priv *aic32x4;
-	struct aic32x4_pdata *pdata = dev->platform_data;
 	struct device_node *np = dev->of_node;
 	int ret;
 
@@ -1363,13 +1362,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");

-- 
2.37.1

Re: [PATCH 1/2] ASoC: codec: tlv320aic32x4: Drop aic32x4_pdata usage

[Alexander Stein]

Hi,

Am Sonntag, 6. Juli 2025, 03:04:23 CEST schrieb Peng Fan:
> There is no machine is using aic32x4_pdata as platform_data, so
> remove the dead code.
> 
> Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
>  include/sound/tlv320aic32x4.h    | 9 ---------
>  sound/soc/codecs/tlv320aic32x4.c | 9 +--------
>  2 files changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/include/sound/tlv320aic32x4.h b/include/sound/tlv320aic32x4.h
> index 0abf74d7edbd69484c45ad6a1c39b3f67d61bd63..b779d671a99576deadc6e647edff9b1b3a5d33c2 100644
> --- a/include/sound/tlv320aic32x4.h
> +++ b/include/sound/tlv320aic32x4.h
> @@ -40,13 +40,4 @@
>  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 54ea4bc58c276d9ab39a15d312287dfb300dbab9..7dbcf7f7130b04a27f58f20beb83eb3676c79c3d 100644
> --- a/sound/soc/codecs/tlv320aic32x4.c
> +++ b/sound/soc/codecs/tlv320aic32x4.c
> @@ -1346,7 +1346,6 @@ int aic32x4_probe(struct device *dev, struct regmap *regmap,
>  		  enum aic32x4_type type)
>  {
>  	struct aic32x4_priv *aic32x4;
> -	struct aic32x4_pdata *pdata = dev->platform_data;
>  	struct device_node *np = dev->of_node;
>  	int ret;
>  
> @@ -1363,13 +1362,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");
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

[PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to GPIO descriptors

[Peng Fan]

of_gpio.h is deprecated, update the driver to use GPIO descriptors.
 - Use devm_gpiod_get_optional to get GPIO descriptor, and set consumer
   name.
 - Use gpiod_set_value to configure output value.

While at here, reorder the included headers.

Checking the DTS that use the device, all are using GPIOD_ACTIVE_LOW
polarity for reset-gpios, so all should work as expected with this patch.

Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 sound/soc/codecs/tlv320aic32x4.c | 44 ++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c
index 7dbcf7f7130b04a27f58f20beb83eb3676c79c3d..1423186f5a6c181a20dd2dd552679d33174edaee 100644
--- a/sound/soc/codecs/tlv320aic32x4.c
+++ b/sound/soc/codecs/tlv320aic32x4.c
@@ -9,27 +9,26 @@
  * Based on sound/soc/codecs/wm8974 and TI driver for kernel 2.6.27.
  */
 
-#include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/init.h>
-#include <linux/delay.h>
-#include <linux/pm.h>
-#include <linux/gpio.h>
-#include <linux/of_gpio.h>
 #include <linux/cdev.h>
-#include <linux/slab.h>
 #include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/of_clk.h>
+#include <linux/pm.h>
 #include <linux/regulator/consumer.h>
+#include <linux/slab.h>
 
-#include <sound/tlv320aic32x4.h>
 #include <sound/core.h>
+#include <sound/initval.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
-#include <sound/initval.h>
 #include <sound/tlv.h>
+#include <sound/tlv320aic32x4.h>
 
 #include "tlv320aic32x4.h"
 
@@ -38,7 +37,7 @@ struct aic32x4_priv {
 	u32 power_cfg;
 	u32 micpga_routing;
 	bool swapdacs;
-	int rstn_gpio;
+	struct gpio_desc *rstn_gpio;
 	const char *mclk_name;
 
 	struct regulator *supply_ldo;
@@ -1236,7 +1235,14 @@ 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);
+	/* Assert reset using GPIOD_OUT_HIGH, because reset is GPIO_ACTIVE_LOW */
+	aic32x4->rstn_gpio = devm_gpiod_get_optional(aic32x4->dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(aic32x4->rstn_gpio)) {
+		return dev_err_probe(aic32x4->dev, PTR_ERR(aic32x4->rstn_gpio),
+				     "Failed to get reset gpio\n");
+	} else {
+		gpiod_set_consumer_name(aic32x4->rstn_gpio, "tlv320aic32x4_rstn");
+	}
 
 	if (of_property_read_u32_array(np, "aic32x4-gpio-func",
 				aic32x4_setup->gpio_func, 5) >= 0)
@@ -1372,26 +1378,20 @@ 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->rstn_gpio = ERR_PTR(-ENOENT);
 		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;
-	}
-
 	ret = aic32x4_setup_regulators(dev, aic32x4);
 	if (ret) {
 		dev_err(dev, "Failed to setup regulators\n");
 		return ret;
 	}
 
-	if (gpio_is_valid(aic32x4->rstn_gpio)) {
+	if (!IS_ERR(aic32x4->rstn_gpio)) {
 		ndelay(10);
-		gpio_set_value_cansleep(aic32x4->rstn_gpio, 1);
+		/* deassert reset */
+		gpiod_set_value_cansleep(aic32x4->rstn_gpio, 0);
 		mdelay(1);
 	}
 

-- 
2.37.1

Re: [PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to GPIO descriptors

[Alexander Stein]

Hi,

Am Sonntag, 6. Juli 2025, 03:04:24 CEST schrieb Peng Fan:
> of_gpio.h is deprecated, update the driver to use GPIO descriptors.
>  - Use devm_gpiod_get_optional to get GPIO descriptor, and set consumer
>    name.
>  - Use gpiod_set_value to configure output value.
> 
> While at here, reorder the included headers.
> 
> Checking the DTS that use the device, all are using GPIOD_ACTIVE_LOW
> polarity for reset-gpios, so all should work as expected with this patch.
> 
> Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  sound/soc/codecs/tlv320aic32x4.c | 44 ++++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c
> index 7dbcf7f7130b04a27f58f20beb83eb3676c79c3d..1423186f5a6c181a20dd2dd552679d33174edaee 100644
> --- a/sound/soc/codecs/tlv320aic32x4.c
> +++ b/sound/soc/codecs/tlv320aic32x4.c
> @@ -9,27 +9,26 @@
>   * Based on sound/soc/codecs/wm8974 and TI driver for kernel 2.6.27.
>   */
>  
> -#include <linux/module.h>
> -#include <linux/moduleparam.h>
> -#include <linux/init.h>
> -#include <linux/delay.h>
> -#include <linux/pm.h>
> -#include <linux/gpio.h>
> -#include <linux/of_gpio.h>
>  #include <linux/cdev.h>
> -#include <linux/slab.h>
>  #include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
>  #include <linux/of_clk.h>
> +#include <linux/pm.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
>  
> -#include <sound/tlv320aic32x4.h>
>  #include <sound/core.h>
> +#include <sound/initval.h>
>  #include <sound/pcm.h>
>  #include <sound/pcm_params.h>
>  #include <sound/soc.h>
>  #include <sound/soc-dapm.h>
> -#include <sound/initval.h>
>  #include <sound/tlv.h>
> +#include <sound/tlv320aic32x4.h>

Mh, maybe create a single commit sorting these headers.

>  
>  #include "tlv320aic32x4.h"
>  
> @@ -38,7 +37,7 @@ struct aic32x4_priv {
>  	u32 power_cfg;
>  	u32 micpga_routing;
>  	bool swapdacs;
> -	int rstn_gpio;
> +	struct gpio_desc *rstn_gpio;
>  	const char *mclk_name;
>  
>  	struct regulator *supply_ldo;
> @@ -1236,7 +1235,14 @@ 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);
> +	/* Assert reset using GPIOD_OUT_HIGH, because reset is GPIO_ACTIVE_LOW */
> +	aic32x4->rstn_gpio = devm_gpiod_get_optional(aic32x4->dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(aic32x4->rstn_gpio)) {
> +		return dev_err_probe(aic32x4->dev, PTR_ERR(aic32x4->rstn_gpio),
> +				     "Failed to get reset gpio\n");
> +	} else {
> +		gpiod_set_consumer_name(aic32x4->rstn_gpio, "tlv320aic32x4_rstn");
> +	}
>  
>  	if (of_property_read_u32_array(np, "aic32x4-gpio-func",
>  				aic32x4_setup->gpio_func, 5) >= 0)
> @@ -1372,26 +1378,20 @@ 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->rstn_gpio = ERR_PTR(-ENOENT);

Shouldn't this be NULL similar to when devm_gpiod_get_optional() doesn't
find any reset GPIO?

Despite that, looks good and works as intended:
Tested-By: Alexander Stein <alexander.stein@ew.tq-group.com>

>  		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;
> -	}
> -
>  	ret = aic32x4_setup_regulators(dev, aic32x4);
>  	if (ret) {
>  		dev_err(dev, "Failed to setup regulators\n");
>  		return ret;
>  	}
>  
> -	if (gpio_is_valid(aic32x4->rstn_gpio)) {
> +	if (!IS_ERR(aic32x4->rstn_gpio)) {
>  		ndelay(10);
> -		gpio_set_value_cansleep(aic32x4->rstn_gpio, 1);
> +		/* deassert reset */
> +		gpiod_set_value_cansleep(aic32x4->rstn_gpio, 0);
>  		mdelay(1);
>  	}
>  
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

RE: [PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to GPIO descriptors

[Peng Fan]

> Subject: Re: [PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to GPIO
> descriptors
> 
> Hi,
> 
> Am Sonntag, 6. Juli 2025, 03:04:24 CEST schrieb Peng Fan:
> > of_gpio.h is deprecated, update the driver to use GPIO descriptors.
> >  - Use devm_gpiod_get_optional to get GPIO descriptor, and set
> consumer
> >    name.
> >  - Use gpiod_set_value to configure output value.
> >
> > While at here, reorder the included headers.
> >
> > Checking the DTS that use the device, all are using
> GPIOD_ACTIVE_LOW
> > polarity for reset-gpios, so all should work as expected with this patch.
> >
> > Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
> > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  sound/soc/codecs/tlv320aic32x4.c | 44
> > ++++++++++++++++++++--------------------
> >  1 file changed, 22 insertions(+), 22 deletions(-)
> >
> > diff --git a/sound/soc/codecs/tlv320aic32x4.c
> > b/sound/soc/codecs/tlv320aic32x4.c
> > index
> >
> 7dbcf7f7130b04a27f58f20beb83eb3676c79c3d..1423186f5a6c181a2
> 0dd2dd55267
> > 9d33174edaee 100644
> > --- a/sound/soc/codecs/tlv320aic32x4.c
> > +++ b/sound/soc/codecs/tlv320aic32x4.c
> > @@ -9,27 +9,26 @@
> >   * Based on sound/soc/codecs/wm8974 and TI driver for kernel
> 2.6.27.
> >   */
> >
> > -#include <linux/module.h>
> > -#include <linux/moduleparam.h>
> > -#include <linux/init.h>
> > -#include <linux/delay.h>
> > -#include <linux/pm.h>
> > -#include <linux/gpio.h>
> > -#include <linux/of_gpio.h>
> >  #include <linux/cdev.h>
> > -#include <linux/slab.h>
> >  #include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/moduleparam.h>
> >  #include <linux/of_clk.h>
> > +#include <linux/pm.h>
> >  #include <linux/regulator/consumer.h>
> > +#include <linux/slab.h>
> >
> > -#include <sound/tlv320aic32x4.h>
> >  #include <sound/core.h>
> > +#include <sound/initval.h>
> >  #include <sound/pcm.h>
> >  #include <sound/pcm_params.h>
> >  #include <sound/soc.h>
> >  #include <sound/soc-dapm.h>
> > -#include <sound/initval.h>
> >  #include <sound/tlv.h>
> > +#include <sound/tlv320aic32x4.h>
> 
> Mh, maybe create a single commit sorting these headers.

ok. Let me do a v2 for this.

> 
> >
> >  #include "tlv320aic32x4.h"
> >
> > @@ -38,7 +37,7 @@ struct aic32x4_priv {
> >  	u32 power_cfg;
> >  	u32 micpga_routing;
> >  	bool swapdacs;
> > -	int rstn_gpio;
> > +	struct gpio_desc *rstn_gpio;
> >  	const char *mclk_name;
> >
> >  	struct regulator *supply_ldo;
> > @@ -1236,7 +1235,14 @@ 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);
> > +	/* Assert reset using GPIOD_OUT_HIGH, because reset is
> GPIO_ACTIVE_LOW */
> > +	aic32x4->rstn_gpio = devm_gpiod_get_optional(aic32x4->dev,
> "reset", GPIOD_OUT_HIGH);
> > +	if (IS_ERR(aic32x4->rstn_gpio)) {
> > +		return dev_err_probe(aic32x4->dev, PTR_ERR(aic32x4-
> >rstn_gpio),
> > +				     "Failed to get reset gpio\n");
> > +	} else {
> > +		gpiod_set_consumer_name(aic32x4->rstn_gpio,
> "tlv320aic32x4_rstn");
> > +	}
> >
> >  	if (of_property_read_u32_array(np, "aic32x4-gpio-func",
> >  				aic32x4_setup->gpio_func, 5) >= 0)
> @@ -1372,26 +1378,20 @@ 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->rstn_gpio = ERR_PTR(-ENOENT);
> 
> Shouldn't this be NULL similar to when devm_gpiod_get_optional()
> doesn't find any reset GPIO?

There is a check in driver, so NULL not work here.

        if (!IS_ERR(aic32x4->rstn_gpio)) {                                                          
                ndelay(10);                                                                         
                /* deassert reset */                                                                
                gpiod_set_value_cansleep(aic32x4->rstn_gpio, 0);                                    
                mdelay(1);                                                                          
        }

> 
> Despite that, looks good and works as intended:
> Tested-By: Alexander Stein <alexander.stein@ew.tq-group.com>

Appreciate!

Thanks,
Peng.

Re: [PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to GPIO descriptors

[Alexander Stein]

Am Montag, 7. Juli 2025, 07:40:58 CEST schrieb Peng Fan:
> ********************
> Achtung externe E-Mail: Öffnen Sie Anhänge und Links nur, wenn Sie wissen, dass diese aus einer sicheren Quelle stammen und sicher sind. Leiten Sie die E-Mail im Zweifelsfall zur Prüfung an den IT-Helpdesk weiter.
> Attention external email: Open attachments and links only if you know that they are from a secure source and are safe. In doubt forward the email to the IT-Helpdesk to check it.
> ********************
> 
> > Subject: Re: [PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to GPIO
> > descriptors
> > 
> > Hi,
> > 
> > Am Sonntag, 6. Juli 2025, 03:04:24 CEST schrieb Peng Fan:
> > > of_gpio.h is deprecated, update the driver to use GPIO descriptors.
> > >  - Use devm_gpiod_get_optional to get GPIO descriptor, and set
> > consumer
> > >    name.
> > >  - Use gpiod_set_value to configure output value.
> > >
> > > While at here, reorder the included headers.
> > >
> > > Checking the DTS that use the device, all are using
> > GPIOD_ACTIVE_LOW
> > > polarity for reset-gpios, so all should work as expected with this patch.
> > >
> > > Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
> > > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  sound/soc/codecs/tlv320aic32x4.c | 44
> > > ++++++++++++++++++++--------------------
> > >  1 file changed, 22 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/sound/soc/codecs/tlv320aic32x4.c
> > > b/sound/soc/codecs/tlv320aic32x4.c
> > > index
> > >
> > 7dbcf7f7130b04a27f58f20beb83eb3676c79c3d..1423186f5a6c181a2
> > 0dd2dd55267
> > > 9d33174edaee 100644
> > > --- a/sound/soc/codecs/tlv320aic32x4.c
> > > +++ b/sound/soc/codecs/tlv320aic32x4.c
> > > @@ -9,27 +9,26 @@
> > >   * Based on sound/soc/codecs/wm8974 and TI driver for kernel
> > 2.6.27.
> > >   */
> > >
> > > -#include <linux/module.h>
> > > -#include <linux/moduleparam.h>
> > > -#include <linux/init.h>
> > > -#include <linux/delay.h>
> > > -#include <linux/pm.h>
> > > -#include <linux/gpio.h>
> > > -#include <linux/of_gpio.h>
> > >  #include <linux/cdev.h>
> > > -#include <linux/slab.h>
> > >  #include <linux/clk.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/init.h>
> > > +#include <linux/module.h>
> > > +#include <linux/moduleparam.h>
> > >  #include <linux/of_clk.h>
> > > +#include <linux/pm.h>
> > >  #include <linux/regulator/consumer.h>
> > > +#include <linux/slab.h>
> > >
> > > -#include <sound/tlv320aic32x4.h>
> > >  #include <sound/core.h>
> > > +#include <sound/initval.h>
> > >  #include <sound/pcm.h>
> > >  #include <sound/pcm_params.h>
> > >  #include <sound/soc.h>
> > >  #include <sound/soc-dapm.h>
> > > -#include <sound/initval.h>
> > >  #include <sound/tlv.h>
> > > +#include <sound/tlv320aic32x4.h>
> > 
> > Mh, maybe create a single commit sorting these headers.
> 
> ok. Let me do a v2 for this.
> 
> > 
> > >
> > >  #include "tlv320aic32x4.h"
> > >
> > > @@ -38,7 +37,7 @@ struct aic32x4_priv {
> > >  	u32 power_cfg;
> > >  	u32 micpga_routing;
> > >  	bool swapdacs;
> > > -	int rstn_gpio;
> > > +	struct gpio_desc *rstn_gpio;
> > >  	const char *mclk_name;
> > >
> > >  	struct regulator *supply_ldo;
> > > @@ -1236,7 +1235,14 @@ 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);
> > > +	/* Assert reset using GPIOD_OUT_HIGH, because reset is
> > GPIO_ACTIVE_LOW */
> > > +	aic32x4->rstn_gpio = devm_gpiod_get_optional(aic32x4->dev,
> > "reset", GPIOD_OUT_HIGH);
> > > +	if (IS_ERR(aic32x4->rstn_gpio)) {
> > > +		return dev_err_probe(aic32x4->dev, PTR_ERR(aic32x4-
> > >rstn_gpio),
> > > +				     "Failed to get reset gpio\n");
> > > +	} else {
> > > +		gpiod_set_consumer_name(aic32x4->rstn_gpio,
> > "tlv320aic32x4_rstn");
> > > +	}
> > >
> > >  	if (of_property_read_u32_array(np, "aic32x4-gpio-func",
> > >  				aic32x4_setup->gpio_func, 5) >= 0)
> > @@ -1372,26 +1378,20 @@ 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->rstn_gpio = ERR_PTR(-ENOENT);
> > 
> > Shouldn't this be NULL similar to when devm_gpiod_get_optional()
> > doesn't find any reset GPIO?
> 
> There is a check in driver, so NULL not work here.
> 
>         if (!IS_ERR(aic32x4->rstn_gpio)) {                                                          

I don't like the fact that both paths have different values for rstn_gpio
for the information there is no rstn GPIO. How about setting NULL in the without
DT node case and checking if (aic32x4->rstn_gpio) here?

Best regards
Alexander

>                 ndelay(10);                                                                         
>                 /* deassert reset */                                                                
>                 gpiod_set_value_cansleep(aic32x4->rstn_gpio, 0);                                    
>                 mdelay(1);                                                                          
>         }
> 
> > 
> > Despite that, looks good and works as intended:
> > Tested-By: Alexander Stein <alexander.stein@ew.tq-group.com>
> 
> Appreciate!
> 
> Thanks,
> Peng.
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

RE: [PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to GPIO descriptors

[Peng Fan]

> Subject: Re: [PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to GPIO
> descriptors
> 
> Am Montag, 7. Juli 2025, 07:40:58 CEST schrieb Peng Fan:
> > ********************
> > Achtung externe E-Mail: Öffnen Sie Anhänge und Links nur, wenn Sie
> wissen, dass diese aus einer sicheren Quelle stammen und sicher sind.
> Leiten Sie die E-Mail im Zweifelsfall zur Prüfung an den IT-Helpdesk
> weiter.
> > Attention external email: Open attachments and links only if you
> know that they are from a secure source and are safe. In doubt forward
> the email to the IT-Helpdesk to check it.
> > ********************
> >
> > > Subject: Re: [PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to
> GPIO
> > > descriptors
> > >
> > > Hi,
> > >
> > > Am Sonntag, 6. Juli 2025, 03:04:24 CEST schrieb Peng Fan:
> > > > of_gpio.h is deprecated, update the driver to use GPIO descriptors.
> > > >  - Use devm_gpiod_get_optional to get GPIO descriptor, and set
> > > consumer
> > > >    name.
> > > >  - Use gpiod_set_value to configure output value.
> > > >
> > > > While at here, reorder the included headers.
> > > >
> > > > Checking the DTS that use the device, all are using
> > > GPIOD_ACTIVE_LOW
> > > > polarity for reset-gpios, so all should work as expected with this
> patch.
> > > >
> > > > Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
> > > > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >  sound/soc/codecs/tlv320aic32x4.c | 44
> > > > ++++++++++++++++++++--------------------
> > > >  1 file changed, 22 insertions(+), 22 deletions(-)
> > > >
> > > > diff --git a/sound/soc/codecs/tlv320aic32x4.c
> > > > b/sound/soc/codecs/tlv320aic32x4.c
> > > > index
> > > >
> > >
> 7dbcf7f7130b04a27f58f20beb83eb3676c79c3d..1423186f5a6c181a2
> > > 0dd2dd55267
> > > > 9d33174edaee 100644
> > > > --- a/sound/soc/codecs/tlv320aic32x4.c
> > > > +++ b/sound/soc/codecs/tlv320aic32x4.c
> > > > @@ -9,27 +9,26 @@
> > > >   * Based on sound/soc/codecs/wm8974 and TI driver for kernel
> > > 2.6.27.
> > > >   */
> > > >
> > > > -#include <linux/module.h>
> > > > -#include <linux/moduleparam.h>
> > > > -#include <linux/init.h>
> > > > -#include <linux/delay.h>
> > > > -#include <linux/pm.h>
> > > > -#include <linux/gpio.h>
> > > > -#include <linux/of_gpio.h>
> > > >  #include <linux/cdev.h>
> > > > -#include <linux/slab.h>
> > > >  #include <linux/clk.h>
> > > > +#include <linux/delay.h>
> > > > +#include <linux/gpio/consumer.h>
> > > > +#include <linux/init.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/moduleparam.h>
> > > >  #include <linux/of_clk.h>
> > > > +#include <linux/pm.h>
> > > >  #include <linux/regulator/consumer.h>
> > > > +#include <linux/slab.h>
> > > >
> > > > -#include <sound/tlv320aic32x4.h>
> > > >  #include <sound/core.h>
> > > > +#include <sound/initval.h>
> > > >  #include <sound/pcm.h>
> > > >  #include <sound/pcm_params.h>
> > > >  #include <sound/soc.h>
> > > >  #include <sound/soc-dapm.h>
> > > > -#include <sound/initval.h>
> > > >  #include <sound/tlv.h>
> > > > +#include <sound/tlv320aic32x4.h>
> > >
> > > Mh, maybe create a single commit sorting these headers.
> >
> > ok. Let me do a v2 for this.
> >
> > >
> > > >
> > > >  #include "tlv320aic32x4.h"
> > > >
> > > > @@ -38,7 +37,7 @@ struct aic32x4_priv {
> > > >  	u32 power_cfg;
> > > >  	u32 micpga_routing;
> > > >  	bool swapdacs;
> > > > -	int rstn_gpio;
> > > > +	struct gpio_desc *rstn_gpio;
> > > >  	const char *mclk_name;
> > > >
> > > >  	struct regulator *supply_ldo;
> > > > @@ -1236,7 +1235,14 @@ 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);
> > > > +	/* Assert reset using GPIOD_OUT_HIGH, because reset is
> > > GPIO_ACTIVE_LOW */
> > > > +	aic32x4->rstn_gpio = devm_gpiod_get_optional(aic32x4->dev,
> > > "reset", GPIOD_OUT_HIGH);
> > > > +	if (IS_ERR(aic32x4->rstn_gpio)) {
> > > > +		return dev_err_probe(aic32x4->dev, PTR_ERR(aic32x4-
> > > >rstn_gpio),
> > > > +				     "Failed to get reset gpio\n");
> > > > +	} else {
> > > > +		gpiod_set_consumer_name(aic32x4->rstn_gpio,
> > > "tlv320aic32x4_rstn");
> > > > +	}
> > > >
> > > >  	if (of_property_read_u32_array(np, "aic32x4-gpio-func",
> > > >  				aic32x4_setup->gpio_func, 5) >= 0)
> > > @@ -1372,26 +1378,20 @@ 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->rstn_gpio = ERR_PTR(-ENOENT);
> > >
> > > Shouldn't this be NULL similar to when devm_gpiod_get_optional()
> > > doesn't find any reset GPIO?
> >
> > There is a check in driver, so NULL not work here.
> >
> >         if (!IS_ERR(aic32x4->rstn_gpio)) {
> 
> I don't like the fact that both paths have different values for rstn_gpio
> for the information there is no rstn GPIO. How about setting NULL in
> the without DT node case and checking if (aic32x4->rstn_gpio) here?

That's ok. V2 will cover this.

Thanks,
Peng.

> 
> Best regards
> Alexander
> 
> >                 ndelay(10);
> >                 /* deassert reset */
> >                 gpiod_set_value_cansleep(aic32x4->rstn_gpio, 0);
> >                 mdelay(1);
> >         }
> >
> > >
> > > Despite that, looks good and works as intended:
> > > Tested-By: Alexander Stein <alexander.stein@ew.tq-group.com>
> >
> > Appreciate!
> >
> > Thanks,
> > Peng.
> >
> >
> 
> 
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld,
> Germany Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> www.tq-
> group.com%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C119e001
> 7ac9344b010b208ddbd196353%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C0%7C638874638907645114%7CUnknown%7CTWFpbG
> Zsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXa
> W4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata
> =CZqJLhaQG1HSnEYnfg3v83dmZO9huNeEDCK2SZhKYho%3D&reserved
> =0
>