[PATCH 6/8] arm: mach-armada: add support for Armada XP board with device tree

[Ben Dooks]

On 15/05/12 14:31, Rob Herring wrote:
> On 05/15/2012 04:58 AM, Ben Dooks wrote:
>> On 15/05/12 09:54, Thomas Petazzoni wrote:
>>> From: Gregory CLEMENT<gregory.clement at free-electrons.com>
>>>
>>> Signed-off-by: Gregory CLEMENT<gregory.clement at free-electrons.com>
>>> Signed-off-by: Thomas Petazzoni<thomas.petazzoni at free-electrons.com>
>>> Signed-off-by: Lior Amsalem<alior at marvell.com>
>>> ---
>>>    arch/arm/boot/dts/axp-db.dts  |   40
>>> ++++++++++++++++++++++++++++++++++++++++
>>>    arch/arm/mach-armada/Kconfig  |    6 ++++++
>>>    arch/arm/mach-armada/Makefile |    2 +-
>>>    arch/arm/mach-armada/axp-dt.c |   41
>>> +++++++++++++++++++++++++++++++++++++++++
>>>    4 files changed, 88 insertions(+), 1 deletion(-)
>>>    create mode 100644 arch/arm/boot/dts/axp-db.dts
>>>    create mode 100644 arch/arm/mach-armada/axp-dt.c
>>>
>>> diff --git a/arch/arm/boot/dts/axp-db.dts b/arch/arm/boot/dts/axp-db.dts
>>> new file mode 100644
>>> index 0000000..916be09
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/axp-db.dts
>>
>> see previous comments.
>>
>>> diff --git a/arch/arm/mach-armada/Kconfig b/arch/arm/mach-armada/Kconfig
>>> index 7995813..2903096 100644
>>> --- a/arch/arm/mach-armada/Kconfig
>>> +++ b/arch/arm/mach-armada/Kconfig
>>> @@ -8,6 +8,12 @@ config MACH_ARMADA_370_DT
>>>          Say 'Y' here if you want your kernel to support
>>>          boards based on Marvell Armada 370 with device tree.
>>>
>>> +config MACH_ARMADA_XP_DT
>>> +    bool "Marvell Armada XP boards with device-tree support"
>>> +    help
>>> +      Say 'Y' here if you want your kernel to support
>>> +      boards based on Marvell Armada XP with device tree.
>>> +
>>>    endmenu
>>>
>>>    endif
>>> diff --git a/arch/arm/mach-armada/Makefile
>>> b/arch/arm/mach-armada/Makefile
>>> index ac39fae..7a3c438 100644
>>> --- a/arch/arm/mach-armada/Makefile
>>> +++ b/arch/arm/mach-armada/Makefile
>>> @@ -1,3 +1,3 @@
>>>    obj-y += common.o irq.o time.o
>>>    obj-$(CONFIG_MACH_ARMADA_370_DT) += a370-dt.o
>>> -
>>> +obj-$(CONFIG_MACH_ARMADA_XP_DT)     += axp-dt.o
>>> diff --git a/arch/arm/mach-armada/axp-dt.c
>>> b/arch/arm/mach-armada/axp-dt.c
>>> new file mode 100644
>>> index 0000000..2c13d4e
>>> --- /dev/null
>>> +++ b/arch/arm/mach-armada/axp-dt.c
>>> @@ -0,0 +1,41 @@
>>> +/*
>>> + * Device Tree support for Armada XP platforms.
>>> + *
>>> + * Copyright (C) 2012 Marvell
>>> + *
>>> + * Lior Amsalem<alior at marvell.com>
>>> + * Gregory CLEMENT<gregory.clement at free-electrons.com>
>>> + * Thomas Petazzoni<thomas.petazzoni at free-electrons.com>
>>> + *
>>> + * This file is licensed under the terms of the GNU General Public
>>> + * License version 2.  This program is licensed "as is" without any
>>> + * warranty of any kind, whether express or implied.
>>> + */
>>> +
>>> +#include<linux/kernel.h>
>>> +#include<linux/init.h>
>>> +#include<linux/of_platform.h>
>>> +#include<asm/mach-types.h>
>>> +#include<asm/mach/arch.h>
>>> +#include<mach/armada.h>
>>> +#include "common.h"
>>> +
>>> +static void __init axp_dt_init(void)
>>> +{
>>> +    of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>>> +}
>>> +
>>> +static const char * const axp_dt_board_dt_compat[] = {
>>> +    "marvell,axp-db",
>>> +    NULL,
>>
>> This is where the device support becomes confusing.
>>
>> I would strongly advise on using "mrvl,mv78230" as the binding
>> name. It isn't as if you can't have multiple names associated
>> with one binding.
>
> The name here should uniquely identify the board, not the chip. If you
> additionally want to have the chip name, then that should be after the
> board name. However, "marvell,armada" is too generic to be useful.
>
> marvell is the documented vendor string, not mrvl.

I could swear I read it as mrvl in the documentation for device-tree

>> Also, given how close they are, do we really need separate machine
>> support files?
>
> There does need to be justification why they need to be separate.

I'd say for the "core" support that they could all be handled by this
case catching the "marvell,mv...." and initialising all the core bits
such as timer, io, irq, etc.

Since the two board support files are identical, except for the names
they print, I'd say this is the job of one file. It can always be split
later.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius