clk: tegra210: Fix default rates for HDA clocks
diff mbox series

Message ID 1559121501-8566-1-git-send-email-jonathanh@nvidia.com
State New
Headers show
Series
  • clk: tegra210: Fix default rates for HDA clocks
Related show

Commit Message

Jon Hunter May 29, 2019, 9:18 a.m. UTC
Currently the default clock rates for the HDA and HDA2CODEC_2X clocks
are both 19.2MHz. However, the default rates for these clocks should
actually be 51MHz and 48MHz, respectively. Correct the default clock
rates for these clocks by specifying them in the clock init table for
Tegra210.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/clk/tegra/clk-tegra210.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Thierry Reding May 29, 2019, 1:46 p.m. UTC | #1
On Wed, May 29, 2019 at 10:18:21AM +0100, Jon Hunter wrote:
> Currently the default clock rates for the HDA and HDA2CODEC_2X clocks
> are both 19.2MHz. However, the default rates for these clocks should
> actually be 51MHz and 48MHz, respectively. Correct the default clock
> rates for these clocks by specifying them in the clock init table for
> Tegra210.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/clk/tegra/clk-tegra210.c | 2 ++
>  1 file changed, 2 insertions(+)

Does this fix anything? Should this be backported to stable releases?

Acked-by: Thierry Reding <treding@nvidia.com>
Jon Hunter May 31, 2019, 2:58 p.m. UTC | #2
On 29/05/2019 14:46, Thierry Reding wrote:
> On Wed, May 29, 2019 at 10:18:21AM +0100, Jon Hunter wrote:
>> Currently the default clock rates for the HDA and HDA2CODEC_2X clocks
>> are both 19.2MHz. However, the default rates for these clocks should
>> actually be 51MHz and 48MHz, respectively. Correct the default clock
>> rates for these clocks by specifying them in the clock init table for
>> Tegra210.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/clk/tegra/clk-tegra210.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
> Does this fix anything? Should this be backported to stable releases?

Good point. We are aligning the clock configuration with what we ship.
So I thought for completeness it would be good to test HDA playback
across the various sample-rates we support (32kHz to 192kHz) but with or
without this patch I am not hearing anything. Let me check on this with
Sameer as I would like to see if we need to mark this for stable or not.

> Acked-by: Thierry Reding <treding@nvidia.com>

Thanks
Jon
Jon Hunter June 5, 2019, 11:30 a.m. UTC | #3
On 31/05/2019 15:58, Jon Hunter wrote:
> 
> On 29/05/2019 14:46, Thierry Reding wrote:
>> On Wed, May 29, 2019 at 10:18:21AM +0100, Jon Hunter wrote:
>>> Currently the default clock rates for the HDA and HDA2CODEC_2X clocks
>>> are both 19.2MHz. However, the default rates for these clocks should
>>> actually be 51MHz and 48MHz, respectively. Correct the default clock
>>> rates for these clocks by specifying them in the clock init table for
>>> Tegra210.
>>>
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> ---
>>>  drivers/clk/tegra/clk-tegra210.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>
>> Does this fix anything? Should this be backported to stable releases?
> 
> Good point. We are aligning the clock configuration with what we ship.
> So I thought for completeness it would be good to test HDA playback
> across the various sample-rates we support (32kHz to 192kHz) but with or
> without this patch I am not hearing anything. Let me check on this with
> Sameer as I would like to see if we need to mark this for stable or not.
> 
>> Acked-by: Thierry Reding <treding@nvidia.com>

I have confirmed that this does fix HDA playback on Tegra210. Without
this fix, I am seeing the following messages during playback and
playback is distorted ...

Write error: -32,Broken pipe
[   15.069335] tegra-mc 70019000.memory-controller: hdar: read
@0x0000000000000000: EMEM address decode error (EMEM decode error)
Write error: -32,Broken pipe
[   15.465362] tegra-mc 70019000.memory-controller: hdar: read
@0x0000000000000000: EMEM address decode error (EMEM decode error)
Write error: -32,Broken pipe
[   15.858615] tegra-mc 70019000.memory-controller: hdar: read
@0x0000000000000000: EMEM address decode error (EMEM decode error)
W

Do you want me to update the change and resend?

Jon
Thierry Reding June 5, 2019, 12:38 p.m. UTC | #4
On Wed, Jun 05, 2019 at 12:30:31PM +0100, Jon Hunter wrote:
> 
> On 31/05/2019 15:58, Jon Hunter wrote:
> > 
> > On 29/05/2019 14:46, Thierry Reding wrote:
> >> On Wed, May 29, 2019 at 10:18:21AM +0100, Jon Hunter wrote:
> >>> Currently the default clock rates for the HDA and HDA2CODEC_2X clocks
> >>> are both 19.2MHz. However, the default rates for these clocks should
> >>> actually be 51MHz and 48MHz, respectively. Correct the default clock
> >>> rates for these clocks by specifying them in the clock init table for
> >>> Tegra210.
> >>>
> >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> >>> ---
> >>>  drivers/clk/tegra/clk-tegra210.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>
> >> Does this fix anything? Should this be backported to stable releases?
> > 
> > Good point. We are aligning the clock configuration with what we ship.
> > So I thought for completeness it would be good to test HDA playback
> > across the various sample-rates we support (32kHz to 192kHz) but with or
> > without this patch I am not hearing anything. Let me check on this with
> > Sameer as I would like to see if we need to mark this for stable or not.
> > 
> >> Acked-by: Thierry Reding <treding@nvidia.com>
> 
> I have confirmed that this does fix HDA playback on Tegra210. Without
> this fix, I am seeing the following messages during playback and
> playback is distorted ...
> 
> Write error: -32,Broken pipe
> [   15.069335] tegra-mc 70019000.memory-controller: hdar: read
> @0x0000000000000000: EMEM address decode error (EMEM decode error)
> Write error: -32,Broken pipe
> [   15.465362] tegra-mc 70019000.memory-controller: hdar: read
> @0x0000000000000000: EMEM address decode error (EMEM decode error)
> Write error: -32,Broken pipe
> [   15.858615] tegra-mc 70019000.memory-controller: hdar: read
> @0x0000000000000000: EMEM address decode error (EMEM decode error)
> W
> 
> Do you want me to update the change and resend?

Honestly I'm not sure if it's worth it. I haven't seen any bug reports
for this and we haven't had audio over HDMI support for very long, so a
backport may not be necessary.

I guess there'd be some use to backport this so that our stable kernel
testing passes these. So it's really up to you. I have a slight tendency
towards backporting, because it's really tiny and then we just have it
out of the way and it's not going to haunt us.

Thierry

Patch
diff mbox series

diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index ed3c7df75d1e..8b3b3d771813 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -3377,6 +3377,8 @@  static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA210_CLK_I2S3_SYNC, TEGRA210_CLK_CLK_MAX, 24576000, 0 },
 	{ TEGRA210_CLK_I2S4_SYNC, TEGRA210_CLK_CLK_MAX, 24576000, 0 },
 	{ TEGRA210_CLK_VIMCLK_SYNC, TEGRA210_CLK_CLK_MAX, 24576000, 0 },
+	{ TEGRA210_CLK_HDA, TEGRA210_CLK_PLL_P, 51000000, 0 },
+	{ TEGRA210_CLK_HDA2CODEC_2X, TEGRA210_CLK_PLL_P, 48000000, 0 },
 	/* This MUST be the last entry. */
 	{ TEGRA210_CLK_CLK_MAX, TEGRA210_CLK_CLK_MAX, 0, 0 },
 };