[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMRc=MdgaiJGJt8UainKG=rKTeZjd4sAdaPbQcYEz1pxZGfYKQ@mail.gmail.com>
Date: Mon, 7 Apr 2025 14:06:55 +0200
From: Bartosz Golaszewski <brgl@...ev.pl>
To: "Peng Fan (OSS)" <peng.fan@....nxp.com>
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
Linus Walleij <linus.walleij@...aro.org>, Peng Fan <peng.fan@....com>,
"open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." <linux-sound@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>,
"open list:GPIO SUBSYSTEM:Keyword:(devm_)?gpio_(request|free|direction|get|set)" <linux-gpio@...r.kernel.org>, andriy.shevchenko@...el.com
Subject: Re: [PATCH V2] ASoC: codec: ak5386: Convert to GPIO descriptors
On Sun, Apr 6, 2025 at 3:06 AM Peng Fan (OSS) <peng.fan@....nxp.com> wrote:
>
> From: Peng Fan <peng.fan@....com>
>
> of_gpio.h is deprecated, update the driver to use GPIO descriptors.
> - Use devm_gpiod_get_optional to get GPIO descriptor.
> - Use gpiod_set_value to configure output value.
>
> With legacy of_gpio API, the driver set GPIO value 1 to power up
> AK5386, and set value 0 to power down.
> Per datasheet for PDN(reset_gpio in the driver):
> Power Down & Reset Mode Pin
> “H”: Power up, “L”: Power down & Reset
> The AK5386 must be reset once upon power-up.
>
> There is no in-tree DTS using this codec, and the bindings does not
> specify polarity. Per driver and datasheet, the GPIO polarity should be
> active-high which is to power up the codec. So using GPIOD_OUT_LOW
> when get the GPIO descriptor matches GPIOF_OUT_INIT_LOW when using
> of_gpio API.
>
> Signed-off-by: Peng Fan <peng.fan@....com>
> ---
>
> V2:
> Typo fixes in commit log
> Drop uneeded gpio check before gpiod_set_value
> Include more headers
>
> sound/soc/codecs/ak5386.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/sound/soc/codecs/ak5386.c b/sound/soc/codecs/ak5386.c
> index 21a44476f48d..6525d50b7ab2 100644
> --- a/sound/soc/codecs/ak5386.c
> +++ b/sound/soc/codecs/ak5386.c
> @@ -6,11 +6,13 @@
> * (c) 2013 Daniel Mack <zonque@...il.com>
> */
>
> +#include <linux/device.h>
> +#include <linux/dev_printk.h>
> +#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/regulator/consumer.h>
> +#include <linux/slab.h>
> #include <sound/soc.h>
> #include <sound/pcm.h>
> #include <sound/initval.h>
> @@ -20,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)];
> };
>
> @@ -110,8 +112,7 @@ 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);
> + gpiod_set_value(priv->reset_gpio, 1);
>
> return 0;
> }
> @@ -122,8 +123,7 @@ 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);
> + gpiod_set_value(priv->reset_gpio, 0);
>
> return 0;
> }
> @@ -177,14 +177,12 @@ static int ak5386_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - priv->reset_gpio = of_get_named_gpio(dev->of_node,
> - "reset-gpio", 0);
> + priv->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(priv->reset_gpio))
> + return dev_err_probe(dev, PTR_ERR(priv->reset_gpio),
> + "Failed to get AK5386 reset GPIO\n");
>
> - 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);
> --
> 2.37.1
>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Powered by blists - more mailing lists