Otherwise GCC 4.2.4 complains that panic() marked noreturn does return.
Signed-off-by: Andreas Färber andreas.faerber@web.de --- arch/ppc/qemu/kernel.h | 2 +- arch/ppc/qemu/qemu.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/ppc/qemu/kernel.h b/arch/ppc/qemu/kernel.h index e8ae364..fe9be83 100644 --- a/arch/ppc/qemu/kernel.h +++ b/arch/ppc/qemu/kernel.h @@ -17,7 +17,7 @@
/* misc.c */ extern void fatal_error( const char *str ); -extern void exit( int status ); +extern void exit( int status ) __attribute__ ((noreturn));
/* start.S */ extern void flush_icache_range( char *start, char *stop ); diff --git a/arch/ppc/qemu/qemu.c b/arch/ppc/qemu/qemu.c index 1f785ac..208669c 100644 --- a/arch/ppc/qemu/qemu.c +++ b/arch/ppc/qemu/qemu.c @@ -35,7 +35,7 @@ void exit( int status __attribute__ ((unused))) { for (;;); -} __attribute__ ((noreturn)) +}
void fatal_error( const char *err )
On 17.10.2010, at 14:05, Andreas Färber wrote:
Otherwise GCC 4.2.4 complains that panic() marked noreturn does return.
Thanks, that one hit me too and I didn't know how to fix it properly :). Applied.
Alex
Am 17.10.2010 um 14:16 schrieb Alexander Graf:
On 17.10.2010, at 14:05, Andreas Färber wrote:
Otherwise GCC 4.2.4 complains that panic() marked noreturn does return.
Thanks, that one hit me too and I didn't know how to fix it properly :). Applied.
Thanks. In case you're playing with ppc64 I'm attaching my current diff. It made it compile but it hangs both ppc and ppc64, no time to investigate further right now.
Most problems were caused by the assumption that a conversion between ucell und void* is possible; I added (unsigned long) casts or reused the cell2pointer/pointer2cell macros. I also played with a new addr_t type as suggested earlier, to allow 64-bit PCI addresses in the drivers; ofmem is still missing this capability. I ran into the same relocation problem for the vectors 0x300 and 0x400 I posted for - mcpu=powerpc64; I hacked around it by temporarily placing the epilogue into the vector itself.
Not sure if we need to worry about ppc64 function descriptors somewhere?
And might we need to turn on LF=1 before entering 64-bit C code?
Andreas
Am 17.10.2010 um 15:20 schrieb Andreas Färber:
I'm attaching my current diff. It made it compile but it hangs both ppc and ppc64, no time to investigate further right now.
Noticed that I was still using 0 in place of t+3 in internal.c (the question I posted elsewhere); undoing that for ppc doesn't help.
Andreas
diff --git a/kernel/internal.c b/kernel/internal.c index bc928bf..5a293e5 100644 --- a/kernel/internal.c +++ b/kernel/internal.c @@ -50,7 +50,11 @@ char xtname[MAXNFALEN]; /* instead of pointing to an explicit 0 variable we * point behind the pointer. */ +#ifdef __powerpc64__ static ucell t[] = { DOCOL, 0, /*(ucell)(t+3)*/0, 0 }; +#else +static ucell t[] = { DOCOL, 0, (ucell)(t+3), 0 }; +#endif static ucell *trampoline = t; #endif
On Sun, Oct 17, 2010 at 1:20 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 17.10.2010 um 14:16 schrieb Alexander Graf:
On 17.10.2010, at 14:05, Andreas Färber wrote:
Otherwise GCC 4.2.4 complains that panic() marked noreturn does return.
Thanks, that one hit me too and I didn't know how to fix it properly :). Applied.
Thanks. In case you're playing with ppc64 I'm attaching my current diff. It made it compile but it hangs both ppc and ppc64, no time to investigate further right now.
Most problems were caused by the assumption that a conversion between ucell und void* is possible; I added (unsigned long) casts or reused the cell2pointer/pointer2cell macros.
Those should be merged with kernel/cross.h ones and used everywhere.
I also played with a new addr_t type as
Please use the standard name intptr_t and remove the casts where possible: - unsigned char *dest = (unsigned char *)POP(); + unsigned char *dest = cell2pointer(POP());
Am 17.10.2010 um 15:45 schrieb Blue Swirl:
On Sun, Oct 17, 2010 at 1:20 PM, Andreas Färber <andreas.faerber@web.de
wrote: Am 17.10.2010 um 14:16 schrieb Alexander Graf:
On 17.10.2010, at 14:05, Andreas Färber wrote:
Otherwise GCC 4.2.4 complains that panic() marked noreturn does return.
Thanks, that one hit me too and I didn't know how to fix it properly :). Applied.
Thanks. In case you're playing with ppc64 I'm attaching my current diff. It made it compile but it hangs both ppc and ppc64, no time to investigate further right now.
Most problems were caused by the assumption that a conversion between ucell und void* is possible; I added (unsigned long) casts or reused the cell2pointer/pointer2cell macros.
Those should be merged with kernel/cross.h ones and used everywhere.
Any particular wishes how? Adding the include path for cross.h everywhere would have required to re-run switch-arch and rebuild everything, so I avoided it. Should the macros be moved into include/ kernel/stack.h alongside PUSH/POP maybe?
I also played with a new addr_t type as
Please use the standard name intptr_t and remove the casts where possible:
unsigned char *dest = (unsigned char *)POP();
unsigned char *dest = cell2pointer(POP());
Thanks for the suggestion!
Andreas
On Sun, Oct 17, 2010 at 5:03 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 17.10.2010 um 15:45 schrieb Blue Swirl:
On Sun, Oct 17, 2010 at 1:20 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 17.10.2010 um 14:16 schrieb Alexander Graf:
On 17.10.2010, at 14:05, Andreas Färber wrote:
Otherwise GCC 4.2.4 complains that panic() marked noreturn does return.
Thanks, that one hit me too and I didn't know how to fix it properly :). Applied.
Thanks. In case you're playing with ppc64 I'm attaching my current diff. It made it compile but it hangs both ppc and ppc64, no time to investigate further right now.
Most problems were caused by the assumption that a conversion between ucell und void* is possible; I added (unsigned long) casts or reused the cell2pointer/pointer2cell macros.
Those should be merged with kernel/cross.h ones and used everywhere.
Any particular wishes how? Adding the include path for cross.h everywhere would have required to re-run switch-arch and rebuild everything, so I avoided it. Should the macros be moved into include/kernel/stack.h alongside PUSH/POP maybe?
Perhaps, or have that include cross.h.
Cc: Blue Swirl blauwirbel@gmail.com Signed-off-by: Andreas Färber andreas.faerber@web.de --- include/arch/ppc/types.h | 2 ++ include/arch/sparc32/types.h | 2 ++ include/arch/sparc64/types.h | 2 ++ 3 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/include/arch/ppc/types.h b/include/arch/ppc/types.h index cb999dc..d6df2a1 100644 --- a/include/arch/ppc/types.h +++ b/include/arch/ppc/types.h @@ -18,11 +18,13 @@ typedef unsigned char uint8_t; typedef unsigned short uint16_t; typedef unsigned int uint32_t; typedef unsigned long long uint64_t; +typedef unsigned long uintptr_t;
typedef signed char int8_t; typedef short int16_t; typedef int int32_t; typedef long long int64_t; +typedef long intptr_t; #endif
/* endianess */ diff --git a/include/arch/sparc32/types.h b/include/arch/sparc32/types.h index 3f89da4..500bcd1 100644 --- a/include/arch/sparc32/types.h +++ b/include/arch/sparc32/types.h @@ -18,11 +18,13 @@ typedef unsigned char uint8_t; typedef unsigned short uint16_t; typedef unsigned int uint32_t; typedef unsigned long long uint64_t; +typedef unsigned long uintptr_t;
typedef signed char int8_t; typedef short int16_t; typedef int int32_t; typedef long long int64_t; +typedef long intptr_t; #endif
/* endianess */ diff --git a/include/arch/sparc64/types.h b/include/arch/sparc64/types.h index 67fea5c..112b53e 100644 --- a/include/arch/sparc64/types.h +++ b/include/arch/sparc64/types.h @@ -18,11 +18,13 @@ typedef unsigned char uint8_t; typedef unsigned short uint16_t; typedef unsigned int uint32_t; typedef unsigned long long uint64_t; +typedef unsigned long uintptr_t;
typedef signed char int8_t; typedef short int16_t; typedef int int32_t; typedef long long int64_t; +typedef long intptr_t; #endif
/* endianess */
These will be needed elsewhere for ppc64.
Cc: Blue Swirl blauwirbel@gmail.com Signed-off-by: Andreas Färber andreas.faerber@web.de --- NB: macros are not yet changed here.
include/kernel/stack.h | 12 ++++++++++++ kernel/cross.h | 6 ------ 2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/include/kernel/stack.h b/include/kernel/stack.h index 9fe52ce..809ffe9 100644 --- a/include/kernel/stack.h +++ b/include/kernel/stack.h @@ -30,6 +30,18 @@ typedef ucell phandle_t;
+#ifdef NATIVE_BITWIDTH_EQUALS_HOST_BITWIDTH +#define pointer2cell(x) ((ucell)(x)) +#define cell2pointer(x) ((u8 *)(x)) +#endif +#ifdef NATIVE_BITWIDTH_SMALLER_THAN_HOST_BITWIDTH +#define pointer2cell(x) ((ucell)(((unsigned long)(x))-base_address)) +#define cell2pointer(x) ((u8 *)(((unsigned long)(x))+base_address)) +#endif +#ifdef NATIVE_BITWIDTH_LARGER_THAN_HOST_BITWIDTH +#define pointer2cell(x) ((ucell)(unsigned long)(x)) +#define cell2pointer(x) ((u8 *)((unsigned long)(x)&0xFFFFFFFFUL)) +#endif
static inline void PUSH(ucell value) { dstack[++dstackcnt] = (value); diff --git a/kernel/cross.h b/kernel/cross.h index d4cfff1..1b7d448 100644 --- a/kernel/cross.h +++ b/kernel/cross.h @@ -103,8 +103,6 @@ /* bit width handling */
#ifdef NATIVE_BITWIDTH_EQUALS_HOST_BITWIDTH -#define pointer2cell(x) ((ucell)(x)) -#define cell2pointer(x) ((u8 *)(x)) #if BITS==32 #define FMT_CELL_x "x" #define FMT_CELL_d "d" @@ -116,15 +114,11 @@
#ifdef NATIVE_BITWIDTH_SMALLER_THAN_HOST_BITWIDTH extern unsigned long base_address; -#define pointer2cell(x) ((ucell)(((unsigned long)(x))-base_address)) -#define cell2pointer(x) ((u8 *)(((unsigned long)(x))+base_address)) #define FMT_CELL_x "x" #define FMT_CELL_d "d" #endif
#ifdef NATIVE_BITWIDTH_LARGER_THAN_HOST_BITWIDTH -#define pointer2cell(x) ((ucell)(unsigned long)(x)) -#define cell2pointer(x) ((u8 *)((unsigned long)(x)&0xFFFFFFFFUL)) #define FMT_CELL_x "llx" #define FMT_CELL_d "lld" #endif
On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, direct casts result in warnings, treated as errors.
Use [u]intptr_t cast or cell2pointer and pointer2cell macros as necessary.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Mark Cave-Ayland mark.cave-ayland@siriusit.co.uk Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de --- Hey,
I've compile-tested ppc, ppc64, sparc32, sparc64; they all appeared to boot as before now (Haiku, AIX, Debian, Solaris, Milax guests).
@Mark: I've taken the liberty of using push_str() in ofmem_set_property() to avoid duplicating the conversion.
Cheers, Andreas
arch/ppc/qemu/kernel.c | 16 ++++++++-------- arch/ppc/qemu/main.c | 2 +- arch/ppc/qemu/methods.c | 4 ++-- arch/ppc/qemu/ofmem.c | 2 +- drivers/adb_kbd.c | 6 +++--- drivers/cuda.c | 4 ++-- drivers/escc.c | 20 ++++++++++---------- drivers/ide.c | 2 +- drivers/macio.c | 8 ++++---- drivers/macio.h | 6 +++--- fs/ext2/ext2_fs.c | 8 ++++---- fs/grubfs/grubfs_fs.c | 8 ++++---- fs/hfs/hfs_fs.c | 10 +++++----- fs/hfsplus/hfsp_fs.c | 10 +++++----- fs/iso9660/iso9660_fs.c | 4 ++-- include/arch/ppc/io.h | 18 +++++++++--------- include/drivers/drivers.h | 6 +++--- include/kernel/stack.h | 4 ++-- kernel/internal.c | 18 +++++++++--------- libc/diskio.c | 8 ++++---- libc/extra.c | 2 +- libopenbios/bindings.c | 20 ++++++++++---------- libopenbios/bootinfo_load.c | 2 +- libopenbios/elf_load.c | 4 ++-- libopenbios/initprogram.c | 12 ++++++------ libopenbios/ofmem_common.c | 7 +++---- libopenbios/xcoff_load.c | 14 +++++++------- packages/deblocker.c | 4 ++-- packages/disk-label.c | 4 ++-- packages/mac-parts.c | 4 ++-- packages/nvram.c | 8 ++++---- packages/pc-parts.c | 4 ++-- packages/video.c | 4 ++-- 33 files changed, 126 insertions(+), 127 deletions(-)
diff --git a/arch/ppc/qemu/kernel.c b/arch/ppc/qemu/kernel.c index cafa167..dbfca57 100644 --- a/arch/ppc/qemu/kernel.c +++ b/arch/ppc/qemu/kernel.c @@ -40,13 +40,13 @@ forth_segv_handler( char *segv_addr ) { ucell addr = 0xdeadbeef;
- if( PC >= (ucell) dict && PC <= (ucell) dict + dicthead ) - addr = *(ucell *) PC; + if( PC >= pointer2cell(dict) && PC <= pointer2cell(dict) + dicthead ) + addr = *(ucell *)cell2pointer(PC);
- printk("panic: segmentation violation at %x\n", (int)segv_addr); - printk("dict=0x%x here=0x%x(dict+0x%x) pc=0x%x(dict+0x%x)\n", - (int)dict, (int)(dict + dicthead), dicthead, - PC, PC - (ucell) dict); + printk("panic: segmentation violation at 0x%p\n", segv_addr); + printk("dict=0x%p here=0x%p(dict+0x%x) pc=0x%x(dict+0x%x)\n", + dict, (char*)dict + dicthead, dicthead, + PC, PC - pointer2cell(dict)); printk("dstackcnt=%d rstackcnt=%d instruction=%x\n", dstackcnt, rstackcnt, addr);
@@ -75,8 +75,8 @@ init_memory( void ) * to initialize the memory allocator */
- PUSH( (ucell)memory ); - PUSH( (ucell)memory + MEMORY_SIZE ); + PUSH( pointer2cell(memory) ); + PUSH( pointer2cell(memory) + MEMORY_SIZE ); }
int diff --git a/arch/ppc/qemu/main.c b/arch/ppc/qemu/main.c index 2d4ae5a..f9c1a53 100644 --- a/arch/ppc/qemu/main.c +++ b/arch/ppc/qemu/main.c @@ -183,7 +183,7 @@ static void check_preloaded_kernel(void) kernel_size = fw_cfg_read_i32(FW_CFG_KERNEL_SIZE); if (kernel_size) { kernel_image = fw_cfg_read_i32(FW_CFG_KERNEL_ADDR); - kernel_cmdline = (const char *) fw_cfg_read_i32(FW_CFG_KERNEL_CMDLINE); + kernel_cmdline = (const char *)(uintptr_t) fw_cfg_read_i32(FW_CFG_KERNEL_CMDLINE); initrd_image = fw_cfg_read_i32(FW_CFG_INITRD_ADDR); initrd_size = fw_cfg_read_i32(FW_CFG_INITRD_SIZE); printk("[ppc] Kernel already loaded (0x%8.8lx + 0x%8.8lx) " diff --git a/arch/ppc/qemu/methods.c b/arch/ppc/qemu/methods.c index 71a364f..bbb101f 100644 --- a/arch/ppc/qemu/methods.c +++ b/arch/ppc/qemu/methods.c @@ -71,7 +71,7 @@ static void tty_read( void ) { int ch, len = POP(); - char *p = (char*)POP(); + char *p = (char*)cell2pointer(POP()); int ret=0;
if( len > 0 ) { @@ -91,7 +91,7 @@ static void tty_write( void ) { int i, len = POP(); - char *p = (char*)POP(); + char *p = (char*)cell2pointer(POP()); for( i=0; i<len; i++ ) putchar( *p++ ); RET( len ); diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c index 189dae3..a507009 100644 --- a/arch/ppc/qemu/ofmem.c +++ b/arch/ppc/qemu/ofmem.c @@ -105,7 +105,7 @@ static inline size_t ALIGN_SIZE(size_t x, size_t a)
ofmem_t* ofmem_arch_get_private(void) { - return (ofmem_t*)(get_heap_top() - OFMEM_SIZE); + return (ofmem_t*)cell2pointer(get_heap_top() - OFMEM_SIZE); }
void* ofmem_arch_get_malloc_base(void) diff --git a/drivers/adb_kbd.c b/drivers/adb_kbd.c index e65ba71..10a5197 100644 --- a/drivers/adb_kbd.c +++ b/drivers/adb_kbd.c @@ -483,7 +483,7 @@ static int adb_kbd_read (void *private) int key; int ret;
- kbd = (void *)dev->state; + kbd = (void *)(uintptr_t)dev->state;
if (kbd->len > 0) { ret = kbd->sequence[kbd->len-- - 1]; @@ -531,7 +531,7 @@ void *adb_kbd_new (char *path, void *private) ADB_kbd_us, ADB_sequences); kbd->next_key = -1; kbd->len = 0; - dev->state = (int32_t)kbd; + dev->state = (uint32_t)(uintptr_t)kbd; my_adb_dev = dev; }
@@ -556,7 +556,7 @@ static void keyboard_read(void) char *addr; int len, key, i; len=POP(); - addr=(char *)POP(); + addr=(char *)cell2pointer(POP());
for (i = 0; i < len; i++) { key = adb_kbd_read(my_adb_dev); diff --git a/drivers/cuda.c b/drivers/cuda.c index c254720..4d784de 100644 --- a/drivers/cuda.c +++ b/drivers/cuda.c @@ -65,12 +65,12 @@
static uint8_t cuda_readb (cuda_t *dev, int reg) { - return *(volatile uint8_t *)(dev->base + reg); + return *(volatile uint8_t *)(uintptr_t)(dev->base + reg); }
static void cuda_writeb (cuda_t *dev, int reg, uint8_t val) { - *(volatile uint8_t *)(dev->base + reg) = val; + *(volatile uint8_t *)(uintptr_t)(dev->base + reg) = val; }
static void cuda_wait_irq (cuda_t *dev) diff --git a/drivers/escc.c b/drivers/escc.c index dbf8262..c70e7ea 100644 --- a/drivers/escc.c +++ b/drivers/escc.c @@ -54,19 +54,19 @@ static volatile unsigned char *serial_dev; #define Rx_CH_AV 0x1 /* Rx Character Available */ #define Tx_BUF_EMP 0x4 /* Tx Buffer empty */
-int uart_charav(int port) +int uart_charav(uintptr_t port) { return (CTRL(port) & Rx_CH_AV) != 0; }
-char uart_getchar(int port) +char uart_getchar(uintptr_t port) { while (!uart_charav(port)) ; return DATA(port) & 0177; }
-static void uart_putchar(int port, unsigned char c) +static void uart_putchar(uintptr_t port, unsigned char c) { if (!serial_dev) return; @@ -102,7 +102,7 @@ static void uart_init_line(volatile unsigned char *port, unsigned long baud)
}
-int uart_init(uint64_t port, unsigned long speed) +int uart_init(uintptr_t port, unsigned long speed) { #ifdef CONFIG_DRIVER_ESCC_SUN serial_dev = map_io(port & ~7ULL, ZS_REGS); @@ -116,7 +116,7 @@ int uart_init(uint64_t port, unsigned long speed)
void serial_putchar(int c) { - uart_putchar((int)serial_dev, (unsigned char) (c & 0xff)); + uart_putchar((uintptr_t)serial_dev, (unsigned char) (c & 0xff)); }
void serial_cls(void) @@ -137,10 +137,10 @@ escc_read(unsigned long *address) int len;
len = POP(); - addr = (char *)POP(); + addr = (char *)cell2pointer(POP());
if (len < 1) - printk("escc_read: bad len, addr %x len %x\n", (unsigned int)addr, len); + printk("escc_read: bad len, addr %p len %x\n", addr, len);
if (uart_charav(*address)) { *addr = (char)uart_getchar(*address); @@ -158,7 +158,7 @@ escc_write(unsigned long *address) int i, len;
len = POP(); - addr = (unsigned char *)POP(); + addr = (unsigned char *)cell2pointer(POP());
for (i = 0; i < len; i++) { uart_putchar(*address, addr[i]); @@ -369,7 +369,7 @@ ob_zs_init(uint64_t base, uint64_t offset, int intr, int slave, int keyboard) #else
static void -escc_add_channel(const char *path, const char *node, uint32_t addr, +escc_add_channel(const char *path, const char *node, uintptr_t addr, uint32_t offset) { char buf[64], tty[32]; @@ -430,7 +430,7 @@ escc_add_channel(const char *path, const char *node, uint32_t addr, }
void -escc_init(const char *path, unsigned long addr) +escc_init(const char *path, uintptr_t addr) { char buf[64]; int props[2]; diff --git a/drivers/ide.c b/drivers/ide.c index 7006720..0d09bcd 100644 --- a/drivers/ide.c +++ b/drivers/ide.c @@ -1190,7 +1190,7 @@ ob_ide_read_blocks(int *idx) { cell n = POP(), cnt=n; ucell blk = POP(); - unsigned char *dest = (unsigned char *)POP(); + unsigned char *dest = (unsigned char *)cell2pointer(POP()); struct ide_drive *drive = *(struct ide_drive **)idx;
IDE_DPRINTF("ob_ide_read_blocks %lx block=%ld n=%ld\n", diff --git a/drivers/macio.c b/drivers/macio.c index d6d1696..55528c1 100644 --- a/drivers/macio.c +++ b/drivers/macio.c @@ -43,7 +43,7 @@ arch_nvram_size( void ) return NW_IO_NVRAM_SIZE >> NW_IO_NVRAM_SHIFT; }
-void macio_nvram_init(const char *path, uint32_t addr) +void macio_nvram_init(const char *path, uintptr_t addr) { phandle_t chosen, aliases; phandle_t dnode; @@ -145,7 +145,7 @@ arch_nvram_get( char *buf ) }
static void -openpic_init(const char *path, uint32_t addr) +openpic_init(const char *path, uintptr_t addr) { phandle_t target_node; phandle_t dnode; @@ -238,7 +238,7 @@ NODE_METHODS(ob_macio) = { };
void -ob_macio_heathrow_init(const char *path, uint32_t addr) +ob_macio_heathrow_init(const char *path, uintptr_t addr) { phandle_t aliases;
@@ -253,7 +253,7 @@ ob_macio_heathrow_init(const char *path, uint32_t addr) }
void -ob_macio_keylargo_init(const char *path, uint32_t addr) +ob_macio_keylargo_init(const char *path, uintptr_t addr) { phandle_t aliases;
diff --git a/drivers/macio.h b/drivers/macio.h index d580387..9fcd167 100644 --- a/drivers/macio.h +++ b/drivers/macio.h @@ -1,5 +1,5 @@ extern phandle_t pic_handle;
-void ob_macio_heathrow_init(const char *path, uint32_t addr); -void ob_macio_keylargo_init(const char *path, uint32_t addr); -void macio_nvram_init(const char *path, uint32_t addr); +void ob_macio_heathrow_init(const char *path, uintptr_t addr); +void ob_macio_keylargo_init(const char *path, uintptr_t addr); +void macio_nvram_init(const char *path, uintptr_t addr); diff --git a/fs/ext2/ext2_fs.c b/fs/ext2/ext2_fs.c index 970fb4d..66eb0b4 100644 --- a/fs/ext2/ext2_fs.c +++ b/fs/ext2/ext2_fs.c @@ -144,7 +144,7 @@ static void ext2_files_read( ext2_info_t *mi ) { int count = POP(); - char *buf = (char *)POP(); + char *buf = (char *)cell2pointer(POP());
ext2_COMMON *common = mi->common; if (common->type != FILE) @@ -177,7 +177,7 @@ ext2_files_seek( ext2_info_t *mi ) static void ext2_files_load( ext2_info_t *mi ) { - char *buf = (char *)POP(); + char *buf = (char *)cell2pointer(POP()); int count;
ext2_COMMON *common = mi->common; @@ -201,14 +201,14 @@ ext2_files_get_path( ext2_info_t *mi ) if (common->type != FILE) RET( 0 );
- RET( (ucell) strdup(common->file->path) ); + RET( pointer2cell(strdup(common->file->path)) ); }
/* ( -- cstr ) */ static void ext2_files_get_fstype( ext2_info_t *mi ) { - PUSH( (ucell)strdup("ext2") ); + PUSH( pointer2cell(strdup("ext2")) ); }
/* static method, ( pathstr len ihandle -- ) */ diff --git a/fs/grubfs/grubfs_fs.c b/fs/grubfs/grubfs_fs.c index fc61943..acd2a64 100644 --- a/fs/grubfs/grubfs_fs.c +++ b/fs/grubfs/grubfs_fs.c @@ -238,7 +238,7 @@ static void grubfs_files_read( grubfs_info_t *mi ) { int count = POP(); - char *buf = (char *)POP(); + char *buf = (char *)cell2pointer(POP());
grubfile_t *file = mi->gfs->fd; int ret; @@ -295,7 +295,7 @@ grubfs_files_seek( grubfs_info_t *mi ) static void grubfs_files_load( grubfs_info_t *mi ) { - char *buf = (char *)POP(); + char *buf = (char *)cell2pointer(POP()); int count, ret;
grubfile_t *file = mi->gfs->fd; @@ -314,7 +314,7 @@ grubfs_files_get_path( grubfs_info_t *mi ) grubfile_t *file = mi->gfs->fd; const char *path = file->path;
- RET( (ucell) strdup(path) ); + RET( pointer2cell(strdup(path)) ); }
/* ( -- cstr ) */ @@ -323,7 +323,7 @@ grubfs_files_get_fstype( grubfs_info_t *mi ) { grubfs_t *gfs = mi->gfs;
- PUSH( (ucell)strdup(gfs->fsys->name) ); + PUSH( pointer2cell(strdup(gfs->fsys->name)) ); }
diff --git a/fs/hfs/hfs_fs.c b/fs/hfs/hfs_fs.c index 293dc18..2d62ec0 100644 --- a/fs/hfs/hfs_fs.c +++ b/fs/hfs/hfs_fs.c @@ -359,7 +359,7 @@ static void hfs_files_read( hfs_info_t *mi ) { int count = POP(); - char *buf = (char *)POP(); + char *buf = (char *)cell2pointer(POP());
hfscommon *common = mi->common; if (common->type != FILE) @@ -402,7 +402,7 @@ hfs_files_seek( hfs_info_t *mi ) static void hfs_files_load( hfs_info_t *mi ) { - char *buf = (char *)POP(); + char *buf = (char *)cell2pointer(POP()); int count;
hfscommon *common = mi->common; @@ -461,14 +461,14 @@ hfs_files_get_path( hfs_info_t *mi ) if( strlen(buf) >= sizeof(buf) ) RET( 0 );
- RET( (ucell) strdup(buf+start) ); + RET( pointer2cell(strdup(buf+start)) ); }
/* ( -- cstr ) */ static void hfs_files_get_fstype( hfs_info_t *mi ) { - PUSH( (ucell)strdup("HFS") ); + PUSH( pointer2cell(strdup("HFS")) ); }
/* ( -- cstr|0 ) */ @@ -486,7 +486,7 @@ hfs_files_volume_name( hfs_info_t *mi ) volname[0] = '\0'; }
- PUSH ((ucell)volname); + PUSH(pointer2cell(volname)); }
/* static method, ( pathstr len ihandle -- ) */ diff --git a/fs/hfsplus/hfsp_fs.c b/fs/hfsplus/hfsp_fs.c index 9c3deb3..83e4d8d 100644 --- a/fs/hfsplus/hfsp_fs.c +++ b/fs/hfsplus/hfsp_fs.c @@ -264,7 +264,7 @@ static void hfsp_files_read( hfsp_info_t *mi ) { int count = POP(); - char *buf = (char *)POP(); + char *buf = (char *)cell2pointer(POP());
hfsp_file_t *t = mi->hfspfile; volume *vol = t->rec.tree->vol; @@ -350,7 +350,7 @@ hfsp_files_seek( hfsp_info_t *mi ) static void hfsp_files_load( hfsp_info_t *mi ) { - char *buf = (char *)POP(); + char *buf = (char *)cell2pointer(POP());
hfsp_file_t *t = mi->hfspfile; volume *vol = t->rec.tree->vol; @@ -388,7 +388,7 @@ hfsp_files_load( hfsp_info_t *mi ) static void hfsp_files_get_fstype( hfsp_info_t *mi ) { - PUSH( (ucell)strdup("HFS+") ); + PUSH( pointer2cell(strdup("HFS+")) ); }
/* ( -- cstr ) */ @@ -405,7 +405,7 @@ hfsp_files_get_path( hfsp_info_t *mi ) strncpy( buf, t->path, strlen(t->path) ); buf[strlen(t->path)] = 0;
- PUSH ((ucell)buf); + PUSH(pointer2cell(buf)); }
/* ( -- success? ) */ @@ -434,7 +434,7 @@ hfsp_files_volume_name( hfsp_info_t *mi ) volname[0] = '\0'; }
- PUSH ((ucell)volname); + PUSH(pointer2cell(volname)); }
/* static method, ( pathstr len ihandle -- ) */ diff --git a/fs/iso9660/iso9660_fs.c b/fs/iso9660/iso9660_fs.c index 3fb5c16..d6f547b 100644 --- a/fs/iso9660/iso9660_fs.c +++ b/fs/iso9660/iso9660_fs.c @@ -90,7 +90,7 @@ static void iso9660_files_read( iso9660_info_t *mi ) { int count = POP(); - char *buf = (char *)POP(); + char *buf = (char *)cell2pointer(POP()); int ret;
if ( mi->common->type != FILE ) @@ -137,7 +137,7 @@ iso9660_files_offset( iso9660_info_t *mi ) static void iso9660_files_load( iso9660_info_t *mi) { - char *buf = (char*)POP(); + char *buf = (char*)cell2pointer(POP()); int ret, size;
if ( mi->common->type != FILE ) diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index d7c78a0..e5180f2 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -22,15 +22,15 @@ extern uint32_t isa_io_base; * are arrays of bytes, and byte-swapping is not appropriate in * that case. - paulus */ -#define insw(port, buf, ns) _insw((uint16_t *)((port)+isa_io_base), (buf), (ns)) -#define outsw(port, buf, ns) _outsw((uint16_t *)((port)+isa_io_base), (buf), (ns)) - -#define inb(port) in_8((uint8_t *)((port)+isa_io_base)) -#define outb(val, port) out_8((uint8_t *)((port)+isa_io_base), (val)) -#define inw(port) in_le16((uint16_t *)((port)+isa_io_base)) -#define outw(val, port) out_le16((uint16_t *)((port)+isa_io_base), (val)) -#define inl(port) in_le32((uint32_t *)((port)+isa_io_base)) -#define outl(val, port) out_le32((uint32_t *)((port)+isa_io_base), (val)) +#define insw(port, buf, ns) _insw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) +#define outsw(port, buf, ns) _outsw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) + +#define inb(port) in_8((uint8_t *)(uintptr_t)((port)+isa_io_base)) +#define outb(val, port) out_8((uint8_t *)(uintptr_t)((port)+isa_io_base), (val)) +#define inw(port) in_le16((uint16_t *)(uintptr_t)((port)+isa_io_base)) +#define outw(val, port) out_le16((uint16_t *)(uintptr_t)((port)+isa_io_base), (val)) +#define inl(port) in_le32((uint32_t *)(uintptr_t)((port)+isa_io_base)) +#define outl(val, port) out_le32((uint32_t *)(uintptr_t)((port)+isa_io_base), (val))
/* * 8, 16 and 32 bit, big and little endian I/O operations, with barrier. diff --git a/include/drivers/drivers.h b/include/drivers/drivers.h index 6829572..cf17d8e 100644 --- a/include/drivers/drivers.h +++ b/include/drivers/drivers.h @@ -113,9 +113,9 @@ char uart_getchar(int port); void serial_putchar(int c); #endif #ifdef CONFIG_DRIVER_ESCC -int uart_init(uint64_t port, unsigned long speed); -int uart_charav(int port); -char uart_getchar(int port); +int uart_init(uintptr_t port, unsigned long speed); +int uart_charav(uintptr_t port); +char uart_getchar(uintptr_t port); void serial_putchar(int c); void serial_cls(void); #ifdef CONFIG_DRIVER_ESCC_SUN diff --git a/include/kernel/stack.h b/include/kernel/stack.h index 809ffe9..44fef0f 100644 --- a/include/kernel/stack.h +++ b/include/kernel/stack.h @@ -31,8 +31,8 @@ typedef ucell phandle_t;
#ifdef NATIVE_BITWIDTH_EQUALS_HOST_BITWIDTH -#define pointer2cell(x) ((ucell)(x)) -#define cell2pointer(x) ((u8 *)(x)) +#define pointer2cell(x) ((ucell)(uintptr_t)(x)) +#define cell2pointer(x) ((u8 *)(uintptr_t)(x)) #endif #ifdef NATIVE_BITWIDTH_SMALLER_THAN_HOST_BITWIDTH #define pointer2cell(x) ((ucell)(((unsigned long)(x))-base_address)) diff --git a/kernel/internal.c b/kernel/internal.c index be1f216..0235737 100644 --- a/kernel/internal.c +++ b/kernel/internal.c @@ -208,7 +208,7 @@ static void call(void) exit(1); #else void (*funcptr) (void); - funcptr=(void *)POP(); + funcptr=(void *)cell2pointer(POP()); dbg_interp_printk("call: %x", funcptr); funcptr(); #endif @@ -329,7 +329,7 @@ static void doplusloop(void) #ifndef FCOMPILER static ucell get_myself(void) { - static ucell **myself = NULL; + static ucell **myself = NULL; if( !myself ) myself = (ucell**)findword("my-self") + 1; return (*myself && **myself) ? (ucell)**myself : 0; @@ -337,35 +337,35 @@ static ucell get_myself(void)
static void doivar(void) { - ucell r, *p = (ucell *)(*(ucell *) PC + sizeof(ucell)); + ucell r, *p = (ucell *)(*(ucell *) cell2pointer(PC) + sizeof(ucell)); ucell ibase = get_myself();
dbg_interp_printk("ivar, offset: %d size: %d (ibase %d)\n", p[0], p[1], ibase );
- r = ibase ? ibase + p[0] : (ucell)&p[2]; + r = ibase ? ibase + p[0] : pointer2cell(&p[2]); PUSH( r ); }
static void doival(void) { - ucell r, *p = (ucell *)(*(ucell *) PC + sizeof(ucell)); + ucell r, *p = (ucell *)(*(ucell *) cell2pointer(PC) + sizeof(ucell)); ucell ibase = get_myself();
dbg_interp_printk("ivar, offset: %d size: %d\n", p[0], p[1] );
- r = ibase ? ibase + p[0] : (ucell)&p[2]; - PUSH( *(ucell *)r ); + r = ibase ? ibase + p[0] : pointer2cell(&p[2]); + PUSH( *(ucell *)cell2pointer(r) ); }
static void doidefer(void) { - ucell *p = (ucell *)(*(ucell *) PC + sizeof(ucell)); + ucell *p = (ucell *)(*(ucell *) cell2pointer(PC) + sizeof(ucell)); ucell ibase = get_myself();
dbg_interp_printk("doidefer, offset: %d size: %d\n", p[0], p[1] );
PUSHR(PC); - PC = ibase ? ibase + p[0] : (ucell)&p[2]; + PC = ibase ? ibase + p[0] : pointer2cell(&p[2]); PC -= sizeof(ucell); } #else diff --git a/libc/diskio.c b/libc/diskio.c index 9f0fd0a..23f38eb 100644 --- a/libc/diskio.c +++ b/libc/diskio.c @@ -150,7 +150,7 @@ get_file_path( int fd ) if( lookup_xt(fdp->ih, "get-path", &fdp->get_path_xt) ) return NULL; call_package( fdp->get_path_xt, fdp->ih ); - return (char*)POP(); + return (char*)cell2pointer(POP()); }
const char * @@ -160,7 +160,7 @@ get_volume_name( int fd ) if( lookup_xt(fdp->ih, "volume-name", &fdp->volume_name_xt) ) return NULL; call_package( fdp->volume_name_xt, fdp->ih ); - return (char*)POP(); + return (char*)cell2pointer(POP()); }
const char * @@ -170,7 +170,7 @@ get_fstype( int fd ) if( lookup_xt(fdp->ih, "get-fstype", &fdp->get_fstype_xt) ) return NULL; call_package( fdp->get_fstype_xt, fdp->ih ); - return (char*)POP(); + return (char*)cell2pointer(POP()); }
int @@ -183,7 +183,7 @@ read_io( int fd, void *buf, size_t cnt ) if (fd != -1) { fdp = file_descriptors[fd];
- PUSH( (ucell)buf ); + PUSH( pointer2cell(buf) ); PUSH( cnt ); call_package( fdp->read_xt, fdp->ih ); ret = POP(); diff --git a/libc/extra.c b/libc/extra.c index 5823afb..85731ad 100644 --- a/libc/extra.c +++ b/libc/extra.c @@ -39,7 +39,7 @@ int forth_printf( const char *fmt, ... ) i = vsnprintf(buf, sizeof(buf), fmt, args); va_end(args);
- PUSH((ucell)buf); + PUSH(pointer2cell(buf)); PUSH(i); fword("type");
diff --git a/libopenbios/bindings.c b/libopenbios/bindings.c index f6bc8de..5323421 100644 --- a/libopenbios/bindings.c +++ b/libopenbios/bindings.c @@ -28,7 +28,7 @@ void push_str( const char *str ) { - PUSH( (ucell)str ); + PUSH( pointer2cell(str) ); PUSH( str ? strlen(str) : 0 ); }
@@ -101,7 +101,7 @@ _parword( const char *method, xt_t *cache_xt ) void bind_func( const char *name, void (*func)(void) ) { - PUSH( (ucell)func ); + PUSH( pointer2cell(func) ); push_str( name ); fword("is-cfunc"); } @@ -111,7 +111,7 @@ bind_xtfunc( const char *name, xt_t xt, ucell arg, void (*func)(void) ) { PUSH_xt( xt ); PUSH( arg ); - PUSH( (cell)func ); + PUSH( pointer2cell(func) ); push_str( name ); fword("is-xt-cfunc"); } @@ -119,7 +119,7 @@ bind_xtfunc( const char *name, xt_t xt, ucell arg, void (*func)(void) ) xt_t bind_noname_func( void (*func)(void) ) { - PUSH( (ucell)func ); + PUSH( pointer2cell(func) ); fword("is-noname-cfunc"); return POP_xt(); } @@ -249,7 +249,7 @@ char * pop_fstr_copy( void ) { int len = POP(); - char *str, *p = (char*)POP(); + char *str, *p = (char*)cell2pointer(POP()); if( !len ) return NULL; str = malloc( len + 1 ); @@ -279,7 +279,7 @@ set_property( phandle_t ph, const char *name, const char *buf, int len ) printk("set_property: NULL phandle\n"); return; } - PUSH((ucell)buf); + PUSH(pointer2cell(buf)); PUSH(len); push_str( name ); PUSH_ph(ph); @@ -309,7 +309,7 @@ get_property( phandle_t ph, const char *name, int *retlen ) len = POP(); if( retlen ) *retlen = len; - return (char*)POP(); + return (char*)cell2pointer(POP()); }
u32 @@ -426,7 +426,7 @@ static void call1_func( void ) { void (*func)(cell v); - func = (void*)POP(); + func = (void*)cell2pointer(POP());
(*func)( POP() ); } @@ -455,7 +455,7 @@ add_methods( int flags, int size, const method_t *methods, int nmet ) char *buf = NULL; if( xt ) { enterforth( xt ); - buf = (char*)POP(); + buf = (char*)cell2pointer(POP()); } (*(initfunc)methods[i].func)( buf ); continue; @@ -463,7 +463,7 @@ add_methods( int flags, int size, const method_t *methods, int nmet ) if( !size ) bind_func( methods[i].name, methods[i].func ); else - bind_xtfunc( methods[i].name, xt, (ucell)methods[i].func, + bind_xtfunc( methods[i].name, xt, pointer2cell(methods[i].func), &call1_func ); }
diff --git a/libopenbios/bootinfo_load.c b/libopenbios/bootinfo_load.c index ceabdeb..c4dbbe5 100644 --- a/libopenbios/bootinfo_load.c +++ b/libopenbios/bootinfo_load.c @@ -156,7 +156,7 @@ bootinfo_init_program(void) filename = get_filename(bootpath, &directory);
feval("load-base"); - base = (char*)POP(); + base = (char*)cell2pointer(POP());
feval("load-size"); size = POP(); diff --git a/libopenbios/elf_load.c b/libopenbios/elf_load.c index 36ae449..9c7850e 100644 --- a/libopenbios/elf_load.c +++ b/libopenbios/elf_load.c @@ -478,12 +478,12 @@ elf_init_program(void) Elf_phdr *phdr; size_t size, total_size = 0; char *addr; - cell tmp; + uintptr_t tmp;
/* TODO: manage ELF notes section */ feval("0 state-valid !"); feval("load-base"); - base = (char*)POP(); + base = (char*)cell2pointer(POP());
ehdr = (Elf_ehdr *)base;
diff --git a/libopenbios/initprogram.c b/libopenbios/initprogram.c index 01be705..1fa33ba 100644 --- a/libopenbios/initprogram.c +++ b/libopenbios/initprogram.c @@ -41,42 +41,42 @@ void init_program(void) addr = POP();
#ifdef CONFIG_LOADER_AOUT - if (is_aout((struct exec *)addr)) { + if (is_aout((struct exec *)cell2pointer(addr))) { aout_init_program(); return; } #endif
#ifdef CONFIG_LOADER_BOOTINFO - if (is_bootinfo((char *)addr)) { + if (is_bootinfo((char *)cell2pointer(addr))) { bootinfo_init_program(); return; } #endif
#ifdef CONFIG_LOADER_ELF - if (is_elf((Elf_ehdr *)addr)) { + if (is_elf((Elf_ehdr *)cell2pointer(addr))) { elf_init_program(); return; } #endif
#ifdef CONFIG_LOADER_FCODE - if (is_fcode((unsigned char *)addr)) { + if (is_fcode((unsigned char *)cell2pointer(addr))) { fcode_init_program(); return; } #endif
#ifdef CONFIG_LOADER_FORTH - if (is_forth((char *)addr)) { + if (is_forth((char *)cell2pointer(addr))) { forth_init_program(); return; } #endif
#ifdef CONFIG_LOADER_XCOFF - if (is_xcoff((COFF_filehdr_t *)addr)) { + if (is_xcoff((COFF_filehdr_t *)cell2pointer(addr))) { xcoff_init_program(); return; } diff --git a/libopenbios/ofmem_common.c b/libopenbios/ofmem_common.c index ceacbb6..22b268d 100644 --- a/libopenbios/ofmem_common.c +++ b/libopenbios/ofmem_common.c @@ -110,7 +110,7 @@ void* ofmem_malloc( size_t size )
top = ofmem_arch_get_heap_top();
- if( (ucell)ofmem->next_malloc + size > top ) { + if( pointer2cell(ofmem->next_malloc) + size > top ) { printk("out of malloc memory (%x)!\n", size ); return NULL; } @@ -183,10 +183,9 @@ ofmem_set_property( phandle_t ph, const char *name, const char *buf, int len ) printk("ofmem_set_property: NULL phandle\n"); return; } - PUSH((ucell)buf); + PUSH(pointer2cell(buf)); PUSH(len); - PUSH((ucell)name); - PUSH(strlen(name)); + push_str(name); PUSH_ph(ph); fword("encode-property"); } diff --git a/libopenbios/xcoff_load.c b/libopenbios/xcoff_load.c index 9868463..0dcb28c 100644 --- a/libopenbios/xcoff_load.c +++ b/libopenbios/xcoff_load.c @@ -64,7 +64,7 @@ xcoff_init_program(void) feval("0 state-valid !");
feval("load-base"); - base = (char*)POP(); + base = (char*)cell2pointer(POP());
fhdr = (COFF_filehdr_t*)base;
@@ -111,22 +111,22 @@ xcoff_init_program(void)
if (strcmp(shdr->s_name, ".text") == 0) {
- memcpy((char*)shdr->s_vaddr, base + shdr->s_scnptr, + memcpy((char*)(uintptr_t)shdr->s_vaddr, base + shdr->s_scnptr, shdr->s_size); total_size += shdr->s_size; #ifdef CONFIG_PPC - flush_icache_range((char*)shdr->s_vaddr, - (char*)(shdr->s_vaddr + shdr->s_size)); + flush_icache_range((char*)(uintptr_t)shdr->s_vaddr, + (char*)(uintptr_t)(shdr->s_vaddr + shdr->s_size)); #endif } else if (strcmp(shdr->s_name, ".data") == 0) {
- memcpy((char*)shdr->s_vaddr, base + shdr->s_scnptr, + memcpy((char*)(uintptr_t)shdr->s_vaddr, base + shdr->s_scnptr, shdr->s_size); total_size += shdr->s_size;
} else if (strcmp(shdr->s_name, ".bss") == 0) {
- memset((void *)shdr->s_vaddr, 0, shdr->s_size); + memset((void *)(uintptr_t)shdr->s_vaddr, 0, shdr->s_size); total_size += shdr->s_size; } else { DPRINTF(" Skip '%s' section\n", shdr->s_name); @@ -137,7 +137,7 @@ xcoff_init_program(void) DPRINTF("XCOFF entry point: %x\n", *(uint32_t*)ahdr->entry);
// Initialise saved-program-state - PUSH(*(uint32_t*)ahdr->entry); + PUSH(*(uint32_t*)(uintptr_t)ahdr->entry); feval("saved-program-state >sps.entry !"); PUSH(total_size); feval("saved-program-state >sps.file-size !"); diff --git a/packages/deblocker.c b/packages/deblocker.c index db66d12..5007185 100644 --- a/packages/deblocker.c +++ b/packages/deblocker.c @@ -96,7 +96,7 @@ deblk_tell( deblk_info_t *di )
#define DO_IO( xt, buf, blk, n ) \ - ({ PUSH3((ucell)(buf), blk, n); call_parent(xt); POP(); }) + ({ PUSH3(pointer2cell(buf), blk, n); call_parent(xt); POP(); })
typedef struct { /* block operation */ @@ -141,7 +141,7 @@ static int do_readwrite( deblk_info_t *di, int is_write, xt_t xt ) { int blk, i, n, len = POP(); - char *dest = (char*)POP(); + char *dest = (char*)cell2pointer(POP()); int last=0, retlen=0; work_t w[3]; ducell mark = ((ducell)di->mark_hi << BITS) | di->mark_lo; diff --git a/packages/disk-label.c b/packages/disk-label.c index c7b894a..44b9f9e 100644 --- a/packages/disk-label.c +++ b/packages/disk-label.c @@ -80,7 +80,7 @@ dlabel_open( dlabel_info_t *di ) call_package(di->parent_seek_xt, my_parent()); POP();
- PUSH((ucell)block0); + PUSH(pointer2cell(block0)); PUSH(sizeof(block0)); call_package(di->parent_read_xt, my_parent()); status = POP(); @@ -88,7 +88,7 @@ dlabel_open( dlabel_info_t *di ) goto out;
/* Find partition handler */ - PUSH( (ucell)block0 ); + PUSH( pointer2cell(block0) ); selfword("find-part-handler"); ph = POP_ph(); if( ph ) { diff --git a/packages/mac-parts.c b/packages/mac-parts.c index ee28ce8..4747a55 100644 --- a/packages/mac-parts.c +++ b/packages/mac-parts.c @@ -42,7 +42,7 @@ typedef struct { DECLARE_NODE( macparts, INSTALL_OPEN, sizeof(macparts_info_t), "+/packages/mac-parts" );
#define SEEK( pos ) ({ DPUSH(pos); call_parent(di->seek_xt); POP(); }) -#define READ( buf, size ) ({ PUSH((ucell)buf); PUSH(size); call_parent(di->read_xt); POP(); }) +#define READ( buf, size ) ({ PUSH(pointer2cell(buf)); PUSH(size); call_parent(di->read_xt); POP(); })
/* ( open -- flag ) */ static void @@ -267,7 +267,7 @@ out: static void macparts_probe( macparts_info_t *dummy ) { - desc_map_t *dmap = (desc_map_t*)POP(); + desc_map_t *dmap = (desc_map_t*)(uintptr_t)POP();
DPRINTF("macparts_probe %x ?= %x\n", dmap->sbSig, DESC_MAP_SIGNATURE); if( __be16_to_cpu(dmap->sbSig) != DESC_MAP_SIGNATURE ) diff --git a/packages/nvram.c b/packages/nvram.c index 4052655..3182edf 100644 --- a/packages/nvram.c +++ b/packages/nvram.c @@ -167,7 +167,7 @@ show_partitions( void ) void update_nvram( void ) { - PUSH( (ucell)nvram.config->data ); + PUSH( pointer2cell(nvram.config->data) ); PUSH( nvram.config_size ); fword("nvram-store-configs"); arch_nvram_put( nvram.data ); @@ -202,7 +202,7 @@ nvconf_init( void ) nvram.config_size = nvpart_size(p) - 0x10;
if( !once++ ) { - PUSH( (ucell)p->data ); + PUSH( pointer2cell(p->data) ); PUSH( nvram.config_size ); fword("nvram-load-configs"); } @@ -256,7 +256,7 @@ static void nvram_read( nvram_ibuf_t *nd ) { int len = POP(); - char *p = (char*)POP(); + char *p = (char*)cell2pointer(POP()); int n=0;
while( nd->mark_lo < nvram.size && n < len ) { @@ -272,7 +272,7 @@ static void nvram_write( nvram_ibuf_t *nd ) { int len = POP(); - char *p = (char*)POP(); + char *p = (char*)cell2pointer(POP()); int n=0;
while( nd->mark_lo < nvram.size && n < len ) { diff --git a/packages/pc-parts.c b/packages/pc-parts.c index 2ec6707..d3adb50 100644 --- a/packages/pc-parts.c +++ b/packages/pc-parts.c @@ -38,7 +38,7 @@ typedef struct { DECLARE_NODE( pcparts, INSTALL_OPEN, sizeof(pcparts_info_t), "+/packages/pc-parts" );
#define SEEK( pos ) ({ DPUSH(pos); call_parent(di->seek_xt); POP(); }) -#define READ( buf, size ) ({ PUSH((ucell)buf); PUSH(size); call_parent(di->read_xt); POP(); }) +#define READ( buf, size ) ({ PUSH(pointer2cell(buf)); PUSH(size); call_parent(di->read_xt); POP(); })
/* three helper functions */
@@ -294,7 +294,7 @@ pcparts_open( pcparts_info_t *di ) static void pcparts_probe( pcparts_info_t *dummy ) { - unsigned char *buf = (unsigned char *)POP(); + unsigned char *buf = (unsigned char *)cell2pointer(POP());
DPRINTF("probing for PC partitions\n");
diff --git a/packages/video.c b/packages/video.c index 32a4469..9eddd0f 100644 --- a/packages/video.c +++ b/packages/video.c @@ -244,7 +244,7 @@ video_set_colors( void ) { int count = POP(); int start = POP(); - unsigned char *p = (unsigned char*)POP(); + unsigned char *p = (unsigned char*)cell2pointer(POP()); int i;
for( i=0; i<count; i++, p+=3 ) { @@ -289,7 +289,7 @@ video_write(void) int len;
len = POP(); - addr = (char *)POP(); + addr = (char *)cell2pointer(POP());
console_draw_fstr(addr, len); PUSH(len);
Andreas Färber wrote:
Hey,
I've compile-tested ppc, ppc64, sparc32, sparc64; they all appeared to boot as before now (Haiku, AIX, Debian, Solaris, Milax guests).
That's a good sign :) I'm happy with the patch in principle, but probably Blue/Alex should have the main say on this one.
@Mark: I've taken the liberty of using push_str() in ofmem_set_property() to avoid duplicating the conversion.
Ah yes, good spot. I'll leave it as it is at the moment so it won't interfere with your patchset.
ATB,
Mark.
On 18.10.2010, at 13:28, Mark Cave-Ayland wrote:
Andreas Färber wrote:
Hey, I've compile-tested ppc, ppc64, sparc32, sparc64; they all appeared to boot as before now (Haiku, AIX, Debian, Solaris, Milax guests).
That's a good sign :) I'm happy with the patch in principle, but probably Blue/Alex should have the main say on this one.
The changes look pretty mechanical, so I don't think there's too much chance to break things. What we really need is a set of test images + cases to verify that everything still works. Preferably automated far enough to run automatically after every commit for easy bisecting.
Also, looking at the amount of work Andreas is putting into OpenBIOS atm, wouldn't it make sense to give him commit rights? Larger changes could still go through RFC cycles, but for mechanical parts I'm not sure the peer review is all that helpful.
Alex
On Wed, Oct 20, 2010 at 8:04 AM, Alexander Graf agraf@suse.de wrote:
On 18.10.2010, at 13:28, Mark Cave-Ayland wrote:
Andreas Färber wrote:
Hey, I've compile-tested ppc, ppc64, sparc32, sparc64; they all appeared to boot as before now (Haiku, AIX, Debian, Solaris, Milax guests).
That's a good sign :) I'm happy with the patch in principle, but probably Blue/Alex should have the main say on this one.
The changes look pretty mechanical, so I don't think there's too much chance to break things. What we really need is a set of test images + cases to verify that everything still works. Preferably automated far enough to run automatically after every commit for easy bisecting.
I tried to use kvm-autotest for Sparc, it sort of worked but it was a bit heavyweight. If someone has a server farm to run the tests, that could be a good approach.
Also, looking at the amount of work Andreas is putting into OpenBIOS atm, wouldn't it make sense to give him commit rights? Larger changes could still go through RFC cycles, but for mechanical parts I'm not sure the peer review is all that helpful.
He should probably discuss about that with Stefan.
On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber andreas.faerber@web.de wrote:
On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, direct casts result in warnings, treated as errors.
Use [u]intptr_t cast or cell2pointer and pointer2cell macros as necessary.
-int uart_init(uint64_t port, unsigned long speed) +int uart_init(uintptr_t port, unsigned long speed)
This is not correct. The physical addresses on Sparc32 are really 36 bits wide, so uintptr_t is not wide enough. For that, we'd need a physical address type. We could reuse for example QEMU name, target_phys_addr_t.
Am 20.10.2010 um 18:19 schrieb Blue Swirl:
On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber <andreas.faerber@web.de
wrote: On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, direct casts result in warnings, treated as errors.
Use [u]intptr_t cast or cell2pointer and pointer2cell macros as necessary.
-int uart_init(uint64_t port, unsigned long speed) +int uart_init(uintptr_t port, unsigned long speed)
This is not correct. The physical addresses on Sparc32 are really 36 bits wide, so uintptr_t is not wide enough.
Okay, then I'll revert this one. The patch may be mechanical to _review_ but it's pretty invasive, so I'd rather have some parts of it committed before running into larger merge conflicts.
For that, we'd need a physical address type. We could reuse for example QEMU name, target_phys_addr_t.
Sounds good. Should we go ahead with my patch first and do the uint64_t -> target_phys_addr_t change as a follow-up patch? Or should I strip all changes to drivers/ from my patch?
Andreas
On Wed, Oct 20, 2010 at 6:36 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 20.10.2010 um 18:19 schrieb Blue Swirl:
On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber andreas.faerber@web.de wrote:
On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, direct casts result in warnings, treated as errors.
Use [u]intptr_t cast or cell2pointer and pointer2cell macros as necessary.
-int uart_init(uint64_t port, unsigned long speed) +int uart_init(uintptr_t port, unsigned long speed)
This is not correct. The physical addresses on Sparc32 are really 36 bits wide, so uintptr_t is not wide enough.
Okay, then I'll revert this one. The patch may be mechanical to _review_ but it's pretty invasive, so I'd rather have some parts of it committed before running into larger merge conflicts.
For that, we'd need a physical address type. We could reuse for example QEMU name, target_phys_addr_t.
Sounds good. Should we go ahead with my patch first and do the uint64_t -> target_phys_addr_t change as a follow-up patch? Or should I strip all changes to drivers/ from my patch?
The cell2pointer etc. changes look OK. The address type changes (drivers/*) probably need a bit more thought, we are probably using a lot of wrong types (int vs. unsigned long vs. uint64_t).
Am 20.10.2010 um 21:04 schrieb Blue Swirl:
On Wed, Oct 20, 2010 at 6:36 PM, Andreas Färber <andreas.faerber@web.de
wrote: [...] Should we go ahead with my patch first and do the uint64_t -> target_phys_addr_t change as a follow-up patch? Or should I strip all changes to drivers/ from my patch?
The cell2pointer etc. changes look OK.
If there's no veto, I'll try committing patches 1-2 tonight.
Andreas
The address type changes (drivers/*) probably need a bit more thought, we are probably using a lot of wrong types (int vs. unsigned long vs. uint64_t).
Am 20.10.2010 um 21:04 schrieb Blue Swirl:
On Wed, Oct 20, 2010 at 6:36 PM, Andreas Färber <andreas.faerber@web.de
wrote: Am 20.10.2010 um 18:19 schrieb Blue Swirl:
On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber <andreas.faerber@web.de
wrote:
On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, direct casts result in warnings, treated as errors.
Use [u]intptr_t cast or cell2pointer and pointer2cell macros as necessary.
-int uart_init(uint64_t port, unsigned long speed) +int uart_init(uintptr_t port, unsigned long speed)
This is not correct. The physical addresses on Sparc32 are really 36 bits wide, so uintptr_t is not wide enough.
Okay, then I'll revert this one. The patch may be mechanical to _review_ but it's pretty invasive, so I'd rather have some parts of it committed before running into larger merge conflicts.
For that, we'd need a physical address type. We could reuse for example QEMU name, target_phys_addr_t.
Sounds good. Should we go ahead with my patch first and do the uint64_t -> target_phys_addr_t change as a follow-up patch? Or should I strip all changes to drivers/ from my patch?
The cell2pointer etc. changes look OK.
Thanks, applied in r922, with one additional (uintptr_t) -> cell2pointer() fix in mac-parts.c that I forgot to mention.
Attached is what I have not applied from the original patch. I'm thinking we should better avoid the native-bitwidth-equals-host- bitwidth path on ppc64 than fixing the symptoms of using it. For the rest, target_phys_addr_t would seem a nice solution. Additionally, the double-dereference of "myself" and the trampoline issue still need proper fixes.
Andreas
On Mon, Oct 25, 2010 at 9:08 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 20.10.2010 um 21:04 schrieb Blue Swirl:
On Wed, Oct 20, 2010 at 6:36 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 20.10.2010 um 18:19 schrieb Blue Swirl:
On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber andreas.faerber@web.de wrote:
On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, direct casts result in warnings, treated as errors.
Use [u]intptr_t cast or cell2pointer and pointer2cell macros as necessary.
-int uart_init(uint64_t port, unsigned long speed) +int uart_init(uintptr_t port, unsigned long speed)
This is not correct. The physical addresses on Sparc32 are really 36 bits wide, so uintptr_t is not wide enough.
Okay, then I'll revert this one. The patch may be mechanical to _review_ but it's pretty invasive, so I'd rather have some parts of it committed before running into larger merge conflicts.
For that, we'd need a physical address type. We could reuse for example QEMU name, target_phys_addr_t.
Sounds good. Should we go ahead with my patch first and do the uint64_t -> target_phys_addr_t change as a follow-up patch? Or should I strip all changes to drivers/ from my patch?
The cell2pointer etc. changes look OK.
Thanks, applied in r922, with one additional (uintptr_t) -> cell2pointer() fix in mac-parts.c that I forgot to mention.
Attached is what I have not applied from the original patch. I'm thinking we should better avoid the native-bitwidth-equals-host-bitwidth path on ppc64 than fixing the symptoms of using it. For the rest, target_phys_addr_t would seem a nice solution. Additionally, the double-dereference of "myself" and the trampoline issue still need proper fixes.
In some cases, instead of casts, the structure fields (in adb_kbd.c: dev->state) should be widened to uintptr_t or pointer.
Am 26.10.2010 um 20:53 schrieb Blue Swirl:
On Mon, Oct 25, 2010 at 9:08 PM, Andreas Färber <andreas.faerber@web.de
wrote: Am 20.10.2010 um 21:04 schrieb Blue Swirl:
On Wed, Oct 20, 2010 at 6:36 PM, Andreas Färber <andreas.faerber@web.de
wrote:
Am 20.10.2010 um 18:19 schrieb Blue Swirl:
On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber andreas.faerber@web.de wrote:
On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, direct casts result in warnings, treated as errors.
Use [u]intptr_t cast or cell2pointer and pointer2cell macros as necessary.
-int uart_init(uint64_t port, unsigned long speed) +int uart_init(uintptr_t port, unsigned long speed)
This is not correct. The physical addresses on Sparc32 are really 36 bits wide, so uintptr_t is not wide enough.
Okay, then I'll revert this one. The patch may be mechanical to _review_ but it's pretty invasive, so I'd rather have some parts of it committed before running into larger merge conflicts.
For that, we'd need a physical address type. We could reuse for example QEMU name, target_phys_addr_t.
Sounds good. Should we go ahead with my patch first and do the uint64_t -> target_phys_addr_t change as a follow-up patch? Or should I strip all changes to drivers/ from my patch?
The cell2pointer etc. changes look OK.
Thanks, applied in r922, with one additional (uintptr_t) -> cell2pointer() fix in mac-parts.c that I forgot to mention.
Attached is what I have not applied from the original patch. I'm thinking we should better avoid the native-bitwidth-equals-host-bitwidth path on ppc64 than fixing the symptoms of using it. For the rest, target_phys_addr_t would seem a nice solution. Additionally, the double-dereference of "myself" and the trampoline issue still need proper fixes.
In some cases, instead of casts, the structure fields (in adb_kbd.c: dev->state) should be widened to uintptr_t or pointer.
I was rather thinking of phys_addr_t.
Andreas
On Tue, Oct 26, 2010 at 7:14 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 26.10.2010 um 20:53 schrieb Blue Swirl:
On Mon, Oct 25, 2010 at 9:08 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 20.10.2010 um 21:04 schrieb Blue Swirl:
On Wed, Oct 20, 2010 at 6:36 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 20.10.2010 um 18:19 schrieb Blue Swirl:
On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber andreas.faerber@web.de wrote: > > On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, direct > casts > result in warnings, treated as errors. > > Use [u]intptr_t cast or cell2pointer and pointer2cell macros as > necessary.
> -int uart_init(uint64_t port, unsigned long speed) > +int uart_init(uintptr_t port, unsigned long speed)
This is not correct. The physical addresses on Sparc32 are really 36 bits wide, so uintptr_t is not wide enough.
Okay, then I'll revert this one. The patch may be mechanical to _review_ but it's pretty invasive, so I'd rather have some parts of it committed before running into larger merge conflicts.
For that, we'd need a physical address type. We could reuse for example QEMU name, target_phys_addr_t.
Sounds good. Should we go ahead with my patch first and do the uint64_t -> target_phys_addr_t change as a follow-up patch? Or should I strip all changes to drivers/ from my patch?
The cell2pointer etc. changes look OK.
Thanks, applied in r922, with one additional (uintptr_t) -> cell2pointer() fix in mac-parts.c that I forgot to mention.
Attached is what I have not applied from the original patch. I'm thinking we should better avoid the native-bitwidth-equals-host-bitwidth path on ppc64 than fixing the symptoms of using it. For the rest, target_phys_addr_t would seem a nice solution. Additionally, the double-dereference of "myself" and the trampoline issue still need proper fixes.
In some cases, instead of casts, the structure fields (in adb_kbd.c: dev->state) should be widened to uintptr_t or pointer.
I was rather thinking of phys_addr_t.
That should be used for physical addresses. But if the address is "virtual" (for example a pointer to a structure, like in the adb_kbd.c case), then a pointer type would be better. There's in those cases an assumption that the addressed data can be accessed via pointer dereference, so CPU word length is enough.
In other places the cast just extends the address (from 32 to 64 bits), but the upper bits have been lost already earlier. Then the structures should be changed (to target_phys_addr_t) to contain a full physical address.
Am 26.10.2010 um 21:26 schrieb Blue Swirl:
On Tue, Oct 26, 2010 at 7:14 PM, Andreas Färber <andreas.faerber@web.de
wrote: Am 26.10.2010 um 20:53 schrieb Blue Swirl:
On Mon, Oct 25, 2010 at 9:08 PM, Andreas Färber <andreas.faerber@web.de
wrote:
Am 20.10.2010 um 21:04 schrieb Blue Swirl:
On Wed, Oct 20, 2010 at 6:36 PM, Andreas Färber <andreas.faerber@web.de
wrote:
Am 20.10.2010 um 18:19 schrieb Blue Swirl:
> On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber > andreas.faerber@web.de > wrote: >> >> On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, >> direct >> casts >> result in warnings, treated as errors. >> >> Use [u]intptr_t cast or cell2pointer and pointer2cell macros as >> necessary. > >> -int uart_init(uint64_t port, unsigned long speed) >> +int uart_init(uintptr_t port, unsigned long speed) > > This is not correct. The physical addresses on Sparc32 are > really 36 > bits wide, so uintptr_t is not wide enough.
Okay, then I'll revert this one. The patch may be mechanical to _review_ but it's pretty invasive, so I'd rather have some parts of it committed before running into larger merge conflicts.
> For that, we'd need a > physical address type. We could reuse for example QEMU name, > target_phys_addr_t.
Sounds good. Should we go ahead with my patch first and do the uint64_t -> target_phys_addr_t change as a follow-up patch? Or should I strip all changes to drivers/ from my patch?
The cell2pointer etc. changes look OK.
Thanks, applied in r922, with one additional (uintptr_t) -> cell2pointer() fix in mac-parts.c that I forgot to mention.
Attached is what I have not applied from the original patch. I'm thinking we should better avoid the native-bitwidth-equals-host-bitwidth path on ppc64 than fixing the symptoms of using it. For the rest, target_phys_addr_t would seem a nice solution. Additionally, the double-dereference of "myself" and the trampoline issue still need proper fixes.
In some cases, instead of casts, the structure fields (in adb_kbd.c: dev->state) should be widened to uintptr_t or pointer.
I was rather thinking of phys_addr_t.
That should be used for physical addresses. But if the address is "virtual" (for example a pointer to a structure, like in the adb_kbd.c case), then a pointer type would be better. There's in those cases an assumption that the addressed data can be accessed via pointer dereference, so CPU word length is enough.
In other places the cast just extends the address (from 32 to 64 bits), but the upper bits have been lost already earlier. Then the structures should be changed (to target_phys_addr_t) to contain a full physical address.
You're right, I mixed it up with cuda where phys_addr_t is necessary for ->base.
Only QEMU needs to distinguish between host and target though, so I don't think target_ makes sense here. Attached FYI.
Andreas
On Tue, Oct 26, 2010 at 8:04 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 26.10.2010 um 21:26 schrieb Blue Swirl:
On Tue, Oct 26, 2010 at 7:14 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 26.10.2010 um 20:53 schrieb Blue Swirl:
On Mon, Oct 25, 2010 at 9:08 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 20.10.2010 um 21:04 schrieb Blue Swirl:
On Wed, Oct 20, 2010 at 6:36 PM, Andreas Färber andreas.faerber@web.de wrote: > > Am 20.10.2010 um 18:19 schrieb Blue Swirl: > >> On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber >> andreas.faerber@web.de >> wrote: >>> >>> On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, >>> direct >>> casts >>> result in warnings, treated as errors. >>> >>> Use [u]intptr_t cast or cell2pointer and pointer2cell macros as >>> necessary. >> >>> -int uart_init(uint64_t port, unsigned long speed) >>> +int uart_init(uintptr_t port, unsigned long speed) >> >> This is not correct. The physical addresses on Sparc32 are really 36 >> bits wide, so uintptr_t is not wide enough. > > Okay, then I'll revert this one. The patch may be mechanical to > _review_ > but > it's pretty invasive, so I'd rather have some parts of it committed > before > running into larger merge conflicts. > >> For that, we'd need a >> physical address type. We could reuse for example QEMU name, >> target_phys_addr_t. > > Sounds good. Should we go ahead with my patch first and do the > uint64_t > -> > target_phys_addr_t change as a follow-up patch? Or should I strip all > changes to drivers/ from my patch?
The cell2pointer etc. changes look OK.
Thanks, applied in r922, with one additional (uintptr_t) -> cell2pointer() fix in mac-parts.c that I forgot to mention.
Attached is what I have not applied from the original patch. I'm thinking we should better avoid the native-bitwidth-equals-host-bitwidth path on ppc64 than fixing the symptoms of using it. For the rest, target_phys_addr_t would seem a nice solution. Additionally, the double-dereference of "myself" and the trampoline issue still need proper fixes.
In some cases, instead of casts, the structure fields (in adb_kbd.c: dev->state) should be widened to uintptr_t or pointer.
I was rather thinking of phys_addr_t.
That should be used for physical addresses. But if the address is "virtual" (for example a pointer to a structure, like in the adb_kbd.c case), then a pointer type would be better. There's in those cases an assumption that the addressed data can be accessed via pointer dereference, so CPU word length is enough.
In other places the cast just extends the address (from 32 to 64 bits), but the upper bits have been lost already earlier. Then the structures should be changed (to target_phys_addr_t) to contain a full physical address.
You're right, I mixed it up with cuda where phys_addr_t is necessary for ->base.
Only QEMU needs to distinguish between host and target though, so I don't think target_ makes sense here. Attached FYI.
Looks OK. For x86, there's PAE mode with 36 bits but that's probably not supported by OpenBIOS or QEMU.
Do the double-dereference in two steps to avoid garbage in the high 32 address bits on ppc64.
Signed-off-by: Andreas Färber andreas.faerber@web.de --- kernel/internal.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/kernel/internal.c b/kernel/internal.c index 0235737..760c66d 100644 --- a/kernel/internal.c +++ b/kernel/internal.c @@ -329,10 +329,11 @@ static void doplusloop(void) #ifndef FCOMPILER static ucell get_myself(void) { - static ucell **myself = NULL; - if( !myself ) - myself = (ucell**)findword("my-self") + 1; - return (*myself && **myself) ? (ucell)**myself : 0; + static ucell *myselfptr = NULL; + if (myselfptr == NULL) + myselfptr = (ucell*)cell2pointer(findword("my-self")) + 1; + ucell *myself = (ucell*)cell2pointer(*myselfptr); + return (myself != NULL) ? *myself : 0; }
static void doivar(void)
--- include/kernel/stack.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/kernel/stack.h b/include/kernel/stack.h index 809ffe9..44fef0f 100644 --- a/include/kernel/stack.h +++ b/include/kernel/stack.h @@ -31,8 +31,8 @@ typedef ucell phandle_t;
#ifdef NATIVE_BITWIDTH_EQUALS_HOST_BITWIDTH -#define pointer2cell(x) ((ucell)(x)) -#define cell2pointer(x) ((u8 *)(x)) +#define pointer2cell(x) ((ucell)(uintptr_t)(x)) +#define cell2pointer(x) ((u8 *)(uintptr_t)(x)) #endif #ifdef NATIVE_BITWIDTH_SMALLER_THAN_HOST_BITWIDTH #define pointer2cell(x) ((ucell)(((unsigned long)(x))-base_address))
v2: * Change field type to pointer to avoid casts.
Signed-off-by: Andreas Färber andreas.faerber@web.de --- drivers/adb_bus.h | 2 +- drivers/adb_kbd.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/adb_bus.h b/drivers/adb_bus.h index e897fe4..205b375 100644 --- a/drivers/adb_bus.h +++ b/drivers/adb_bus.h @@ -32,7 +32,7 @@ struct adb_dev_t { adb_bus_t *bus; uint8_t addr; uint8_t type; - uint32_t state; + void *state; };
#define ADB_BUF_SIZE 8 diff --git a/drivers/adb_kbd.c b/drivers/adb_kbd.c index 0784ec8..e38798a 100644 --- a/drivers/adb_kbd.c +++ b/drivers/adb_kbd.c @@ -483,7 +483,7 @@ static int adb_kbd_read (void *private) int key; int ret;
- kbd = (void *)dev->state; + kbd = dev->state;
if (kbd->len > 0) { ret = kbd->sequence[kbd->len-- - 1]; @@ -531,7 +531,7 @@ void *adb_kbd_new (char *path, void *private) ADB_kbd_us, ADB_sequences); kbd->next_key = -1; kbd->len = 0; - dev->state = (int32_t)kbd; + dev->state = kbd; my_adb_dev = dev; }
Signed-off-by: Andreas Färber andreas.faerber@web.de --- TODO: Add a comment on PAE needing 36 bits.
include/arch/amd64/types.h | 3 +++ include/arch/ia64/types.h | 3 +++ include/arch/ppc/types.h | 7 +++++++ include/arch/sparc32/types.h | 3 +++ include/arch/sparc64/types.h | 3 +++ include/arch/x86/types.h | 3 +++ 6 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/include/arch/amd64/types.h b/include/arch/amd64/types.h index ffe8194..672f02c 100644 --- a/include/arch/amd64/types.h +++ b/include/arch/amd64/types.h @@ -16,6 +16,9 @@ /* endianess */ #include "autoconf.h"
+/* physical address */ +typedef uint64_t phys_addr_t; + /* cell based types */
typedef long long cell; diff --git a/include/arch/ia64/types.h b/include/arch/ia64/types.h index 5bf065a..d423461 100644 --- a/include/arch/ia64/types.h +++ b/include/arch/ia64/types.h @@ -17,6 +17,9 @@
#include <endian.h>
+/* physical address */ +typedef uint64_t phys_addr_t; + /* cell based types */
typedef int64_t cell; diff --git a/include/arch/ppc/types.h b/include/arch/ppc/types.h index d6df2a1..aaa66fc 100644 --- a/include/arch/ppc/types.h +++ b/include/arch/ppc/types.h @@ -30,6 +30,13 @@ typedef long intptr_t; /* endianess */ #include "autoconf.h"
+/* physical address */ +#if defined(__powerpc64__) +typedef uint64_t phys_addr_t; +#else +typedef uint32_t phys_addr_t; +#endif + /* cell based types */
typedef int32_t cell; diff --git a/include/arch/sparc32/types.h b/include/arch/sparc32/types.h index 500bcd1..bf96f57 100644 --- a/include/arch/sparc32/types.h +++ b/include/arch/sparc32/types.h @@ -30,6 +30,9 @@ typedef long intptr_t; /* endianess */ #include "autoconf.h"
+/* physical address: 36 bits */ +typedef uint64_t phys_addr_t; + /* cell based types */
typedef int32_t cell; diff --git a/include/arch/sparc64/types.h b/include/arch/sparc64/types.h index 112b53e..8baa5ee 100644 --- a/include/arch/sparc64/types.h +++ b/include/arch/sparc64/types.h @@ -30,6 +30,9 @@ typedef long intptr_t; /* endianess */ #include "autoconf.h"
+/* physical address */ +typedef uint64_t phys_addr_t; + /* cell based types */ typedef long long cell; typedef unsigned long long ucell; diff --git a/include/arch/x86/types.h b/include/arch/x86/types.h index 9491eae..16f355f 100644 --- a/include/arch/x86/types.h +++ b/include/arch/x86/types.h @@ -17,6 +17,9 @@
#include "autoconf.h"
+/* physical address */ +typedef uint32_t phys_addr_t; + /* cell based types */
typedef int32_t cell;
v2: * Use phys_addr_t. --- NB: The use of phys_addr_t implies for ppc that the physical address width needs to be at least as large as the virtual address width.
On sparc32 due to sizeof(void*) < sizeof(phys_addr_t) we can't use phys_addr_t in some places, so I've used pointers instead.
drivers/cuda.c | 2 +- drivers/cuda.h | 4 ++-- drivers/escc.c | 40 ++++++++++++++++++++-------------------- drivers/escc.h | 4 ++-- drivers/macio.c | 8 ++++---- drivers/macio.h | 6 +++--- include/arch/ppc/io.h | 18 +++++++++--------- include/drivers/drivers.h | 6 +++--- 8 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/drivers/cuda.c b/drivers/cuda.c index c254720..e4319d5 100644 --- a/drivers/cuda.c +++ b/drivers/cuda.c @@ -369,7 +369,7 @@ rtc_init(char *path)
}
-cuda_t *cuda_init (const char *path, uint32_t base) +cuda_t *cuda_init (const char *path, phys_addr_t base) { cuda_t *cuda; char buf[64]; diff --git a/drivers/cuda.h b/drivers/cuda.h index 66662b5..d3818c0 100644 --- a/drivers/cuda.h +++ b/drivers/cuda.h @@ -1,7 +1,7 @@ #include "adb_bus.h"
struct cuda_t { - uint32_t base; + phys_addr_t base; adb_bus_t *adb_bus; }; typedef struct cuda_t cuda_t; @@ -14,4 +14,4 @@ enum { CHARDEV_LAST, };
-cuda_t *cuda_init (const char *path, uint32_t base); +cuda_t *cuda_init (const char *path, phys_addr_t base); diff --git a/drivers/escc.c b/drivers/escc.c index 809cfa8..e22d245 100644 --- a/drivers/escc.c +++ b/drivers/escc.c @@ -13,11 +13,11 @@
static volatile unsigned char *serial_dev;
-#define CTRL(addr) (*(volatile unsigned char *)(addr)) +#define CTRL(addr) (*(volatile unsigned char *)(uintptr_t)(addr)) #ifdef CONFIG_DRIVER_ESCC_SUN -#define DATA(addr) (*(volatile unsigned char *)(addr + 2)) +#define DATA(addr) (*(volatile unsigned char *)(uintptr_t)(addr + 2)) #else -#define DATA(addr) (*(volatile unsigned char *)(addr + 16)) +#define DATA(addr) (*(volatile unsigned char *)(uintptr_t)(addr + 16)) #endif
/* Conversion routines to/from brg time constants from/to bits @@ -54,19 +54,19 @@ static volatile unsigned char *serial_dev; #define Rx_CH_AV 0x1 /* Rx Character Available */ #define Tx_BUF_EMP 0x4 /* Tx Buffer empty */
-int uart_charav(int port) +int uart_charav(uintptr_t port) { return (CTRL(port) & Rx_CH_AV) != 0; }
-char uart_getchar(int port) +char uart_getchar(uintptr_t port) { while (!uart_charav(port)) ; return DATA(port) & 0177; }
-static void uart_putchar(int port, unsigned char c) +static void uart_putchar(uintptr_t port, unsigned char c) { if (!serial_dev) return; @@ -102,13 +102,13 @@ static void uart_init_line(volatile unsigned char *port, unsigned long baud)
}
-int uart_init(uint64_t port, unsigned long speed) +int uart_init(phys_addr_t port, unsigned long speed) { #ifdef CONFIG_DRIVER_ESCC_SUN serial_dev = map_io(port & ~7ULL, ZS_REGS); serial_dev += port & 7ULL; #else - serial_dev = (unsigned char *)(unsigned long)port; + serial_dev = (unsigned char *)(uintptr_t)port; #endif uart_init_line(serial_dev, speed); return -1; @@ -116,7 +116,7 @@ int uart_init(uint64_t port, unsigned long speed)
void serial_putchar(int c) { - uart_putchar((int)serial_dev, (unsigned char) (c & 0xff)); + uart_putchar((uintptr_t)serial_dev, (unsigned char) (c & 0xff)); }
void serial_cls(void) @@ -131,7 +131,7 @@ void serial_cls(void)
/* ( addr len -- actual ) */ static void -escc_read(unsigned long *address) +escc_read(phys_addr_t *address) { char *addr; int len; @@ -140,7 +140,7 @@ escc_read(unsigned long *address) addr = (char *)cell2pointer(POP());
if (len < 1) - printk("escc_read: bad len, addr %x len %x\n", (unsigned int)addr, len); + printk("escc_read: bad len, addr %p len %x\n", addr, len);
if (uart_charav(*address)) { *addr = (char)uart_getchar(*address); @@ -152,7 +152,7 @@ escc_read(unsigned long *address)
/* ( addr len -- actual ) */ static void -escc_write(unsigned long *address) +escc_write(phys_addr_t *address) { unsigned char *addr; int i, len; @@ -172,7 +172,7 @@ escc_close(void) }
static void -escc_open(unsigned long *address) +escc_open(phys_addr_t *address) { #ifdef CONFIG_DRIVER_ESCC_SUN int len; @@ -199,7 +199,7 @@ escc_open(unsigned long *address) RET ( -1 ); }
-DECLARE_UNNAMED_NODE(escc, INSTALL_OPEN, sizeof(unsigned long)); +DECLARE_UNNAMED_NODE(escc, INSTALL_OPEN, sizeof(phys_addr_t));
NODE_METHODS(escc) = { { "open", escc_open }, @@ -211,7 +211,7 @@ NODE_METHODS(escc) = { #ifdef CONFIG_DRIVER_ESCC_SUN static volatile unsigned char *kbd_dev;
-void kbd_init(uint64_t base) +void kbd_init(phys_addr_t base) { kbd_dev = map_io(base, 2 * 4); kbd_dev += 4; @@ -295,7 +295,7 @@ escc_read_keyboard(void) addr = (unsigned char *)POP();
if (len < 1) - printk("escc_read: bad len, addr %x len %x\n", (unsigned int)addr, len); + printk("escc_read: bad len, addr %p len %x\n", addr, len);
if (keyboard_dataready()) { *addr = keyboard_readdata(); @@ -305,7 +305,7 @@ escc_read_keyboard(void) } }
-DECLARE_UNNAMED_NODE(escc_keyboard, INSTALL_OPEN, sizeof(unsigned long)); +DECLARE_UNNAMED_NODE(escc_keyboard, INSTALL_OPEN, sizeof(phys_addr_t));
NODE_METHODS(escc_keyboard) = { { "open", escc_open }, @@ -314,7 +314,7 @@ NODE_METHODS(escc_keyboard) = { };
void -ob_zs_init(uint64_t base, uint64_t offset, int intr, int slave, int keyboard) +ob_zs_init(phys_addr_t base, uint64_t offset, int intr, int slave, int keyboard) { char nodebuff[256]; phandle_t aliases; @@ -369,7 +369,7 @@ ob_zs_init(uint64_t base, uint64_t offset, int intr, int slave, int keyboard) #else
static void -escc_add_channel(const char *path, const char *node, uint32_t addr, +escc_add_channel(const char *path, const char *node, phys_addr_t addr, uint32_t offset) { char buf[64], tty[32]; @@ -430,7 +430,7 @@ escc_add_channel(const char *path, const char *node, uint32_t addr, }
void -escc_init(const char *path, unsigned long addr) +escc_init(const char *path, phys_addr_t addr) { char buf[64]; int props[2]; diff --git a/drivers/escc.h b/drivers/escc.h index a5516db..caaf00d 100644 --- a/drivers/escc.h +++ b/drivers/escc.h @@ -4,6 +4,6 @@
#define ZS_REGS 8
-void escc_init(const char *path, unsigned long addr); -void ob_zs_init(uint64_t base, uint64_t offset, int intr, int slave, +void escc_init(const char *path, phys_addr_t addr); +void ob_zs_init(phys_addr_t base, uint64_t offset, int intr, int slave, int keyboard); diff --git a/drivers/macio.c b/drivers/macio.c index d6d1696..2baf295 100644 --- a/drivers/macio.c +++ b/drivers/macio.c @@ -43,7 +43,7 @@ arch_nvram_size( void ) return NW_IO_NVRAM_SIZE >> NW_IO_NVRAM_SHIFT; }
-void macio_nvram_init(const char *path, uint32_t addr) +void macio_nvram_init(const char *path, phys_addr_t addr) { phandle_t chosen, aliases; phandle_t dnode; @@ -145,7 +145,7 @@ arch_nvram_get( char *buf ) }
static void -openpic_init(const char *path, uint32_t addr) +openpic_init(const char *path, phys_addr_t addr) { phandle_t target_node; phandle_t dnode; @@ -238,7 +238,7 @@ NODE_METHODS(ob_macio) = { };
void -ob_macio_heathrow_init(const char *path, uint32_t addr) +ob_macio_heathrow_init(const char *path, phys_addr_t addr) { phandle_t aliases;
@@ -253,7 +253,7 @@ ob_macio_heathrow_init(const char *path, uint32_t addr) }
void -ob_macio_keylargo_init(const char *path, uint32_t addr) +ob_macio_keylargo_init(const char *path, phys_addr_t addr) { phandle_t aliases;
diff --git a/drivers/macio.h b/drivers/macio.h index d580387..925b881 100644 --- a/drivers/macio.h +++ b/drivers/macio.h @@ -1,5 +1,5 @@ extern phandle_t pic_handle;
-void ob_macio_heathrow_init(const char *path, uint32_t addr); -void ob_macio_keylargo_init(const char *path, uint32_t addr); -void macio_nvram_init(const char *path, uint32_t addr); +void ob_macio_heathrow_init(const char *path, phys_addr_t addr); +void ob_macio_keylargo_init(const char *path, phys_addr_t addr); +void macio_nvram_init(const char *path, phys_addr_t addr); diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index d7c78a0..e5180f2 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -22,15 +22,15 @@ extern uint32_t isa_io_base; * are arrays of bytes, and byte-swapping is not appropriate in * that case. - paulus */ -#define insw(port, buf, ns) _insw((uint16_t *)((port)+isa_io_base), (buf), (ns)) -#define outsw(port, buf, ns) _outsw((uint16_t *)((port)+isa_io_base), (buf), (ns)) - -#define inb(port) in_8((uint8_t *)((port)+isa_io_base)) -#define outb(val, port) out_8((uint8_t *)((port)+isa_io_base), (val)) -#define inw(port) in_le16((uint16_t *)((port)+isa_io_base)) -#define outw(val, port) out_le16((uint16_t *)((port)+isa_io_base), (val)) -#define inl(port) in_le32((uint32_t *)((port)+isa_io_base)) -#define outl(val, port) out_le32((uint32_t *)((port)+isa_io_base), (val)) +#define insw(port, buf, ns) _insw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) +#define outsw(port, buf, ns) _outsw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) + +#define inb(port) in_8((uint8_t *)(uintptr_t)((port)+isa_io_base)) +#define outb(val, port) out_8((uint8_t *)(uintptr_t)((port)+isa_io_base), (val)) +#define inw(port) in_le16((uint16_t *)(uintptr_t)((port)+isa_io_base)) +#define outw(val, port) out_le16((uint16_t *)(uintptr_t)((port)+isa_io_base), (val)) +#define inl(port) in_le32((uint32_t *)(uintptr_t)((port)+isa_io_base)) +#define outl(val, port) out_le32((uint32_t *)(uintptr_t)((port)+isa_io_base), (val))
/* * 8, 16 and 32 bit, big and little endian I/O operations, with barrier. diff --git a/include/drivers/drivers.h b/include/drivers/drivers.h index 6829572..d139ace 100644 --- a/include/drivers/drivers.h +++ b/include/drivers/drivers.h @@ -113,9 +113,9 @@ char uart_getchar(int port); void serial_putchar(int c); #endif #ifdef CONFIG_DRIVER_ESCC -int uart_init(uint64_t port, unsigned long speed); -int uart_charav(int port); -char uart_getchar(int port); +int uart_init(phys_addr_t port, unsigned long speed); +int uart_charav(uintptr_t port); +char uart_getchar(uintptr_t port); void serial_putchar(int c); void serial_cls(void); #ifdef CONFIG_DRIVER_ESCC_SUN
On Tue, Oct 26, 2010 at 11:08 PM, Andreas Färber andreas.faerber@web.de wrote:
v2:
- Use phys_addr_t.
Looks OK. Again, it would be nice to make inline functions of CTRL and DATA macros in escc.c as well as in/out macros in include/arch/*/io.h, but that can be done later.
Am 27.10.2010 um 21:53 schrieb Blue Swirl:
On Tue, Oct 26, 2010 at 11:08 PM, Andreas Färber <andreas.faerber@web.de
wrote: v2:
- Use phys_addr_t.
Looks OK. Again, it would be nice to make inline functions of CTRL and DATA macros in escc.c as well as in/out macros in include/arch/*/io.h, but that can be done later.
Applied remaining ppc64 patches in r930-r933.
Some prettifications remain to be evaluated. Will look into the above suggestion later tonight.
Current ppc64 build progress should be:
CC target/kernel/primitives.o cc1: warnings being treated as errors In file included from ../kernel/primitives.c:25: ../kernel/internal.c:53: warning: cast from pointer to integer of different size ../kernel/internal.c:53: error: initializer element is not constant ../kernel/internal.c:53: error: (near initialization for 't[2]') make: *** [target/kernel/primitives.o] Error 1
This is the trampoline initialization issue.
Also still unsolved is the ppc64 relocation issue in start.S. Ideas welcome.
Not to mention of course that some assembler code will need to be changed for things to work properly at runtime.
Cheers, Andreas
Suggested by Blue.
Signed-off-by: Andreas Färber andreas.faerber@web.de --- include/arch/ppc/io.h | 66 +++++++++++++++++++++++++++++++++++++------------ 1 files changed, 50 insertions(+), 16 deletions(-)
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..84198f2 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -17,22 +17,6 @@ extern unsigned long virt_offset; extern uint32_t isa_io_base;
/* - * The insw/outsw/insl/outsl macros don't do byte-swapping. - * They are only used in practice for transferring buffers which - * are arrays of bytes, and byte-swapping is not appropriate in - * that case. - paulus - */ -#define insw(port, buf, ns) _insw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) -#define outsw(port, buf, ns) _outsw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) - -#define inb(port) in_8((uint8_t *)(uintptr_t)((port)+isa_io_base)) -#define outb(val, port) out_8((uint8_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inw(port) in_le16((uint16_t *)(uintptr_t)((port)+isa_io_base)) -#define outw(val, port) out_le16((uint16_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inl(port) in_le32((uint32_t *)(uintptr_t)((port)+isa_io_base)) -#define outl(val, port) out_le32((uint32_t *)(uintptr_t)((port)+isa_io_base), (val)) - -/* * 8, 16 and 32 bit, big and little endian I/O operations, with barrier. */ static inline int in_8(volatile unsigned char *addr) @@ -145,6 +129,56 @@ static inline void _outsw(volatile uint16_t * port, const void *buf, ns--; } } + + +/* + * The insw/outsw/insl/outsl functions don't do byte-swapping. + * They are only used in practice for transferring buffers which + * are arrays of bytes, and byte-swapping is not appropriate in + * that case. - paulus + */ + +static inline void insw(phys_addr_t port, void *buf, int ns) +{ + _insw((uint16_t *)(port + isa_io_base), buf, ns); +} + +static inline void outsw(phys_addr_t port, void *buf, int ns) +{ + _outsw((uint16_t *)(port + isa_io_base), buf, ns); +} + + +static inline int inb(phys_addr_t port) +{ + return in_8((uint8_t *)(port + isa_io_base)); +} + +static inline void outb(int val, phys_addr_t port) +{ + out_8((uint8_t *)(port + isa_io_base), val); +} + +static inline int inw(phys_addr_t port) +{ + return in_le16((uint16_t *)(port + isa_io_base)); +} + +static inline void outw(int val, phys_addr_t port) +{ + out_le16((uint16_t *)(port + isa_io_base), val); +} + +static inline unsigned inl(phys_addr_t port) +{ + return in_le32((uint32_t *)(port + isa_io_base)); +} + +static inline void outl(int val, phys_addr_t port) +{ + out_le32((uint32_t *)(port + isa_io_base), val); +} + #else /* BOOTSTRAP */ #ifdef FCOMPILER #define inb(reg) ((u8)0xff)
Am 30.10.2010 um 21:51 schrieb Andreas Färber:
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..84198f2 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h
+static inline void insw(phys_addr_t port, void *buf, int ns) +{
- _insw((uint16_t *)(port + isa_io_base), buf, ns);
+}
I'm wondering whether the use of phys_addr_t is right here or whether it should be uintptr_t? Would like to get this out of my queue.
Andreas
On 11.11.2010, at 00:01, Andreas Färber andreas.faerber@web.de wrote:
Am 30.10.2010 um 21:51 schrieb Andreas Färber:
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..84198f2 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h
+static inline void insw(phys_addr_t port, void *buf, int ns) +{
- _insw((uint16_t *)(port + isa_io_base), buf, ns);
+}
I'm wondering whether the use of phys_addr_t is right here or whether it should be uintptr_t? Would like to get this out of my queue.
PIO only defines 16 bit number of ports anyways, no?
Alex
Am 11.11.2010 um 08:39 schrieb Alexander Graf:
On 11.11.2010, at 00:01, Andreas Färber andreas.faerber@web.de wrote:
Am 30.10.2010 um 21:51 schrieb Andreas Färber:
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..84198f2 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h
+static inline void insw(phys_addr_t port, void *buf, int ns) +{
- _insw((uint16_t *)(port + isa_io_base), buf, ns);
+}
I'm wondering whether the use of phys_addr_t is right here or whether it should be uintptr_t? Would like to get this out of my queue.
PIO only defines 16 bit number of ports anyways, no?
If you say so. :) I'll try uint16_t then. We'll either need to upcast isa_io_base from uint32_t then or change that variable in qemu/ init.c(?) to either phys_addr_t or uintptr_t...
Do we want to consider a phys_addr_t an address where devices are located, whether accessed through MMU or real mode, or is phys_addr_t just supposed to be the MMU counterpart to the effective address? If the latter, do we need a new addr_t type or something or is uintptr_t the type to use since we want to cast it to pointer ultimately?
Andreas
On Thu, Nov 11, 2010 at 6:21 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 11.11.2010 um 08:39 schrieb Alexander Graf:
On 11.11.2010, at 00:01, Andreas Färber andreas.faerber@web.de wrote:
Am 30.10.2010 um 21:51 schrieb Andreas Färber:
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..84198f2 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h
+static inline void insw(phys_addr_t port, void *buf, int ns) +{
- _insw((uint16_t *)(port + isa_io_base), buf, ns);
+}
I'm wondering whether the use of phys_addr_t is right here or whether it should be uintptr_t? Would like to get this out of my queue.
PIO only defines 16 bit number of ports anyways, no?
If you say so. :) I'll try uint16_t then. We'll either need to upcast isa_io_base from uint32_t then or change that variable in qemu/init.c(?) to either phys_addr_t or uintptr_t...
Do we want to consider a phys_addr_t an address where devices are located, whether accessed through MMU or real mode, or is phys_addr_t just supposed to be the MMU counterpart to the effective address? If the latter, do we need a new addr_t type or something or is uintptr_t the type to use since we want to cast it to pointer ultimately?
I'm not sure about PPC terminology, but on Sparc the physical address space (if not mapped by MMU) can be accessed only with special alternate loads or stores. This is what Sparc64 IO macros do.
If an area is mapped it can be accessed with pointer dereference, then a pointer type or uintptr_t would be correct.
On 11.11.2010, at 19:33, Blue Swirl blauwirbel@gmail.com wrote:
On Thu, Nov 11, 2010 at 6:21 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 11.11.2010 um 08:39 schrieb Alexander Graf:
On 11.11.2010, at 00:01, Andreas Färber andreas.faerber@web.de wrote:
Am 30.10.2010 um 21:51 schrieb Andreas Färber:
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..84198f2 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h
+static inline void insw(phys_addr_t port, void *buf, int ns) +{
- _insw((uint16_t *)(port + isa_io_base), buf, ns);
+}
I'm wondering whether the use of phys_addr_t is right here or whether it should be uintptr_t? Would like to get this out of my queue.
PIO only defines 16 bit number of ports anyways, no?
If you say so. :) I'll try uint16_t then. We'll either need to upcast isa_io_base from uint32_t then or change that variable in qemu/init.c(?) to either phys_addr_t or uintptr_t...
Do we want to consider a phys_addr_t an address where devices are located, whether accessed through MMU or real mode, or is phys_addr_t just supposed to be the MMU counterpart to the effective address? If the latter, do we need a new addr_t type or something or is uintptr_t the type to use since we want to cast it to pointer ultimately?
I'm not sure about PPC terminology, but on Sparc the physical address space (if not mapped by MMU) can be accessed only with special alternate loads or stores. This is what Sparc64 IO macros do.
If an area is mapped it can be accessed with pointer dereference, then a pointer type or uintptr_t would be correct.
Any reason we can't just make it void* and map it for real where necessary?
Alex
Am 11.11.2010 um 20:07 schrieb Alexander Graf:
On 11.11.2010, at 19:33, Blue Swirl blauwirbel@gmail.com wrote:
On Thu, Nov 11, 2010 at 6:21 PM, Andreas Färber <andreas.faerber@web.de
wrote: Am 11.11.2010 um 08:39 schrieb Alexander Graf:
On 11.11.2010, at 00:01, Andreas Färber andreas.faerber@web.de wrote:
Am 30.10.2010 um 21:51 schrieb Andreas Färber:
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..84198f2 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h
+static inline void insw(phys_addr_t port, void *buf, int ns) +{
- _insw((uint16_t *)(port + isa_io_base), buf, ns);
+}
I'm wondering whether the use of phys_addr_t is right here or whether it should be uintptr_t? Would like to get this out of my queue.
PIO only defines 16 bit number of ports anyways, no?
If you say so. :) I'll try uint16_t then. We'll either need to upcast isa_io_base from uint32_t then or change that variable in qemu/ init.c(?) to either phys_addr_t or uintptr_t...
Do we want to consider a phys_addr_t an address where devices are located, whether accessed through MMU or real mode, or is phys_addr_t just supposed to be the MMU counterpart to the effective address? If the latter, do we need a new addr_t type or something or is uintptr_t the type to use since we want to cast it to pointer ultimately?
I'm not sure about PPC terminology, but on Sparc the physical address space (if not mapped by MMU) can be accessed only with special alternate loads or stores. This is what Sparc64 IO macros do.
This seems different from ppc.
If an area is mapped it can be accessed with pointer dereference, then a pointer type or uintptr_t would be correct.
Any reason we can't just make it void*
The reason for address types elsewhere (e.g., Haiku) is that offsets are easier to calculate with non-pointer types (a + b vs. (sometype*)a + bdependingontype). I'd prefer uintptr_t then.
and map it for real where necessary?
We could use void* but please don't rely on MMU mappings. As mentioned before, we need to support real mode too and the weird ROM-into-RAM mapping via ISI/DSI is problematic enough already. (Any particular reason there's no real OF-visible MMU translation btw?)
Andreas
On 11.11.2010, at 20:59, Andreas Färber andreas.faerber@web.de wrote:
Am 11.11.2010 um 20:07 schrieb Alexander Graf:
On 11.11.2010, at 19:33, Blue Swirl blauwirbel@gmail.com wrote:
On Thu, Nov 11, 2010 at 6:21 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 11.11.2010 um 08:39 schrieb Alexander Graf:
On 11.11.2010, at 00:01, Andreas Färber andreas.faerber@web.de wrote:
Am 30.10.2010 um 21:51 schrieb Andreas Färber:
> diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h > index e5180f2..84198f2 100644 > --- a/include/arch/ppc/io.h > +++ b/include/arch/ppc/io.h
> +static inline void insw(phys_addr_t port, void *buf, int ns) > +{ > + _insw((uint16_t *)(port + isa_io_base), buf, ns); > +}
I'm wondering whether the use of phys_addr_t is right here or whether it should be uintptr_t? Would like to get this out of my queue.
PIO only defines 16 bit number of ports anyways, no?
If you say so. :) I'll try uint16_t then. We'll either need to upcast isa_io_base from uint32_t then or change that variable in qemu/init.c(?) to either phys_addr_t or uintptr_t...
Do we want to consider a phys_addr_t an address where devices are located, whether accessed through MMU or real mode, or is phys_addr_t just supposed to be the MMU counterpart to the effective address? If the latter, do we need a new addr_t type or something or is uintptr_t the type to use since we want to cast it to pointer ultimately?
I'm not sure about PPC terminology, but on Sparc the physical address space (if not mapped by MMU) can be accessed only with special alternate loads or stores. This is what Sparc64 IO macros do.
This seems different from ppc.
If an area is mapped it can be accessed with pointer dereference, then a pointer type or uintptr_t would be correct.
Any reason we can't just make it void*
The reason for address types elsewhere (e.g., Haiku) is that offsets are easier to calculate with non-pointer types (a + b vs. (sometype*)a + bdependingontype). I'd prefer uintptr_t then.
and map it for real where necessary?
We could use void* but please don't rely on MMU mappings. As mentioned before, we need to support real mode too and the weird ROM-into-RAM mapping via ISI/DSI is problematic enough already. (Any particular reason there's no real OF-visible MMU translation btw?)
Well I was talking about isa_io_base. Port would still be uint16_t.
Alex
On Thu, Nov 11, 2010 at 7:07 PM, Alexander Graf agraf@suse.de wrote:
On 11.11.2010, at 19:33, Blue Swirl blauwirbel@gmail.com wrote:
On Thu, Nov 11, 2010 at 6:21 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 11.11.2010 um 08:39 schrieb Alexander Graf:
On 11.11.2010, at 00:01, Andreas Färber andreas.faerber@web.de wrote:
Am 30.10.2010 um 21:51 schrieb Andreas Färber:
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..84198f2 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h
+static inline void insw(phys_addr_t port, void *buf, int ns) +{
- _insw((uint16_t *)(port + isa_io_base), buf, ns);
+}
I'm wondering whether the use of phys_addr_t is right here or whether it should be uintptr_t? Would like to get this out of my queue.
PIO only defines 16 bit number of ports anyways, no?
If you say so. :) I'll try uint16_t then. We'll either need to upcast isa_io_base from uint32_t then or change that variable in qemu/init.c(?) to either phys_addr_t or uintptr_t...
Do we want to consider a phys_addr_t an address where devices are located, whether accessed through MMU or real mode, or is phys_addr_t just supposed to be the MMU counterpart to the effective address? If the latter, do we need a new addr_t type or something or is uintptr_t the type to use since we want to cast it to pointer ultimately?
I'm not sure about PPC terminology, but on Sparc the physical address space (if not mapped by MMU) can be accessed only with special alternate loads or stores. This is what Sparc64 IO macros do.
If an area is mapped it can be accessed with pointer dereference, then a pointer type or uintptr_t would be correct.
Any reason we can't just make it void* and map it for real where necessary?
It's possible, but then all drivers which use in/out functions should be changed.
Suggested by Blue.
v2: * Make port uint16_t, suggested by Alex. * Adapt isa_io_base for ppc64.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de --- arch/ppc/qemu/init.c | 2 +- include/arch/ppc/io.h | 68 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c index ba215bc..ab1ac69 100644 --- a/arch/ppc/qemu/init.c +++ b/arch/ppc/qemu/init.c @@ -152,7 +152,7 @@ static const pci_arch_t known_arch[] = { .irqs = { 21, 22, 23, 24 } }, }; -uint32_t isa_io_base; +uintptr_t isa_io_base;
void entry( void ) diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..d8f9dcc 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -14,23 +14,7 @@ extern unsigned long virt_offset;
#ifndef BOOTSTRAP
-extern uint32_t isa_io_base; - -/* - * The insw/outsw/insl/outsl macros don't do byte-swapping. - * They are only used in practice for transferring buffers which - * are arrays of bytes, and byte-swapping is not appropriate in - * that case. - paulus - */ -#define insw(port, buf, ns) _insw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) -#define outsw(port, buf, ns) _outsw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) - -#define inb(port) in_8((uint8_t *)(uintptr_t)((port)+isa_io_base)) -#define outb(val, port) out_8((uint8_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inw(port) in_le16((uint16_t *)(uintptr_t)((port)+isa_io_base)) -#define outw(val, port) out_le16((uint16_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inl(port) in_le32((uint32_t *)(uintptr_t)((port)+isa_io_base)) -#define outl(val, port) out_le32((uint32_t *)(uintptr_t)((port)+isa_io_base), (val)) +extern uintptr_t isa_io_base;
/* * 8, 16 and 32 bit, big and little endian I/O operations, with barrier. @@ -145,6 +129,56 @@ static inline void _outsw(volatile uint16_t * port, const void *buf, ns--; } } + + +/* + * The insw/outsw/insl/outsl functions don't do byte-swapping. + * They are only used in practice for transferring buffers which + * are arrays of bytes, and byte-swapping is not appropriate in + * that case. - paulus + */ + +static inline void insw(uint16_t port, void *buf, int ns) +{ + _insw((uint16_t *)(port + isa_io_base), buf, ns); +} + +static inline void outsw(uint16_t port, void *buf, int ns) +{ + _outsw((uint16_t *)(port + isa_io_base), buf, ns); +} + + +static inline int inb(uint16_t port) +{ + return in_8((uint8_t *)(port + isa_io_base)); +} + +static inline void outb(int val, uint16_t port) +{ + out_8((uint8_t *)(port + isa_io_base), val); +} + +static inline int inw(uint16_t port) +{ + return in_le16((uint16_t *)(port + isa_io_base)); +} + +static inline void outw(int val, uint16_t port) +{ + out_le16((uint16_t *)(port + isa_io_base), val); +} + +static inline unsigned inl(uint16_t port) +{ + return in_le32((uint32_t *)(port + isa_io_base)); +} + +static inline void outl(int val, uint16_t port) +{ + out_le32((uint32_t *)(port + isa_io_base), val); +} + #else /* BOOTSTRAP */ #ifdef FCOMPILER #define inb(reg) ((u8)0xff)
On Fri, Nov 12, 2010 at 11:54 PM, Andreas Färber andreas.faerber@web.de wrote:
Suggested by Blue.
v2:
- Make port uint16_t, suggested by Alex.
- Adapt isa_io_base for ppc64.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de
Please also adjust isa_io_base in arch/unix/unix.c: #ifdef CONFIG_PPC uint32_t isa_io_base; #elif defined(CONFIG_SPARC64) unsigned long isa_io_base; #endif
It's also defined as uint32_t in arch/ppc/{briq,mol,pearpc}/init.c. Sparc64 should be fine with uintptr_t or phys_addr_t.
To avoid cast warnings, use a 64-bit ISA I/O base on ppc64.
v3: * Adjust unix target, pointed out by Blue. Unify ppc and sparc64, using unsigned long instead of uintptr_t. It is initialized from pci_arch_t, which uses unsigned long. * Update other ppc targets, pointed out by Blue.
v2: Initial.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de --- arch/ppc/briq/init.c | 2 +- arch/ppc/mol/init.c | 2 +- arch/ppc/pearpc/init.c | 2 +- arch/ppc/qemu/init.c | 2 +- arch/unix/unix.c | 4 +--- include/arch/ppc/io.h | 2 +- 6 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/arch/ppc/briq/init.c b/arch/ppc/briq/init.c index 0b2b3b9..b32e97a 100644 --- a/arch/ppc/briq/init.c +++ b/arch/ppc/briq/init.c @@ -53,7 +53,7 @@ unexpected_excep( int vector ) ; }
-uint32_t isa_io_base; +unsigned long isa_io_base;
void entry( void ) diff --git a/arch/ppc/mol/init.c b/arch/ppc/mol/init.c index 3d24352..15333b4 100644 --- a/arch/ppc/mol/init.c +++ b/arch/ppc/mol/init.c @@ -49,7 +49,7 @@ unexpected_excep( int vector ) ; }
-uint32_t isa_io_base; +unsigned long isa_io_base;
void entry( void ) diff --git a/arch/ppc/pearpc/init.c b/arch/ppc/pearpc/init.c index b2c11c9..ca6da0a 100644 --- a/arch/ppc/pearpc/init.c +++ b/arch/ppc/pearpc/init.c @@ -56,7 +56,7 @@ unexpected_excep( int vector ) ; }
-uint32_t isa_io_base; +unsigned long isa_io_base;
void entry( void ) diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c index ba215bc..0b781d9 100644 --- a/arch/ppc/qemu/init.c +++ b/arch/ppc/qemu/init.c @@ -152,7 +152,7 @@ static const pci_arch_t known_arch[] = { .irqs = { 21, 22, 23, 24 } }, }; -uint32_t isa_io_base; +unsigned long isa_io_base;
void entry( void ) diff --git a/arch/unix/unix.c b/arch/unix/unix.c index f1c18df..a8b9f78 100644 --- a/arch/unix/unix.c +++ b/arch/unix/unix.c @@ -63,9 +63,7 @@ static int diskemu; static int segfault = 0; static int verbose = 0;
-#ifdef CONFIG_PPC -uint32_t isa_io_base; -#elif defined(CONFIG_SPARC64) +#if defined(CONFIG_PPC) || defined(CONFIG_SPARC64) unsigned long isa_io_base; #endif
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..267f167 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -14,7 +14,7 @@ extern unsigned long virt_offset;
#ifndef BOOTSTRAP
-extern uint32_t isa_io_base; +extern unsigned long isa_io_base;
/* * The insw/outsw/insl/outsl macros don't do byte-swapping.
Suggested by Blue.
v3: * Split off isa_io_base changes.
v2: * Make port uint16_t, suggested by Alex. * Adapt isa_io_base for ppc64.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de --- include/arch/ppc/io.h | 66 +++++++++++++++++++++++++++++++++++++------------ 1 files changed, 50 insertions(+), 16 deletions(-)
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index 267f167..3479a87 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -17,22 +17,6 @@ extern unsigned long virt_offset; extern unsigned long isa_io_base;
/* - * The insw/outsw/insl/outsl macros don't do byte-swapping. - * They are only used in practice for transferring buffers which - * are arrays of bytes, and byte-swapping is not appropriate in - * that case. - paulus - */ -#define insw(port, buf, ns) _insw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) -#define outsw(port, buf, ns) _outsw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) - -#define inb(port) in_8((uint8_t *)(uintptr_t)((port)+isa_io_base)) -#define outb(val, port) out_8((uint8_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inw(port) in_le16((uint16_t *)(uintptr_t)((port)+isa_io_base)) -#define outw(val, port) out_le16((uint16_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inl(port) in_le32((uint32_t *)(uintptr_t)((port)+isa_io_base)) -#define outl(val, port) out_le32((uint32_t *)(uintptr_t)((port)+isa_io_base), (val)) - -/* * 8, 16 and 32 bit, big and little endian I/O operations, with barrier. */ static inline int in_8(volatile unsigned char *addr) @@ -145,6 +129,56 @@ static inline void _outsw(volatile uint16_t * port, const void *buf, ns--; } } + + +/* + * The insw/outsw/insl/outsl functions don't do byte-swapping. + * They are only used in practice for transferring buffers which + * are arrays of bytes, and byte-swapping is not appropriate in + * that case. - paulus + */ + +static inline void insw(uint16_t port, void *buf, int ns) +{ + _insw((uint16_t *)(port + isa_io_base), buf, ns); +} + +static inline void outsw(uint16_t port, void *buf, int ns) +{ + _outsw((uint16_t *)(port + isa_io_base), buf, ns); +} + + +static inline int inb(uint16_t port) +{ + return in_8((uint8_t *)(port + isa_io_base)); +} + +static inline void outb(int val, uint16_t port) +{ + out_8((uint8_t *)(port + isa_io_base), val); +} + +static inline int inw(uint16_t port) +{ + return in_le16((uint16_t *)(port + isa_io_base)); +} + +static inline void outw(int val, uint16_t port) +{ + out_le16((uint16_t *)(port + isa_io_base), val); +} + +static inline unsigned inl(uint16_t port) +{ + return in_le32((uint32_t *)(port + isa_io_base)); +} + +static inline void outl(int val, uint16_t port) +{ + out_le32((uint32_t *)(port + isa_io_base), val); +} + #else /* BOOTSTRAP */ #ifdef FCOMPILER #define inb(reg) ((u8)0xff)
Am 13.11.2010 um 10:25 schrieb Andreas Färber:
To avoid cast warnings, use a 64-bit ISA I/O base on ppc64.
v3:
- Adjust unix target, pointed out by Blue.
Unify ppc and sparc64, using unsigned long instead of uintptr_t. It is initialized from pci_arch_t, which uses unsigned long.
- Update other ppc targets, pointed out by Blue.
Ping. Opinions on which type is more correct to use here? Should we go with uintptr_t and change PCI instead?
Andreas
v2: Initial.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de
arch/ppc/briq/init.c | 2 +- arch/ppc/mol/init.c | 2 +- arch/ppc/pearpc/init.c | 2 +- arch/ppc/qemu/init.c | 2 +- arch/unix/unix.c | 4 +--- include/arch/ppc/io.h | 2 +- 6 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/arch/ppc/briq/init.c b/arch/ppc/briq/init.c index 0b2b3b9..b32e97a 100644 --- a/arch/ppc/briq/init.c +++ b/arch/ppc/briq/init.c @@ -53,7 +53,7 @@ unexpected_excep( int vector ) ; }
-uint32_t isa_io_base; +unsigned long isa_io_base;
void entry( void ) diff --git a/arch/ppc/mol/init.c b/arch/ppc/mol/init.c index 3d24352..15333b4 100644 --- a/arch/ppc/mol/init.c +++ b/arch/ppc/mol/init.c @@ -49,7 +49,7 @@ unexpected_excep( int vector ) ; }
-uint32_t isa_io_base; +unsigned long isa_io_base;
void entry( void ) diff --git a/arch/ppc/pearpc/init.c b/arch/ppc/pearpc/init.c index b2c11c9..ca6da0a 100644 --- a/arch/ppc/pearpc/init.c +++ b/arch/ppc/pearpc/init.c @@ -56,7 +56,7 @@ unexpected_excep( int vector ) ; }
-uint32_t isa_io_base; +unsigned long isa_io_base;
void entry( void ) diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c index ba215bc..0b781d9 100644 --- a/arch/ppc/qemu/init.c +++ b/arch/ppc/qemu/init.c @@ -152,7 +152,7 @@ static const pci_arch_t known_arch[] = { .irqs = { 21, 22, 23, 24 } }, }; -uint32_t isa_io_base; +unsigned long isa_io_base;
void entry( void ) diff --git a/arch/unix/unix.c b/arch/unix/unix.c index f1c18df..a8b9f78 100644 --- a/arch/unix/unix.c +++ b/arch/unix/unix.c @@ -63,9 +63,7 @@ static int diskemu; static int segfault = 0; static int verbose = 0;
-#ifdef CONFIG_PPC -uint32_t isa_io_base; -#elif defined(CONFIG_SPARC64) +#if defined(CONFIG_PPC) || defined(CONFIG_SPARC64) unsigned long isa_io_base; #endif
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..267f167 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -14,7 +14,7 @@ extern unsigned long virt_offset;
#ifndef BOOTSTRAP
-extern uint32_t isa_io_base; +extern unsigned long isa_io_base;
/*
- The insw/outsw/insl/outsl macros don't do byte-swapping.
-- 1.7.3
-- OpenBIOS http://openbios.org/ Mailinglist: http://lists.openbios.org/mailman/listinfo Free your System - May the Forth be with you
On Sat, Nov 20, 2010 at 4:42 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 13.11.2010 um 10:25 schrieb Andreas Färber:
To avoid cast warnings, use a 64-bit ISA I/O base on ppc64.
v3:
- Adjust unix target, pointed out by Blue.
Unify ppc and sparc64, using unsigned long instead of uintptr_t. It is initialized from pci_arch_t, which uses unsigned long.
- Update other ppc targets, pointed out by Blue.
Ping. Opinions on which type is more correct to use here? Should we go with uintptr_t and change PCI instead?
For Sparc64, isa_io_base should be 64 bits. Any one of unsigned long, uintptr_t or phys_addr_t would be correct. Sparc32 does not use ISA, so that leaves the decision to PPC/PPC64.
On 21.11.2010, at 14:00, Blue Swirl wrote:
On Sat, Nov 20, 2010 at 4:42 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 13.11.2010 um 10:25 schrieb Andreas Färber:
To avoid cast warnings, use a 64-bit ISA I/O base on ppc64.
v3:
- Adjust unix target, pointed out by Blue.
Unify ppc and sparc64, using unsigned long instead of uintptr_t. It is initialized from pci_arch_t, which uses unsigned long.
- Update other ppc targets, pointed out by Blue.
Ping. Opinions on which type is more correct to use here? Should we go with uintptr_t and change PCI instead?
For Sparc64, isa_io_base should be 64 bits. Any one of unsigned long, uintptr_t or phys_addr_t would be correct. Sparc32 does not use ISA, so that leaves the decision to PPC/PPC64.
I'm reasonably indifferent. On PPC32 we don't support memory > 32 bits IIRC. On PPC64, there's the theoretical possibility to have the isa base above 32 bits, so anything works really :).
Alex
Am 21.11.2010 um 14:09 schrieb Alexander Graf:
On 21.11.2010, at 14:00, Blue Swirl wrote:
On Sat, Nov 20, 2010 at 4:42 PM, Andreas Färber <andreas.faerber@web.de
wrote: Am 13.11.2010 um 10:25 schrieb Andreas Färber:
To avoid cast warnings, use a 64-bit ISA I/O base on ppc64.
v3:
- Adjust unix target, pointed out by Blue.
Unify ppc and sparc64, using unsigned long instead of uintptr_t. It is initialized from pci_arch_t, which uses unsigned long.
- Update other ppc targets, pointed out by Blue.
Ping. Opinions on which type is more correct to use here? Should we go with uintptr_t and change PCI instead?
For Sparc64, isa_io_base should be 64 bits. Any one of unsigned long, uintptr_t or phys_addr_t would be correct. Sparc32 does not use ISA, so that leaves the decision to PPC/PPC64.
I'm reasonably indifferent. On PPC32 we don't support memory > 32 bits IIRC. On PPC64, there's the theoretical possibility to have the isa base above 32 bits, so anything works really :).
Thanks guys, I went with the least invasive version then and applied it in r964.
Alex, if you could add an Acked-by or Reviewed-by for the uint16_t port patch, I'd apply that one as well. Getting closer to SLBs then!
Andreas
On 13.11.2010, at 00:54, Andreas Färber wrote:
Suggested by Blue.
v2:
- Make port uint16_t, suggested by Alex.
- Adapt isa_io_base for ppc64.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de
arch/ppc/qemu/init.c | 2 +- include/arch/ppc/io.h | 68 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c index ba215bc..ab1ac69 100644 --- a/arch/ppc/qemu/init.c +++ b/arch/ppc/qemu/init.c @@ -152,7 +152,7 @@ static const pci_arch_t known_arch[] = { .irqs = { 21, 22, 23, 24 } }, }; -uint32_t isa_io_base; +uintptr_t isa_io_base;
void entry( void ) diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..d8f9dcc 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -14,23 +14,7 @@ extern unsigned long virt_offset;
#ifndef BOOTSTRAP
-extern uint32_t isa_io_base;
-/*
- The insw/outsw/insl/outsl macros don't do byte-swapping.
- They are only used in practice for transferring buffers which
- are arrays of bytes, and byte-swapping is not appropriate in
- that case. - paulus
- */
-#define insw(port, buf, ns) _insw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) -#define outsw(port, buf, ns) _outsw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns))
-#define inb(port) in_8((uint8_t *)(uintptr_t)((port)+isa_io_base)) -#define outb(val, port) out_8((uint8_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inw(port) in_le16((uint16_t *)(uintptr_t)((port)+isa_io_base)) -#define outw(val, port) out_le16((uint16_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inl(port) in_le32((uint32_t *)(uintptr_t)((port)+isa_io_base)) -#define outl(val, port) out_le32((uint32_t *)(uintptr_t)((port)+isa_io_base), (val)) +extern uintptr_t isa_io_base;
/*
- 8, 16 and 32 bit, big and little endian I/O operations, with barrier.
@@ -145,6 +129,56 @@ static inline void _outsw(volatile uint16_t * port, const void *buf, ns--; } }
+/*
- The insw/outsw/insl/outsl functions don't do byte-swapping.
- They are only used in practice for transferring buffers which
- are arrays of bytes, and byte-swapping is not appropriate in
- that case. - paulus
- */
+static inline void insw(uint16_t port, void *buf, int ns) +{
- _insw((uint16_t *)(port + isa_io_base), buf, ns);
+}
+static inline void outsw(uint16_t port, void *buf, int ns) +{
- _outsw((uint16_t *)(port + isa_io_base), buf, ns);
+}
+static inline int inb(uint16_t port)
Shouldn't inb return an uint8_t?
+{
- return in_8((uint8_t *)(port + isa_io_base));
+}
+static inline void outb(int val, uint16_t port) +{
- out_8((uint8_t *)(port + isa_io_base), val);
+}
+static inline int inw(uint16_t port)
I'd expect uint16_t to be returned here.
+{
- return in_le16((uint16_t *)(port + isa_io_base));
+}
+static inline void outw(int val, uint16_t port) +{
- out_le16((uint16_t *)(port + isa_io_base), val);
+}
+static inline unsigned inl(uint16_t port)
and uint32_t here.
+{
- return in_le32((uint32_t *)(port + isa_io_base));
+}
+static inline void outl(int val, uint16_t port)
The same goes for the out values. They should only be as big as they can really be and preferably unsigned :).
The rest of it looks very nice!
Alex
Am 21.11.2010 um 14:51 schrieb Alexander Graf:
On 13.11.2010, at 00:54, Andreas Färber wrote:
Suggested by Blue.
v2:
- Make port uint16_t, suggested by Alex.
- Adapt isa_io_base for ppc64.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de
arch/ppc/qemu/init.c | 2 +- include/arch/ppc/io.h | 68 +++++++++++++++++++++++++++++++++++ +------------ 2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c index ba215bc..ab1ac69 100644 --- a/arch/ppc/qemu/init.c +++ b/arch/ppc/qemu/init.c @@ -152,7 +152,7 @@ static const pci_arch_t known_arch[] = { .irqs = { 21, 22, 23, 24 } }, }; -uint32_t isa_io_base; +uintptr_t isa_io_base;
void entry( void ) diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..d8f9dcc 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -14,23 +14,7 @@ extern unsigned long virt_offset;
#ifndef BOOTSTRAP
-extern uint32_t isa_io_base;
-/*
- The insw/outsw/insl/outsl macros don't do byte-swapping.
- They are only used in practice for transferring buffers which
- are arrays of bytes, and byte-swapping is not appropriate in
- that case. - paulus
- */
-#define insw(port, buf, ns) _insw((uint16_t *)(uintptr_t)((port) +isa_io_base), (buf), (ns)) -#define outsw(port, buf, ns) _outsw((uint16_t *)(uintptr_t)((port) +isa_io_base), (buf), (ns))
-#define inb(port) in_8((uint8_t *)(uintptr_t)((port)+isa_io_base)) -#define outb(val, port) out_8((uint8_t *)(uintptr_t)((port) +isa_io_base), (val)) -#define inw(port) in_le16((uint16_t *)(uintptr_t)((port) +isa_io_base)) -#define outw(val, port) out_le16((uint16_t *)(uintptr_t)((port) +isa_io_base), (val)) -#define inl(port) in_le32((uint32_t *)(uintptr_t)((port) +isa_io_base)) -#define outl(val, port) out_le32((uint32_t *)(uintptr_t)((port) +isa_io_base), (val)) +extern uintptr_t isa_io_base;
/*
- 8, 16 and 32 bit, big and little endian I/O operations, with
barrier. @@ -145,6 +129,56 @@ static inline void _outsw(volatile uint16_t * port, const void *buf, ns--; } }
+/*
- The insw/outsw/insl/outsl functions don't do byte-swapping.
- They are only used in practice for transferring buffers which
- are arrays of bytes, and byte-swapping is not appropriate in
- that case. - paulus
- */
+static inline void insw(uint16_t port, void *buf, int ns) +{
- _insw((uint16_t *)(port + isa_io_base), buf, ns);
+}
+static inline void outsw(uint16_t port, void *buf, int ns) +{
- _outsw((uint16_t *)(port + isa_io_base), buf, ns);
+}
+static inline int inb(uint16_t port)
Shouldn't inb return an uint8_t?
+{
- return in_8((uint8_t *)(port + isa_io_base));
+}
+static inline void outb(int val, uint16_t port) +{
- out_8((uint8_t *)(port + isa_io_base), val);
+}
+static inline int inw(uint16_t port)
I'd expect uint16_t to be returned here.
+{
- return in_le16((uint16_t *)(port + isa_io_base));
+}
+static inline void outw(int val, uint16_t port) +{
- out_le16((uint16_t *)(port + isa_io_base), val);
+}
+static inline unsigned inl(uint16_t port)
and uint32_t here.
+{
- return in_le32((uint32_t *)(port + isa_io_base));
+}
+static inline void outl(int val, uint16_t port)
The same goes for the out values. They should only be as big as they can really be and preferably unsigned :).
I used the types used by the functions my functions wrap. I noticed that myself but wanted to keep the patch as small as possible. I'll change the existing functions then.
Andreas
The rest of it looks very nice!
Alex
-- OpenBIOS http://openbios.org/ Mailinglist: http://lists.openbios.org/mailman/listinfo Free your System - May the Forth be with you
On 21.11.2010, at 14:56, Andreas Färber wrote:
Am 21.11.2010 um 14:51 schrieb Alexander Graf:
On 13.11.2010, at 00:54, Andreas Färber wrote:
Suggested by Blue.
v2:
- Make port uint16_t, suggested by Alex.
- Adapt isa_io_base for ppc64.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de
arch/ppc/qemu/init.c | 2 +- include/arch/ppc/io.h | 68 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c index ba215bc..ab1ac69 100644 --- a/arch/ppc/qemu/init.c +++ b/arch/ppc/qemu/init.c @@ -152,7 +152,7 @@ static const pci_arch_t known_arch[] = { .irqs = { 21, 22, 23, 24 } }, }; -uint32_t isa_io_base; +uintptr_t isa_io_base;
void entry( void ) diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index e5180f2..d8f9dcc 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -14,23 +14,7 @@ extern unsigned long virt_offset;
#ifndef BOOTSTRAP
-extern uint32_t isa_io_base;
-/*
- The insw/outsw/insl/outsl macros don't do byte-swapping.
- They are only used in practice for transferring buffers which
- are arrays of bytes, and byte-swapping is not appropriate in
- that case. - paulus
- */
-#define insw(port, buf, ns) _insw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) -#define outsw(port, buf, ns) _outsw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns))
-#define inb(port) in_8((uint8_t *)(uintptr_t)((port)+isa_io_base)) -#define outb(val, port) out_8((uint8_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inw(port) in_le16((uint16_t *)(uintptr_t)((port)+isa_io_base)) -#define outw(val, port) out_le16((uint16_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inl(port) in_le32((uint32_t *)(uintptr_t)((port)+isa_io_base)) -#define outl(val, port) out_le32((uint32_t *)(uintptr_t)((port)+isa_io_base), (val)) +extern uintptr_t isa_io_base;
/*
- 8, 16 and 32 bit, big and little endian I/O operations, with barrier.
@@ -145,6 +129,56 @@ static inline void _outsw(volatile uint16_t * port, const void *buf, ns--; } }
+/*
- The insw/outsw/insl/outsl functions don't do byte-swapping.
- They are only used in practice for transferring buffers which
- are arrays of bytes, and byte-swapping is not appropriate in
- that case. - paulus
- */
+static inline void insw(uint16_t port, void *buf, int ns) +{
- _insw((uint16_t *)(port + isa_io_base), buf, ns);
+}
+static inline void outsw(uint16_t port, void *buf, int ns) +{
- _outsw((uint16_t *)(port + isa_io_base), buf, ns);
+}
+static inline int inb(uint16_t port)
Shouldn't inb return an uint8_t?
+{
- return in_8((uint8_t *)(port + isa_io_base));
+}
+static inline void outb(int val, uint16_t port) +{
- out_8((uint8_t *)(port + isa_io_base), val);
+}
+static inline int inw(uint16_t port)
I'd expect uint16_t to be returned here.
+{
- return in_le16((uint16_t *)(port + isa_io_base));
+}
+static inline void outw(int val, uint16_t port) +{
- out_le16((uint16_t *)(port + isa_io_base), val);
+}
+static inline unsigned inl(uint16_t port)
and uint32_t here.
+{
- return in_le32((uint32_t *)(port + isa_io_base));
+}
+static inline void outl(int val, uint16_t port)
The same goes for the out values. They should only be as big as they can really be and preferably unsigned :).
I used the types used by the functions my functions wrap. I noticed that myself but wanted to keep the patch as small as possible. I'll change the existing functions then.
Ah, makes sense :). Yeah, just change them while at it :). The closer we are to reality, the better, right? :)
Alex
Suggested by Blue.
Clean up function signatures while at it.
v4: * Convert all I/O functions to use POSIX types for value. Suggested by Alex.
v3: * Split off isa_io_base changes.
v2: * Make port uint16_t, suggested by Alex. * Adapt isa_io_base for ppc64.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de --- include/arch/ppc/io.h | 96 +++++++++++++++++++++++++++++++++---------------- 1 files changed, 65 insertions(+), 31 deletions(-)
diff --git a/include/arch/ppc/io.h b/include/arch/ppc/io.h index 267f167..3449c5b 100644 --- a/include/arch/ppc/io.h +++ b/include/arch/ppc/io.h @@ -17,89 +17,73 @@ extern unsigned long virt_offset; extern unsigned long isa_io_base;
/* - * The insw/outsw/insl/outsl macros don't do byte-swapping. - * They are only used in practice for transferring buffers which - * are arrays of bytes, and byte-swapping is not appropriate in - * that case. - paulus - */ -#define insw(port, buf, ns) _insw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) -#define outsw(port, buf, ns) _outsw((uint16_t *)(uintptr_t)((port)+isa_io_base), (buf), (ns)) - -#define inb(port) in_8((uint8_t *)(uintptr_t)((port)+isa_io_base)) -#define outb(val, port) out_8((uint8_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inw(port) in_le16((uint16_t *)(uintptr_t)((port)+isa_io_base)) -#define outw(val, port) out_le16((uint16_t *)(uintptr_t)((port)+isa_io_base), (val)) -#define inl(port) in_le32((uint32_t *)(uintptr_t)((port)+isa_io_base)) -#define outl(val, port) out_le32((uint32_t *)(uintptr_t)((port)+isa_io_base), (val)) - -/* * 8, 16 and 32 bit, big and little endian I/O operations, with barrier. */ -static inline int in_8(volatile unsigned char *addr) +static inline uint8_t in_8(volatile uint8_t *addr) { - int ret; + uint8_t ret;
__asm__ __volatile__("lbz%U1%X1 %0,%1; eieio":"=r"(ret):"m"(*addr)); return ret; }
-static inline void out_8(volatile unsigned char *addr, int val) +static inline void out_8(volatile uint8_t *addr, uint8_t val) { __asm__ __volatile__("stb%U0%X0 %1,%0; eieio":"=m"(*addr):"r"(val)); }
-static inline int in_le16(volatile unsigned short *addr) +static inline uint16_t in_le16(volatile uint16_t *addr) { - int ret; + uint16_t ret;
__asm__ __volatile__("lhbrx %0,0,%1; eieio":"=r"(ret): "r"(addr), "m"(*addr)); return ret; }
-static inline int in_be16(volatile unsigned short *addr) +static inline uint16_t in_be16(volatile uint16_t *addr) { - int ret; + uint16_t ret;
__asm__ __volatile__("lhz%U1%X1 %0,%1; eieio":"=r"(ret):"m"(*addr)); return ret; }
-static inline void out_le16(volatile unsigned short *addr, int val) +static inline void out_le16(volatile uint16_t *addr, uint16_t val) { __asm__ __volatile__("sthbrx %1,0,%2; eieio":"=m"(*addr):"r"(val), "r"(addr)); }
-static inline void out_be16(volatile unsigned short *addr, int val) +static inline void out_be16(volatile uint16_t *addr, uint16_t val) { __asm__ __volatile__("sth%U0%X0 %1,%0; eieio":"=m"(*addr):"r"(val)); }
-static inline unsigned in_le32(volatile unsigned *addr) +static inline uint32_t in_le32(volatile uint32_t *addr) { - unsigned ret; + uint32_t ret;
__asm__ __volatile__("lwbrx %0,0,%1; eieio":"=r"(ret): "r"(addr), "m"(*addr)); return ret; }
-static inline unsigned in_be32(volatile unsigned *addr) +static inline uint32_t in_be32(volatile uint32_t *addr) { - unsigned ret; + uint32_t ret;
__asm__ __volatile__("lwz%U1%X1 %0,%1; eieio":"=r"(ret):"m"(*addr)); return ret; }
-static inline void out_le32(volatile unsigned *addr, int val) +static inline void out_le32(volatile uint32_t *addr, uint32_t val) { __asm__ __volatile__("stwbrx %1,0,%2; eieio":"=m"(*addr):"r"(val), "r"(addr)); }
-static inline void out_be32(volatile unsigned *addr, int val) +static inline void out_be32(volatile unsigned *addr, uint32_t val) { __asm__ __volatile__("stw%U0%X0 %1,%0; eieio":"=m"(*addr):"r"(val)); } @@ -145,6 +129,56 @@ static inline void _outsw(volatile uint16_t * port, const void *buf, ns--; } } + + +/* + * The insw/outsw/insl/outsl functions don't do byte-swapping. + * They are only used in practice for transferring buffers which + * are arrays of bytes, and byte-swapping is not appropriate in + * that case. - paulus + */ + +static inline void insw(uint16_t port, void *buf, int ns) +{ + _insw((uint16_t *)(port + isa_io_base), buf, ns); +} + +static inline void outsw(uint16_t port, void *buf, int ns) +{ + _outsw((uint16_t *)(port + isa_io_base), buf, ns); +} + + +static inline uint8_t inb(uint16_t port) +{ + return in_8((uint8_t *)(port + isa_io_base)); +} + +static inline void outb(uint8_t val, uint16_t port) +{ + out_8((uint8_t *)(port + isa_io_base), val); +} + +static inline uint16_t inw(uint16_t port) +{ + return in_le16((uint16_t *)(port + isa_io_base)); +} + +static inline void outw(uint16_t val, uint16_t port) +{ + out_le16((uint16_t *)(port + isa_io_base), val); +} + +static inline uint32_t inl(uint16_t port) +{ + return in_le32((uint32_t *)(port + isa_io_base)); +} + +static inline void outl(uint32_t val, uint16_t port) +{ + out_le32((uint32_t *)(port + isa_io_base), val); +} + #else /* BOOTSTRAP */ #ifdef FCOMPILER #define inb(reg) ((u8)0xff)
On 21.11.2010, at 19:19, Andreas Färber wrote:
Suggested by Blue.
Clean up function signatures while at it.
v4:
- Convert all I/O functions to use POSIX types for value.
Suggested by Alex.
v3:
- Split off isa_io_base changes.
v2:
- Make port uint16_t, suggested by Alex.
- Adapt isa_io_base for ppc64.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de
Now, that's what I call clean :)
Signed-off-by: Alexander Graf agraf@suse.de
Alex
Am 21.11.2010 um 20:10 schrieb Alexander Graf:
On 21.11.2010, at 19:19, Andreas Färber wrote:
Suggested by Blue.
Clean up function signatures while at it.
v4:
- Convert all I/O functions to use POSIX types for value.
Suggested by Alex.
v3:
- Split off isa_io_base changes.
v2:
- Make port uint16_t, suggested by Alex.
- Adapt isa_io_base for ppc64.
Cc: Blue Swirl blauwirbel@gmail.com Cc: Alexander Graf agraf@suse.de Signed-off-by: Andreas Färber andreas.faerber@web.de
Now, that's what I call clean :)
Signed-off-by: Alexander Graf agraf@suse.de
Thanks, applied in r965.
Andreas
Am 27.10.2010 um 21:53 schrieb Blue Swirl:
On Tue, Oct 26, 2010 at 11:08 PM, Andreas Färber <andreas.faerber@web.de
wrote: v2:
- Use phys_addr_t.
Looks OK. Again, it would be nice to make inline functions of CTRL and DATA macros in escc.c
These macros are being used on the left side. We'd have to dump the whole concept then, duplicating the conversions in separate in and out functions - I fail to see the advantage here...
Andreas
as well as in/out macros in include/arch/*/io.h, but that can be done later.
-- OpenBIOS http://openbios.org/ Mailinglist: http://lists.openbios.org/mailman/listinfo Free your System - May the Forth be with you
Hello Alex,
Are you able to ack this patch? Or are there any reasons why the state must be uint32_t? The ppc64 mac99 machine still uses ADB and apparently OpenBIOS doesn't support USB keyboards yet.
Thanks, Andreas
Am 27.10.2010 um 01:08 schrieb Andreas Färber:
v2:
- Change field type to pointer to avoid casts.
Signed-off-by: Andreas Färber andreas.faerber@web.de
drivers/adb_bus.h | 2 +- drivers/adb_kbd.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/adb_bus.h b/drivers/adb_bus.h index e897fe4..205b375 100644 --- a/drivers/adb_bus.h +++ b/drivers/adb_bus.h @@ -32,7 +32,7 @@ struct adb_dev_t { adb_bus_t *bus; uint8_t addr; uint8_t type;
- uint32_t state;
- void *state;
};
#define ADB_BUF_SIZE 8 diff --git a/drivers/adb_kbd.c b/drivers/adb_kbd.c index 0784ec8..e38798a 100644 --- a/drivers/adb_kbd.c +++ b/drivers/adb_kbd.c @@ -483,7 +483,7 @@ static int adb_kbd_read (void *private) int key; int ret;
- kbd = (void *)dev->state;
kbd = dev->state;
if (kbd->len > 0) { ret = kbd->sequence[kbd->len-- - 1];
@@ -531,7 +531,7 @@ void *adb_kbd_new (char *path, void *private) ADB_kbd_us, ADB_sequences); kbd->next_key = -1; kbd->len = 0;
- dev->state = (int32_t)kbd;
- dev->state = kbd; my_adb_dev = dev; }
-- 1.7.3
Hey Andreas,
On 30.10.2010, at 04:51, Andreas Färber wrote:
Hello Alex,
Are you able to ack this patch? Or are there any reasons why the state must be uint32_t?
Uh, looks good to me. It seems to only be a temporary storage for a pointer anyways, so I have no idea why it'd be kept as u32.
The ppc64 mac99 machine still uses ADB and apparently OpenBIOS doesn't support USB keyboards yet.
Yeah, that piece is very broken though. Linux doesn't even allow you to compile in support for the ADB controller we have there on ppc64 Linux :(.
Acked-by: Alexander Graf agraf@suse.de
Alex
Am 30.10.2010 um 13:56 schrieb Alexander Graf:
On 30.10.2010, at 04:51, Andreas Färber wrote:
Hello Alex,
Are you able to ack this patch? Or are there any reasons why the state must be uint32_t?
Uh, looks good to me. It seems to only be a temporary storage for a pointer anyways, so I have no idea why it'd be kept as u32.
The ppc64 mac99 machine still uses ADB and apparently OpenBIOS doesn't support USB keyboards yet.
Yeah, that piece is very broken though. Linux doesn't even allow you to compile in support for the ADB controller we have there on ppc64 Linux :(.
Acked-by: Alexander Graf agraf@suse.de
Thanks, applied in r924.
Concerning ADB on ppc64, I think this is related to the OHCI warning on the serial console and complete lack of USB controller support in drivers/pci_database.c except for usb_iface definitions. Any help fixing this would be appreciated. I guess we would need the equivalent of sys_devices mapping the PCI IDs to OF property strings and values?
Andreas
On Tue, Oct 26, 2010 at 11:08 PM, Andreas Färber andreas.faerber@web.de wrote:
include/kernel/stack.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/kernel/stack.h b/include/kernel/stack.h index 809ffe9..44fef0f 100644 --- a/include/kernel/stack.h +++ b/include/kernel/stack.h @@ -31,8 +31,8 @@ typedef ucell phandle_t;
#ifdef NATIVE_BITWIDTH_EQUALS_HOST_BITWIDTH -#define pointer2cell(x) ((ucell)(x)) -#define cell2pointer(x) ((u8 *)(x)) +#define pointer2cell(x) ((ucell)(uintptr_t)(x)) +#define cell2pointer(x) ((u8 *)(uintptr_t)(x))
Perhaps this should be changed to inline function which returns void *, then we may avoid a few casts.
Am 27.10.2010 um 21:48 schrieb Blue Swirl:
On Tue, Oct 26, 2010 at 11:08 PM, Andreas Färber <andreas.faerber@web.de
wrote:
include/kernel/stack.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/kernel/stack.h b/include/kernel/stack.h index 809ffe9..44fef0f 100644 --- a/include/kernel/stack.h +++ b/include/kernel/stack.h @@ -31,8 +31,8 @@ typedef ucell phandle_t;
#ifdef NATIVE_BITWIDTH_EQUALS_HOST_BITWIDTH -#define pointer2cell(x) ((ucell)(x)) -#define cell2pointer(x) ((u8 *)(x)) +#define pointer2cell(x) ((ucell)(uintptr_t)(x)) +#define cell2pointer(x) ((u8 *)(uintptr_t)(x))
Perhaps this should be changed to inline function which returns void *, then we may avoid a few casts.
I wouldn't mind inline functions either.
I doubt this quickfix is right at all though. It makes things compile with minimal impact, but these decisions seem seriously flawed. switch- arch and these macros all assume that just pointer size matters. IMO we would need CELL_BITWIDTH_EQUALS_POINTER_BITWIDTH instead and should be using CELL_BITWIDTH_SMALLER_THAN_POINTER_BITWIDTH (two #ifdefs below) for ppc64 instead of making the equality code path (hardcoded in Makefile.target) handle the smaller-than case as well now. If we want to go ahead with my above patch I should at least add a TODO that this is wrong, no?
Andreas
On Wed, Oct 27, 2010 at 8:56 PM, Andreas Färber andreas.faerber@web.de wrote:
Am 27.10.2010 um 21:48 schrieb Blue Swirl:
On Tue, Oct 26, 2010 at 11:08 PM, Andreas Färber andreas.faerber@web.de wrote:
include/kernel/stack.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/kernel/stack.h b/include/kernel/stack.h index 809ffe9..44fef0f 100644 --- a/include/kernel/stack.h +++ b/include/kernel/stack.h @@ -31,8 +31,8 @@ typedef ucell phandle_t;
#ifdef NATIVE_BITWIDTH_EQUALS_HOST_BITWIDTH -#define pointer2cell(x) ((ucell)(x)) -#define cell2pointer(x) ((u8 *)(x)) +#define pointer2cell(x) ((ucell)(uintptr_t)(x)) +#define cell2pointer(x) ((u8 *)(uintptr_t)(x))
Perhaps this should be changed to inline function which returns void *, then we may avoid a few casts.
I wouldn't mind inline functions either.
I doubt this quickfix is right at all though. It makes things compile with minimal impact, but these decisions seem seriously flawed. switch-arch and these macros all assume that just pointer size matters. IMO we would need CELL_BITWIDTH_EQUALS_POINTER_BITWIDTH instead and should be using CELL_BITWIDTH_SMALLER_THAN_POINTER_BITWIDTH (two #ifdefs below) for ppc64 instead of making the equality code path (hardcoded in Makefile.target) handle the smaller-than case as well now.
Sounds about right.
If we want to go ahead with my above patch I should at least add a TODO that this is wrong, no?
I think it's your decision if you want to create even better patches, but I don't think this is too wrong, or at least we are not going to wrong direction.
Use const void* for pointer2cell() due to const char* arguments. Fix a misuse of pointer2cell().
Suggested by Blue.
Cc: Blue Swirl blauwirbel@gmail.com Signed-off-by: Andreas Färber andreas.faerber@web.de --- include/kernel/stack.h | 14 +++++++++++--- kernel/internal.c | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/kernel/stack.h b/include/kernel/stack.h index 2a78c62..a87c647 100644 --- a/include/kernel/stack.h +++ b/include/kernel/stack.h @@ -31,9 +31,17 @@ typedef ucell phandle_t;
#ifdef NATIVE_BITWIDTH_EQUALS_HOST_BITWIDTH -// XXX Check whether we can take the larger-than code path for ppc64 instead. -#define pointer2cell(x) ((ucell)(uintptr_t)(x)) -#define cell2pointer(x) ((u8 *)(uintptr_t)(x)) + +static inline ucell pointer2cell(const void* x) +{ + return (ucell)(uintptr_t)x; +} + +static inline void* cell2pointer(ucell x) +{ + return (void*)(uintptr_t)x; +} + #endif
static inline void PUSH(ucell value) { diff --git a/kernel/internal.c b/kernel/internal.c index 760c66d..91ab440 100644 --- a/kernel/internal.c +++ b/kernel/internal.c @@ -645,7 +645,7 @@ do_source_dbg( struct debug_xt *debug_xt_item ) case 'F': /* Start subordinate Forth interpreter */ PUSHR(PC - sizeof(cell)); - PC = pointer2cell(findword("outer-interpreter")) + sizeof(ucell); + PC = findword("outer-interpreter") + sizeof(ucell);
/* Save rstack position for when we return */ dbgrstackcnt = rstackcnt;
Blue,
Am 31.10.2010 um 18:44 schrieb Andreas Färber:
Use const void* for pointer2cell() due to const char* arguments. Fix a misuse of pointer2cell().
Suggested by Blue.
Cc: Blue Swirl blauwirbel@gmail.com Signed-off-by: Andreas Färber andreas.faerber@web.de
Discussing about #defines elsewhere, are you planning to review this one? If not, I'd go ahead and apply it since it found one erroneous caller.
The other patch I'm pretty neutral about. Is there a general rule of thumb in this fight against macros?
Andreas
On Mon, Nov 8, 2010 at 9:59 PM, Andreas Färber andreas.faerber@web.de wrote:
Blue,
Am 31.10.2010 um 18:44 schrieb Andreas Färber:
Use const void* for pointer2cell() due to const char* arguments. Fix a misuse of pointer2cell().
Suggested by Blue.
Cc: Blue Swirl blauwirbel@gmail.com Signed-off-by: Andreas Färber andreas.faerber@web.de
Discussing about #defines elsewhere, are you planning to review this one? If not, I'd go ahead and apply it since it found one erroneous caller.
Looks OK.
The other patch I'm pretty neutral about. Is there a general rule of thumb in this fight against macros?
Not really, I just suggested that out of common sense.
Am 27.10.2010 um 01:08 schrieb Andreas Färber:
Do the double-dereference in two steps to avoid garbage in the high 32 address bits on ppc64.
Signed-off-by: Andreas Färber andreas.faerber@web.de
If no one objects, I'll commit this one and some switch-arch support to try it on ppc64 later today.
kernel/internal.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/kernel/internal.c b/kernel/internal.c index 0235737..760c66d 100644 --- a/kernel/internal.c +++ b/kernel/internal.c @@ -329,10 +329,11 @@ static void doplusloop(void) #ifndef FCOMPILER static ucell get_myself(void) {
- static ucell **myself = NULL;
- if( !myself )
myself = (ucell**)findword("my-self") + 1;- return (*myself && **myself) ? (ucell)**myself : 0;
- static ucell *myselfptr = NULL;
- if (myselfptr == NULL)
myselfptr = (ucell*)cell2pointer(findword("my-self")) + 1;- ucell *myself = (ucell*)cell2pointer(*myselfptr);
- return (myself != NULL) ? *myself : 0;
}
static void doivar(void)
1.7.3
-- OpenBIOS http://openbios.org/ Mailinglist: http://lists.openbios.org/mailman/listinfo Free your System - May the Forth be with you
Am 17.10.2010 um 15:45 schrieb Blue Swirl:
Please [...] remove the casts where possible:
unsigned char *dest = (unsigned char *)POP();
unsigned char *dest = cell2pointer(POP());
I wasn't able to remove most char* casts due to pointer signedness warnings but I managed to drop some more ucell casts.
Andreas
Blue Swirl wrote:
Most problems were caused by the assumption that a conversion between ucell und void* is possible; I added (unsigned long) casts or reused the cell2pointer/pointer2cell macros.
Those should be merged with kernel/cross.h ones and used everywhere.
Indeed. Is there a way we can make the compiler throw an error if (u)cells are assigned directly to pointers for everyone?
ATB,
Mark.
On Mon, Oct 18, 2010 at 11:19 AM, Mark Cave-Ayland mark.cave-ayland@siriusit.co.uk wrote:
Blue Swirl wrote:
Most problems were caused by the assumption that a conversion between ucell und void* is possible; I added (unsigned long) casts or reused the cell2pointer/pointer2cell macros.
Those should be merged with kernel/cross.h ones and used everywhere.
Indeed. Is there a way we can make the compiler throw an error if (u)cells are assigned directly to pointers for everyone?
'typedef cell struct something' should do the trick. This could be useful also for the 128 bit ducell case.
-
Alexander Graf -
Andreas Färber -
Blue Swirl -
Mark Cave-Ayland