On Mon, Apr 09, 2012 at 02:20:26PM +0100, Jamie Lentin wrote: > On Fri, 6 Apr 2012, Grant Likely wrote: > > >On Tue, 27 Mar 2012 22:54:11 +0100, Jamie Lentin <[email protected]> wrote: > >>Add support for the DNS-320 and DNS-325. Describe as much as currently > >>possible > >>in the devicetree files, create a board-dnskw.c for everything else. > >> > >>Use IEEE-compliant "okay", rather than "ok" > >> > >>Acked-by: Arnd Bergmann <[email protected]> > >>Acked-by: Jason Cooper <[email protected]> > >>Signed-off-by: Jamie Lentin <[email protected]> > >>--- > >> arch/arm/boot/dts/kirkwood-dns320.dts | 29 +++ > >> arch/arm/boot/dts/kirkwood-dns325.dts | 24 +++ > >> arch/arm/mach-kirkwood/Kconfig | 23 +++ > >> arch/arm/mach-kirkwood/Makefile | 1 + > >> arch/arm/mach-kirkwood/Makefile.boot | 2 + > >> arch/arm/mach-kirkwood/board-dnskw.c | 306 > >> +++++++++++++++++++++++++++++++++ > >> arch/arm/mach-kirkwood/board-dt.c | 5 + > >> arch/arm/mach-kirkwood/common.h | 6 + > >> 8 files changed, 396 insertions(+), 0 deletions(-) > >> create mode 100644 arch/arm/boot/dts/kirkwood-dns320.dts > >> create mode 100644 arch/arm/boot/dts/kirkwood-dns325.dts > >> create mode 100644 arch/arm/mach-kirkwood/board-dnskw.c > >> > >>diff --git a/arch/arm/boot/dts/kirkwood-dns320.dts > >>b/arch/arm/boot/dts/kirkwood-dns320.dts > >>new file mode 100644 > >>index 0000000..78c834f > >>--- /dev/null > >>+++ b/arch/arm/boot/dts/kirkwood-dns320.dts > >>@@ -0,0 +1,29 @@ > >>+/dts-v1/; > >>+ > >>+/include/ "kirkwood.dtsi" > >>+ > >>+/ { > >>+ model = "D-Link DNS-320 NAS (Rev A1)"; > >>+ compatible = "dlink,dns-320-a1", "dlink,dns-320", "dlink,dns-kirkwood", > >>"mrvl,kirkwood-88f6281", "mrvl,kirkwood"; > >>+ > >>+ memory { > >>+ device_type = "memory"; > >>+ reg = <0x00000000 0x8000000>; > >>+ }; > >>+ > >>+ chosen { > >>+ bootargs = "console=ttyS0,115200n8 earlyprintk"; > >>+ }; > >>+ > >>+ ocp@f1000000 { > >>+ serial@12000 { > >>+ clock-frequency = <166666667>; > >>+ status = "okay"; > >>+ }; > >>+ > >>+ serial@12100 { > >>+ clock-frequency = <166666667>; > >>+ status = "okay"; > >>+ }; > >>+ }; > >>+}; > >>diff --git a/arch/arm/boot/dts/kirkwood-dns325.dts > >>b/arch/arm/boot/dts/kirkwood-dns325.dts > >>new file mode 100644 > >>index 0000000..23241ab > >>--- /dev/null > >>+++ b/arch/arm/boot/dts/kirkwood-dns325.dts > >>@@ -0,0 +1,24 @@ > >>+/dts-v1/; > >>+ > >>+/include/ "kirkwood.dtsi" > >>+ > >>+/ { > >>+ model = "D-Link DNS-325 NAS (Rev A1)"; > >>+ compatible = "dlink,dns-325-a1", "dlink,dns-325", "dlink,dns-kirkwood", > >>"mrvl,kirkwood-88f6281", "mrvl,kirkwood"; > >>+ > >>+ memory { > >>+ device_type = "memory"; > >>+ reg = <0x00000000 0x10000000>; > >>+ }; > >>+ > >>+ chosen { > >>+ bootargs = "console=ttyS0,115200n8 earlyprintk"; > >>+ }; > >>+ > >>+ ocp@f1000000 { > >>+ serial@12000 { > >>+ clock-frequency = <200000000>; > >>+ status = "okay"; > >>+ }; > >>+ }; > >>+}; > >>diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig > >>index 90ceab7..d594b6e 100644 > >>--- a/arch/arm/mach-kirkwood/Kconfig > >>+++ b/arch/arm/mach-kirkwood/Kconfig > >>@@ -58,6 +58,29 @@ config MACH_DREAMPLUG_DT > >> Say 'Y' here if you want your kernel to support the > >> Marvell DreamPlug (Flattened Device Tree). > >> > >>+config MACH_DNSKW_DT > >>+ bool > >>+ > >>+config MACH_DNS320_DT > >>+ bool "D-Link DNS-320 (Flattened Device Tree)" > >>+ select ARCH_KIRKWOOD_DT > >>+ select MACH_DNSKW_DT > >>+ select CONFIG_MTD_OF_PARTS > >>+ select CONFIG_SERIAL_OF_PLATFORM > > > >These two lines are dangerous. It is not safe to 'select' Kconfig > >symbols that have 'depends' constraints. Otherwise, the symbol will > >get forced on without it's dependencies. > > > >Typically other code handles this by creating blank "HAVE_*" symbols > >that the needed symbol can do something like "default y if HAVE_*" > > > > Okay I didn't realise this, thanks. The options in question aren't > hard dependencies per-se, but no serial or NAND support is probably a > mistake, so added them here to avoid a certain amount of shooting in foot. > > What's the etiquette in this situation? Adding HAVE_* seems excessive, > would selecting them in kirkwood_defconfig make more sense? Or would it > simply be covered by having a reasonable config on my website or suchlike?
The latter sounds good. kirkwood_defconfig is in need of some love, and I haven't had time to get to it. > Jason, given you've already applied this patch, would you rather me > submit something else to ditch these, or resumbit the entire series, > combining feedback? I only applied it locally, I was waiting for comments like this before doing a pull request. Go ahead and redo the series based on the input and repost. I'll replace what I have. > > >>+ help > >>+ Say 'Y' here if you want your kernel to support the > >>+ D-Link DNS-320 NAS, using Flattened Device Tree. > >>+ > >>+config MACH_DNS325_DT > >>+ bool "D-Link DNS-325 (Flattened Device Tree)" > >>+ select ARCH_KIRKWOOD_DT > >>+ select MACH_DNSKW_DT > >>+ select CONFIG_MTD_OF_PARTS > >>+ select CONFIG_SERIAL_OF_PLATFORM > >>+ help > >>+ Say 'Y' here if you want your kernel to support the > >>+ D-Link DNS-325 NAS, using Flattened Device Tree. > >>+ > > > >The *only* difference between these two configs is the .dtb file that > >gets built. Don't create a separate Kconfig entry for each dnskw > >board. Just build all the dnskw dtb files when MACH_DNSKW_DT is > >selected. Building .dtb files is cheap. > > > > The rationale for this is the code in dnskw.c is (hopefully!) temporary, > and when it goes there isn't any need for MACH_DNSKW_DT either. > > In a future land where everything has been converted to devicetree, what > would be best? An option to "Build all kirkwood-based .dtb's", an option > to build all D-link .dtb's, Q-QNAP .dtb's, etc. or an option for each > board? I've not got any strong opinion, so will reformat the above to > whatever is considered best. Based on Grant's comment, I'll probably be working towards a MACH_GLOBALSCALE_DT option to catch dreamplug,sheevaplug,guruplug, etc. Perhaps this should be MACH_BUFFALO_DT? > >> config MACH_TS219 > >> bool "QNAP TS-110, TS-119, TS-119P+, TS-210, TS-219, TS-219P and > >> TS-219P+ Turbo NAS" > >> help > >>diff --git a/arch/arm/mach-kirkwood/Makefile > >>b/arch/arm/mach-kirkwood/Makefile > >>index e299a95..b092af5 100644 > >>--- a/arch/arm/mach-kirkwood/Makefile > >>+++ b/arch/arm/mach-kirkwood/Makefile > >>@@ -22,3 +22,4 @@ obj-$(CONFIG_MACH_T5325) += t5325-setup.o > >> obj-$(CONFIG_CPU_IDLE) += cpuidle.o > >> obj-$(CONFIG_ARCH_KIRKWOOD_DT) += board-dt.o > >> obj-$(CONFIG_MACH_DREAMPLUG_DT) += board-dreamplug.o > >>+obj-$(CONFIG_MACH_DNSKW_DT) += board-dnskw.o > >>diff --git a/arch/arm/mach-kirkwood/Makefile.boot > >>b/arch/arm/mach-kirkwood/Makefile.boot > >>index 16f9385..9c5e45f 100644 > >>--- a/arch/arm/mach-kirkwood/Makefile.boot > >>+++ b/arch/arm/mach-kirkwood/Makefile.boot > >>@@ -3,3 +3,5 @@ params_phys-y := 0x00000100 > >> initrd_phys-y := 0x00800000 > >> > >> dtb-$(CONFIG_MACH_DREAMPLUG_DT) += kirkwood-dreamplug.dtb > >>+dtb-$(CONFIG_MACH_DNS320_DT) += kirkwood-dns320.dtb > >>+dtb-$(CONFIG_MACH_DNS325_DT) += kirkwood-dns325.dtb > >>diff --git a/arch/arm/mach-kirkwood/board-dnskw.c > >>b/arch/arm/mach-kirkwood/board-dnskw.c > >>new file mode 100644 > >>index 0000000..7cb7f6a > >>--- /dev/null > >>+++ b/arch/arm/mach-kirkwood/board-dnskw.c > >>@@ -0,0 +1,306 @@ > >>+/* > >>+ * Copyright 2012 (C), Jamie Lentin <[email protected]> > >>+ * > >>+ * arch/arm/mach-kirkwood/board-dnskw.c > >>+ * > >>+ * D-link DNS-320 & DNS-325 NAS Init for drivers not converted to > >>+ * flattened device tree yet. > >>+ * > >>+ * 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/platform_device.h> > >>+#include <linux/i2c.h> > >>+#include <linux/ata_platform.h> > >>+#include <linux/mv643xx_eth.h> > >>+#include <linux/of.h> > >>+#include <linux/gpio.h> > >>+#include <linux/input.h> > >>+#include <linux/gpio_keys.h> > >>+#include <linux/gpio-fan.h> > >>+#include <linux/leds.h> > >>+#include <linux/mtd/physmap.h> > >>+#include <asm/mach-types.h> > >>+#include <asm/mach/arch.h> > >>+#include <asm/mach/map.h> > >>+#include <mach/kirkwood.h> > >>+#include <mach/bridge-regs.h> > >>+#include "common.h" > >>+#include "mpp.h" > >>+ > >>+static struct mtd_partition dnskw_nand_parts[] = { > >>+ { > >>+ .name = "u-boot", > >>+ .offset = 0, > >>+ .size = SZ_1M, > >>+ .mask_flags = MTD_WRITEABLE > >>+ }, { > >>+ .name = "uImage", > >>+ .offset = MTDPART_OFS_NXTBLK, > >>+ .size = 5 * SZ_1M > >>+ }, { > >>+ .name = "ramdisk", > >>+ .offset = MTDPART_OFS_NXTBLK, > >>+ .size = 5 * SZ_1M > >>+ }, { > >>+ .name = "image", > >>+ .offset = MTDPART_OFS_NXTBLK, > >>+ .size = 102 * SZ_1M > >>+ }, { > >>+ .name = "mini firmware", > >>+ .offset = MTDPART_OFS_NXTBLK, > >>+ .size = 10 * SZ_1M > >>+ }, { > >>+ .name = "config", > >>+ .offset = MTDPART_OFS_NXTBLK, > >>+ .size = 5 * SZ_1M > >>+ }, > >>+}; > > > >This patch adds a static partition mapping, and then a later patch in > >this same series removes it again. Please don't do that. Just squash > >the patches together. Or, if sqashing is not appropriate, then just > >omit the partitions. > > > >I also suggest omiting similar data when there are patches already in > >progess to do it properly. It is okay for the inital support to go in > >to be incomplete when enhancements will be ready soon. > > > > Noted. If the entire patchset needs resubmitting, I'll combine the patch > that moves this definition to the .dtb See my comment in my other reply, just don't add nand/partition support in the beginning. thx, Jason. _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
