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

[PATCH] 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.

Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
---
 sound/soc/codecs/tlv320dac33.c | 56 +++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c
index 423b9264a..b8e833efb 100644
--- a/sound/soc/codecs/tlv320dac33.c
+++ b/sound/soc/codecs/tlv320dac33.c
@@ -15,6 +15,7 @@
 #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>
@@ -109,6 +110,8 @@ struct tlv320dac33_priv {
 
 	enum dac33_state state;
 	struct i2c_client *i2c;
+
+	struct gpio_desc *power_gpiod;
 };
 
 static const u8 dac33_reg[DAC33_CACHEREGNUM] = {
@@ -383,14 +386,28 @@ 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->power_gpio >= 0) {
+			ret = gpiod_set_value(dac33->power_gpiod, 1);
+			if (ret < 0) {
+				dev_err(&dac33->i2c->dev,
+					"Failed to set reset GPIO %d: %d\n",
+					dac33->power_gpio, 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->power_gpio >= 0) {
+			ret = gpiod_set_value(dac33->power_gpiod, 0);
+			if (ret < 0) {
+				dev_err(&dac33->i2c->dev,
+					"Failed to set reset GPIO %d: %d\n",
+					dac33->power_gpio, ret);
+				goto exit;
+			}
+		}
 
 		ret = regulator_bulk_disable(ARRAY_SIZE(dac33->supplies),
 					     dac33->supplies);
@@ -1500,14 +1517,22 @@ static int dac33_i2c_probe(struct i2c_client *client)
 
 	/* Check if the reset GPIO number is valid and request it */
 	if (dac33->power_gpio >= 0) {
-		ret = gpio_request(dac33->power_gpio, "tlv320dac33 reset");
+		ret = devm_gpio_request_one(&client->dev, dac33->power_gpio,
+					    GPIOF_OUT_INIT_LOW,
+					    "tlv320dac33 reset");
 		if (ret < 0) {
 			dev_err(&client->dev,
-				"Failed to request reset GPIO (%d)\n",
-				dac33->power_gpio);
-			goto err_gpio;
+				"Failed to request reset GPIO %d: %d\n",
+				dac33->power_gpio, ret);
+			goto err;
+		}
+
+		dac33->power_gpiod = gpio_to_desc(dac33->power_gpio);
+		if (!dac33->power_gpiod) {
+			dev_err(&client->dev,
+				"Failed to get reset GPIO descriptor\n");
+			return -EINVAL;
 		}
-		gpio_direction_output(dac33->power_gpio, 0);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(dac33->supplies); i++)
@@ -1518,19 +1543,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 +1563,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] ASoC: codecs: tlv320dac33: switch to gpiod api

[Bartosz Golaszewski]

On Sun, Aug 31, 2025 at 5:09 AM Alex Tran <alex.t.tran@gmail.com> 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.
>
> Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
> ---

The fact that the call to gpio_request() is still there made me think
immediately that the conversion is incomplete. I looked into why you
didn't change that and noticed that the global GPIO number comes
through platform data, specifically: struct tlv320dac33_platform_data.
That platform data struct however is not used in the kernel -and even
the header that defines it - sound/tlv320dac33-plat.h - is never
included outside of the driver. Seems to me like the main obstacle to
completing the conversion is not even used in mainline and can be
dropped?

Bart

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

[Alex Tran]

On Sun, Aug 31, 2025 at 5:55 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> The fact that the call to gpio_request() is still there made me think
> immediately that the conversion is incomplete. I looked into why you
> didn't change that and noticed that the global GPIO number comes
> through platform data, specifically: struct tlv320dac33_platform_data.
> That platform data struct however is not used in the kernel -and even
> the header that defines it - sound/tlv320dac33-plat.h - is never
> included outside of the driver. Seems to me like the main obstacle to
> completing the conversion is not even used in mainline and can be
> dropped?
>
> Bart

Since struct tlv320dac33_platform_data isn't used in the kernel and the
corresponding header file it's in isn't used outside of the driver,
I'll go ahead
and drop them and send in a v2. Thanks for the review.

-- 
Alex Tran