From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1L3Z3c-0004u4-G6 for qemu-devel@nongnu.org; Fri, 21 Nov 2008 11:40:36 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1L3Z3b-0004tU-Hs for qemu-devel@nongnu.org; Fri, 21 Nov 2008 11:40:36 -0500 Received: from [199.232.76.173] (port=57881 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1L3Z3b-0004tO-Cf for qemu-devel@nongnu.org; Fri, 21 Nov 2008 11:40:35 -0500 Received: from e2.ny.us.ibm.com ([32.97.182.142]:59318) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1L3Z3a-0003tI-RK for qemu-devel@nongnu.org; Fri, 21 Nov 2008 11:40:35 -0500 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by e2.ny.us.ibm.com (8.13.1/8.13.1) with ESMTP id mALGeE60011341 for ; Fri, 21 Nov 2008 11:40:14 -0500 Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id mALGeWZM181912 for ; Fri, 21 Nov 2008 11:40:32 -0500 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id mALGeWRc027619 for ; Fri, 21 Nov 2008 11:40:32 -0500 Received: from [9.53.41.42] (slate.austin.ibm.com [9.53.41.42]) by d01av03.pok.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id mALGeW3t027585 for ; Fri, 21 Nov 2008 11:40:32 -0500 Subject: Re: [Qemu-devel] DHT Walnut board support From: Hollis Blanchard In-Reply-To: <169263.53603.qm@web27203.mail.ukl.yahoo.com> References: <169263.53603.qm@web27203.mail.ukl.yahoo.com> Content-Type: text/plain Date: Fri, 21 Nov 2008 10:40:31 -0600 Message-Id: <1227285631.26513.30.camel@localhost.localdomain> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Fri, 2008-11-21 at 01:53 +0000, Salvatore Lionetti wrote: > Source modified > =============== > - code factoring for ppc405GPr, ppc405EP, ppc405GPe > - reorganization in i2c code, with possibility of board customization callback > - new board added for 'DHT Walnut board', fw in pc-bios/walnut_uboot.bin > - cfi flash now support 'buffer mode', write N {byte, halfword, word} consecutively. > - emac/mal now work with vlan mode First, you *really* need to split these into separate patches. That will make it possible to actually review all this code. Each bullet you list above should be at least one separate patch (and in some cases maybe more). > Board Walnut: > ============= > http://elinux.org/DHT-Walnut-Flameman > > CPU PowerPC 405 GPe running at 266Mhz > LAN On-chip 405GP ethernet, namely emac/mal > ROM 512k of boot flash, AMD 29LV040B > PID eeprom via i2c > > Full Tested (eth, flash, pid) > ============================= > - u-boot 1.1.6 on walnut board (with few patch, see ppc405_board.c) > - OSE on proprietary NSN 4G radio module > - Can anyone test ref405ep and taihu? > > P.S: > I've tryed to have patch for bin file but no luck > Since i can not version file, does diff support binary file?does attachment work? > > > > Index: Makefile.target > =================================================================== > --- Makefile.target (revision 5720) > +++ Makefile.target (working copy) > @@ -679,7 +679,7 @@ > # NewWorld PowerMac > OBJS+= unin_pci.o ppc_chrp.o > # PowerPC 4xx boards > -OBJS+= pflash_cfi02.o ppc4xx_devs.o ppc405_uc.o ppc405_boards.o > +OBJS+= pflash_cfi02.o emac.o ppc4xx_devs.o ppc405_uc.o ppc405_boards.o > endif > ifeq ($(TARGET_BASE_ARCH), mips) > OBJS+= mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o > Index: osdep.c > =================================================================== > --- osdep.c (revision 5720) > +++ osdep.c (working copy) > @@ -32,6 +32,7 @@ > #include > #include > #endif > +#include > > #include "qemu-common.h" > #include "sysemu.h" > @@ -310,3 +311,49 @@ > fcntl(fd, F_SETFL, f | O_NONBLOCK); > } > #endif > + > +void *qemu_mmap(void * start, size_t length, int prot, int flags, int fd, off_t offset) { > + void *ret; > +#ifndef _WIN32 > + ret = mmap(start, length, prot, flags, fd, offset); > +#else > + { /* TO TEST!!! */ > + static char name[15]; > + static char ind=0; > + HANDLE fd_map; > + LPVOID fd_buf; > + char* buf; > + > + assert(fd!=INVALID_HANDLE_VALUE); > + > + snprintf(name, 15, "qemu_mmap%d", ind++); > + fd_map = CreateFileMapping(fd, > + NULL, /* Security attr */ > + PAGE_READWRITE, > + 0, TOTAL_SIZE, > + name); > + > + fd_buf = MapViewOfFile(fd_map, > + FILE_MAP_ALL_ACCESS, > + 0, 0, /* Offset */ > + TOTAL_SIZE); > + assert(fd_buf); > + ret = fd_buf; > + > + } > +#endif > + return ret; > +} > +int qemu_munmap(void *start, size_t length) { > + int ret; > +#ifndef _WIN32 > + ret = munmap(start, length); > +#else > + { /* TO TEST!!! */ > + UnmapViewOfFile(start); > + /*CloseHandle(fd_map); > + CloseHandle(fd);*/ > + } > +#endif > + return ret; > +} Why is this here? You seem to have created a new function that nobody calls. You should remove it completely. > Index: target-ppc/machine.c > =================================================================== > --- target-ppc/machine.c (revision 5720) > +++ target-ppc/machine.c (working copy) > @@ -8,6 +8,7 @@ > qemu_register_machine(&prep_machine); > qemu_register_machine(&ref405ep_machine); > qemu_register_machine(&taihu_machine); > + qemu_register_machine(&walnut_machine); > } > > void cpu_save(QEMUFile *f, void *opaque) > Index: qemu-common.h > =================================================================== > --- qemu-common.h (revision 5720) > +++ qemu-common.h (working copy) > @@ -152,4 +152,11 @@ > /* Force QEMU to stop what it's doing and service IO */ > void qemu_service_io(void); > > +/* KR function prototype. */ > +typedef size_t KRHandler(int fd, char* buf, size_t size); Aside from the fact that I have no idea what "KR" means, this does not belong in qemu-common.h. > +/* mmap like function > + * (cygwin seem !have aio yet so no linux env under Winzozz) */ > +void *qemu_mmap(void * start, size_t length, int prot, int flags, int fd, off_t offset); > +int qemu_munmap(void *start, size_t length); > #endif > Index: hw/ppc405_boards.c > =================================================================== > --- hw/ppc405_boards.c (revision 5720) > +++ hw/ppc405_boards.c (working copy) > @@ -206,7 +206,7 @@ > #ifdef DEBUG_BOARD_INIT > printf("%s: register cpu\n", __func__); > #endif > - env = ppc405ep_init(ram_bases, ram_sizes, 33333333, &pic, &sram_offset, > + env = ppc405xp_init("405ep", ram_bases, ram_sizes, 33333333, &pic, &sram_offset, > kernel_filename == NULL ? 0 : 1); > /* allocate SRAM */ > #ifdef DEBUG_BOARD_INIT There is no such thing as a 405XP. > @@ -353,7 +353,6 @@ > printf("bdloc %016lx %s\n", > (unsigned long)bdloc, (char *)(phys_ram_base + bdloc)); > } > - > QEMUMachine ref405ep_machine = { > .name = "ref405ep", > .desc = "ref405ep", > @@ -361,6 +360,349 @@ > .ram_require = (128 * 1024 * 1024 + 4096 + 512 * 1024 + BIOS_SIZE) | RAMSIZE_FIXED, > }; > > +/* > + * Walnut board > + * > + * Starting point from official uboot 1.1.6 with following patch: > + * - compilation error: cmd_bootm.c:470 manually expand #if > + * - mac address not in nvram: include/configs/walnut.h add "ethaddr=52:54:00:12:34:56\0" in CONFIG_EXTRA_ENV_SETTING > + * - remove trailer from recv packet(fcs?): cpu/ppc4xx/4xx_enet.c:1478 call NetReceive with 'length' as 2nd parameter. > + */ > +/* Addedum related to 405GP: is it only for such device? */ > +static target_ulong walnut_dcr_read (void *opaque, int dcrn) > +{ > + target_ulong ret=0; > + switch (dcrn) { > + case 0xAA: /* CPCO_ECR */ > + break; > + case 0xB0: /* CPC0_PLLMR */ > + /* |--> {ForwardA, Backward} Divisor > + * ---- |--> Opb > + * ---> | x2 |-|--> ExtBus > + * ---- | ---- > + * CPU ---> | /4 | ---> PLB ------------> PCI > + * Clock ---- > + */ > + ret = -2 << 29;/* Forward divisor is 2 */ > + ret |= 2 << 25; /* Backward divisor is 2 */ > + ret |= 3 << 17; /* Cpu/Plb is 4 */ > + ret |= 0 << 13; /* Pci/Plb is 1 */ > + ret |= 0 << 11; /* ExtBus/Plb is 2 */ > + ret |= 1 << 15; /* Opb/Plb is 2 */ > + break; > + case 0xB1: /* CPC0_CR0 */ > + break; > + case 0xB2: /* CPC0_CR1 */ > + break; > + case 0xB4: /* CPC0_PSR: For same uP model, could be different! */ > + break; > + default: > + break; > + } > +/* printf("walnut_dcr [%x] => %x\n", dcrn, ret);*/ > + return ret; > +} > +static void walnut_dcr_write (void *opaque, int dcrn, target_ulong val) > +{ > +/* printf("walnut_dcr [%x] <= %x\n", dcrn, val);*/ > +} The CPC is an SoC device, not a board device. Accordingly, these functions shouldn't have "walnut" in the name. Also, some of these registers (CPC0_CR0, CR1, and PLLMR) are common with 405GP, and some (ECR, PSR) are specific to 405GPr. Some better code sharing is needed. > +/* > + * I2C Customization > + * > + * A file. > + */ > +static char PID_content[128]; /* If no file present */ > +static uint32_t PID_cursor; At the very least, these should be part of an I2C structure, so that multiple I2C controllers can be created. > +size_t PID_read(int addr, char *buf, size_t len) { > + int ret; > + ret = len; > + if (len+PID_cursor>128) > + len = 128 - PID_cursor; > + memcpy(buf, &PID_content[PID_cursor], len); Use a #define for "128". > +#if 0 > + { > + int i; > + printf("PID_Read dev(%x) len %d offs %d =>(", addr, ret, PID_cursor); > + for (i=0; i + printf(" %x,", buf[i]); > + } > + printf(") ret %d\n", ret); > + } > +#endif > + return ret; > +} > +size_t PID_write(int addr, char *buf, size_t len) { > + int ret; > + ret = len; > + /* buf = {offset} if write to set position (a read'll follow) > + * = {offset,data0, data1 ...} if write data different from position */ > + if (len>0) { > + PID_cursor = buf[0]; > + len--; > + if (len>0) { > + if (len+PID_cursor>128) > + len = 128 - PID_cursor; > + memcpy(&PID_content[PID_cursor], buf+1, len); > + } > + } > +#if 0 > + { > + int i; > + printf("PID_Write dev(0x%x) len %d, off %d <=(", addr, ret, PID_cursor); > + for (i=0; i + printf(" %x,", buf[i]); > + } > + printf(")\n"); > + } > +#endif > + > + return ret; > +} > +static void walnut_init (ram_addr_t ram_size, int vga_ram_size, > + const char *boot_device, DisplayState *ds, > + const char *kernel_filename, > + const char *kernel_cmdline, > + const char *initrd_filename, > + const char *cpu_model) > +{ > + ppc4xx_bd_info_t bd; > + CPUPPCState *env; > + qemu_irq *pic; > + ram_addr_t sram_offset, bios_offset, bdloc; > + target_phys_addr_t ram_bases[2], ram_sizes[2]; > + target_ulong sram_size, bios_size; > + //int phy_addr = 0; > + //static int phy_addr = 1; > + target_ulong kernel_base, kernel_size, initrd_base, initrd_size; > + int linux_boot; > + int fl_idx, fl_sectors, len; > + int ppc_boot_device = boot_device[0]; > + int index; > + > + /* XXX: fix this */ > + ram_bases[0] = 0x00000000; > + ram_sizes[0] = 0x08000000; > + ram_bases[1] = 0x00000000; > + ram_sizes[1] = 0x00000000; > + ram_size = 128 * 1024 * 1024; > +#ifdef DEBUG_BOARD_INIT > + printf("%s: register cpu\n", __func__); > +#endif > + env = ppc405xp_init("405GPe", ram_bases, ram_sizes, 33333333, &pic, &sram_offset, > + kernel_filename == NULL ? 0 : 1); > + /* Customize some dcr */ > + ppc_dcr_register(env, 0xAA/* CPC0_ECR */, NULL, &walnut_dcr_read, &walnut_dcr_write); > + ppc_dcr_register(env, 0xB0/* CPC0_PLLMR */, NULL, &walnut_dcr_read, NULL); > + ppc_dcr_register(env, 0xB1/* CPC0_CR0 */, NULL, &walnut_dcr_read, &walnut_dcr_write); > + ppc_dcr_register(env, 0xB2/* CPC0_CR1 */, NULL, &walnut_dcr_read, &walnut_dcr_write); > + ppc_dcr_register(env, 0xB4/* CPC0_PSR */, NULL, &walnut_dcr_read, NULL); > + > + /* Customize I2C access */ > + { > + extern KRHandler* readF; > + extern KRHandler* writeF; Get rid of these nested braces. "readF" and "writeF" are terrible names, especially considering that they're global! I'm not sure why you even have them; could you explain? It looks like you're trying to extend the I2C emulation so that some chips (405GPr) support functionality that other chips (405GP?) don't? > + /* PID */ > + //load_init_image("pid.bin", NULL, 256, 0xff); > + int PID_fd; > + memset(PID_content, 0, 128); > + if ((PID_fd=open("pid.bin", O_RDWR | O_BINARY, 0666))>=0) { /*"r+b");*/ > + int n = read(PID_fd, PID_content, 128); > + if (n<0) { > + printf("Error reading file pid.bin (ret %d)\n", n); > + } else if (n<128) { > + printf("Warning reading file pid.bin: only %d/128 bytes loaded\n", n); > + } > + close(PID_fd); > + } You should use load_image() here. Also, it looks like pid.bin is something that should live inside 'bios_dir', just like your "bios" loading below. What *is* pid.bin, anyways? How would I obtain/create my own, if I wanted to run your code? > + if (PID_fd<0) { > + /* Built in eeprom */ > + printf("Warning using builtin pid content!\n"); > + PID_content[2] = 4; /* Magic word, use sdram */ > + PID_content[3] = 12; /* Row word, use sdram */ > + PID_content[4] = 10; /* Col word, use sdram */ > + PID_content[5] = 1; /* 1 bank */ > + PID_content[6] = 32; /* Module width */ > + PID_content[31] = 0x10; /* How many 4MB is long */ > + PID_content[127]= 0 | 2 /* Cas latency */; > + } > + > + readF = &PID_read; > + writeF = &PID_write; > + } > + /* allocate SRAM */ > +#ifdef DEBUG_BOARD_INIT > + printf("%s: register SRAM at offset %08lx\n", __func__, sram_offset); > +#endif > + /* Last 48 (max 128) bytes) used for bd info */ This comment doesn't seem related to this code. > + sram_size = 8 * 1024; > + cpu_register_physical_memory(0x40000000, sram_size, > + sram_offset | IO_MEM_RAM); > + /* allocate and load BIOS */ > +#ifdef DEBUG_BOARD_INIT > + printf("%s: register BIOS\n", __func__); > +#endif > + bios_offset = sram_offset + sram_size; > + fl_idx = 0; > +#ifdef USE_FLASH_BIOS > + index = drive_get_index(IF_PFLASH, 0, fl_idx); > + if (index != -1) { > + bios_size = bdrv_getlength(drives_table[index].bdrv); > + fl_sectors = (bios_size + 65535) >> 16; > +#ifdef DEBUG_BOARD_INIT > + printf("Register parallel flash %d size " ADDRX " at offset %08lx " > + " addr " ADDRX " '%s' %d\n", > + fl_idx, bios_size, bios_offset, -bios_size, > + bdrv_get_device_name(drives_table[index].bdrv), fl_sectors); > +#endif > + pflash_cfi02_register((uint32_t)(-bios_size), bios_offset, > + drives_table[index].bdrv, 65536, fl_sectors, 1, > + 2, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA); > + fl_idx++; > + } else > +#endif > + { > + char buf[1024]; > + ram_addr_t flash_offset; > + target_ulong flash_size; > + > + flash_offset = bios_offset; > + flash_size = 512*1024; > +#ifdef DEBUG_BOARD_INIT > + printf("Load BIOS from file\n"); > +#endif > + if (bios_name == NULL) > + bios_name = BIOS_FILENAME; > + snprintf(buf, sizeof(buf), "%s/%s", bios_dir, bios_name); > + bios_size = get_image_size(buf); > + if (bios_size < 0 || bios_size > BIOS_SIZE) { > + fprintf(stderr, "qemu: could not load PowerPC bios '%s'\n", buf); > + exit(1); > + } > + bios_size = (bios_size + 0xfff) & ~0xfff; > +#ifdef DEBUG_BOARD_INIT > + printf("Loading bios@" ADDRX " %d bytes\n", (uint32_t)(-bios_size), bios_size); > +#endif > +#if 1 > + pflash_cfi02_register((uint32_t)(-flash_size), flash_offset, > + NULL, 65536 /*sector len*/, flash_size/65536 /* nblocks */, 1, > + 1 /* width */, 0x0001/* AMD */, 0x4F/* Am29LV040B */, 0x0000, 0x0000, 0x555/* unlock addrs */, 0x2AA); > +#else > + cpu_register_physical_memory((uint32_t)(-flash_size), > + flash_size, flash_offset | IO_MEM_ROM); > +#endif > + > + bios_size = load_image_targphys(buf, (-bios_size), BIOS_SIZE/* max sz */); > + if (bios_size < 0 || bios_size > BIOS_SIZE) { > + fprintf(stderr, "qemu: could not load PowerPC bios '%s'\n", buf); > + exit(1); > + } > + bios_size = flash_size; > + } > + bios_offset += bios_size; > + /* Register FPGA */ > +#ifdef DEBUG_BOARD_INIT > + printf("%s: register FPGA\n", __func__); > +#endif > + ref405ep_fpga_init(0xF0300000); > + /* Register NVRAM */ > +#ifdef DEBUG_BOARD_INIT > + printf("%s: register NVRAM\n", __func__); > +#endif > + m48t59_init(NULL, 0xF0000000, 0, 8192, 8); > + /* Load kernel */ > + linux_boot = (kernel_filename != NULL); > + if (0) { > +#ifdef DEBUG_BOARD_INIT > + printf("%s: load kernel\n", __func__); > +#endif > + memset(&bd, 0, sizeof(bd)); > + bd.bi_memstart = 0x00000000; > + bd.bi_memsize = ram_size; > + bd.bi_flashstart = -bios_size; > + bd.bi_flashsize = -bios_size; > + bd.bi_flashoffset = 0; > + bd.bi_sramstart = 0xFFF00000; > + bd.bi_sramsize = sram_size; > + bd.bi_bootflags = 0; > + bd.bi_intfreq = 133333333; > + bd.bi_busfreq = 33333333; > + bd.bi_baudrate = 115200; > + bd.bi_s_version[0] = 'Q'; > + bd.bi_s_version[1] = 'M'; > + bd.bi_s_version[2] = 'U'; > + bd.bi_s_version[3] = '\0'; > + bd.bi_r_version[0] = 'Q'; > + bd.bi_r_version[1] = 'E'; > + bd.bi_r_version[2] = 'M'; > + bd.bi_r_version[3] = 'U'; > + bd.bi_r_version[4] = '\0'; > + bd.bi_procfreq = 133333333; > + bd.bi_plb_busfreq = 33333333; > + bd.bi_pci_busfreq = 33333333; > + bd.bi_opbfreq = 33333333; > + bdloc = ppc405_set_bootinfo(env, &bd, 0x00000001); > + env->gpr[3] = bdloc; > + kernel_base = KERNEL_LOAD_ADDR; > + /* now we can load the kernel */ > + kernel_size = load_image(kernel_filename, phys_ram_base + kernel_base); > + if (kernel_size < 0) { > + fprintf(stderr, "qemu: could not load kernel '%s'\n", > + kernel_filename); > + exit(1); > + } > + printf("Load kernel size " TARGET_FMT_ld " at " TARGET_FMT_lx > + " %02x %02x %02x %02x\n", kernel_size, kernel_base, > + *(char *)(phys_ram_base + kernel_base), > + *(char *)(phys_ram_base + kernel_base + 1), > + *(char *)(phys_ram_base + kernel_base + 2), > + *(char *)(phys_ram_base + kernel_base + 3)); > + /* load initrd */ > + if (initrd_filename) { > + initrd_base = INITRD_LOAD_ADDR; > + initrd_size = load_image(initrd_filename, > + phys_ram_base + initrd_base); > + if (initrd_size < 0) { > + fprintf(stderr, "qemu: could not load initial ram disk '%s'\n", > + initrd_filename); > + exit(1); > + } > + } else { > + initrd_base = 0; > + initrd_size = 0; > + } > + env->gpr[4] = initrd_base; > + env->gpr[5] = initrd_size; > + ppc_boot_device = 'm'; > + if (kernel_cmdline != NULL) { > + len = strlen(kernel_cmdline); > + bdloc -= ((len + 255) & ~255); > + memcpy(phys_ram_base + bdloc, kernel_cmdline, len + 1); > + env->gpr[6] = bdloc; > + env->gpr[7] = bdloc + len; > + } else { > + env->gpr[6] = 0; > + env->gpr[7] = 0; > + } > + env->nip = KERNEL_LOAD_ADDR; > + } else { > + env->nip = 0xFFFFFFFC; > + } > +#ifdef DEBUG_BOARD_INIT > + printf("%s: Done\n", __func__); > +#endif > + /*printf("bdloc %016lx %s\n", > + (unsigned long)bdloc, (char *)(phys_ram_base + bdloc));*/ > +} A *ton* of this code seems to be nearly identical to ref405ep_init(). How can you better share it? > +QEMUMachine walnut_machine = { > + .name = "walnut", > + .desc = "DHT Walnut board, based on ppc405GPe", > + .init = walnut_init, > + .ram_require = (128 * 1024 * 1024 + 4096 + 512 * 1024 + BIOS_SIZE) | RAMSIZE_FIXED, > +}; Is that really the *minimum* memory possible? Why couldn't I use less RAM? I'll make comments on your EMAC code in another email. -- Hollis Blanchard IBM Linux Technology Center