ARM: Kirkwood: Fix crash when neither NAND nor SPI

Both NAND and SPI make use of the RUNIT clk. However, if neither NAND nor SPI is used in the system, RUNIT clock gets turned off, and the SoC hard locks. It appears something else in the SoC, which is not documented, is also using RUNIT. So prepare and enable RUNIT clock in kirkwood_clk_init().

---
 arch/arm/mach-kirkwood/common.c |    4 ++++
 1 file changed, 4 insertions(+)

index 25fb3fd..97f7d36 100644
@@ -238,6 +238,10 @@ void __init kirkwood_clk_init(void)
 	orion_clkdev_add("0", "pcie", pex0);
 	orion_clkdev_add("1", "pcie", pex1);
 	orion_clkdev_add(NULL, "kirkwood-i2s", audio);
+
+	/* Something other than SPI and NAND needs runit, so make sure
+	   it never gets turned off. */
+	clk_prepare_enable(runit);
 }
 
 /*****************************************************************************

1.7.10

This looks bogus, because if you really need to enable that clock all the time, we don't even need a way to turn it off.

Do you need a similar change with the 3.4 kernel before the big change to the clock framework? If not, I would guess that there is some other problem.

Arnd

This is a generic gated clock, so the ability to turn it off comes from the framework. The framework also turns off all clk which are unused in a late init, unless flagged otherwise.

3.4 did not have code to ensure it was not turned off.

However, all current systems have either SPI FLASH or NAND, since that is how these devices boot. So, under normal conditions, this clock is always used, and so never gets turned off as being unused. However, while porting boards to DT, a few of us have hit situations where we get the DT wrong, the spi/nand does not match, and the clock is unused and so turned off. This is particularly true as we implement DT support in the SPI and NAND drivers. This is not so easy to debug, since it just stops, rather than panic when the root file system is missing.

I also tried to save a little bit of power when the SPI device was inactive, turning the clock off between transactions. This resulted in the same lockup.

By preparing and enabling the clock, we reflect that there is probably something else inside RUNIT, and it makes it easier to debug DT bugs. The two alternatives would be to set the flag CLK_IGNORE_UNUSED, or just leave it the way it is, now that DT SPI and NAND support is mostly developed, and we are less likely to hit this situation.

Andrew

What if your filesystems are on devices other than flash (I use an SD card on my Sheevaplugs). In that case flash isn't used at all by the Linux kernel and could be safely turned off. I'm not saying that this is worth the hassle, just pointing out that flash may be unused by Linux on some systems.

Tixy

The current sheevaplug code sets up the nand driver and mtd partitions. That is sufficient to ensure the clock is kept on. It does not matter if a partitions are used or not.

I in fact use a similar system to you. I typically use kirkwood devices which are NAS boxes and i have the root file system on the disk and i tftpboot the kernel. The SPI flash is unused, but its still made available via mtd.

Problems will however come when somebody creates a box which really does boot from disk and skips NAND and SPI-FLASH in the DT description of the device. That is why i like claiming this clock in the core code, just to say there is something in the CPU core using it.

Andrew

Hi Andrew,

Am 08.06.2012 16:09, schrieb Andrew Lunn:

O.K. never tried that. I only tested with kirkwood_defconfig.

It is probably infer_hw_params().

The correct fix is going to be interesting. I assume that either ge0 or ge1 is sufficient to make the shared hardware work. So one option would be to register a clock with name MV643XX_ETH_SHARED_NAME, which is ge0. The shared probe code can turn it on/off during the shared probe function. When the specific interface is probed, the correct clock for that interface will be enabled, and it will also driver the shared hardware.

I will try this out tomorrow.

Do you have any other drivers built as modules? I was conservative and turned the clock on in the probe function, so i expect that most work as modules. However XOR has a similar sort of structure, so might also be broken...

Thanks
Andrew

...

I don't use XOR as module. However, in addition to the Ethernet one, I have the following modules that enable clocks:

mv_cesa
sata_mv
ehci_hcd

I did not encounter any problem with these so far (with the exception of data corruption in mv_cesa, which is completely unrelated (and is no problem of mv_cesa, but a problem of the current implementation of flush_kernel_dcache_page in my view)).

- Simon

I debugged this further. As i guessed, it is interesting, but not how i expected.

By enabling a clock while performing shared_probe, i stopped it locking up. I also needed to add a short delay in the individual interface probe, otherwise it locked up there.

However, this leads to another problem. It appears to forget its MAC address, as set by u-boot, while the clock is off :-(

I'm not sure how to handle this. One option is simply to not allow the two clocks to be turned off. The module can then be loaded/unloaded at any time, and the MAC address set by u-boot will still be available.

Or i could enable_prepare() the clock in kirkwood_ge00_init() and kirkwood_ge01_init() when the platform device structure is created. These functions indicate at some time in the future, the driver might be loaded to driver one or the other interface, so keeping the clk ticking to preserve the MAC address is a good idea. This will however not work for DT boards, since they won't in the future call these functions. We would need additional code to look into the DT during boot, see if there are nodes for the ethernet interfaces, and if so, enable_prepare() the clocks.

I'm tempted to go with this last solution, since it is more in keeping with the way it was done before i replaced the code with the generic clock framework.

Andrew