From mboxrd@z Thu Jan 1 00:00:00 1970 From: nicolas.ferre@atmel.com (Nicolas Ferre) Date: Wed, 13 Mar 2013 15:35:41 +0100 Subject: [PATCH 4/4] ARM: at91: introduce SAMA5 support In-Reply-To: References: <1362755638-20092-1-git-send-email-ludovic.desroches@atmel.com> <1362755638-20092-5-git-send-email-ludovic.desroches@atmel.com> <513a1756.e94aec0a.1f22.6694SMTPIN_ADDED_BROKEN@mx.google.com> <20130308185625.GF4590@game.jcrosoft.org> Message-ID: <51408EBD.6080101@atmel.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 03/08/2013 10:27 PM, Joachim Eastwood : > On 8 March 2013 19:56, Jean-Christophe PLAGNIOL-VILLARD > wrote: >> On 18:18 Fri 08 Mar , Joachim Eastwood wrote: >>> On 8 March 2013 17:52, Ludovic Desroches wrote: > > > >>>>> >>>>>> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c >>>>>> new file mode 100644 >>>>>> index 0000000..705305e >>>>>> --- /dev/null >>>>>> +++ b/arch/arm/mach-at91/board-dt-sama5.c >>>>>> @@ -0,0 +1,86 @@ >>>>>> +/* >>>>>> + * Setup code for SAMA5 Evaluation Kits with Device Tree support >>>>>> + * >>>>>> + * Copyright (C) 2013 Atmel, >>>>>> + * 2013 Ludovic Desroches >>>>>> + * >>>>>> + * Licensed under GPLv2 or later. >>>>>> + */ >>>>>> + >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> + >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> + >>>>>> +#include "at91_aic.h" >>>>>> +#include "generic.h" >>>>>> + >>>>>> + >>>>>> +static const struct of_device_id irq_of_match[] __initconst = { >>>>>> + >>>>>> + { .compatible = "atmel,sama5d3-aic", .data = at91_aic5_of_init }, >>>>>> + { /*sentinel*/ } >>>>>> +}; >>>>>> + >>>>>> +static void __init at91_dt_init_irq(void) >>>>>> +{ >>>>>> + of_irq_init(irq_of_match); >>>>>> +} >>>>>> + >>>>>> +static int ksz9021rn_phy_fixup(struct phy_device *phy) >>>>>> +{ >>>>>> + int value; >>>>>> + >>>>>> +#define GMII_RCCPSR 260 >>>>>> +#define GMII_RRDPSR 261 >>>>>> +#define GMII_ERCR 11 >>>>>> +#define GMII_ERDWR 12 >>>>>> + >>>>>> + /* Set delay values */ >>>>>> + value = GMII_RCCPSR | 0x8000; >>>>>> + phy_write(phy, GMII_ERCR, value); >>>>>> + value = 0xF2F4; >>>>>> + phy_write(phy, GMII_ERDWR, value); >>>>>> + value = GMII_RRDPSR | 0x8000; >>>>>> + phy_write(phy, GMII_ERCR, value); >>>>>> + value = 0x2222; >>>>>> + phy_write(phy, GMII_ERDWR, value); >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static void __init sama5_dt_device_init(void) >>>>>> +{ >>>>>> + if (of_machine_is_compatible("atmel,sama5d3xcm")) >>>>>> + phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK, >>>>>> + ksz9021rn_phy_fixup); >>>>>> + >>>>>> + of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); >>>>>> +} >>>>>> + >>>>>> +static const char *sama5_dt_board_compat[] __initdata = { >>>>>> + "atmel,sama5", >>>>>> + NULL >>>>>> +}; >>>>>> + >>>>>> +DT_MACHINE_START(sama5_dt, "Atmel SAMA5 (Device Tree)") >>>>>> + /* Maintainer: Atmel */ >>>>>> + .init_time = at91sam926x_pit_init, >>>>>> + .map_io = at91_map_io, >>>>>> + .handle_irq = at91_aic5_handle_irq, >>>>>> + .init_early = at91_dt_initialize, >>>>>> + .init_irq = at91_dt_init_irq, >>>>>> + .init_machine = sama5_dt_device_init, >>>>>> + .dt_compat = sama5_dt_board_compat, >>>>>> +MACHINE_END >>>>> >>>>> Do we really need board-dt-sama5.c? >>>>> Now that we have both irqchip_init and clocksource_of_init it >>>>> shouldn't be necessary with more than one board-dt.c. >>>>> >>>> >>>> At the beginning, I had the same point of view but we have a new >>>> architecture, we can't build a single kernel image for both AT91SAM9 and >>>> SAMA5. So why not splitting it? >>> >>> It does add more lines to mach-at91 and I don't see a reason to have >>> two near identical board-dt files. board-dt could be used for SAMA5 >>> even without irqchip_init and clocksource_of_init so what's the point >>> in splitting it? >>> >>> When the at91 clocksource and irqchip driver are converted/moved we >>> could support all AT91 SoC from RM9200 to SAMA5 in one board-dt. I >>> don't see the reason for 3 board-dt files or even 2. >>> >>> That we can't build a kernel that supports both ARMv4-5 and ARMv7 >>> doesn't make a difference. >> my answer is no I do not want to if compatible_is on the c code so no >> different files > > But you have this already on the above code: >>>>>> + if (of_machine_is_compatible("atmel,sama5d3xcm")) > > I don't see the point to have different board-dt files. > If a board turns out to need lots of specific C code to work I think > it's fine to have it in its own board file. But right now you are just > duplicating code unnecessary. Well, in fact it turns out that we already have quite a few DT tests in our sama5d3 kernel (github/linux4sam) because all subsystem are not converted to DT yet. If we put even an extract of this in a common board file it will soon become a mess. I also do understand the need for limiting the testing of several compatibility strings before booting... Moreover, as discussed with Jean-Christophe, if we join board-dt-rm9200.c and the sam9 one, we will need to compile at91rm9200_time.c ST driver, even for sam9s and sama5s which is kind of useless. Even if I hate duplicating code, I tend to prefer the "multi-file" solution as it adds a tiny overhead but leads to a clearer result. Ludovic, can you please re-spin the whole series with updated patches and the removal of patch #3 which I plan to queue for 3.9 fixes. Best regards, -- Nicolas Ferre