On Sunday, 03 Mar 2013 11:33:42 Jeremy Bennett wrote:
> 1. Could you update the documentation (in the doc folder) to match.

hmmm, i thought the changes i submitted for doc/or1ksim.texi were sufficient.  
what more do you want updated?

> 2. Could you supply a git patch if at all possible. The main repo is at
> https://github.com/openrisc/or1ksim.

yep, can do.  i'll wait on the answer regarding the docs before submitting.

> One day the problem will go away, when we can drop the or32 branch.

so maybe i'll submit against just or1k to accelerate the process a tiny bit.

> I have some questions/commentary on your code. You don't need to do
> anything about these - it's just to help my understanding.
> 
> 1. peripheral/memory.c: I'm curious about the use of "seed" for the file
> handle name. Personally I'd have defined a variable "fh" (or similar) to
> avoid confusion.

i thought about adding another variable as well but realized the file is 
seeding the memory so it works.

> 2. peripheral/memory.c: I'm not sure worrying about endianness is a good
> thing here. I would be inclined to say that the file is just a byte
> image, and it is up to the user to ensure the sequence of bytes matches
> the endianness of the machine. It will mean the file can always be used
> for other tools as a true image of memory for the particular machine. It
> is trivial for a user to have an external utility to swap byte order to
> create a new file if that is what they need.

if i seed memory with a file containing "AbCd" and use a simple program like 
the attached dumpROM.c, i expect to get "AbCd".  on my x86_64-unknown-linux-gnu 
host without swabbing i get "dCbA".  that will be confusing for users not 
familiar with endianness.  we could send them haring off through the user 
manual and the file could be converted offline but it seems kinder to the user 
to do it for them.

> 3. Is a straight binary file what you want? Had you considered
> supporting the image formats used by Verilog $readmemh and $readmemb.
> Maybe these should be further memory options, "readmemh" and "readmemb"
> for the future.

well, to an old Un*x hacker, _ALL_ files are binary  :{)}.  my source data is a 
pcap file so definitely "straight binary".

i am not that familiar with Verilog, barely learning VHDL right now to tell the 
truth, but i did look up these with Google and they are definitely not what i 
want.  i agree they deserve to be implemented but i expect the endianness 
problem to show up here as well so better to have a correct implementation 
already coded in memory.c.

:{)}
#include <stdio.h>

/* adjust according to configuration */
#define ROMADDR ((void*)0xe0000000)
#define ROMSIZE (64*1024*1024)


int main(int argc, char* argv[]) {
  unsigned char* ROM = ROMADDR;
  for (int i = ROMSIZE; --i >= 0; )
    putchar(*ROM++);
  return 0;
}
_______________________________________________
OpenRISC mailing list
[email protected]
http://lists.openrisc.net/listinfo/openrisc

Reply via email to