sound/soc/codecs/ak5386.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-)
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
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
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 > >
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
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 > > >
© 2016 - 2025 Red Hat, Inc.