[U-Boot] [PATCH v8 1/9] mips: add base support for QCA/Atheros ath79 SOCs

[Daniel Schwierzeck]

2016-03-17 13:20 GMT+01:00 Marek Vasut <marex at denx.de>:
> On 03/17/2016 05:02 AM, Wills Wang wrote:
>>
>>
>> On Thursday, March 17, 2016 11:44 AM, Marek Vasut wrote:
>>> On 03/17/2016 04:39 AM, Wills Wang wrote:
>>>>
>>>> On Thursday, March 17, 2016 05:35 AM, Marek Vasut wrote:
>>>>> On 03/16/2016 09:59 AM, Wills Wang wrote:
>>>>>> This patch add some common code for QCA/Atheros ath79 SOCs such as
>>>>>> DDR tuning, chip reset and CPU detection.
>>>>>>
>>>>>> Signed-off-by: Wills Wang <wills.wang at live.com>
>>>>>> ---
>>>>>>
>>>>>> Changes in v8:
>>>>>> - Use setbits_be32
>>>>>> - Use lookup-table instead of big switch statement for CPU detection
>>>>>>
>>>>> Good stuff, minor nits below.
>>> [...]
>>>
>>>>>> +phys_size_t initdram(int board_type)
>>>>>> +{
>>>>>> +    ddr_tap_tuning();
>>>>> Is the DDR tap tuning needed on all AR7xxx/AR9xxx systems ?
>>>> Yes, it's for optimizing DDR timing according to hardware.
>>>> Sometimes, the hard code value is not ideal.
>>> AR934x doesn't seem to need this.
>> I seem the following code in u-boot from Atheros LSDK:
>> #ifdef CONFIG_AP123
>>     ath_ddr_tap_cal();
>
> Where can I get the LSDK ? I'd like to take a look and add it into my
> ar934x support patch.
>
>>>>>> +    return get_ram_size((void *)KSEG1, SZ_256M);
>>>>>> +}
>>>>>> diff --git a/arch/mips/mach-ath79/include/mach/ar71xx_regs.h
>>>>>> b/arch/mips/mach-ath79/include/mach/ar71xx_regs.h
>>>>>> new file mode 100644
>>>>>> index 0000000..893dedc
>>>>>> --- /dev/null
>>>>>> +++ b/arch/mips/mach-ath79/include/mach/ar71xx_regs.h
>>>>>> @@ -0,0 +1,1184 @@
>>>>>> +/*
>>>>>> + * Atheros AR71XX/AR724X/AR913X SoC register definitions
>>>>>> + *
>>>>>> + * Copyright (C) 2015-2016 Wills Wang <wills.wang at live.com>
>>>>>> + * Copyright (C) 2010-2011 Jaiganesh Narayanan
>>>>>> <jnarayanan at atheros.com>
>>>>>> + * Copyright (C) 2008-2010 Gabor Juhos <juhosg at openwrt.org>
>>>>>> + * Copyright (C) 2008 Imre Kaloz <kaloz at openwrt.org>
>>>>>> + *
>>>>>> + * SPDX-License-Identifier:     GPL-2.0+
>>>>>> + */
>>>>>> +
>>>>>> +#ifndef __ASM_MACH_AR71XX_REGS_H
>>>>>> +#define __ASM_MACH_AR71XX_REGS_H
>>>>>> +
>>>>>> +#ifndef __ASSEMBLY__
>>>>>> +#include <linux/bitops.h>
>>>>>> +#else
>>>>>> +#ifndef BIT
>>>>>> +#define BIT(nr)                 (1 << (nr))
>>>>> This should really go into some common header.
>>>> This header is also included by some assembly code,
>>>> but the BIT macro in linux/bitops.h isn't compatiable with assembler.
>>> Because of the 1UL in it ?
>> Yes, there are also a few other C keywords and syntax, such as in-line
>> function.
>
> Hmmmmm, it pains me to see such duplication, but I have no better idea
> how to deal with it without introducing ifdefs.
>

actually such register definitions files are obsolete because register
base adresses and sizes are taken from device-tree. The only
exceptions are early startup or assembly code, but then you only need
to add the definitions whose are needed by a driver. Thus I suggest
you remove all the crap from this file you don't need.

-- 
- Daniel