| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<PAXPR04MB8459F40C66870CE6FFFADF4988BD2@PAXPR04MB8459.eurprd04.prod.outlook.com>
Date: Wed, 16 Apr 2025 08:31:10 +0000
From: Peng Fan <peng.fan@....com>
To: Alexander Stein <alexander.stein@...tq-group.com>, Shenghao Ding
<shenghao-ding@...com>, Kevin Lu <kevin-lu@...com>, Baojun Xu
<baojun.xu@...com>, Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai
<tiwai@...e.com>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown
<broonie@...nel.org>, Linus Walleij <linus.walleij@...aro.org>, Bartosz
Golaszewski <brgl@...ev.pl>, Peter Ujfalusi <peter.ujfalusi@...il.com>, David
Rhodes <david.rhodes@...rus.com>, Richard Fitzgerald
<rf@...nsource.cirrus.com>, "Peng Fan (OSS)" <peng.fan@....nxp.com>
CC: "linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
"patches@...nsource.cirrus.com" <patches@...nsource.cirrus.com>, Markus
Niebel <Markus.Niebel@...tq-group.com>
Subject: RE: [PATCH 2/7] ASoC: codec: tlv320aic32x4: Convert to GPIO
descriptors
> Subject: Re: [PATCH 2/7] ASoC: codec: tlv320aic32x4: Convert to GPIO
> descriptors
>
> Hi,
>
> Am Dienstag, 8. April 2025, 03:39:58 CEST schrieb Peng Fan (OSS):
> > 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, and set
> consumer
> > name.
> > - Use gpiod_set_value to configure output value.
> >
> > reset pin is active low, so when request the gpio, set
> GPIOD_OUT_HIGH
> > to assert reset and later deassert reset with value set to 0.
>
> IMHO it shouldn't matter if that pin is active-low or not. You want to
> activate that reset, so GPIOD_OUT_HIGH is correct. If the GPIO is
> active-low then nice, everything will be taken careof.
>
> > 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@...tq-group.com>
> > Cc: Alexander Stein <alexander.stein@...tq-group.com>
> > Signed-off-by: Peng Fan <peng.fan@....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..1423186f5a6c181a2
> 0dd2dd55267
> > 9d33174edaee 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>
> >
> > #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 */
>
> As stated above this comment shouldn't be necessary, it might be even
> confusing if there is some external inverter to the GPIO.
I just write down why use GPIOD_OUT_HIGH here.
of gpio API not take polarity into consideration, but gpiod API takes
polarity.
By adding external inverter, the reset for the codec is still low active,
but the gpio output from SoC should be high for the codec to stay
in reset state.
Then need a driver or whatever to describe the converter.
Here the reset-gpios is for codec.
>
> > + 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");
>
> Any reason to not set the consumer name to "tlv320aic32x4_reset"? I
> assume the 'n' implies active-low. But this is something for the
> devicetree, not the driver.
I not change the name, it is same as original driver
- ret = devm_gpio_request_one(dev, aic32x4->rstn_gpio,
- GPIOF_OUT_INIT_LOW, "tlv320aic32x4 rstn");
Regards,
Peng.
>
> Best regards,
> Alexander
>
> > + }
> >
> > 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);
> > 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
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> www.tq-
> group.com%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C85c2dca
> 8a42049d66eb808dd7c24eac6%7C686ea1d3bc2b4c6fa92cd99c5c301
> 635%7C0%7C0%7C638803220194936731%7CUnknown%7CTWFpbGZ
> sb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW
> 4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=5
> MgiAxA1ccqvepsjo1%2BPD%2Fv2dMiMNe%2F%2Fmy37xvSk5Wc%3D
> &reserved=0
>
Powered by blists - more mailing lists