On 01/30/2013 01:03 AM, Simon Baatz wrote:

On Tue, Jan 29, 2013 at 09:08:46PM +0100, Sebastian Hesselbarth wrote:

quoted

Well, if there is no device/driver using runit it should be safe to disable
it. If there is a driver using it, we need a clocks property for it. And
as you already stated, serial is using it. And you want serial in every
minimal kernel, don't you?

 From f479db4:

  Marvell engineers tell us:

  It seems that many units use the RUNIT clock.
  SPI, UART, NAND, TWSI, ...
  So it's not possible to clock gate it.

  Currently the SPI, NAND and TWSI driver will clk_prepaure_enable()
  this clk, but since we have no idea what ... is, and turning this clk
  off results in a hard lock, unconditionally enable runit.


Thus, if we know better than that, that's fine, but please don't tell
me that this is "blindly" enabling clocks.

Hmm, should have seen that comment ealier. Based on your/Andrew's statement
above using CLK_IGNORE_UNUSED on runit maybe is the best.

So I guess we take patch 2/2 and extend DT clock gating for .flags later?

So, we have a statement from Marvell not to gate it, we know that it
is needed for "...", can we please not gate it?

Ack.

quoted

quoted

quoted

quoted

3.8-rc5 + runit + Sebastians get smi clock patch (modified to use
legacy clock names):

DT: Boots, but no MAC
non-DT: Boots ok

This is the behavior originally found by Andrew. The clock patch
eliminates the lockup, but the hardware forgets its MAC address.

For the smi clock issue: DT is fixed by the smi clock patch, right?
non-DT should be fixed in kirkwood_clk_init() with an orion_clkdev_add()
alias for MV643XX_ETH_SHARED_NAME ".0" and ".1" respectively.

For the MAC address issue: non-DT doesn't need to be fixed, right?
DT clock gates should be fixed in kirkwood_legacy_clk_init which will
also save the clk_get_sys() call. We can easily remove it when mv643xx_eth
catches the MAC address from either local-mac-address or ATAG.

Fine with all of that. But: I am talking about 3.8 all the time. We
have three options for issues 2 and 3 from my point of view:

1. We do proper fixes in 3.8 for issues 2 and 3.

2. We fix this regression by not gating the clock in both the DT and
the non-DT case, preferably by using the correct clocks in
kirkwood_ge0[01]_init().  Additionally, Jason promises that all gets
well with the DT aware driver in 3.9.  ;-)

3. Do nothing. Simply accept that we broke modular Ethernet for DT
because some code relies on two pointers being set.  When we
introduced a new code path for DT, we forgot about these pointers.
Bad luck, the solution was not nice anyway and we will do proper
fixes in 3.9.

As should be clear by now, I think we should go for option 2.

Agreed, do you think all issues you are suffering will be solved with:

- [PATCH v2 2/2] clk: mvebu: Do not gate runit clock on Kirkwood
   (no lockup for minimal kernel configs)

- [PATCH] NET: mv643xx: get smi clock from device tree
   (no lockup for modular DT ethernet)

- Some patch that adds MV643XX_ETH_SHARED_NAME ".0" and ".1" clk aliases
   (no lockup for modular non-DT ethernet)

- Some patch that adds clk_prepare_enable to ge0/ge1 clocks to
   kirkwood_legacy_clk_init()
   (retain MAC address for modular DT ethernet)

I'll prepare the latter patches and post them.

Sebastian