| 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: <5004055.31r3eYUQgx@steina-w>
Date: Tue, 15 Apr 2025 15:53:19 +0200
From: Alexander Stein <alexander.stein@...tq-group.com>
To: 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-kernel@...r.kernel.org,
linux-gpio@...r.kernel.org, patches@...nsource.cirrus.com,
Peng Fan <peng.fan@....com>, Markus Niebel <Markus.Niebel@...tq-group.com>
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..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>
>
> #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.
> + 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.
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
http://www.tq-group.com/
Powered by blists - more mailing lists