[PATCH v2] ASoC: codecs: tlv320dac33: switch to gpiod api

[PATCH v2] ASoC: codecs: tlv320dac33: switch to gpiod api

[Alex Tran]

Changelog:
- Changed reset GPIO setup that uses 'gpio_request' and
  'gpio_direction_output' to use 'devm_gpio_request_one' instead 
  for legacy support.
- Convert to gpio descriptor for use.
- Better error handling with 'gpiod_set_value'.
- Removed cleanup of reset gpio as gpiod api is now used.


- Performed full conversion to gpiod with 'devm_gpiod_get_optional'.
- Removed struct 'tlv320dac33_platform_data' as it is
  not used in the kernel.
- Removed file 'tlv320dac33-plat.h' as it was not included
  anywhere outside this driver.
- Removed 'power_gpio' and added 'reset_gpiod'.
- Added default value for dac33->burst_bclkdiv as it can't be 0 (2-17).
  See <https://www.ti.com/lit/ds/symlink/tlv320dac32.pdf>

Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
---
 include/sound/tlv320dac33-plat.h | 21 ----------
 sound/soc/codecs/tlv320dac33.c   | 68 +++++++++++++++-----------------
 2 files changed, 31 insertions(+), 58 deletions(-)
 delete mode 100644 include/sound/tlv320dac33-plat.h

diff --git a/include/sound/tlv320dac33-plat.h b/include/sound/tlv320dac33-plat.h
deleted file mode 100644
index 7a7249a89..000000000
--- a/include/sound/tlv320dac33-plat.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Platform header for Texas Instruments TLV320DAC33 codec driver
- *
- * Author: Peter Ujfalusi <peter.ujfalusi@ti.com>
- *
- * Copyright:   (C) 2009 Nokia Corporation
- */
-
-#ifndef __TLV320DAC33_PLAT_H
-#define __TLV320DAC33_PLAT_H
-
-struct tlv320dac33_platform_data {
-	int power_gpio;
-	int mode1_latency; /* latency caused by the i2c writes in us */
-	int auto_fifo_config; /* FIFO config based on the period size */
-	int keep_bclk;	/* Keep the BCLK running in FIFO modes */
-	u8 burst_bclkdiv;
-};
-
-#endif /* __TLV320DAC33_PLAT_H */
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c
index 423b9264a..43d372dd6 100644
--- a/sound/soc/codecs/tlv320dac33.c
+++ b/sound/soc/codecs/tlv320dac33.c
@@ -14,7 +14,7 @@
 #include <linux/pm.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <sound/core.h>
@@ -24,7 +24,6 @@
 #include <sound/initval.h>
 #include <sound/tlv.h>
 
-#include <sound/tlv320dac33-plat.h>
 #include "tlv320dac33.h"
 
 /*
@@ -80,7 +79,7 @@ struct tlv320dac33_priv {
 	struct snd_soc_component *component;
 	struct regulator_bulk_data supplies[DAC33_NUM_SUPPLIES];
 	struct snd_pcm_substream *substream;
-	int power_gpio;
+	struct gpio_desc *reset_gpiod;
 	int chip_power;
 	int irq;
 	unsigned int refclk;
@@ -383,14 +382,26 @@ static int dac33_hard_power(struct snd_soc_component *component, int power)
 			goto exit;
 		}
 
-		if (dac33->power_gpio >= 0)
-			gpio_set_value(dac33->power_gpio, 1);
+		if (dac33->reset_gpiod) {
+			ret = gpiod_set_value(dac33->reset_gpiod, 1);
+			if (ret < 0) {
+				dev_err(&dac33->i2c->dev,
+					"Failed to set reset GPIO: %d\n", ret);
+				goto exit;
+			}
+		}
 
 		dac33->chip_power = 1;
 	} else {
 		dac33_soft_power(component, 0);
-		if (dac33->power_gpio >= 0)
-			gpio_set_value(dac33->power_gpio, 0);
+		if (dac33->reset_gpiod) {
+			ret = gpiod_set_value(dac33->reset_gpiod, 0);
+			if (ret < 0) {
+				dev_err(&dac33->i2c->dev,
+					"Failed to set reset GPIO: %d\n", ret);
+				goto exit;
+			}
+		}
 
 		ret = regulator_bulk_disable(ARRAY_SIZE(dac33->supplies),
 					     dac33->supplies);
@@ -1462,16 +1473,9 @@ static struct snd_soc_dai_driver dac33_dai = {
 
 static int dac33_i2c_probe(struct i2c_client *client)
 {
-	struct tlv320dac33_platform_data *pdata;
 	struct tlv320dac33_priv *dac33;
 	int ret, i;
 
-	if (client->dev.platform_data == NULL) {
-		dev_err(&client->dev, "Platform data not set\n");
-		return -ENODEV;
-	}
-	pdata = client->dev.platform_data;
-
 	dac33 = devm_kzalloc(&client->dev, sizeof(struct tlv320dac33_priv),
 			     GFP_KERNEL);
 	if (dac33 == NULL)
@@ -1488,26 +1492,21 @@ static int dac33_i2c_probe(struct i2c_client *client)
 
 	i2c_set_clientdata(client, dac33);
 
-	dac33->power_gpio = pdata->power_gpio;
-	dac33->burst_bclkdiv = pdata->burst_bclkdiv;
-	dac33->keep_bclk = pdata->keep_bclk;
-	dac33->mode1_latency = pdata->mode1_latency;
+	if (!dac33->burst_bclkdiv)
+		dac33->burst_bclkdiv = 8;
 	if (!dac33->mode1_latency)
 		dac33->mode1_latency = 10000; /* 10ms */
 	dac33->irq = client->irq;
 	/* Disable FIFO use by default */
 	dac33->fifo_mode = DAC33_FIFO_BYPASS;
 
-	/* Check if the reset GPIO number is valid and request it */
-	if (dac33->power_gpio >= 0) {
-		ret = gpio_request(dac33->power_gpio, "tlv320dac33 reset");
-		if (ret < 0) {
-			dev_err(&client->dev,
-				"Failed to request reset GPIO (%d)\n",
-				dac33->power_gpio);
-			goto err_gpio;
-		}
-		gpio_direction_output(dac33->power_gpio, 0);
+	/* request optional reset GPIO */
+	dac33->reset_gpiod =
+		devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(dac33->reset_gpiod)) {
+		dev_err_probe(&client->dev, PTR_ERR(dac33->reset_gpiod),
+			      "Failed to get reset GPIO\n");
+		goto err;
 	}
 
 	for (i = 0; i < ARRAY_SIZE(dac33->supplies); i++)
@@ -1518,19 +1517,17 @@ static int dac33_i2c_probe(struct i2c_client *client)
 
 	if (ret != 0) {
 		dev_err(&client->dev, "Failed to request supplies: %d\n", ret);
-		goto err_get;
+		goto err;
 	}
 
 	ret = devm_snd_soc_register_component(&client->dev,
 			&soc_component_dev_tlv320dac33, &dac33_dai, 1);
 	if (ret < 0)
-		goto err_get;
+		goto err;
 
 	return ret;
-err_get:
-	if (dac33->power_gpio >= 0)
-		gpio_free(dac33->power_gpio);
-err_gpio:
+
+err:
 	return ret;
 }
 
@@ -1540,9 +1537,6 @@ static void dac33_i2c_remove(struct i2c_client *client)
 
 	if (unlikely(dac33->chip_power))
 		dac33_hard_power(dac33->component, 0);
-
-	if (dac33->power_gpio >= 0)
-		gpio_free(dac33->power_gpio);
 }
 
 static const struct i2c_device_id tlv320dac33_i2c_id[] = {
-- 
2.51.0

Re: [PATCH v2] ASoC: codecs: tlv320dac33: switch to gpiod api

[Mark Brown]

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

On Sun, Aug 31, 2025 at 08:59:56PM -0700, Alex Tran wrote:
> Changelog:
> - Changed reset GPIO setup that uses 'gpio_request' and
>   'gpio_direction_output' to use 'devm_gpio_request_one' instead 
>   for legacy support.
> - Convert to gpio descriptor for use.
> - Better error handling with 'gpiod_set_value'.
> - Removed cleanup of reset gpio as gpiod api is now used.
> 
> 
> - Performed full conversion to gpiod with 'devm_gpiod_get_optional'.
> - Removed struct 'tlv320dac33_platform_data' as it is
>   not used in the kernel.
> - Removed file 'tlv320dac33-plat.h' as it was not included
>   anywhere outside this driver.
> - Removed 'power_gpio' and added 'reset_gpiod'.
> - Added default value for dac33->burst_bclkdiv as it can't be 0 (2-17).
>   See <https://www.ti.com/lit/ds/symlink/tlv320dac32.pdf>

This is a set of separate changes which as covered in
submitting-patches.rst should each be split into individual patches,
this makes things much easier to review.

As also covered in submitting-patches.rst any inter-version changelogs
should go after the --- so they can be removed by tooling.

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

Re: [PATCH v2] ASoC: codecs: tlv320dac33: switch to gpiod api

[kernel test robot]

Hi Alex,

kernel test robot noticed the following build warnings:

[auto build test WARNING on broonie-sound/for-next]
[also build test WARNING on tiwai-sound/for-next tiwai-sound/for-linus linus/master v6.17-rc4 next-20250901]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Alex-Tran/ASoC-codecs-tlv320dac33-switch-to-gpiod-api/20250901-120204
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
patch link:    https://lore.kernel.org/r/20250901035956.1195081-1-alex.t.tran%40gmail.com
patch subject: [PATCH v2] ASoC: codecs: tlv320dac33: switch to gpiod api
config: loongarch-randconfig-002-20250901 (https://download.01.org/0day-ci/archive/20250902/202509020014.XuEeJtZr-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250902/202509020014.XuEeJtZr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509020014.XuEeJtZr-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> sound/soc/codecs/tlv320dac33.c:1506:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    1506 |         if (IS_ERR(dac33->reset_gpiod)) {
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   sound/soc/codecs/tlv320dac33.c:1531:9: note: uninitialized use occurs here
    1531 |         return ret;
         |                ^~~
   sound/soc/codecs/tlv320dac33.c:1506:2: note: remove the 'if' if its condition is always false
    1506 |         if (IS_ERR(dac33->reset_gpiod)) {
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    1507 |                 dev_err_probe(&client->dev, PTR_ERR(dac33->reset_gpiod),
         |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    1508 |                               "Failed to get reset GPIO\n");
         |                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    1509 |                 goto err;
         |                 ~~~~~~~~~
    1510 |         }
         |         ~
   sound/soc/codecs/tlv320dac33.c:1477:9: note: initialize the variable 'ret' to silence this warning
    1477 |         int ret, i;
         |                ^
         |                 = 0
   1 warning generated.


vim +1506 sound/soc/codecs/tlv320dac33.c

  1473	
  1474	static int dac33_i2c_probe(struct i2c_client *client)
  1475	{
  1476		struct tlv320dac33_priv *dac33;
  1477		int ret, i;
  1478	
  1479		dac33 = devm_kzalloc(&client->dev, sizeof(struct tlv320dac33_priv),
  1480				     GFP_KERNEL);
  1481		if (dac33 == NULL)
  1482			return -ENOMEM;
  1483	
  1484		dac33->reg_cache = devm_kmemdup_array(&client->dev, dac33_reg, ARRAY_SIZE(dac33_reg),
  1485						      sizeof(dac33_reg[0]), GFP_KERNEL);
  1486		if (!dac33->reg_cache)
  1487			return -ENOMEM;
  1488	
  1489		dac33->i2c = client;
  1490		mutex_init(&dac33->mutex);
  1491		spin_lock_init(&dac33->lock);
  1492	
  1493		i2c_set_clientdata(client, dac33);
  1494	
  1495		if (!dac33->burst_bclkdiv)
  1496			dac33->burst_bclkdiv = 8;
  1497		if (!dac33->mode1_latency)
  1498			dac33->mode1_latency = 10000; /* 10ms */
  1499		dac33->irq = client->irq;
  1500		/* Disable FIFO use by default */
  1501		dac33->fifo_mode = DAC33_FIFO_BYPASS;
  1502	
  1503		/* request optional reset GPIO */
  1504		dac33->reset_gpiod =
  1505			devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_LOW);
> 1506		if (IS_ERR(dac33->reset_gpiod)) {
  1507			dev_err_probe(&client->dev, PTR_ERR(dac33->reset_gpiod),
  1508				      "Failed to get reset GPIO\n");
  1509			goto err;
  1510		}
  1511	
  1512		for (i = 0; i < ARRAY_SIZE(dac33->supplies); i++)
  1513			dac33->supplies[i].supply = dac33_supply_names[i];
  1514	
  1515		ret = devm_regulator_bulk_get(&client->dev, ARRAY_SIZE(dac33->supplies),
  1516					 dac33->supplies);
  1517	
  1518		if (ret != 0) {
  1519			dev_err(&client->dev, "Failed to request supplies: %d\n", ret);
  1520			goto err;
  1521		}
  1522	
  1523		ret = devm_snd_soc_register_component(&client->dev,
  1524				&soc_component_dev_tlv320dac33, &dac33_dai, 1);
  1525		if (ret < 0)
  1526			goto err;
  1527	
  1528		return ret;
  1529	
  1530	err:
  1531		return ret;
  1532	}
  1533	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2] ASoC: codecs: tlv320dac33: switch to gpiod api

[Alex Tran]

On Mon, Sep 1, 2025 at 3:35 AM Mark Brown <broonie@kernel.org> wrote:
> This is a set of separate changes which as covered in
> submitting-patches.rst should each be split into individual patches,
> this makes things much easier to review.
>
> As also covered in submitting-patches.rst any inter-version changelogs
> should go after the --- so they can be removed by tooling.

I'll split it up into 3 separate patches: the removal of the struct
tlv320dac33_platform_data,
addition of burst_bclkdiv default value, and the conversion to gpiod.

Thanks for the feedback,
-- 
Alex Tran