linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* omap4 pandaboard: clock input for TiWi module inactive
@ 2013-03-07 20:37 Levi Pearson
  2013-03-11  8:58 ` Roger Quadros
  0 siblings, 1 reply; 6+ messages in thread
From: Levi Pearson @ 2013-03-07 20:37 UTC (permalink / raw)
  To: linux-arm-kernel

I recently upgraded from a 3.4 kernel to a 3.8 kernel and found that
the drivers for the functionality provided by the TiWi module on the
pandaboard (Bluetooth, WiFI, etc.) could no longer probe the device.
After some investigation, it turned out that the TiWi clock is
provided by the twl6030 PMIC chip.

The clock output of the twl6030 was first enabled via these commits:

https://patchwork.kernel.org/patch/546381/
https://patchwork.kernel.org/patch/546401/

Since most of the functionality of the TWL6030 is power regulation and
the clock is enabled via a similar mechanism to the power outputs, the
clock was added as a 'pseudo-regulator' to
drivers/regulator/twl-regulator.c.

Some time later, a series of commits removed the clock functionality
from twl-regulator.c, apparently due to being out-of-place in the
Device Tree listings or something.

The first of these is recorded here: https://lkml.org/lkml/2012/6/4/330
The next two just clean up the code that was orphaned by the previous:
 e76ab829 "regulator: twl: Remove references to the twl4030 regulator"
 029dd3ce "regulator: twl: Remove another unused variable warning"

Luciano Cuelho, who maintains the driver for the WiFi portion of the
TiWi modules, reported the resulting breakage some time later on the
LKML: https://lkml.org/lkml/2012/11/15/72

Despite the protestation of Mark Brown that his code removal wasn't
the cause of the problem, reverting his changes does indeed enable the
clock.  However, it also seems to be the case that there ought to be a
better home for the clock than twl-regulator.c.  At the time of the
previous discussion, it wasn't clear if the framework was in place to
support a clock driver for the twl6030's clock, but it seems like the
support exists now, since there's a clock driver for the twl6030's
companion, the twl6040 audio chip.

So: Is there any chance of simply reverting the 3 commits that broke
the clock?  I have done this locally, but I hate to see pandaboard
support in the mainstream kernel continue to be broken in this way.

If for whatever reason the commits cannot be reverted, would creating
a driver like drivers/clk/clk-twl6040.c that enabled the twl6030's
clock output be a suitable alternative?  I'd be willing to take a stab
at writing it if there's not something like that in the works already.

---
Levi Pearson

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

* omap4 pandaboard: clock input for TiWi module inactive
  2013-03-07 20:37 omap4 pandaboard: clock input for TiWi module inactive Levi Pearson
@ 2013-03-11  8:58 ` Roger Quadros
  2013-03-11  9:51   ` Peter Ujfalusi
  0 siblings, 1 reply; 6+ messages in thread
From: Roger Quadros @ 2013-03-11  8:58 UTC (permalink / raw)
  To: linux-arm-kernel

(+ Peter, Luca, Balaji)

else your e-mail might just go unnoticed ;)

On 03/07/2013 10:37 PM, Levi Pearson wrote:
> I recently upgraded from a 3.4 kernel to a 3.8 kernel and found that
> the drivers for the functionality provided by the TiWi module on the
> pandaboard (Bluetooth, WiFI, etc.) could no longer probe the device.
> After some investigation, it turned out that the TiWi clock is
> provided by the twl6030 PMIC chip.
> 
> The clock output of the twl6030 was first enabled via these commits:
> 
> https://patchwork.kernel.org/patch/546381/
> https://patchwork.kernel.org/patch/546401/
> 
> Since most of the functionality of the TWL6030 is power regulation and
> the clock is enabled via a similar mechanism to the power outputs, the
> clock was added as a 'pseudo-regulator' to
> drivers/regulator/twl-regulator.c.
> 
> Some time later, a series of commits removed the clock functionality
> from twl-regulator.c, apparently due to being out-of-place in the
> Device Tree listings or something.
> 
> The first of these is recorded here: https://lkml.org/lkml/2012/6/4/330
> The next two just clean up the code that was orphaned by the previous:
>  e76ab829 "regulator: twl: Remove references to the twl4030 regulator"
>  029dd3ce "regulator: twl: Remove another unused variable warning"
> 
> Luciano Cuelho, who maintains the driver for the WiFi portion of the
> TiWi modules, reported the resulting breakage some time later on the
> LKML: https://lkml.org/lkml/2012/11/15/72
> 
> Despite the protestation of Mark Brown that his code removal wasn't
> the cause of the problem, reverting his changes does indeed enable the
> clock.  However, it also seems to be the case that there ought to be a
> better home for the clock than twl-regulator.c.  At the time of the
> previous discussion, it wasn't clear if the framework was in place to
> support a clock driver for the twl6030's clock, but it seems like the
> support exists now, since there's a clock driver for the twl6030's
> companion, the twl6040 audio chip.
> 
> So: Is there any chance of simply reverting the 3 commits that broke
> the clock?  I have done this locally, but I hate to see pandaboard
> support in the mainstream kernel continue to be broken in this way.
> 
> If for whatever reason the commits cannot be reverted, would creating
> a driver like drivers/clk/clk-twl6040.c that enabled the twl6030's
> clock output be a suitable alternative?  I'd be willing to take a stab
> at writing it if there's not something like that in the works already.
> 
> ---
> Levi Pearson
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* omap4 pandaboard: clock input for TiWi module inactive
  2013-03-11  8:58 ` Roger Quadros
@ 2013-03-11  9:51   ` Peter Ujfalusi
  2013-03-12 16:33     ` Levi Pearson
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Ujfalusi @ 2013-03-11  9:51 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/11/2013 09:58 AM, Roger Quadros wrote:
> (+ Peter, Luca, Balaji)
> 
> else your e-mail might just go unnoticed ;)
> 
> On 03/07/2013 10:37 PM, Levi Pearson wrote:
>> I recently upgraded from a 3.4 kernel to a 3.8 kernel and found that
>> the drivers for the functionality provided by the TiWi module on the
>> pandaboard (Bluetooth, WiFI, etc.) could no longer probe the device.
>> After some investigation, it turned out that the TiWi clock is
>> provided by the twl6030 PMIC chip.
>>
>> The clock output of the twl6030 was first enabled via these commits:
>>
>> https://patchwork.kernel.org/patch/546381/
>> https://patchwork.kernel.org/patch/546401/
>>
>> Since most of the functionality of the TWL6030 is power regulation and
>> the clock is enabled via a similar mechanism to the power outputs, the
>> clock was added as a 'pseudo-regulator' to
>> drivers/regulator/twl-regulator.c.
>>
>> Some time later, a series of commits removed the clock functionality
>> from twl-regulator.c, apparently due to being out-of-place in the
>> Device Tree listings or something.
>>
>> The first of these is recorded here: https://lkml.org/lkml/2012/6/4/330
>> The next two just clean up the code that was orphaned by the previous:
>>  e76ab829 "regulator: twl: Remove references to the twl4030 regulator"
>>  029dd3ce "regulator: twl: Remove another unused variable warning"
>>
>> Luciano Cuelho, who maintains the driver for the WiFi portion of the
>> TiWi modules, reported the resulting breakage some time later on the
>> LKML: https://lkml.org/lkml/2012/11/15/72
>>
>> Despite the protestation of Mark Brown that his code removal wasn't
>> the cause of the problem, reverting his changes does indeed enable the
>> clock.  However, it also seems to be the case that there ought to be a
>> better home for the clock than twl-regulator.c.  At the time of the
>> previous discussion, it wasn't clear if the framework was in place to
>> support a clock driver for the twl6030's clock, but it seems like the
>> support exists now, since there's a clock driver for the twl6030's
>> companion, the twl6040 audio chip.

IMHO the 'hack' in the twl-regulator should not have been removed before we
provide proper support for the clocks via the common clock framework.

>> So: Is there any chance of simply reverting the 3 commits that broke
>> the clock?  I have done this locally, but I hate to see pandaboard
>> support in the mainstream kernel continue to be broken in this way.

We should have the clock driver under drivers/clk/ via clk-twl6030.c driver
for all the clocks twl6030 provides and use that in drivers (updating the
clock tree as well).

>> If for whatever reason the commits cannot be reverted, would creating
>> a driver like drivers/clk/clk-twl6040.c that enabled the twl6030's
>> clock output be a suitable alternative?  I'd be willing to take a stab
>> at writing it if there's not something like that in the works already.

If you can take this it would be great. It is still in my to-do list to
integrate the clk-twl6040 clock with the omap-mcpdm driver. I already got
working code, but it is not working in a way I would really wanted to be.
Integrating external clock to the SoC's clock tree does not seams to be that
straight forward: at boot time when the clock tree is initialized the external
chip is yet to be probed -> the root clock is not there, but later on for some
reason I can not get the CCF to enable the clock tree correctly.

-- 
P?ter

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

* omap4 pandaboard: clock input for TiWi module inactive
  2013-03-11  9:51   ` Peter Ujfalusi
@ 2013-03-12 16:33     ` Levi Pearson
  2013-03-13  8:29       ` Peter Ujfalusi
  0 siblings, 1 reply; 6+ messages in thread
From: Levi Pearson @ 2013-03-12 16:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Mar 11, 2013 at 3:51 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> On 03/11/2013 09:58 AM, Roger Quadros wrote:

>
> IMHO the 'hack' in the twl-regulator should not have been removed before we
> provide proper support for the clocks via the common clock framework.
>

I was surprised that this happened in the first place, but I guess it
really wasn't
obvious that it was still in use.

>
> We should have the clock driver under drivers/clk/ via clk-twl6030.c driver
> for all the clocks twl6030 provides and use that in drivers (updating the
> clock tree as well).
>

I spent some more time looking at clk-twl6030.c, and it seems it just turns the
twl6030 chip on when you ask it to enable the clock, and as far as I
can tell that
happens based on module dependencies?  There aren't any explicit references to
it that I can find, so I'm not entirely sure how a clk-twl6040 would
cause a clock
to be enabled.

>>> If for whatever reason the commits cannot be reverted, would creating
>>> a driver like drivers/clk/clk-twl6040.c that enabled the twl6030's
>>> clock output be a suitable alternative?  I'd be willing to take a stab
>>> at writing it if there's not something like that in the works already.
>
> If you can take this it would be great. It is still in my to-do list to
> integrate the clk-twl6040 clock with the omap-mcpdm driver. I already got
> working code, but it is not working in a way I would really wanted to be.
> Integrating external clock to the SoC's clock tree does not seams to be that
> straight forward: at boot time when the clock tree is initialized the external
> chip is yet to be probed -> the root clock is not there, but later on for some
> reason I can not get the CCF to enable the clock tree correctly.

I looked into the early boot clock setup code, and that does look a lot less
straightforward.  There are already a few 32k clocks listed there, and
I *think* they
all source from the twl6040; do they just come up automatically at power-on?
>From what I can tell, clk32kg is just a gated fork of the regular 32k
clock that is
available at boot from twl6040.  I am surprised it doesn't come on by default.

I also don't have a TRM for twl6040 that matches up with what twl-regulator.c is
doing to enable the clk32kg output; it looks like it's adding it to
one of the power
modes, but I'm not entirely sure how the registers it's writing to work, so my
confidence level in being able to get it transplanted to a separate
driver has lowered
quite a bit.

---

--Levi

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

* omap4 pandaboard: clock input for TiWi module inactive
  2013-03-12 16:33     ` Levi Pearson
@ 2013-03-13  8:29       ` Peter Ujfalusi
  2013-03-13 16:12         ` Levi Pearson
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Ujfalusi @ 2013-03-13  8:29 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/12/2013 05:33 PM, Levi Pearson wrote:
> I spent some more time looking at clk-twl6030.c, and it seems it just turns the
> twl6030 chip on when you ask it to enable the clock, and as far as I
> can tell that
> happens based on module dependencies?  There aren't any explicit references to
> it that I can find, so I'm not entirely sure how a clk-twl6040 would
> cause a clock
> to be enabled.

The clk-twl6040 is not really a good example for you start with. We only need
it to turn on in order to get the clock on the output. But the principle is
the same at the end. you need to enable/disable the clock from twl6030...

>>>> If for whatever reason the commits cannot be reverted, would creating
>>>> a driver like drivers/clk/clk-twl6040.c that enabled the twl6030's
>>>> clock output be a suitable alternative?  I'd be willing to take a stab
>>>> at writing it if there's not something like that in the works already.
>>
>> If you can take this it would be great. It is still in my to-do list to
>> integrate the clk-twl6040 clock with the omap-mcpdm driver. I already got
>> working code, but it is not working in a way I would really wanted to be.
>> Integrating external clock to the SoC's clock tree does not seams to be that
>> straight forward: at boot time when the clock tree is initialized the external
>> chip is yet to be probed -> the root clock is not there, but later on for some
>> reason I can not get the CCF to enable the clock tree correctly.
> 
> I looked into the early boot clock setup code, and that does look a lot less
> straightforward.

Yes, this is what I thought for the twl6040 clock integration with McPDM. It
turned out a bit more complicated.
At boot time you do not have the twl clock driver loaded.
You have the clock tree already defined.
When you boot further you add the clock for twl6040.
You want to hook it up with the existing clock tree - to set it as parent of
one of the clock root (which you already modified to not to be the root clock).
I shall work, but it does not:
for external chips we need to do the configuration/enable in clk_prepare time
(because they are over i2c).
So the driver goes and asks for a clock (which supposed to be at the end
rooted to this new external clock).
Nothing happens.
The relevant clock tree has been already got prepared earlier so new prepare
will not going to go down to the root, leaving the root clock disabled.

I'm still looking a way to somehow fix this, but my time is limited right now.


> There are already a few 32k clocks listed there, and
> I *think* they
> all source from the twl6040; do they just come up automatically at power-on?
> From what I can tell, clk32kg is just a gated fork of the regular 32k
> clock that is
> available at boot from twl6040.  I am surprised it doesn't come on by default.

I have not checked the schematics, but it could be that we have crystal 32K on
board as well. TWL6030 can gate some of it's 32K outputs and this is the case
with WiLink on PandaBoard.

> 
> I also don't have a TRM for twl6040 that matches up with what twl-regulator.c is
> doing to enable the clk32kg output; it looks like it's adding it to
> one of the power
> modes, but I'm not entirely sure how the registers it's writing to work, so my
> confidence level in being able to get it transplanted to a separate
> driver has lowered
> quite a bit.

I'm not sure if I have twl6030 TRM either, but I can try to come up with a
basic driver for the twl6030 clocks.

-- 
P?ter

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

* omap4 pandaboard: clock input for TiWi module inactive
  2013-03-13  8:29       ` Peter Ujfalusi
@ 2013-03-13 16:12         ` Levi Pearson
  0 siblings, 0 replies; 6+ messages in thread
From: Levi Pearson @ 2013-03-13 16:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 13, 2013 at 2:29 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> On 03/12/2013 05:33 PM, Levi Pearson wrote:
>> I spent some more time looking at clk-twl6030.c, and it seems it just turns the
>> twl6030 chip on when you ask it to enable the clock, and as far as I
>> can tell that
>> happens based on module dependencies?  There aren't any explicit references to
>> it that I can find, so I'm not entirely sure how a clk-twl6040 would
>> cause a clock
>> to be enabled.
>
> The clk-twl6040 is not really a good example for you start with. We only need
> it to turn on in order to get the clock on the output. But the principle is
> the same at the end. you need to enable/disable the clock from twl6030...
>

Well, it's at least a nice minimal example of how to interface with the clk API.

>> I looked into the early boot clock setup code, and that does look a lot less
>> straightforward.
>
> Yes, this is what I thought for the twl6040 clock integration with McPDM. It
> turned out a bit more complicated.
> At boot time you do not have the twl clock driver loaded.
> You have the clock tree already defined.
> When you boot further you add the clock for twl6040.

I can't see where exactly this is done, though.  I think I'm just
going to have to
poke around the clock trees for other platforms or something to see how it's
normally done, but unfortunately my time to work on this is pretty limited too!

> You want to hook it up with the existing clock tree - to set it as parent of
> one of the clock root (which you already modified to not to be the root clock).
> I shall work, but it does not:
> for external chips we need to do the configuration/enable in clk_prepare time
> (because they are over i2c).
> So the driver goes and asks for a clock (which supposed to be at the end
> rooted to this new external clock).
> Nothing happens.
> The relevant clock tree has been already got prepared earlier so new prepare
> will not going to go down to the root, leaving the root clock disabled.
>
> I'm still looking a way to somehow fix this, but my time is limited right now.

Is it possible to do static config on the clocks to create the basic
structure during
the early boot phase, and then add the dynamic control functions to
the structure
later after i2c has been set up?  See below for more details, but TWL6030 is the
parent of all 32k clocks on the board, it just enables some of them by
default when
it's powered on.  Seems like that should be reflected in the early set
up structure.

>> There are already a few 32k clocks listed there, and
>> I *think* they
>> all source from the twl6040; do they just come up automatically at power-on?
>> From what I can tell, clk32kg is just a gated fork of the regular 32k
>> clock that is
>> available at boot from twl6040.  I am surprised it doesn't come on by default.
>
> I have not checked the schematics, but it could be that we have crystal 32K on
> board as well. TWL6030 can gate some of it's 32K outputs and this is the case
> with WiLink on PandaBoard.

There is a crystal 32k; it's connected directly to TWL6030 via OSC32KIN and
OSC32KOUT.  There are also 3 gated outputs:
  * CLK32K_AUD  <- this goes to TWL6040 as the CLK32K input of its PLL block
  * CLK32K_MAIN  <- this goes to OMAP4430's SYS32K input
  * CLK32K_GATE  <- this is goes to WL172x chip's SLOW_CLK input

And those are all the 32k clocks on the board.  No one gets a 32k clock except
through TWL6030, though I guess there may be some internal 32k clock gating
inside the OMAP. The question in the back of my mind is why CLK32K_AUD and
CLK32K_MAIN are enabled by default in the power modes we use but
CLK32K_GATE is not.  Or... maybe it is and we are specifically disabling it
somewhere?

>> I also don't have a TRM for twl6040 that matches up with what twl-regulator.c is
>> doing to enable the clk32kg output; it looks like it's adding it to
>> one of the power
>> modes, but I'm not entirely sure how the registers it's writing to work, so my
>> confidence level in being able to get it transplanted to a separate
>> driver has lowered
>> quite a bit.
>
> I'm not sure if I have twl6030 TRM either, but I can try to come up with a
> basic driver for the twl6030 clocks.

I actually found a TRM for it on google, but the register addresses in
the code don't
seem to match up with what's documented in the TRM I have.  Wouldn't
be the first
time that external docs didn't list all the registers....

---

     --Levi

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

end of thread, other threads:[~2013-03-13 16:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-07 20:37 omap4 pandaboard: clock input for TiWi module inactive Levi Pearson
2013-03-11  8:58 ` Roger Quadros
2013-03-11  9:51   ` Peter Ujfalusi
2013-03-12 16:33     ` Levi Pearson
2013-03-13  8:29       ` Peter Ujfalusi
2013-03-13 16:12         ` Levi Pearson

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