Hi John. Oops, you had also posted this patch to the list later, so I'm also forwarding my comments to the list.
Cheers, g. On Sat, Jun 28, 2008 at 3:56 PM, Grant Likely <[EMAIL PROTECTED]> wrote: > Sorry for the late reply on this one, I had gotten rather busy. > > On Wed, Jun 18, 2008 at 03:09:39PM -0600, John Linn wrote: >> 1. I'm not sure why we need to disable the interrupts in the >> bootstrap loader? > > You don't. That's old zImage.raw stuff that you don't need. > >> 2. I see some SecetLab copyright in new files that might be just a >> cut/paste type error? > > If it is mostly based on my code, then it is appropriate to preserve my > copyright and add Xilinx's copyright above it. If it is really heavily > modified, then the Secret lab copyright can be dropped. > >> 3. I don't see the cputable.c up to date with the 440? > >> Thanks, >> John >> >> -----Original Message----- >> From: John Linn [mailto:[EMAIL PROTECTED] >> Sent: Wednesday, June 18, 2008 2:58 PM >> Cc: John Linn >> Subject: [PATCH] powerpc: Xilinx: adding virtex5 powerpc 440 support >> >> >> >> The following files add support for Virtex5 with Powerpc 440. >> >> >> >> Device trees are currently handled differently than other embedded >> >> systems as there may not be a boot loader such that the device >> >> tree is compiled into the kernel or pulled from a BRAM. >> >> >> >> The UART 16550 has extra initialization in the bootstrap loader >> >> since a boot loader is not used many times. >> > > Your mailer seems to have damaged the patch by wrapping lines and > inserting extra blank lines. You'll need to resend. I've removed > them below so I could make comments. > >> >> Signed-off-by: John Linn <[EMAIL PROTECTED]> >> --- >> arch/powerpc/Kconfig | 75 +++ >> arch/powerpc/boot/Makefile | 24 +- >> arch/powerpc/boot/dts/ml507.dts | 254 ++++++++ >> arch/powerpc/boot/io.h | 7 + >> arch/powerpc/boot/virtex.c | 246 ++++++++ >> arch/powerpc/configs/44x/ml507_defconfig | 1010 >> ++++++++++++++++++++++++++++++ >> arch/powerpc/platforms/44x/Kconfig | 62 ++ >> arch/powerpc/platforms/44x/Makefile | 1 + >> arch/powerpc/platforms/44x/virtex.c | 58 ++ >> arch/powerpc/platforms/Kconfig | 7 + >> 10 files changed, 1739 insertions(+), 5 deletions(-) >> create mode 100644 arch/powerpc/boot/dts/ml507.dts >> create mode 100644 arch/powerpc/boot/virtex.c >> create mode 100644 arch/powerpc/configs/44x/ml507_defconfig >> create mode 100644 arch/powerpc/platforms/44x/virtex.c >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index 3934e26..94adfe1 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -483,6 +483,81 @@ config SECCOMP >> >> If unsure, say Y. Only embedded should say N here. >> >> +config WANT_DEVICE_TREE >> + bool >> + default n > > This shouldn't be here. > >> + >> +config BUILD_RAW_IMAGE >> + bool "Build firmware-independent image" >> + select WANT_DEVICE_TREE >> + help >> + If this is enabled, a firmware independent "raw" image will be >> + built, as zImage.raw. This requires a completely filled-in >> + device tree, with the following labels: >> + >> + mem_size_cells: on /#address-cells >> + memsize: on the size portion of /memory/reg >> + timebase: on the boot CPU's timebase property > > Obsolete stuff; replaced with simpleImage > >> +config DEVICE_TREE >> + string "Static device tree source file" >> + depends on WANT_DEVICE_TREE >> + help >> + This specifies the device tree source (.dts) file to be >> + compiled and included when building the bootwrapper. If a >> + relative filename is given, then it will be relative to >> + arch/powerpc/boot/dts. If you are not using the bootwrapper, >> + or do not need to build a dts into the bootwrapper, this >> + field is ignored. >> + >> + For example, this is required when building a cuImage target >> + for an older U-Boot, which cannot pass a device tree itself. >> + Such a kernel will not work with a newer U-Boot that tries to >> + pass a device tree (unless you tell it not to). If your U-Boot >> + does not mention a device tree in "help bootm", then use the >> + cuImage target and specify a device tree here. Otherwise, use >> + the uImage target and leave this field blank. > > Obsolete > >> +config COMPRESSED_DEVICE_TREE >> + bool "Use compressed device tree" >> + depends on XILINX_VIRTEX >> + depends on WANT_DEVICE_TREE >> + help >> + In Xilinx FPGAs, the hardware can change quite dramatically >> while >> + still running the same kernel. In this case and other similar >> + ones, it is preferable to associate the device tree with a >> + particular build of the hardware design. This configuration >> + option assumes that the device tree blob has been compressed and >> + stored in Block RAM in the FPGA design. Typically, such a block >> + ram is available in order to provide a bootloop or other code >> + close to the reset vector at the top of the address space. By >> + default, the parameter options associated with this configuration >> + assumes that exactly one block ram (2KB) of storage is available, >> + which should be sufficient for most designs. If necessary in a >> + particular design, due to boot code requirement or a large number >> + of devices, this address (and the corresponding parameters in the >> + EDK design) must be modified. >> + >> + Note that in some highly area constrained designs, no block rams >> + may be available in the design, and some other mechanism may be >> + used to hold the processor in reset while external memory is >> + initialized with processor code. In such cases, that mechanism >> + should also be used to load the device tree at an appropriate >> + location, and the parameters associated with this configuration >> + option should be modified to point to that location in external >> + memory. > > This is a really interesting option and is probably useful for other > platforms too. I'd like to see this split out into a separate patch and > slightly reworked so that it is usable by non-xilinx targets. > >> +config COMPRESSED_DTB_START >> + hex "Start of compressed device tree" >> + depends on COMPRESSED_DEVICE_TREE >> + default 0xfffff800 >> + >> +config COMPRESSED_DTB_SIZE >> + hex "Size of compressed device tree" >> + depends on COMPRESSED_DEVICE_TREE >> + default 0x800 > > These probably shouldn't be config values. Either they should be > provided in regsiters or memory at boot time, or a data file should be > pulled in when linking the bootwrapper to determine where to look for the > device tree compressed blob. The goal of this is to allow multiple > images to be built (with different dtb locations) for a single kernel > compile. > >> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile >> index 1cee2f9..9de59fb 100644 >> --- a/arch/powerpc/boot/Makefile >> +++ b/arch/powerpc/boot/Makefile >> @@ -66,7 +66,7 @@ src-plat := of.c cuboot-52xx.c cuboot-824x.c >> cuboot-83xx.c cuboot-85xx.c holly.c >> fixed-head.S ep88xc.c ep405.c \ >> cuboot-katmai.c cuboot-rainier.c redboot-8xx.c ep8248e.c \ >> cuboot-warp.c cuboot-85xx-cpm2.c cuboot-yosemite.c >> simpleboot.c \ >> - virtex405-head.S >> + virtex.c virtex405-head.S >> src-boot := $(src-wlib) $(src-plat) empty.c >> >> src-boot := $(addprefix $(obj)/, $(src-boot)) >> @@ -197,6 +197,7 @@ image-$(CONFIG_PPC_HOLLY) += zImage.holly >> image-$(CONFIG_PPC_PRPMC2800) += dtbImage.prpmc2800 >> image-$(CONFIG_PPC_ISERIES) += zImage.iseries >> image-$(CONFIG_DEFAULT_UIMAGE) += uImage >> +image-$(CONFIG_XILINX_VIRTEX) += zImage.virtex >> >> # >> # Targets which embed a device tree blob >> @@ -279,14 +280,24 @@ targets += $(image-y) $(initrd-y) >> >> $(addprefix $(obj)/, $(initrd-y)): $(obj)/ramdisk.image.gz >> >> +# If CONFIG_WANT_DEVICE_TREE is set and CONFIG_DEVICE_TREE isn't an >> +# empty string, define 'dts' to be path to the dts >> +# CONFIG_DEVICE_TREE will have "" around it, make sure to strip them >> +ifeq ($(CONFIG_WANT_DEVICE_TREE),y) >> +ifneq ($(CONFIG_DEVICE_TREE),"") >> +dts = $(if $(shell echo $(CONFIG_DEVICE_TREE) | grep '^/'),\ >> + ,$(srctree)/$(src)/dts/)$(CONFIG_DEVICE_TREE:"%"=%) >> +endif >> +endif >> + > > Obsolete stuff > >> # Don't put the ramdisk on the pattern rule; when its missing make will >> try >> # the pattern rule with less dependencies that also matches (even with >> the >> # hard dependency listed). >> -$(obj)/zImage.initrd.%: vmlinux $(wrapperbits) >> - $(call if_changed,wrap,$*,,,$(obj)/ramdisk.image.gz) >> +$(obj)/zImage.initrd.%: vmlinux $(wrapperbits) $(dts) >> + $(call if_changed,wrap,$*,$(dts),,$(obj)/ramdisk.image.gz) > > Ditto > >> >> -$(obj)/zImage.%: vmlinux $(wrapperbits) >> - $(call if_changed,wrap,$*) >> +$(obj)/zImage.%: vmlinux $(wrapperbits) $(dts) >> + $(call if_changed,wrap,$*,$(dts)) > > Ditto > >> # dtbImage% - a dtbImage is a zImage with an embedded device tree blob >> $(obj)/dtbImage.initrd.%: vmlinux $(wrapperbits) $(obj)/%.dtb >> @@ -325,6 +336,9 @@ $(obj)/treeImage.%: vmlinux $(obj)/%.dtb >> $(wrapperbits) >> $(obj)/%.dtb: $(dtstree)/%.dts $(obj)/dtc >> $(obj)/dtc -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) >> $(dtstree)/$*.dts >> >> +$(obj)/zImage.raw: vmlinux $(dts) $(wrapperbits) >> + $(call if_changed,wrap,raw,$(dts)) >> + > > Ditto > >> # If there isn't a platform selected then just strip the vmlinux. >> ifeq (,$(image-y)) >> image-y := vmlinux.strip >> diff --git a/arch/powerpc/boot/dts/ml507.dts >> b/arch/powerpc/boot/dts/ml507.dts >> new file mode 100644 >> index 0000000..43d8535 >> --- /dev/null >> +++ b/arch/powerpc/boot/dts/ml507.dts >> @@ -0,0 +1,254 @@ >> +/* >> + * (C) Copyright 2007-2008 Xilinx, Inc. >> + * (C) Copyright 2007 Michal Simek >> + * >> + * Michal SIMEK <[EMAIL PROTECTED]> > > I don't think it is appropriate to have Michal's name here; this is a > generated file, not a file that he wrote. (It is, of course, 100% > appropriate for the tool that generated it to have his copyright) > > It *might* be appropriate for Xilinx to claim copyright on this file > because it is the device tree for one of Xilinx's reference designs, but > that ground isn't very stable. > >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation; either version 2 of >> + * the License, or (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, >> + * MA 02111-1307 USA > > These three paragraphs can be dropped; they are redundant. > >> + * >> + * CAUTION: This file is automatically generated by libgen. >> + * Version: Xilinx EDK 10.1.01 EDK_K_SP1.2 > > This is a generated file and so it is debatable about whether or not it > is appropriate for inclusion in the kernel. Personally, I lean toward > including it as long as it is well documented as to what it is. > Specifically, the comment block should state exactly what version of the > reference design is being described here. > > <snipping the entire dts file> > > BTW, the dts file and defconfig should be submitted in separate patch > files. > >> diff --git a/arch/powerpc/boot/io.h b/arch/powerpc/boot/io.h >> index ccaedae..ec57ec9 100644 >> --- a/arch/powerpc/boot/io.h >> +++ b/arch/powerpc/boot/io.h >> @@ -99,4 +99,11 @@ static inline void barrier(void) >> asm volatile("" : : : "memory"); >> } >> >> +static inline void disable_irq(void) >> +{ >> + int dummy; >> + asm volatile("mfmsr %0; rlwinm %0, %0, 0, ~(1<<15); mtmsr %0" : >> + "=r" (dummy) : : "memory"); >> +} >> + > > Drop this; it was part of the old zImage.raw stuff. > >> #endif /* _IO_H */ >> diff --git a/arch/powerpc/boot/virtex.c b/arch/powerpc/boot/virtex.c >> new file mode 100644 >> index 0000000..5d807c6 >> --- /dev/null >> +++ b/arch/powerpc/boot/virtex.c >> @@ -0,0 +1,246 @@ >> +/* >> + * Old U-boot compatibility for Walnut > > Not true! :-) > >> + * Author: Josh Boyer <[EMAIL PROTECTED]> > > Probably not true either! > >> + * >> + * Copyright 2007 IBM Corporation >> + * Based on cuboot-83xx.c, which is: >> + * Copyright (c) 2007 Freescale Semiconductor, Inc. > > You should probably add Xilinx to this list. > >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License version 2 as published >> + * by the Free Software Foundation. > > This paragraph is redundant; remove. > >> + */ >> + >> +#include <stddef.h> >> +#include <stdio.h> >> +#include "ops.h" >> +#include "dcr.h" >> +#include "4xx.h" >> +#include "io.h" >> +#include "reg.h" >> + >> +BSS_STACK(4096); >> + >> +#include "types.h" >> +#include "gunzip_util.h" >> +#include <libfdt.h> >> +#include "../../../include/linux/autoconf.h" >> + >> +#define UART_DLL 0 /* Out: Divisor Latch Low */ >> +#define UART_DLM 1 /* Out: Divisor Latch High */ >> +#define UART_FCR 2 /* Out: FIFO Control Register */ >> +#define UART_FCR_CLEAR_RCVR 0x02 /* Clear the RCVR FIFO */ >> +#define UART_FCR_CLEAR_XMIT 0x04 /* Clear the XMIT FIFO */ >> +#define UART_LCR 3 /* Out: Line Control Register */ >> +#define UART_MCR 4 /* Out: Modem Control Register */ >> +#define UART_MCR_RTS 0x02 /* RTS complement */ >> +#define UART_MCR_DTR 0x01 /* DTR complement */ >> +#define UART_LCR_DLAB 0x80 /* Divisor latch access bit */ >> +#define UART_LCR_WLEN8 0x03 /* Wordlength: 8 bits */ >> + >> +/* This function is only needed when there is no boot loader to >> + initialize the UART >> +*/ >> +static int virtex_ns16550_console_init(void *devp) >> +{ >> + int n; >> + unsigned long reg_phys; >> + unsigned char *regbase; >> + u32 regshift, clk, spd; >> + u16 divisor; >> + >> + n = getprop(devp, "virtual-reg", ®base, sizeof(regbase)); >> + if (n != sizeof(regbase)) { >> + if (!dt_xlate_reg(devp, 0, ®_phys, NULL)) >> + return -1; >> + >> + regbase = (void *)reg_phys + 3; >> + } >> + regshift = 2; >> + >> + n = getprop(devp, "current-speed", (void *)&spd, sizeof(spd)); >> + if (n != sizeof(spd)) >> + spd = 9600; >> + >> + /* should there be a default clock rate?*/ >> + n = getprop(devp, "clock-frequency", (void *)&clk, sizeof(clk)); >> + if (n != sizeof(clk)) >> + return -1; >> + >> + divisor = clk / (16 * spd); >> + >> + /* Access baud rate */ >> + out_8(regbase + (UART_LCR << regshift), UART_LCR_DLAB); >> + >> + /* Baud rate based on input clock */ >> + out_8(regbase + (UART_DLL << regshift), divisor & 0xFF); >> + out_8(regbase + (UART_DLM << regshift), divisor >> 8); >> + >> + /* 8 data, 1 stop, no parity */ >> + out_8(regbase + (UART_LCR << regshift), UART_LCR_WLEN8); >> + >> + /* RTS/DTR */ >> + out_8(regbase + (UART_MCR << regshift), UART_MCR_RTS | UART_MCR_DTR); >> + >> + /* Clear transmitter and receiver */ >> + out_8(regbase + (UART_FCR << regshift), >> + UART_FCR_CLEAR_XMIT | UART_FCR_CLEAR_RCVR); >> + return 0; >> +} >> + >> +/* For virtex, the kernel may be loaded without using a bootloader and if so >> + some UARTs need more setup than is provided in the normal console init >> +*/ >> +static int virtex_serial_console_init(void) >> +{ >> + void *devp; >> + char devtype[MAX_PROP_LEN]; >> + char path[MAX_PATH_LEN]; >> + >> + devp = finddevice("/chosen"); >> + if (devp == NULL) >> + return -1; >> + >> + if (getprop(devp, "linux,stdout-path", path, MAX_PATH_LEN) > 0) { >> + devp = finddevice(path); >> + if (devp == NULL) >> + return -1; >> + >> + if ((getprop(devp, "device_type", devtype, sizeof(devtype)) > 0) >> + && !strcmp(devtype, "serial") >> + && (dt_is_compatible(devp, "ns16550"))) >> + virtex_ns16550_console_init(devp); >> + } >> + return 0; >> +} >> + >> +#ifdef CONFIG_COMPRESSED_DEVICE_TREE >> +static struct gunzip_state gzstate; >> +#endif > > #ifdefs are forbidden in bootwrapper code. Use different zImage targets > to enable/disable particular features. > >> + >> +void platform_init(void) > > Wrong signature for platform_init(). Should be: > > void platform_init(unsigned long r3, unsigned long r4, unsigned long r5, > unsigned long r6, unsigned long r7) > >> +{ >> + u32 memreg[4]; >> + u64 start; >> + u64 size = 0x2000000; >> + int naddr, nsize, i; >> + void *root, *memory; >> + static const unsigned long line_size = 32; >> + static const unsigned long congruence_classes = 256; >> + unsigned long addr; >> + unsigned long dccr; >> + >> +#ifdef CONFIG_COMPRESSED_DEVICE_TREE >> + void *dtbz_start; >> + u32 dtbz_size; >> + void *dtb_addr; >> + u32 dtb_size; >> + struct fdt_header dtb_header; >> + int len; >> +#endif > > Ditto > >> + >> + if((mfpvr() & 0xfffff000) == 0x20011000) { >> + /* PPC errata 213: only for Virtex-4 FX */ >> + __asm__("mfccr0 0\n\t" >> + "oris 0,0,[EMAIL PROTECTED]" >> + "mtccr0 0" >> + : : : "0"); >> + } >> + >> + /* >> + * Invalidate the data cache if the data cache is turned off. >> + * - The 405 core does not invalidate the data cache on power-up >> + * or reset but does turn off the data cache. We cannot assume >> + * that the cache contents are valid. >> + * - If the data cache is turned on this must have been done by >> + * a bootloader and we assume that the cache contents are >> + * valid. >> + */ >> + __asm__("mfdccr %0": "=r" (dccr)); >> + if (dccr == 0) { >> + for (addr = 0; >> + addr < (congruence_classes * line_size); >> + addr += line_size) { >> + __asm__("dccci 0,%0": :"b"(addr)); >> + } >> + } > > Instead of duplicating this in C code, you should instead modify the > wrapper script to also link in virtex405-head.o > >> +#if defined(CONFIG_XILINX_VIRTEX_5_FXT) && defined(CONFIG_MATH_EMULATION) >> + /* Make sure the APU is disabled when using soft FPU emulation */ >> + mtdcr(5, 0); >> +#endif > > Again; remove #ifdefs. Do stuff like this in the equivalent of > virtex405-head.S > >> + >> + disable_irq(); >> + >> +#ifdef CONFIG_COMPRESSED_DEVICE_TREE >> + >> + /** FIXME: flatdevicetrees need the initializer allocated, >> + libfdt will fix this. */ >> + dtbz_start = (void *)CONFIG_COMPRESSED_DTB_START; >> + dtbz_size = CONFIG_COMPRESSED_DTB_SIZE; >> + /** get the device tree */ >> + gunzip_start(&gzstate, dtbz_start, dtbz_size); >> + gunzip_exactly(&gzstate, &dtb_header, sizeof(dtb_header)); >> + >> + dtb_size = dtb_header.totalsize; >> + // printf("Allocating 0x%lx bytes for dtb ...\n\r", dtb_size); > > remove c++ style comments > >> + >> + dtb_addr = _end; // Should be allocated? >> + >> + gunzip_start(&gzstate, dtbz_start, dtbz_size); >> + len = gunzip_finish(&gzstate, dtb_addr, dtb_size); >> + if (len != dtb_size) >> + fatal("ran out of data! only got 0x%x of 0x%lx bytes.\n\r", >> + len, dtb_size); >> + printf("done 0x%x bytes\n\r", len); >> + simple_alloc_init(0x800000, size - (unsigned long)0x800000, 32, 64); >> + fdt_init(dtb_addr); > > Address shouldn't be hard coded. Can you calculate the dtb_addr based on > the _end addr and the size of the dtb? > >> +#else >> + /** FIXME: flatdevicetrees need the initializer allocated, >> + libfdt will fix this. */ >> + simple_alloc_init(_end, size - (unsigned long)_end, 32, 64); >> + fdt_init(_dtb_start); >> +#endif >> + >> + root = finddevice("/"); >> + if (getprop(root, "#address-cells", &naddr, sizeof(naddr)) < 0) >> + naddr = 2; >> + if (naddr < 1 || naddr > 2) >> + fatal("Can't cope with #address-cells == %d in /\n\r", naddr); >> + >> + if (getprop(root, "#size-cells", &nsize, sizeof(nsize)) < 0) >> + nsize = 1; >> + if (nsize < 1 || nsize > 2) >> + fatal("Can't cope with #size-cells == %d in /\n\r", nsize); >> + >> + memory = finddevice("/[EMAIL PROTECTED]"); >> + if (! memory) { >> + fatal("Need a [EMAIL PROTECTED] node!\n\r"); >> + } >> + if (getprop(memory, "reg", memreg, sizeof(memreg)) < 0) >> + fatal("Need a [EMAIL PROTECTED] node!\n\r"); >> + >> + i = 0; >> + start = memreg[i++]; >> + if(naddr == 2) { >> + start = (start << 32) | memreg[i++]; >> + } >> + size = memreg[i++]; >> + if (nsize == 2) >> + size = (size << 32) | memreg[i++]; >> + >> + // timebase_period_ns = 1000000000 / timebase; >> + virtex_serial_console_init(); >> + serial_console_init(); >> + if (console_ops.open) >> + console_ops.open(); >> + >> +#ifdef CONFIG_COMPRESSED_DEVICE_TREE >> + printf("Using compressed device tree at 0x%x\n\r", >> CONFIG_COMPRESSED_DTB_START); >> +#else >> +#endif >> + printf("booting virtex\n\r"); >> + printf("memstart=0x%llx\n\r", start); >> + printf("memsize=0x%llx\n\r", size); >> +} >> diff --git a/arch/powerpc/platforms/44x/Kconfig >> b/arch/powerpc/platforms/44x/Kconfig >> index 6abe913..b90b33d 100644 >> --- a/arch/powerpc/platforms/44x/Kconfig >> +++ b/arch/powerpc/platforms/44x/Kconfig >> @@ -102,6 +102,58 @@ config YOSEMITE >> # help >> # This option enables support for the IBM PPC440GX evaluation board. >> >> +config XILINX_ML507 >> + bool "Xilinx ML507 Reference System" >> + depends on 44x >> + default n >> + select XILINX_ML5XX >> + select WANT_DEVICE_TREE >> + help >> + This option enables support for the Xilinx ML507 board. > > I don't think this is necessary. Follow the lead of virtex4 support and > do something like config XILINX_VIRTEX_440_GENERIC_BOARD. > >> @@ -152,3 +204,13 @@ config 460EX >> # 44x errata/workaround config symbols, selected by the CPU models >> above >> config IBM440EP_ERR42 >> bool >> + >> +# Xilinx specific config options. >> +config XILINX_ML5XX >> + bool >> + select XILINX_VIRTEX > > I think you should drop this. Board specific stuff should be contained > within the .dts files. > >> +config XILINX_VIRTEX_5_FXT >> + bool >> + depends on XILINX_ML507 >> + default y > > Drop the above two lines and may the generic board config option select > XILINX_VIRTEX_5_FXT > >> diff --git a/arch/powerpc/platforms/44x/Makefile >> b/arch/powerpc/platforms/44x/Makefile >> index 774165f..e3a9337 100644 >> --- a/arch/powerpc/platforms/44x/Makefile >> +++ b/arch/powerpc/platforms/44x/Makefile >> @@ -4,6 +4,7 @@ obj-$(CONFIG_TAISHAN) += taishan.o >> obj-$(CONFIG_BAMBOO) += bamboo.o >> obj-$(CONFIG_YOSEMITE) += bamboo.o >> obj-$(CONFIG_SEQUOIA) += sequoia.o >> +obj-$(CONFIG_XILINX_ML507) += virtex.o >> obj-$(CONFIG_KATMAI) += katmai.o >> obj-$(CONFIG_RAINIER) += rainier.o >> obj-$(CONFIG_WARP) += warp.o >> diff --git a/arch/powerpc/platforms/44x/virtex.c >> b/arch/powerpc/platforms/44x/virtex.c >> new file mode 100644 >> index 0000000..42ca337 >> --- /dev/null >> +++ b/arch/powerpc/platforms/44x/virtex.c >> @@ -0,0 +1,58 @@ >> +/* >> + * Xilinx Virtex 5FXT based board support >> + * >> + * Copyright 2007 Secret Lab Technologies Ltd. >> + * >> + * 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/init.h> >> +#include <linux/of_platform.h> >> +#include <asm/machdep.h> >> +#include <asm/prom.h> >> +#include <asm/time.h> >> +#include <asm/xilinx_intc.h> >> +#include <asm/reg.h> >> +#include <asm/ppc4xx.h> >> +#include "44x.h" >> + >> +static struct of_device_id xilinx_of_bus_ids[] __initdata = { >> + { .compatible = "simple-bus", }, >> + { .compatible = "xlnx,plb-v46-1.00.a", }, >> + { .compatible = "xlnx,plb-v46-1.02.a", }, >> + { .compatible = "xlnx,plb-v34-1.01.a", }, >> + { .compatible = "xlnx,plb-v34-1.02.a", }, >> + { .compatible = "xlnx,opb-v20-1.10.c", }, >> + { .compatible = "xlnx,dcr-v29-1.00.a", }, > > Do you need this whole list if the device tree generator is now claiming > "simple-bus" compatibility? > >> + { .compatible = "xlnx,compound", }, >> + {} >> +}; >> + >> +static int __init virtex_device_probe(void) >> +{ >> + of_platform_bus_probe(NULL, xilinx_of_bus_ids, NULL); >> + >> + return 0; >> +} >> +machine_device_initcall(virtex, virtex_device_probe); >> + >> +static int __init virtex_probe(void) >> +{ >> + unsigned long root = of_get_flat_dt_root(); >> + >> + if (!of_flat_dt_is_compatible(root, "xlnx,virtex")) >> + return 0; > > This is probably not good. Granted, it is impossible to build a 405+440 > multiplatform image, but the device tree generator should probably be > modified to claim something like "xlnx,virtex5fxt" so it isn't confused > with an older virtex part. (I realize that this is just following the > lead from virtex2/4 support, but that should probably be changed too.) > >> + >> + return 1; >> +} >> + >> +define_machine(virtex) { >> + .name = "Xilinx Virtex", >> + .probe = virtex_probe, >> + .init_IRQ = xilinx_intc_init_tree, >> + .get_irq = xilinx_intc_get_irq, >> + .calibrate_decr = generic_calibrate_decr, >> + .restart = ppc4xx_reset_system, >> +}; >> diff --git a/arch/powerpc/platforms/Kconfig >> b/arch/powerpc/platforms/Kconfig >> index 87454c5..523388b 100644 >> --- a/arch/powerpc/platforms/Kconfig >> +++ b/arch/powerpc/platforms/Kconfig >> @@ -313,6 +313,13 @@ config FSL_ULI1575 >> config CPM >> bool >> >> +config XILINX_VIRTEX >> + bool >> + select PPC_DCR_MMIO >> + select PPC_DCR_NATIVE >> + help >> + Support for Xilinx Virtex platforms. >> + > > config XILINX_VIRTEX is already defined in > arch/powerpc/platforms/40x/Kconfig. Moving it to this file should be > done in a separate patch. > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev