[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] DHT Walnut board support
|
From: |
Hollis Blanchard |
|
Subject: |
Re: [Qemu-devel] DHT Walnut board support |
|
Date: |
Fri, 21 Nov 2008 10:40:31 -0600 |
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 <sys/types.h>
> #include <sys/statvfs.h>
> #endif
> +#include <assert.h>
>
> #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<ret; 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<ret; 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