[PATCH] ASoC: codec: ak5386: Convert to GPIO descriptors

[PATCH] ASoC: codec: ak5386: Convert to GPIO descriptors

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

 of_gpio.h is deprecated, update the driver to use GPIO descriptors.
 - Use dev_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 dt-bindings 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@nxp.com>
---

V1:
 The Documentation/devicetree/bindings/sound/ak5386.txt not specify
 polarity(this seems bug), so per code and datasheet, I think it
 should be active-high. I could add a quirk in gpiolib-of to force
 active-high or acitive-low if you think needed.

 sound/soc/codecs/ak5386.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/sound/soc/codecs/ak5386.c b/sound/soc/codecs/ak5386.c
index 21a44476f48d..ff635141b37f 100644
--- a/sound/soc/codecs/ak5386.c
+++ b/sound/soc/codecs/ak5386.c
@@ -6,11 +6,10 @@
  * (c) 2013 Daniel Mack <zonque@gmail.com>
  */
 
+#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 +19,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 +109,8 @@ 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);
+	if (priv->reset_gpio)
+		gpiod_set_value(priv->reset_gpio, 1);
 
 	return 0;
 }
@@ -122,8 +121,8 @@ 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);
+	if (priv->reset_gpio)
+		gpiod_set_value(priv->reset_gpio, 0);
 
 	return 0;
 }
@@ -177,14 +176,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

Re: [PATCH] ASoC: codec: ak5386: Convert to GPIO descriptors

[Andy Shevchenko]

On Fri, Mar 28, 2025 at 07:39:17PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
>  of_gpio.h is deprecated, update the driver to use GPIO descriptors.
>  - Use dev_gpiod_get_optional to get GPIO descriptor.

devm

>  - Use gpiod_set_value to configure output value.
> 
> With legacy of_gpio API, the driver set gpio value 1 to power up

sets GPIO

> 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 dt-bindings not

bindings does not

> specify polarity. Per driver and datasheet, the gpio polarity should be

GPIO

> 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.

...

>  The Documentation/devicetree/bindings/sound/ak5386.txt not specify
>  polarity(this seems bug), so per code and datasheet, I think it
>  should be active-high. I could add a quirk in gpiolib-of to force
>  active-high or acitive-low if you think needed.

I don't think we need a quirk as long as the default is the same,
I mean if the DTS is written without setting polarity, would it be
active-high or active-low?

...

> +	if (priv->reset_gpio)

Redundant as it duplicates the one in the below call.

> +		gpiod_set_value(priv->reset_gpio, 1);

...

> +	if (priv->reset_gpio)

Ditto.

> +		gpiod_set_value(priv->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),

+ dev_printk.h // or even device.h, depending on the current code base
+ err.h

> +				     "Failed to get AK5386 reset GPIO\n");

> +	gpiod_set_consumer_name(priv->reset_gpio, "AK5386 Reset");

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] ASoC: codec: ak5386: Convert to GPIO descriptors

[Peng Fan]

On Fri, Mar 28, 2025 at 03:33:11PM +0200, Andy Shevchenko wrote:
>On Fri, Mar 28, 2025 at 07:39:17PM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>> 
>>  of_gpio.h is deprecated, update the driver to use GPIO descriptors.
>>  - Use dev_gpiod_get_optional to get GPIO descriptor.
>
>devm
>
>>  - Use gpiod_set_value to configure output value.
>> 
>> With legacy of_gpio API, the driver set gpio value 1 to power up
>
>sets GPIO
>
>> 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 dt-bindings not
>
>bindings does not
>
>> specify polarity. Per driver and datasheet, the gpio polarity should be
>
>GPIO
>
>> 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.
>
>...
>
>>  The Documentation/devicetree/bindings/sound/ak5386.txt not specify
>>  polarity(this seems bug), so per code and datasheet, I think it
>>  should be active-high. I could add a quirk in gpiolib-of to force
>>  active-high or acitive-low if you think needed.
>
>I don't think we need a quirk as long as the default is the same,
>I mean if the DTS is written without setting polarity, would it be
>active-high or active-low?

Per current gpio driver, of_gpio_n_cells should at least be 2,
Not find any driver using 1 in current linux tree.

Without polarity, I think of_xlate will not work as expected.

Regards,
Peng

>
>...
>
>> +	if (priv->reset_gpio)
>
>Redundant as it duplicates the one in the below call.
>
>> +		gpiod_set_value(priv->reset_gpio, 1);
>
>...
>
>> +	if (priv->reset_gpio)
>
>Ditto.
>
>> +		gpiod_set_value(priv->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),
>
>+ dev_printk.h // or even device.h, depending on the current code base
>+ err.h
>
>> +				     "Failed to get AK5386 reset GPIO\n");
>
>> +	gpiod_set_consumer_name(priv->reset_gpio, "AK5386 Reset");
>
>-- 
>With Best Regards,
>Andy Shevchenko
>
>

Re: [PATCH] ASoC: codec: ak5386: Convert to GPIO descriptors

[Andy Shevchenko]

On Mon, Mar 31, 2025 at 02:03:59PM +0800, Peng Fan wrote:
> On Fri, Mar 28, 2025 at 03:33:11PM +0200, Andy Shevchenko wrote:
> >On Fri, Mar 28, 2025 at 07:39:17PM +0800, Peng Fan (OSS) wrote:

...

> >>  The Documentation/devicetree/bindings/sound/ak5386.txt not specify
> >>  polarity(this seems bug), so per code and datasheet, I think it
> >>  should be active-high. I could add a quirk in gpiolib-of to force
> >>  active-high or acitive-low if you think needed.
> >
> >I don't think we need a quirk as long as the default is the same,
> >I mean if the DTS is written without setting polarity, would it be
> >active-high or active-low?
> 
> Per current gpio driver, of_gpio_n_cells should at least be 2,
> Not find any driver using 1 in current linux tree.

Yes...

> Without polarity, I think of_xlate will not work as expected.

...and yes, but how is it related to my comment? The default should be sane
to make it work, since there is no in-kernel user with wrong polarity there
is nothing to fix.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] ASoC: codec: ak5386: Convert to GPIO descriptors

[Peng Fan]

On Mon, Mar 31, 2025 at 10:19:12AM +0300, Andy Shevchenko wrote:
>On Mon, Mar 31, 2025 at 02:03:59PM +0800, Peng Fan wrote:
>> On Fri, Mar 28, 2025 at 03:33:11PM +0200, Andy Shevchenko wrote:
>> >On Fri, Mar 28, 2025 at 07:39:17PM +0800, Peng Fan (OSS) wrote:
>
>...
>
>> >>  The Documentation/devicetree/bindings/sound/ak5386.txt not specify
>> >>  polarity(this seems bug), so per code and datasheet, I think it
>> >>  should be active-high. I could add a quirk in gpiolib-of to force
>> >>  active-high or acitive-low if you think needed.
>> >
>> >I don't think we need a quirk as long as the default is the same,
>> >I mean if the DTS is written without setting polarity, would it be
>> >active-high or active-low?
>> 
>> Per current gpio driver, of_gpio_n_cells should at least be 2,
>> Not find any driver using 1 in current linux tree.
>
>Yes...
>
>> Without polarity, I think of_xlate will not work as expected.
>
>...and yes, but how is it related to my comment? The default should be sane
>to make it work, since there is no in-kernel user with wrong polarity there
>is nothing to fix.

I see, nothing to fix. Thanks for help reviewing.

Thanks,
Peng

>
>-- 
>With Best Regards,
>Andy Shevchenko
>
>
>