All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Stein <alexander.stein@ew.tq-group.com>
To: Shenghao Ding <shenghao-ding@ti.com>, Kevin Lu <kevin-lu@ti.com>,
	Baojun Xu <baojun.xu@ti.com>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <brgl@bgdev.pl>, Peng Fan <peng.fan@nxp.com>
Cc: linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org, Peng Fan <peng.fan@nxp.com>,
	Markus Niebel <Markus.Niebel@ew.tq-group.com>
Subject: Re: [PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to GPIO descriptors
Date: Mon, 07 Jul 2025 07:22:57 +0200	[thread overview]
Message-ID: <3372384.44csPzL39Z@steina-w> (raw)
In-Reply-To: <20250706-asoc-gpio-1-v1-2-f0912095e923@nxp.com>

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/



  reply	other threads:[~2025-07-07  5:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-06  1:04 [PATCH 0/2] ASoC: codec: Convert to GPIO descriptors for tlv320aic32x4 Peng Fan
2025-07-06  1:04 ` [PATCH 1/2] ASoC: codec: tlv320aic32x4: Drop aic32x4_pdata usage Peng Fan
2025-07-07  5:12   ` Alexander Stein
2025-07-06  1:04 ` [PATCH 2/2] ASoC: codec: tlv320aic32x4: Convert to GPIO descriptors Peng Fan
2025-07-07  5:22   ` Alexander Stein [this message]
2025-07-07  5:40     ` Peng Fan
2025-07-07  5:44       ` Alexander Stein
2025-07-07  6:46         ` Peng Fan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3372384.44csPzL39Z@steina-w \
    --to=alexander.stein@ew.tq-group.com \
    --cc=Markus.Niebel@ew.tq-group.com \
    --cc=baojun.xu@ti.com \
    --cc=brgl@bgdev.pl \
    --cc=broonie@kernel.org \
    --cc=kevin-lu@ti.com \
    --cc=lgirdwood@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=peng.fan@nxp.com \
    --cc=perex@perex.cz \
    --cc=shenghao-ding@ti.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.