linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH next 1/3] ASoC: dt-bindings: tas2781: fix reset polarity
@ 2025-07-03  7:50 Catalin Popescu
  2025-07-03  7:50 ` [PATCH next 2/3] ASoC: tas2781: fix reset-gpio polarity Catalin Popescu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Catalin Popescu @ 2025-07-03  7:50 UTC (permalink / raw)
  To: lgirdwood, broonie, robh, krzk+dt, conor+dt, shenghao-ding,
	kevin-lu, baojun.xu, perex, tiwai, matthias.bgg,
	angelogioacchino.delregno
  Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, m.felsch, bsp-development.geo, Catalin Popescu

Both TAS2563 & TAS2781 have a reset active low. Yet, the binding wrongly
indicates an active high reset.

Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
---
 Documentation/devicetree/bindings/sound/ti,tas2781.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/ti,tas2781.yaml b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
index 5ea1cdc593b5..47f5d3c6339e 100644
--- a/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
+++ b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
@@ -116,7 +116,7 @@ examples:
                   <0x3b>; /* Audio slot 3 */
 
             #sound-dai-cells = <0>;
-            reset-gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;
+            reset-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>;
             interrupt-parent = <&gpio1>;
             interrupts = <15>;
         };

base-commit: 50c8770a42faf8b1c7abe93e7c114337f580a97d
prerequisite-patch-id: 0000000000000000000000000000000000000000
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH next 2/3] ASoC: tas2781: fix reset-gpio polarity
  2025-07-03  7:50 [PATCH next 1/3] ASoC: dt-bindings: tas2781: fix reset polarity Catalin Popescu
@ 2025-07-03  7:50 ` Catalin Popescu
  2025-07-03  8:03   ` Krzysztof Kozlowski
  2025-07-03  7:50 ` [PATCH next 3/3] dts: arm64: mediatek: mt8188: fix audio amplifier reset polarity Catalin Popescu
  2025-07-03  8:04 ` [PATCH next 1/3] ASoC: dt-bindings: tas2781: fix " Krzysztof Kozlowski
  2 siblings, 1 reply; 6+ messages in thread
From: Catalin Popescu @ 2025-07-03  7:50 UTC (permalink / raw)
  To: lgirdwood, broonie, robh, krzk+dt, conor+dt, shenghao-ding,
	kevin-lu, baojun.xu, perex, tiwai, matthias.bgg,
	angelogioacchino.delregno
  Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, m.felsch, bsp-development.geo, Catalin Popescu

Both TAS2563 & TAS2781 have an active low reset, yet the driver assumes
an active high reset. Hence, in order to get the chip out of reset we
need to lie to the devicetree about the reset polarity. The patch fixes
the driver so it could work no matter the polarity and leaves it to the
devicetree to define the correct polarity.

Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
---
 sound/soc/codecs/tas2781-comlib-i2c.c | 4 ++--
 sound/soc/codecs/tas2781-i2c.c        | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/tas2781-comlib-i2c.c b/sound/soc/codecs/tas2781-comlib-i2c.c
index c078bb0a8437..2553af086637 100644
--- a/sound/soc/codecs/tas2781-comlib-i2c.c
+++ b/sound/soc/codecs/tas2781-comlib-i2c.c
@@ -313,9 +313,9 @@ void tasdevice_reset(struct tasdevice_priv *tas_dev)
 	int ret, i;
 
 	if (tas_dev->reset) {
-		gpiod_set_value_cansleep(tas_dev->reset, 0);
-		usleep_range(500, 1000);
 		gpiod_set_value_cansleep(tas_dev->reset, 1);
+		usleep_range(500, 1000);
+		gpiod_set_value_cansleep(tas_dev->reset, 0);
 	} else {
 		for (i = 0; i < tas_dev->ndev; i++) {
 			ret = tasdevice_dev_write(tas_dev, i,
diff --git a/sound/soc/codecs/tas2781-i2c.c b/sound/soc/codecs/tas2781-i2c.c
index 9f4d965a1335..b7202b000247 100644
--- a/sound/soc/codecs/tas2781-i2c.c
+++ b/sound/soc/codecs/tas2781-i2c.c
@@ -1895,7 +1895,7 @@ static void tasdevice_parse_dt(struct tasdevice_priv *tas_priv)
 		tas_priv->tasdevice[i].dev_addr = dev_addrs[i];
 
 	tas_priv->reset = devm_gpiod_get_optional(&client->dev,
-			"reset", GPIOD_OUT_HIGH);
+			"reset", GPIOD_OUT_LOW);
 	if (IS_ERR(tas_priv->reset))
 		dev_err(tas_priv->dev, "%s Can't get reset GPIO\n",
 			__func__);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH next 3/3] dts: arm64: mediatek: mt8188: fix audio amplifier reset polarity
  2025-07-03  7:50 [PATCH next 1/3] ASoC: dt-bindings: tas2781: fix reset polarity Catalin Popescu
  2025-07-03  7:50 ` [PATCH next 2/3] ASoC: tas2781: fix reset-gpio polarity Catalin Popescu
@ 2025-07-03  7:50 ` Catalin Popescu
  2025-07-03  8:04 ` [PATCH next 1/3] ASoC: dt-bindings: tas2781: fix " Krzysztof Kozlowski
  2 siblings, 0 replies; 6+ messages in thread
From: Catalin Popescu @ 2025-07-03  7:50 UTC (permalink / raw)
  To: lgirdwood, broonie, robh, krzk+dt, conor+dt, shenghao-ding,
	kevin-lu, baojun.xu, perex, tiwai, matthias.bgg,
	angelogioacchino.delregno
  Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, m.felsch, bsp-development.geo, Catalin Popescu

A few MT8188 boards defines a wrong polarity for the audio amplifier's
(TAS2563/2781) reset gpio. This is due to the driver assuming a wrong
polarity so the devicetrees have to configure a wrong polarity for the
audio amplifier to be working. Now, that the driver has been fixed, we
can configure the correct polarity in the devicetrees.

Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
---
 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku4.dts | 2 +-
 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku5.dts | 2 +-
 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku6.dts | 2 +-
 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku7.dts | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku4.dts b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku4.dts
index ea953d7e1543..65993d25a3b0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku4.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku4.dts
@@ -21,7 +21,7 @@ &i2c0 {
 	tas2563: amplifier@4f {
 		compatible = "ti,tas2563", "ti,tas2781";
 		reg = <0x4f>, <0x4c>; /* left / right channel */
-		reset-gpios = <&pio 118 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&pio 118 GPIO_ACTIVE_LOW>;
 		#sound-dai-cells = <0>;
 	};
 };
diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku5.dts b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku5.dts
index bf87201ccf27..303d8cc3af69 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku5.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku5.dts
@@ -33,7 +33,7 @@ es8326: audio-codec@19 {
 	tas2563: amplifier@4f {
 		compatible = "ti,tas2563", "ti,tas2781";
 		reg = <0x4f>, <0x4c>; /* left / right channel */
-		reset-gpios = <&pio 118 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&pio 118 GPIO_ACTIVE_LOW>;
 		#sound-dai-cells = <0>;
 	};
 };
diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku6.dts b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku6.dts
index 17d7359dfb6a..f54b116eab1a 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku6.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku6.dts
@@ -33,7 +33,7 @@ es8326: audio-codec@19 {
 	tas2563: amplifier@4f {
 		compatible = "ti,tas2563", "ti,tas2781";
 		reg = <0x4f>, <0x4c>; /* left / right channel */
-		reset-gpios = <&pio 118 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&pio 118 GPIO_ACTIVE_LOW>;
 		#sound-dai-cells = <0>;
 	};
 };
diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku7.dts b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku7.dts
index 825015b452d5..36c3b236dcb9 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku7.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku7.dts
@@ -21,7 +21,7 @@ &i2c0 {
 	tas2563: amplifier@4f {
 		compatible = "ti,tas2563", "ti,tas2781";
 		reg = <0x4f>, <0x4c>; /* left / right channel */
-		reset-gpios = <&pio 118 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&pio 118 GPIO_ACTIVE_LOW>;
 		#sound-dai-cells = <0>;
 	};
 };
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH next 2/3] ASoC: tas2781: fix reset-gpio polarity
  2025-07-03  7:50 ` [PATCH next 2/3] ASoC: tas2781: fix reset-gpio polarity Catalin Popescu
@ 2025-07-03  8:03   ` Krzysztof Kozlowski
  2025-07-03 12:29     ` POPESCU Catalin
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-03  8:03 UTC (permalink / raw)
  To: Catalin Popescu, lgirdwood, broonie, robh, krzk+dt, conor+dt,
	shenghao-ding, kevin-lu, baojun.xu, perex, tiwai, matthias.bgg,
	angelogioacchino.delregno
  Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, m.felsch, bsp-development.geo

On 03/07/2025 09:50, Catalin Popescu wrote:
> Both TAS2563 & TAS2781 have an active low reset, yet the driver assumes
> an active high reset. Hence, in order to get the chip out of reset we
> need to lie to the devicetree about the reset polarity. The patch fixes
> the driver so it could work no matter the polarity and leaves it to the
> devicetree to define the correct polarity.
> 
> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>


This breaks all existing in-tree and out-tree users.

DTS patches are independent, so you cannot fix in-tree that way. Anyway,
you cannot fix other users, so sorry, you are stuck with this or you
need some tricks (I once did for a qcom codec, but I don't know if this
is really the solution and my case was easier about the users).

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH next 1/3] ASoC: dt-bindings: tas2781: fix reset polarity
  2025-07-03  7:50 [PATCH next 1/3] ASoC: dt-bindings: tas2781: fix reset polarity Catalin Popescu
  2025-07-03  7:50 ` [PATCH next 2/3] ASoC: tas2781: fix reset-gpio polarity Catalin Popescu
  2025-07-03  7:50 ` [PATCH next 3/3] dts: arm64: mediatek: mt8188: fix audio amplifier reset polarity Catalin Popescu
@ 2025-07-03  8:04 ` Krzysztof Kozlowski
  2 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-03  8:04 UTC (permalink / raw)
  To: Catalin Popescu, lgirdwood, broonie, robh, krzk+dt, conor+dt,
	shenghao-ding, kevin-lu, baojun.xu, perex, tiwai, matthias.bgg,
	angelogioacchino.delregno
  Cc: linux-sound, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, m.felsch, bsp-development.geo

On 03/07/2025 09:50, Catalin Popescu wrote:
> Both TAS2563 & TAS2781 have a reset active low. Yet, the binding wrongly
> indicates an active high reset.


This is just an example, not a binding. If there is an inverter, then
maybe the example was correct?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH next 2/3] ASoC: tas2781: fix reset-gpio polarity
  2025-07-03  8:03   ` Krzysztof Kozlowski
@ 2025-07-03 12:29     ` POPESCU Catalin
  0 siblings, 0 replies; 6+ messages in thread
From: POPESCU Catalin @ 2025-07-03 12:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski, lgirdwood@gmail.com, broonie@kernel.org,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	shenghao-ding@ti.com, kevin-lu@ti.com, baojun.xu@ti.com,
	perex@perex.cz, tiwai@suse.com, matthias.bgg@gmail.com,
	angelogioacchino.delregno@collabora.com
  Cc: linux-sound@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, m.felsch@pengutronix.de,
	GEO-CHHER-bsp-development

On 03/07/2025 10:03, Krzysztof Kozlowski wrote:
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>
>
> On 03/07/2025 09:50, Catalin Popescu wrote:
>> Both TAS2563 & TAS2781 have an active low reset, yet the driver assumes
>> an active high reset. Hence, in order to get the chip out of reset we
>> need to lie to the devicetree about the reset polarity. The patch fixes
>> the driver so it could work no matter the polarity and leaves it to the
>> devicetree to define the correct polarity.
>>
>> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
>
> This breaks all existing in-tree and out-tree users.
>
> DTS patches are independent, so you cannot fix in-tree that way. Anyway,
> you cannot fix other users, so sorry, you are stuck with this or you
> need some tricks (I once did for a qcom codec, but I don't know if this
> is really the solution and my case was easier about the users).
>
> Best regards,
> Krzysztof

Then, I'll keep lying to my devicetree and stick with the current driver.
Thx for the review!

BR,
Catalin


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-07-03 12:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-03  7:50 [PATCH next 1/3] ASoC: dt-bindings: tas2781: fix reset polarity Catalin Popescu
2025-07-03  7:50 ` [PATCH next 2/3] ASoC: tas2781: fix reset-gpio polarity Catalin Popescu
2025-07-03  8:03   ` Krzysztof Kozlowski
2025-07-03 12:29     ` POPESCU Catalin
2025-07-03  7:50 ` [PATCH next 3/3] dts: arm64: mediatek: mt8188: fix audio amplifier reset polarity Catalin Popescu
2025-07-03  8:04 ` [PATCH next 1/3] ASoC: dt-bindings: tas2781: fix " Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).