Patchwork [2/2] clk: tegra: add ac97 controller clock

login
register
mail settings
Submitter Lucas Stach
Date Jan. 27, 2013, 10:17 p.m.
Message ID <1359325055-5160-2-git-send-email-dev@lynxeye.de>
Download mbox | patch
Permalink /patch/216069/
State Changes Requested, archived
Headers show

Comments

Lucas Stach - Jan. 27, 2013, 10:17 p.m.
AC97 controller clock is hardwired to pll_a_out0.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/clk/tegra/clk-tegra20.c | 8 ++++++++
 1 file changed, 8 insertions(+)
Stephen Warren - Jan. 28, 2013, 6:38 p.m.
On 01/27/2013 03:17 PM, Lucas Stach wrote:
> AC97 controller clock is hardwired to pll_a_out0.

I would like Prashant's/Peter's review here too.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - Jan. 28, 2013, 7:15 p.m.
On 01/27/2013 03:17 PM, Lucas Stach wrote:
> AC97 controller clock is hardwired to pll_a_out0.

One more comment here; don't you need to add this clock into
audio_parents[]?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lucas Stach - Jan. 28, 2013, 7:25 p.m.
Am Montag, den 28.01.2013, 12:15 -0700 schrieb Stephen Warren:
> On 01/27/2013 03:17 PM, Lucas Stach wrote:
> > AC97 controller clock is hardwired to pll_a_out0.
> 
> One more comment here; don't you need to add this clock into
> audio_parents[]?
> 
Yeah it's documented that AC97 can be used as audio_sync_clock, but for
one I haven't tested this and also ac97 clock == pll_a_out0, so you can
just as well use the pll out in that case.

Regards,
Lucas


--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - Jan. 28, 2013, 9:17 p.m.
On 01/28/2013 12:25 PM, Lucas Stach wrote:
> Am Montag, den 28.01.2013, 12:15 -0700 schrieb Stephen Warren:
>> On 01/27/2013 03:17 PM, Lucas Stach wrote:
>>> AC97 controller clock is hardwired to pll_a_out0.
>>
>> One more comment here; don't you need to add this clock into
>> audio_parents[]?
>>
> Yeah it's documented that AC97 can be used as audio_sync_clock, but for
> one I haven't tested this

I expect there are quite a few of the clock driver table entries that
aren't tested. I still think we should add them though, once the clocks
they refer to actually exist.

> and also ac97 clock == pll_a_out0, so you can
> just as well use the pll out in that case.

I suspect that "ac97 clock == pll_a_out0" isn't categorically true; it's
quite possible that the TRM simply doesn't document the mux
register/field for the ac97 clock since the ac97 module is considered
deprecated. The diagram at the end of section 5.2.3 "Audio Clocks"
certainly implies that all of the i2s*, spdifout, and ac97 clocks have
the same structure.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter De Schrijver - Jan. 30, 2013, 9:35 a.m.
> 
> > and also ac97 clock == pll_a_out0, so you can
> > just as well use the pll out in that case.
> 
> I suspect that "ac97 clock == pll_a_out0" isn't categorically true; it's
> quite possible that the TRM simply doesn't document the mux
> register/field for the ac97 clock since the ac97 module is considered
> deprecated. The diagram at the end of section 5.2.3 "Audio Clocks"
> certainly implies that all of the i2s*, spdifout, and ac97 clocks have
> the same structure.

It indeed does, but I can't find any reference to a mux control register for
ac97 in any document...

Cheers,

Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - Jan. 30, 2013, 6:27 p.m.
On 01/30/2013 02:35 AM, Peter De Schrijver wrote:
>>
>>> and also ac97 clock == pll_a_out0, so you can
>>> just as well use the pll out in that case.
>>
>> I suspect that "ac97 clock == pll_a_out0" isn't categorically true; it's
>> quite possible that the TRM simply doesn't document the mux
>> register/field for the ac97 clock since the ac97 module is considered
>> deprecated. The diagram at the end of section 5.2.3 "Audio Clocks"
>> certainly implies that all of the i2s*, spdifout, and ac97 clocks have
>> the same structure.
> 
> It indeed does, but I can't find any reference to a mux control register for
> ac97 in any document...

For both these two issues, I'll try and track down the relevant HW
engineers or other sources of documentation internally...

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lucas Stach - March 21, 2013, 2:01 p.m.
Am Mittwoch, den 30.01.2013, 11:27 -0700 schrieb Stephen Warren:
> On 01/30/2013 02:35 AM, Peter De Schrijver wrote:
> >>
> >>> and also ac97 clock == pll_a_out0, so you can
> >>> just as well use the pll out in that case.
> >>
> >> I suspect that "ac97 clock == pll_a_out0" isn't categorically true; it's
> >> quite possible that the TRM simply doesn't document the mux
> >> register/field for the ac97 clock since the ac97 module is considered
> >> deprecated. The diagram at the end of section 5.2.3 "Audio Clocks"
> >> certainly implies that all of the i2s*, spdifout, and ac97 clocks have
> >> the same structure.
> > 
> > It indeed does, but I can't find any reference to a mux control register for
> > ac97 in any document...
> 
> For both these two issues, I'll try and track down the relevant HW
> engineers or other sources of documentation internally...
> 
Ping.
I would really like to see those two patches in 3.9, as otherwise
Colibri T20 boots up with kernel warnings and a non-functional audio.

Regards,
Lucas

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - March 21, 2013, 5:48 p.m.
On 03/21/2013 08:01 AM, Lucas Stach wrote:
> Am Mittwoch, den 30.01.2013, 11:27 -0700 schrieb Stephen Warren:
>> On 01/30/2013 02:35 AM, Peter De Schrijver wrote:
>>>>
>>>>> and also ac97 clock == pll_a_out0, so you can
>>>>> just as well use the pll out in that case.
>>>>
>>>> I suspect that "ac97 clock == pll_a_out0" isn't categorically true; it's
>>>> quite possible that the TRM simply doesn't document the mux
>>>> register/field for the ac97 clock since the ac97 module is considered
>>>> deprecated. The diagram at the end of section 5.2.3 "Audio Clocks"
>>>> certainly implies that all of the i2s*, spdifout, and ac97 clocks have
>>>> the same structure.
>>>
>>> It indeed does, but I can't find any reference to a mux control register for
>>> ac97 in any document...
>>
>> For both these two issues, I'll try and track down the relevant HW
>> engineers or other sources of documentation internally...
>>
> Ping.
> I would really like to see those two patches in 3.9, as otherwise
> Colibri T20 boots up with kernel warnings and a non-functional audio.

Sorry, I seem to have forgotten about this, and it looks like I wasn't
able to get any kind of answer from HW. I expect it's far too late to
get this into 3.9.

I guess we can go ahead and add the clock without actually resolving
these issues; I don't think their resolution would affect the device
tree at all, so there should be no compatibility issues to worry about.

I suggest reposting to Mike Turqette (clock maintainer) to see if he'll
take it as a fix for 3.9, or ack it so I can. If not, if he can ack it,
I'll take it for 3.10 along with the Tegra clock driver changes.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index f08cffc..1be8c23 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -894,6 +894,14 @@  static void __init tegra20_periph_clk_init(void)
 	struct clk *clk;
 	int i;
 
+	/* ac97 */
+	clk = tegra_clk_register_periph_gate("ac97", "pll_a_out0",
+				    TEGRA_PERIPH_ON_APB,
+				    clk_base, 0, 3, &periph_l_regs,
+				    periph_clk_enb_refcnt);
+	clk_register_clkdev(clk, NULL, "tegra20-ac97");
+	clks[ac97] = clk;
+
 	/* apbdma */
 	clk = tegra_clk_register_periph_gate("apbdma", "pclk", 0, clk_base,
 				    0, 34, &periph_h_regs,