linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Fix Tegra194 HDA regression
@ 2021-12-22  4:35 Sameer Pujar
  2021-12-22  4:35 ` [PATCH v3 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure Sameer Pujar
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Sameer Pujar @ 2021-12-22  4:35 UTC (permalink / raw)
  To: tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex
  Cc: jonathanh, digetx, mkumard, alsa-devel, devicetree, linux-tegra,
	linux-kernel, Sameer Pujar

HDA probe failure is observed on Tegra194 based platforms and this
happens due to reset failure. This series fixes the problem by
skipping the failing reset and DT bindings are updated accordingly.


Changelog
=========
 v2 -> v3:
 ---------
   * Use reset bulk APIs in HDA driver as suggested by Dmitry.


 v1 -> v2:
 ---------
   * Updated HDA driver patch to skip the failing reset instead of
     skipping resets in general for BPMP devices as per comment from
     Dmitry.
   * Used a better strucure name for SoC data as per comment from
     Thierry.
   * Dropped 'Fixes' tag in binding doc patch as per comment from
     Dmitry.

Sameer Pujar (3):
  ALSA: hda/tegra: Fix Tegra194 HDA reset failure
  dt-bindings: sound: tegra: Update HDA resets
  arm64: tegra: Remove non existent Tegra194 reset

 .../bindings/sound/nvidia,tegra30-hda.yaml         | 13 +++++--
 arch/arm64/boot/dts/nvidia/tegra194.dtsi           |  5 +--
 sound/pci/hda/hda_tegra.c                          | 45 +++++++++++++++++-----
 3 files changed, 47 insertions(+), 16 deletions(-)

-- 
2.7.4


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

* [PATCH v3 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure
  2021-12-22  4:35 [PATCH v3 0/3] Fix Tegra194 HDA regression Sameer Pujar
@ 2021-12-22  4:35 ` Sameer Pujar
  2021-12-22 18:40   ` Dmitry Osipenko
  2021-12-22  4:35 ` [PATCH v3 2/3] dt-bindings: sound: tegra: Update HDA resets Sameer Pujar
  2021-12-22  4:35 ` [PATCH v3 3/3] arm64: tegra: Remove non existent Tegra194 reset Sameer Pujar
  2 siblings, 1 reply; 9+ messages in thread
From: Sameer Pujar @ 2021-12-22  4:35 UTC (permalink / raw)
  To: tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex
  Cc: jonathanh, digetx, mkumard, alsa-devel, devicetree, linux-tegra,
	linux-kernel, Sameer Pujar, stable

HDA regression is recently reported on Tegra194 based platforms.
This happens because "hda2codec_2x" reset does not really exist
in Tegra194 and it causes probe failure. All the HDA based audio
tests fail at the moment. This underlying issue is exposed by
commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP
response") which now checks return code of BPMP command response.
Fix this issue by skipping unavailable reset on Tegra194.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Cc: stable@vger.kernel.org
Depends-on: 87f0e46e7559 ("ALSA: hda/tegra: Reset hardware")
---
 sound/pci/hda/hda_tegra.c | 45 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index ea700395..7c3df54 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -68,14 +68,20 @@
  */
 #define TEGRA194_NUM_SDO_LINES	  4
 
+struct hda_tegra_soc {
+	bool has_hda2codec_2x_reset;
+};
+
 struct hda_tegra {
 	struct azx chip;
 	struct device *dev;
-	struct reset_control *reset;
+	struct reset_control_bulk_data resets[3];
 	struct clk_bulk_data clocks[3];
+	unsigned int nresets;
 	unsigned int nclocks;
 	void __iomem *regs;
 	struct work_struct probe_work;
+	const struct hda_tegra_soc *data;
 };
 
 #ifdef CONFIG_PM
@@ -170,7 +176,7 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
 	int rc;
 
 	if (!chip->running) {
-		rc = reset_control_assert(hda->reset);
+		rc = reset_control_bulk_assert(hda->nresets, hda->resets);
 		if (rc)
 			return rc;
 	}
@@ -187,7 +193,7 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
 	} else {
 		usleep_range(10, 100);
 
-		rc = reset_control_deassert(hda->reset);
+		rc = reset_control_bulk_deassert(hda->nresets, hda->resets);
 		if (rc)
 			return rc;
 	}
@@ -427,9 +433,17 @@ static int hda_tegra_create(struct snd_card *card,
 	return 0;
 }
 
+static const struct hda_tegra_soc tegra30_data = {
+	.has_hda2codec_2x_reset = true,
+};
+
+static const struct hda_tegra_soc tegra194_data = {
+	.has_hda2codec_2x_reset = false,
+};
+
 static const struct of_device_id hda_tegra_match[] = {
-	{ .compatible = "nvidia,tegra30-hda" },
-	{ .compatible = "nvidia,tegra194-hda" },
+	{ .compatible = "nvidia,tegra30-hda", .data = &tegra30_data },
+	{ .compatible = "nvidia,tegra194-hda", .data = &tegra194_data },
 	{},
 };
 MODULE_DEVICE_TABLE(of, hda_tegra_match);
@@ -449,6 +463,10 @@ static int hda_tegra_probe(struct platform_device *pdev)
 	hda->dev = &pdev->dev;
 	chip = &hda->chip;
 
+	hda->data = of_device_get_match_data(&pdev->dev);
+	if (!hda->data)
+		return -EINVAL;
+
 	err = snd_card_new(&pdev->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
 			   THIS_MODULE, 0, &card);
 	if (err < 0) {
@@ -456,11 +474,20 @@ static int hda_tegra_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	hda->reset = devm_reset_control_array_get_exclusive(&pdev->dev);
-	if (IS_ERR(hda->reset)) {
-		err = PTR_ERR(hda->reset);
+	hda->resets[hda->nresets++].id = "hda";
+	hda->resets[hda->nresets++].id = "hda2hdmi";
+	/*
+	 * "hda2codec_2x" reset is not present on Tegra194. Though DT would
+	 * be updated to reflect this, but to have backward compatibility
+	 * below is necessary.
+	 */
+	if (hda->data->has_hda2codec_2x_reset)
+		hda->resets[hda->nresets++].id = "hda2codec_2x";
+
+	err = devm_reset_control_bulk_get_exclusive(&pdev->dev, hda->nresets,
+						    hda->resets);
+	if (err)
 		goto out_free;
-	}
 
 	hda->clocks[hda->nclocks++].id = "hda";
 	hda->clocks[hda->nclocks++].id = "hda2hdmi";
-- 
2.7.4


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

* [PATCH v3 2/3] dt-bindings: sound: tegra: Update HDA resets
  2021-12-22  4:35 [PATCH v3 0/3] Fix Tegra194 HDA regression Sameer Pujar
  2021-12-22  4:35 ` [PATCH v3 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure Sameer Pujar
@ 2021-12-22  4:35 ` Sameer Pujar
  2021-12-22 19:37   ` Rob Herring
  2021-12-22  4:35 ` [PATCH v3 3/3] arm64: tegra: Remove non existent Tegra194 reset Sameer Pujar
  2 siblings, 1 reply; 9+ messages in thread
From: Sameer Pujar @ 2021-12-22  4:35 UTC (permalink / raw)
  To: tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex
  Cc: jonathanh, digetx, mkumard, alsa-devel, devicetree, linux-tegra,
	linux-kernel, Sameer Pujar

Tegra194 HDA has only two resets unlike the previous generations of
Tegra SoCs. Hence update the reset list accordingly.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Acked-by: Thierry Reding <treding@nvidia.com>
---
 .../devicetree/bindings/sound/nvidia,tegra30-hda.yaml       | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml
index b55775e..70dbdff5 100644
--- a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml
+++ b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml
@@ -50,13 +50,18 @@ properties:
       - const: hda2codec_2x
 
   resets:
+    minItems: 2
     maxItems: 3
 
   reset-names:
-    items:
-      - const: hda
-      - const: hda2hdmi
-      - const: hda2codec_2x
+    oneOf:
+      - items:
+          - const: hda
+          - const: hda2hdmi
+          - const: hda2codec_2x
+      - items:
+          - const: hda
+          - const: hda2hdmi
 
   power-domains:
     maxItems: 1
-- 
2.7.4


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

* [PATCH v3 3/3] arm64: tegra: Remove non existent Tegra194 reset
  2021-12-22  4:35 [PATCH v3 0/3] Fix Tegra194 HDA regression Sameer Pujar
  2021-12-22  4:35 ` [PATCH v3 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure Sameer Pujar
  2021-12-22  4:35 ` [PATCH v3 2/3] dt-bindings: sound: tegra: Update HDA resets Sameer Pujar
@ 2021-12-22  4:35 ` Sameer Pujar
  2 siblings, 0 replies; 9+ messages in thread
From: Sameer Pujar @ 2021-12-22  4:35 UTC (permalink / raw)
  To: tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex
  Cc: jonathanh, digetx, mkumard, alsa-devel, devicetree, linux-tegra,
	linux-kernel, Sameer Pujar

Tegra194 does not really have "hda2codec_2x" related reset. Hence drop
this entry to reflect actual HW.

Fixes: 4878cc0c9fab ("arm64: tegra: Add HDA controller on Tegra194")
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra194.dtsi | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 8d29b7f..6a1d896 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -976,9 +976,8 @@
 				 <&bpmp TEGRA194_CLK_HDA2CODEC_2X>;
 			clock-names = "hda", "hda2hdmi", "hda2codec_2x";
 			resets = <&bpmp TEGRA194_RESET_HDA>,
-				 <&bpmp TEGRA194_RESET_HDA2HDMICODEC>,
-				 <&bpmp TEGRA194_RESET_HDA2CODEC_2X>;
-			reset-names = "hda", "hda2hdmi", "hda2codec_2x";
+				 <&bpmp TEGRA194_RESET_HDA2HDMICODEC>;
+			reset-names = "hda", "hda2hdmi";
 			power-domains = <&bpmp TEGRA194_POWER_DOMAIN_DISP>;
 			interconnects = <&mc TEGRA194_MEMORY_CLIENT_HDAR &emc>,
 					<&mc TEGRA194_MEMORY_CLIENT_HDAW &emc>;
-- 
2.7.4


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

* Re: [PATCH v3 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure
  2021-12-22  4:35 ` [PATCH v3 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure Sameer Pujar
@ 2021-12-22 18:40   ` Dmitry Osipenko
  2021-12-23  4:34     ` Sameer Pujar
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Osipenko @ 2021-12-22 18:40 UTC (permalink / raw)
  To: Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex
  Cc: jonathanh, mkumard, alsa-devel, devicetree, linux-tegra,
	linux-kernel, stable

22.12.2021 07:35, Sameer Pujar пишет:
> HDA regression is recently reported on Tegra194 based platforms.
> This happens because "hda2codec_2x" reset does not really exist
> in Tegra194 and it causes probe failure. All the HDA based audio
> tests fail at the moment. This underlying issue is exposed by
> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP
> response") which now checks return code of BPMP command response.
> Fix this issue by skipping unavailable reset on Tegra194.
> 
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> Cc: stable@vger.kernel.org
> Depends-on: 87f0e46e7559 ("ALSA: hda/tegra: Reset hardware")

Is "Depends-on" a valid tag? I can't find it in Documentation/.

> ---
>  sound/pci/hda/hda_tegra.c | 45 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
> index ea700395..7c3df54 100644
> --- a/sound/pci/hda/hda_tegra.c
> +++ b/sound/pci/hda/hda_tegra.c
> @@ -68,14 +68,20 @@
>   */
>  #define TEGRA194_NUM_SDO_LINES	  4
>  
> +struct hda_tegra_soc {
> +	bool has_hda2codec_2x_reset;
> +};
> +
>  struct hda_tegra {
>  	struct azx chip;
>  	struct device *dev;
> -	struct reset_control *reset;
> +	struct reset_control_bulk_data resets[3];
>  	struct clk_bulk_data clocks[3];
> +	unsigned int nresets;
>  	unsigned int nclocks;
>  	void __iomem *regs;
>  	struct work_struct probe_work;
> +	const struct hda_tegra_soc *data;
>  };
>  
>  #ifdef CONFIG_PM
> @@ -170,7 +176,7 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
>  	int rc;
>  
>  	if (!chip->running) {
> -		rc = reset_control_assert(hda->reset);
> +		rc = reset_control_bulk_assert(hda->nresets, hda->resets);
>  		if (rc)
>  			return rc;
>  	}
> @@ -187,7 +193,7 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
>  	} else {
>  		usleep_range(10, 100);
>  
> -		rc = reset_control_deassert(hda->reset);
> +		rc = reset_control_bulk_deassert(hda->nresets, hda->resets);
>  		if (rc)
>  			return rc;
>  	}
> @@ -427,9 +433,17 @@ static int hda_tegra_create(struct snd_card *card,
>  	return 0;
>  }
>  
> +static const struct hda_tegra_soc tegra30_data = {
> +	.has_hda2codec_2x_reset = true,
> +};
> +
> +static const struct hda_tegra_soc tegra194_data = {
> +	.has_hda2codec_2x_reset = false,
> +};
> +
>  static const struct of_device_id hda_tegra_match[] = {
> -	{ .compatible = "nvidia,tegra30-hda" },
> -	{ .compatible = "nvidia,tegra194-hda" },
> +	{ .compatible = "nvidia,tegra30-hda", .data = &tegra30_data },
> +	{ .compatible = "nvidia,tegra194-hda", .data = &tegra194_data },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, hda_tegra_match);
> @@ -449,6 +463,10 @@ static int hda_tegra_probe(struct platform_device *pdev)
>  	hda->dev = &pdev->dev;
>  	chip = &hda->chip;
>  
> +	hda->data = of_device_get_match_data(&pdev->dev);
> +	if (!hda->data)
> +		return -EINVAL;

hda->data can't ever be NULL because all hda_tegra_match[] compatibles
above have .data assigned. Technically this check is redundant.

Thierry suggested previously to name it "hda->soc", like we usually do
it in other drivers.

>  	err = snd_card_new(&pdev->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
>  			   THIS_MODULE, 0, &card);
>  	if (err < 0) {
> @@ -456,11 +474,20 @@ static int hda_tegra_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> -	hda->reset = devm_reset_control_array_get_exclusive(&pdev->dev);
> -	if (IS_ERR(hda->reset)) {
> -		err = PTR_ERR(hda->reset);
> +	hda->resets[hda->nresets++].id = "hda";
> +	hda->resets[hda->nresets++].id = "hda2hdmi";
> +	/*
> +	 * "hda2codec_2x" reset is not present on Tegra194. Though DT would
> +	 * be updated to reflect this, but to have backward compatibility
> +	 * below is necessary.
> +	 */
> +	if (hda->data->has_hda2codec_2x_reset)
> +		hda->resets[hda->nresets++].id = "hda2codec_2x";
> +
> +	err = devm_reset_control_bulk_get_exclusive(&pdev->dev, hda->nresets,
> +						    hda->resets);
> +	if (err)
>  		goto out_free;
> -	}
>  
>  	hda->clocks[hda->nclocks++].id = "hda";
>  	hda->clocks[hda->nclocks++].id = "hda2hdmi";
> 

Not sure whether the above nits worth making v4. I'll leave it up to you
and other reviewers to decide.

Overall this patch looks good to me, thank you.

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>

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

* Re: [PATCH v3 2/3] dt-bindings: sound: tegra: Update HDA resets
  2021-12-22  4:35 ` [PATCH v3 2/3] dt-bindings: sound: tegra: Update HDA resets Sameer Pujar
@ 2021-12-22 19:37   ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2021-12-22 19:37 UTC (permalink / raw)
  To: Sameer Pujar
  Cc: tiwai, broonie, lgirdwood, thierry.reding, perex, jonathanh,
	digetx, mkumard, alsa-devel, devicetree, linux-tegra,
	linux-kernel

On Wed, Dec 22, 2021 at 10:05:50AM +0530, Sameer Pujar wrote:
> Tegra194 HDA has only two resets unlike the previous generations of
> Tegra SoCs. Hence update the reset list accordingly.
> 
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> Acked-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../devicetree/bindings/sound/nvidia,tegra30-hda.yaml       | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml
> index b55775e..70dbdff5 100644
> --- a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml
> +++ b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml
> @@ -50,13 +50,18 @@ properties:
>        - const: hda2codec_2x
>  
>    resets:
> +    minItems: 2
>      maxItems: 3
>  
>    reset-names:

Just add 'minItems: 2' here instead.

> -    items:
> -      - const: hda
> -      - const: hda2hdmi
> -      - const: hda2codec_2x
> +    oneOf:
> +      - items:
> +          - const: hda
> +          - const: hda2hdmi
> +          - const: hda2codec_2x
> +      - items:
> +          - const: hda
> +          - const: hda2hdmi
>  
>    power-domains:
>      maxItems: 1
> -- 
> 2.7.4
> 
> 

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

* Re: [PATCH v3 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure
  2021-12-22 18:40   ` Dmitry Osipenko
@ 2021-12-23  4:34     ` Sameer Pujar
  2021-12-23  7:16       ` Greg KH
  0 siblings, 1 reply; 9+ messages in thread
From: Sameer Pujar @ 2021-12-23  4:34 UTC (permalink / raw)
  To: Dmitry Osipenko, tiwai, broonie, lgirdwood, robh+dt,
	thierry.reding, perex
  Cc: jonathanh, mkumard, alsa-devel, devicetree, linux-tegra,
	linux-kernel, stable



On 12/23/2021 12:10 AM, Dmitry Osipenko wrote:
> 22.12.2021 07:35, Sameer Pujar пишет:
>> HDA regression is recently reported on Tegra194 based platforms.
>> This happens because "hda2codec_2x" reset does not really exist
>> in Tegra194 and it causes probe failure. All the HDA based audio
>> tests fail at the moment. This underlying issue is exposed by
>> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP
>> response") which now checks return code of BPMP command response.
>> Fix this issue by skipping unavailable reset on Tegra194.
>>
>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>> Cc: stable@vger.kernel.org
>> Depends-on: 87f0e46e7559 ("ALSA: hda/tegra: Reset hardware")
> Is "Depends-on" a valid tag? I can't find it in Documentation/.

I do find the usage of the tag in many commits though there is no 
reference of this in doc. I always thought it would act as a reference 
when commits get pulled to other branches. If this is not true and it 
does not mean anything, I will drop this.

>
>> ---
>>   sound/pci/hda/hda_tegra.c | 45 ++++++++++++++++++++++++++++++++++++---------
>>   1 file changed, 36 insertions(+), 9 deletions(-)
>>
>> diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
>> index ea700395..7c3df54 100644
>> --- a/sound/pci/hda/hda_tegra.c
>> +++ b/sound/pci/hda/hda_tegra.c
>> @@ -68,14 +68,20 @@
>>    */
>>   #define TEGRA194_NUM_SDO_LINES         4
>>
>> +struct hda_tegra_soc {
>> +     bool has_hda2codec_2x_reset;
>> +};
>> +
>>   struct hda_tegra {
>>        struct azx chip;
>>        struct device *dev;
>> -     struct reset_control *reset;
>> +     struct reset_control_bulk_data resets[3];
>>        struct clk_bulk_data clocks[3];
>> +     unsigned int nresets;
>>        unsigned int nclocks;
>>        void __iomem *regs;
>>        struct work_struct probe_work;
>> +     const struct hda_tegra_soc *data;
>>   };
>>
>>   #ifdef CONFIG_PM
>> @@ -170,7 +176,7 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
>>        int rc;
>>
>>        if (!chip->running) {
>> -             rc = reset_control_assert(hda->reset);
>> +             rc = reset_control_bulk_assert(hda->nresets, hda->resets);
>>                if (rc)
>>                        return rc;
>>        }
>> @@ -187,7 +193,7 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
>>        } else {
>>                usleep_range(10, 100);
>>
>> -             rc = reset_control_deassert(hda->reset);
>> +             rc = reset_control_bulk_deassert(hda->nresets, hda->resets);
>>                if (rc)
>>                        return rc;
>>        }
>> @@ -427,9 +433,17 @@ static int hda_tegra_create(struct snd_card *card,
>>        return 0;
>>   }
>>
>> +static const struct hda_tegra_soc tegra30_data = {
>> +     .has_hda2codec_2x_reset = true,
>> +};
>> +
>> +static const struct hda_tegra_soc tegra194_data = {
>> +     .has_hda2codec_2x_reset = false,
>> +};
>> +
>>   static const struct of_device_id hda_tegra_match[] = {
>> -     { .compatible = "nvidia,tegra30-hda" },
>> -     { .compatible = "nvidia,tegra194-hda" },
>> +     { .compatible = "nvidia,tegra30-hda", .data = &tegra30_data },
>> +     { .compatible = "nvidia,tegra194-hda", .data = &tegra194_data },
>>        {},
>>   };
>>   MODULE_DEVICE_TABLE(of, hda_tegra_match);
>> @@ -449,6 +463,10 @@ static int hda_tegra_probe(struct platform_device *pdev)
>>        hda->dev = &pdev->dev;
>>        chip = &hda->chip;
>>
>> +     hda->data = of_device_get_match_data(&pdev->dev);
>> +     if (!hda->data)
>> +             return -EINVAL;
> hda->data can't ever be NULL because all hda_tegra_match[] compatibles
> above have .data assigned. Technically this check is redundant.

Will remove.

>
> Thierry suggested previously to name it "hda->soc", like we usually do
> it in other drivers.

Previously I renamed strcture, but didn't update the member name. Will 
update.

>>        err = snd_card_new(&pdev->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
>>                           THIS_MODULE, 0, &card);
>>        if (err < 0) {
>> @@ -456,11 +474,20 @@ static int hda_tegra_probe(struct platform_device *pdev)
>>                return err;
>>        }
>>
>> -     hda->reset = devm_reset_control_array_get_exclusive(&pdev->dev);
>> -     if (IS_ERR(hda->reset)) {
>> -             err = PTR_ERR(hda->reset);
>> +     hda->resets[hda->nresets++].id = "hda";
>> +     hda->resets[hda->nresets++].id = "hda2hdmi";
>> +     /*
>> +      * "hda2codec_2x" reset is not present on Tegra194. Though DT would
>> +      * be updated to reflect this, but to have backward compatibility
>> +      * below is necessary.
>> +      */
>> +     if (hda->data->has_hda2codec_2x_reset)
>> +             hda->resets[hda->nresets++].id = "hda2codec_2x";
>> +
>> +     err = devm_reset_control_bulk_get_exclusive(&pdev->dev, hda->nresets,
>> +                                                 hda->resets);
>> +     if (err)
>>                goto out_free;
>> -     }
>>
>>        hda->clocks[hda->nclocks++].id = "hda";
>>        hda->clocks[hda->nclocks++].id = "hda2hdmi";
>>
> Not sure whether the above nits worth making v4. I'll leave it up to you
> and other reviewers to decide.
>
> Overall this patch looks good to me, thank you.
>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>


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

* Re: [PATCH v3 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure
  2021-12-23  4:34     ` Sameer Pujar
@ 2021-12-23  7:16       ` Greg KH
  2021-12-23 11:24         ` Sameer Pujar
  0 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2021-12-23  7:16 UTC (permalink / raw)
  To: Sameer Pujar
  Cc: Dmitry Osipenko, tiwai, broonie, lgirdwood, robh+dt,
	thierry.reding, perex, jonathanh, mkumard, alsa-devel,
	devicetree, linux-tegra, linux-kernel, stable

On Thu, Dec 23, 2021 at 10:04:19AM +0530, Sameer Pujar wrote:
> 
> 
> On 12/23/2021 12:10 AM, Dmitry Osipenko wrote:
> > 22.12.2021 07:35, Sameer Pujar пишет:
> > > HDA regression is recently reported on Tegra194 based platforms.
> > > This happens because "hda2codec_2x" reset does not really exist
> > > in Tegra194 and it causes probe failure. All the HDA based audio
> > > tests fail at the moment. This underlying issue is exposed by
> > > commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP
> > > response") which now checks return code of BPMP command response.
> > > Fix this issue by skipping unavailable reset on Tegra194.
> > > 
> > > Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> > > Cc: stable@vger.kernel.org
> > > Depends-on: 87f0e46e7559 ("ALSA: hda/tegra: Reset hardware")
> > Is "Depends-on" a valid tag? I can't find it in Documentation/.
> 
> I do find the usage of the tag in many commits though there is no reference
> of this in doc. I always thought it would act as a reference when commits
> get pulled to other branches. If this is not true and it does not mean
> anything, I will drop this.

It is not true at all, please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

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

* Re: [PATCH v3 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure
  2021-12-23  7:16       ` Greg KH
@ 2021-12-23 11:24         ` Sameer Pujar
  0 siblings, 0 replies; 9+ messages in thread
From: Sameer Pujar @ 2021-12-23 11:24 UTC (permalink / raw)
  To: Greg KH
  Cc: Dmitry Osipenko, tiwai, broonie, lgirdwood, robh+dt,
	thierry.reding, perex, jonathanh, mkumard, alsa-devel,
	devicetree, linux-tegra, linux-kernel, stable



On 12/23/2021 12:46 PM, Greg KH wrote:
> On Thu, Dec 23, 2021 at 10:04:19AM +0530, Sameer Pujar wrote:
>>
>> On 12/23/2021 12:10 AM, Dmitry Osipenko wrote:
>>> 22.12.2021 07:35, Sameer Pujar пишет:
>>>> HDA regression is recently reported on Tegra194 based platforms.
>>>> This happens because "hda2codec_2x" reset does not really exist
>>>> in Tegra194 and it causes probe failure. All the HDA based audio
>>>> tests fail at the moment. This underlying issue is exposed by
>>>> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP
>>>> response") which now checks return code of BPMP command response.
>>>> Fix this issue by skipping unavailable reset on Tegra194.
>>>>
>>>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>>>> Cc: stable@vger.kernel.org
>>>> Depends-on: 87f0e46e7559 ("ALSA: hda/tegra: Reset hardware")
>>> Is "Depends-on" a valid tag? I can't find it in Documentation/.
>> I do find the usage of the tag in many commits though there is no reference
>> of this in doc. I always thought it would act as a reference when commits
>> get pulled to other branches. If this is not true and it does not mean
>> anything, I will drop this.
> It is not true at all, please read:
>      https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.

Thanks Greg for the pointer. I will drop above tag in v4.

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

end of thread, other threads:[~2021-12-23 11:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-22  4:35 [PATCH v3 0/3] Fix Tegra194 HDA regression Sameer Pujar
2021-12-22  4:35 ` [PATCH v3 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure Sameer Pujar
2021-12-22 18:40   ` Dmitry Osipenko
2021-12-23  4:34     ` Sameer Pujar
2021-12-23  7:16       ` Greg KH
2021-12-23 11:24         ` Sameer Pujar
2021-12-22  4:35 ` [PATCH v3 2/3] dt-bindings: sound: tegra: Update HDA resets Sameer Pujar
2021-12-22 19:37   ` Rob Herring
2021-12-22  4:35 ` [PATCH v3 3/3] arm64: tegra: Remove non existent Tegra194 reset Sameer Pujar

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