From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Subject: Re: [PATCH v2] ARM: SPEAr600: Add device-tree support to SPEAr600 boards Date: Wed, 14 Mar 2012 20:01:58 +0100 Message-ID: <201203142001.58762.sr@denx.de> References: <1331737232-25477-1-git-send-email-sr@denx.de> <201203141749.05899.sr@denx.de> <20120314172435.GP18320@game.jcrosoft.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120314172435.GP18320-RQcB7r2h9QmfDR2tN2SG5Ni2O/JbrIOy@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org To: Jean-Christophe PLAGNIOL-VILLARD Cc: Viresh Kumar , devicetree-discuss-mnsaURCQ41sdnm+yROfE0A@public.gmane.org, spear-devel-nkJGhpqTU55BDgjK7y7TUQ@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org On Wednesday 14 March 2012 18:24:35 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > diff --git a/arch/arm/boot/dts/spear600-evb.dts > > > > b/arch/arm/boot/dts/spear600-evb.dts new file mode 100644 > > > > index 0000000..cbfda8d > > > > --- /dev/null > > > > +++ b/arch/arm/boot/dts/spear600-evb.dts > > > > @@ -0,0 +1,30 @@ > > > > +/* > > > > + * Copyright 2012 Stefan Roese > > > > + * > > > > + * The code contained herein is licensed under the GNU General > > > > Public + * License. You may obtain a copy of the GNU General Public > > > > License + * Version 2 or later at the following locations: > > > > + * > > > > + * http://www.opensource.org/licenses/gpl-license.html > > > > + * http://www.gnu.org/copyleft/gpl.html > > > > + */ > > > > + > > > > +/dts-v1/; > > > > +/include/ "spear600.dtsi" > > > > + > > > > +/ { > > > > + model = "ST SPEAr600 Evaluation Board"; > > > > + compatible = "st,spear600-evb", "st,spear600"; > > > > + #address-cells = <1>; > > > > + #size-cells = <1>; > > > > > > please put the mem size here > > > > Might I ask why? Other *.dtsi files don't have it either. > > do not expect the bootloader to put it. I t's not mandatory But the memory node it is already present in spear600.dtsi (see below). Do we really need to duplicate this in the board specific dts file? > > > > + > > > > + soc { > > > > + gmac0: ethernet@e0800000 { > > > > + phy-mode = "gmii"; > > > > + }; > > > > + > > > > + i2c@d0200000 { > > > > + clock-frequency = <400000>; > > > > + }; > > > > + }; > > > > +}; > > > > diff --git a/arch/arm/boot/dts/spear600.dtsi > > > > b/arch/arm/boot/dts/spear600.dtsi new file mode 100644 > > > > index 0000000..e45e58f > > > > --- /dev/null > > > > +++ b/arch/arm/boot/dts/spear600.dtsi > > > > @@ -0,0 +1,162 @@ > > > > +/* > > > > + * Copyright 2012 Stefan Roese > > > > + * > > > > + * The code contained herein is licensed under the GNU General > > > > Public + * License. You may obtain a copy of the GNU General Public > > > > License + * Version 2 or later at the following locations: > > > > + * > > > > + * http://www.opensource.org/licenses/gpl-license.html > > > > + * http://www.gnu.org/copyleft/gpl.html > > > > + */ > > > > + > > > > +/include/ "skeleton.dtsi" > > > > + > > > > +/ { > > > > + compatible = "st,spear600"; > > > > + > > > > + cpus { > > > > + cpu@0 { > > > > + compatible = "arm,arm926ejs"; > > > > + }; > > > > + }; > > > > + > > > > + memory { > > > > + device_type = "memory"; > > > > + reg = <0 0>; /* Filled by U-Boot */ > > > > > > please put the max mem of the SoC here > > > > Why? It's always updated by the bootloader. And a max. mem size (when not > > updated to the correct mem size by the bootloader) would crash the board > > if it is equipped with less memory. > > yes but we descripbe the soc here and on the dts you reduce it > Never expect the bootloader to update it. Without update to real size we're doomed. > It's a possibility not mandatory Okay, I'll add the max. size here. > > > > + }; > > > > + > > > > + soc { > > > > > > please put the name of the bus not soc > > > > Okay. > > > > > > + #address-cells = <1>; > > > > + #size-cells = <1>; > > > > + compatible = "simple-bus"; > > > > + ranges; > > > > + > > > > + vic0: interrupt-controller@f1100000 { > > > > + compatible = "arm,pl190-vic"; > > > > + interrupt-controller; > > > > + reg = <0xf1100000 0x1000>; > > > > + #interrupt-cells = <1>; > > > > + }; > > > > + > > > > + vic1: interrupt-controller@f1000000 { > > > > + compatible = "arm,pl190-vic"; > > > > + interrupt-controller; > > > > + reg = <0xf1000000 0x1000>; > > > > + #interrupt-cells = <1>; > > > > + }; > > > > + > > > > + serial@d0000000 { > > > > + compatible = "arm,pl011", "arm,primecell"; > > > > + reg = <0xd0000000 0x1000>; > > > > + interrupt-parent = <&vic0>; > > > > + interrupts = <24>; > > > > + }; > > > > + > > > > + serial@d0080000 { > > > > + compatible = "arm,pl011", "arm,primecell"; > > > > + reg = <0xd0080000 0x1000>; > > > > + interrupt-parent = <&vic0>; > > > > + interrupts = <25>; > > > > + }; > > > > + > > > > + /* local/cpu GPIO */ > > > > + gpio0: gpio@f0100000 { > > > > + #gpio-cells = <2>; > > > > + compatible = "arm,pl061", "arm,primecell"; > > > > + gpio-controller; > > > > + reg = <0xf0100000 0x1000>; > > > > + interrupt-parent = <&vic0>; > > > > + interrupts = <18>; > > > > + }; > > > > + > > > > + /* basic GPIO */ > > > > + gpio1: gpio@fc980000 { > > > > + #gpio-cells = <2>; > > > > + compatible = "arm,pl061", "arm,primecell"; > > > > + gpio-controller; > > > > + reg = <0xfc980000 0x1000>; > > > > + interrupt-parent = <&vic1>; > > > > + interrupts = <19>; > > > > + }; > > > > + > > > > + /* appl GPIO */ > > > > + gpio2: gpio@d8100000 { > > > > + #gpio-cells = <2>; > > > > + compatible = "arm,pl061", "arm,primecell"; > > > > + gpio-controller; > > > > + reg = <0xd8100000 0x1000>; > > > > + interrupt-parent = <&vic1>; > > > > + interrupts = <4>; > > > > + }; > > > > + > > > > + gmac0: ethernet@e0800000 { > > > > + compatible = "st,spear600-gmac"; > > > > + reg = <0xe0800000 0x8000>; > > > > + interrupt-parent = <&vic1>; > > > > + interrupts = <24 23>; > > > > + interrupt-names = "macirq", "eth_wake_irq"; > > > > + mac-address = [000000000000]; /* Filled in by U-Boot > > > > */ > > drop this no dummy data Okay. > > > > + }; > > > > + > > > > + i2c@d0200000 { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + compatible = "snps,designware-i2c"; > > > > + reg = <0xd0200000 0x1000>; > > > > + interrupt-parent = <&vic0>; > > > > + interrupts = <28>; > > > > + }; > > > > + > > > > + fsmc0: flash@d1800000 { > > > > + status = "disabled"; > > > > + compatible = "st,spear600-fsmc-nand"; > > > > + #address-cells = <1>; > > > > + #size-cells = <1>; > > > > + reg = <0xd1800000 0x1000 /* FSMC Register */ > > > > + 0xd2000000 0x4000>; /* NAND Base */ > > > > + reg-names = "fsmc_regs", "nand_data"; > > > > + }; > > > > + > > > > + smi0: flash@fc000000 { > > > > + status = "disabled"; > > > > + compatible = "st,spear600-smi"; > > > > + #address-cells = <1>; > > > > + #size-cells = <1>; > > > > + reg = <0xfc000000 0x1000>; > > > > + interrupt-parent = <&vic1>; > > > > + interrupts = <12>; > > > > + }; > > > > + > > > > + ehci@e1800000 { > > > > + status = "disabled"; > > > > + compatible = "st,spear600-ehci", "usb-ehci"; > > > > + reg = <0xe1800000 0x1000>; > > > > + interrupt-parent = <&vic1>; > > > > + interrupts = <27>; > > > > + }; > > > > + > > > > + ehci@e2000000 { > > > > + status = "disabled"; > > > > + compatible = "st,spear600-ehci", "usb-ehci"; > > > > + reg = <0xe2000000 0x1000>; > > > > + interrupt-parent = <&vic1>; > > > > + interrupts = <29>; > > > > + }; > > > > + > > > > + ohci@e1900000 { > > > > + status = "disabled"; > > > > + compatible = "st,spear600-ohci", "usb-ohci"; > > > > + reg = <0xe1900000 0x1000>; > > > > + interrupt-parent = <&vic1>; > > > > + interrupts = <26>; > > > > + }; > > > > + > > > > + ohci@e2100000 { > > > > + status = "disabled"; > > > > + compatible = "st,spear600-ohci", "usb-ohci"; > > > > + reg = <0xe2100000 0x1000>; > > > > + interrupt-parent = <&vic1>; > > > > + interrupts = <28>; > > > > + }; > > > > > > all of this need a Doc for the ST bindings > > > > Sure. I only provided those nodes on Arnd's special request. They are > > currently disabled. And documentation will follow with the DT device > > driver patches. > > no update it after Hmmm. I'm sorry, but I fail to understand you here. Please explain. > > > > + }; > > > > +}; > > > > diff --git a/arch/arm/mach-spear6xx/Kconfig > > > > b/arch/arm/mach-spear6xx/Kconfig index ff4ae5b..7777f72 100644 > > > > --- a/arch/arm/mach-spear6xx/Kconfig > > > > +++ b/arch/arm/mach-spear6xx/Kconfig > > > > @@ -11,6 +11,13 @@ config BOARD_SPEAR600_EVB > > > > > > > > help > > > > > > > > Supports ST SPEAr600 Evaluation Board > > > > > > > > +config BOARD_SPEAR600_DT > > > > + bool "SPEAr600 generic board configured via device-tree" > > > > + select MACH_SPEAR600 > > > > + select USE_OF > > > > + help > > > > + Supports ST SPEAr600 boards configured via the device-tree > > > > + > > > > > > > > endmenu > > > > > > > > config MACH_SPEAR600 > > > > > > > > diff --git a/arch/arm/mach-spear6xx/Makefile > > > > b/arch/arm/mach-spear6xx/Makefile index cc1a4d8..6f87c3a 100644 > > > > --- a/arch/arm/mach-spear6xx/Makefile > > > > +++ b/arch/arm/mach-spear6xx/Makefile > > > > @@ -5,8 +5,5 @@ > > > > > > > > # common files > > > > obj-y += clock.o spear6xx.o > > > > > > > > -# spear600 specific files > > > > -obj-$(CONFIG_MACH_SPEAR600) += spear600.o > > > > - > > > > > > > > # spear600 boards files > > > > obj-$(CONFIG_BOARD_SPEAR600_EVB) += spear600_evb.o > > > > > > > > diff --git a/arch/arm/mach-spear6xx/spear600.c > > > > b/arch/arm/mach-spear6xx/spear600.c deleted file mode 100644 > > > > index d0e6eea..0000000 > > > > --- a/arch/arm/mach-spear6xx/spear600.c > > > > +++ /dev/null > > > > @@ -1,25 +0,0 @@ > > > > -/* > > > > - * arch/arm/mach-spear6xx/spear600.c > > > > - * > > > > - * SPEAr600 machine source file > > > > - * > > > > - * Copyright (C) 2009 ST Microelectronics > > > > - * Rajeev Kumar > > > > - * > > > > - * 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 > > > > -#include > > > > -#include > > > > -#include > > > > - > > > > -/* Add spear600 specific devices here */ > > > > - > > > > -void __init spear600_init(void) > > > > -{ > > > > - /* call spear6xx family common init function */ > > > > - spear6xx_init(); > > > > -} > > > > diff --git a/arch/arm/mach-spear6xx/spear600_evb.c > > > > b/arch/arm/mach-spear6xx/spear600_evb.c index c6e4254..c4949aa 100644 > > > > --- a/arch/arm/mach-spear6xx/spear600_evb.c > > > > +++ b/arch/arm/mach-spear6xx/spear600_evb.c > > > > @@ -32,9 +32,6 @@ static void __init spear600_evb_init(void) > > > > > > > > { > > > > > > > > unsigned int i; > > > > > > > > - /* call spear600 machine init function */ > > > > - spear600_init(); > > > > - > > > > > > > > /* Add Platform Devices */ > > > > platform_add_devices(plat_devs, ARRAY_SIZE(plat_devs)); > > > > > > > > diff --git a/arch/arm/mach-spear6xx/spear6xx.c > > > > b/arch/arm/mach-spear6xx/spear6xx.c index e0f6628..aca20f0 100644 > > > > --- a/arch/arm/mach-spear6xx/spear6xx.c > > > > +++ b/arch/arm/mach-spear6xx/spear6xx.c > > > > @@ -6,6 +6,8 @@ > > > > > > > > * Copyright (C) 2009 ST Microelectronics > > > > * Rajeev Kumar > > > > * > > > > > > > > + * Copyright 2012 Stefan Roese > > > > + * > > > > > > > > * 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. > > > > > > > > @@ -15,6 +17,10 @@ > > > > > > > > #include > > > > #include > > > > #include > > > > > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > > > > > #include > > > > #include > > > > #include > > > > > > > > @@ -99,12 +105,6 @@ struct amba_device gpio_device[] = { > > > > > > > > } > > > > > > > > }; > > > > > > > > -/* This will add devices, and do machine specific tasks */ > > > > -void __init spear6xx_init(void) > > > > -{ > > > > - /* nothing to do for now */ > > > > -} > > > > - > > > > > > > > /* This will initialize vic */ > > > > void __init spear6xx_init_irq(void) > > > > { > > > > > > > > @@ -181,3 +181,60 @@ static void __init spear6xx_timer_init(void) > > > > > > > > struct sys_timer spear6xx_timer = { > > > > > > > > .init = spear6xx_timer_init, > > > > > > > > }; > > > > > > > > + > > > > +/* > > > > + * Devicetree specific init structs/functions: > > > > + * Once the SPEAr600 devicetree support has matured, we can remove > > > > + * spear600_evb.c and the non-devicetree support above in this file. > > > > + */ > > > > + > > > > +/* > > > > + * Add auxdata so that clock name matching doesn't fail in device > > > > + * drivers > > > > + */ > > > > +struct of_dev_auxdata spear600_auxdata_lookup[] __initdata = { > > > > + OF_DEV_AUXDATA("arm,pl011", SPEAR6XX_ICM1_UART0_BASE, > > > > + "uart0", NULL), > > > > + OF_DEV_AUXDATA("arm,pl011", SPEAR6XX_ICM1_UART1_BASE, > > > > + "uart1", NULL), > > > > + OF_DEV_AUXDATA("arm,pl061", SPEAR6XX_CPU_GPIO_BASE, > > > > + "gpio0", NULL), > > > > + OF_DEV_AUXDATA("arm,pl061", SPEAR6XX_ICM3_GPIO_BASE, > > > > + "gpio1", NULL), > > > > + OF_DEV_AUXDATA("arm,pl061", SPEAR6XX_ICM2_GPIO_BASE, > > > > + "gpio2", NULL), > > > > + OF_DEV_AUXDATA("snps,designware-i2c", SPEAR6XX_ICM1_I2C_BASE, > > > > + "i2c_designware.0", NULL), > > > > > > update the clkdev no more OF_DEV_AUXDATA > > > > That will break non-DT boards. We can change it in clkdev later, once DT > > support has stabalized for SPEAr600. And remove all this then. > > it will not just add new entry > > as done on AT91 as example and requested for imx recently too Again, I fail to understand you. Sorry. Please describe in a complete sentence what exactly you mean. Thanks, Stefan