[U-Boot] [PATCH 2/5] arm64: mvebu: a8k: Add support for NAND clock get

[Stefan Roese]

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, kostap at marvell.com wrote:
>>> From: Konstantin Porotchkin <kostap at marvell.com>
>>>
>>> Implement mvebu_get_nand_clock call for A8K family.
>>> This function is used by PXA3XX NAND driver.
>>>
>>> Signed-off-by: Konstantin Porotchkin <kostap at marvell.com>
>>> Cc: Stefan Roese <sr at denx.de>
>>> Cc: Igal Liberman <igall at marvell.com>
>>> Cc: Nadav Haklai <nadavh at 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