Hi Kosta,

On 30.03.2017 17:46, Konstantin Porotchkin wrote:
On 03/30/2017 04:15 PM, Stefan Roese wrote:
On 28.03.2017 17:16, kos...@marvell.com wrote:
From: Konstantin Porotchkin <kos...@marvell.com>

Implement mvebu_get_nand_clock call for A8K family.
This function is used by PXA3XX NAND driver.

Signed-off-by: Konstantin Porotchkin <kos...@marvell.com>
Cc: Stefan Roese <s...@denx.de>
Cc: Igal Liberman <ig...@marvell.com>
Cc: Nadav Haklai <nad...@marvell.com>
---
 arch/arm/mach-mvebu/armada8k/cpu.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/mach-mvebu/armada8k/cpu.c
b/arch/arm/mach-mvebu/armada8k/cpu.c
index 2325e9a..e18ca6b 100644
--- a/arch/arm/mach-mvebu/armada8k/cpu.c
+++ b/arch/arm/mach-mvebu/armada8k/cpu.c
@@ -110,3 +110,19 @@ void reset_cpu(ulong ignored)
     reg &= ~(1 << RFU_SW_RESET_OFFSET);
     writel(reg, RFU_GLOBAL_SW_RST);
 }
+
+#ifdef CONFIG_NAND_PXA3XX

Do we really need to have this code conditionally compiled here?
I can remove it, just wanted not to increase the code size if not
needed. Do you think it is excessive?

Yes, please remove it. These platforms are not that size constraint
as some SPL ones are. The pros for not adding more #ifdef's
overweight the small code size increase - at least for my taste.

+/* Return NAND clock in Hz */
+u32 mvebu_get_nand_clock(void)
+{
+    unsigned long NAND_FLASH_CLK_CTRL = 0xF2440700UL;

I know that some of this code is historically done this way. But
with DT available now, isn't it possible to at least get the base
address of such registers from the DT instead of hardcoding it?
I see what you saying and I agree it is not as elegant as it could be.
The only problem is that the NAND clock register is not part of the SoC
NAND unit, so the IO base taken from DTS entry for NAND device is not
really useful here. NAND unit and its clock configuration register are
only sharing the CP0 base - 0xF2000000.

Right. I wouldn't have expected that this register is located
in the NAND controller space - more likely some clocking controller
/ infrastructure. I'm okay with this patch for now, but perhaps
you could add a small "ToDo" comment here, that this should be
converted at some time, once a mvebu / a7k/8K DM clock driver
is available in U-Boot (see drivers/clk)?

Thanks,
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to