Subject:
Re: [gnupic] gpdasm 0.13.6 alpha question
From:
Marko Kohtala ####@####.####
Date:
21 Nov 2009 13:30:57 -0000
Message-Id: <9cfa10eb0911201255r1ac678dh942fdcf941a75b4c@mail.gmail.com>
I've been looking into byte vs. word addressing in gputils when ever I
have had some spare time. That is not very often, so the progress has
been very slow.
Since it now seems I'll eventually end up with some sort of a patch,
I'll let you know I'm working on it and report my current idea for it.
I've changed MemBlock to byte storage. I'll use the byte address for
everything internally. For instruction handling I always treat an
instruction as two bytes and for instruction writing and reading there
will be functions that write and read the two bytes. Instruction
memory usage is therefore also managed byte by byte. When ever an
instruction address is needed (in COFF or COD headers or messages for
ROM sections, or in assembly) it'll see if its not PIC16E and divide
byte address by two.
I've also turned processor class into a structure that holds what is
needed to remove a number of processor class switch statements around
the code.
Not everything compiles yet, and I expect it'll take some time to
clean it up and get it to pass the tests again. If someone likes to
take a look and comment or continue on it, I can send the patch in
whatever state it happens to be.
Marko
On Tue, Mar 17, 2009 at 3:32 PM, David Barnett ####@####.#### wrote:
> Marko, that sounds right to me. Our handling of byte vs. word addressing is
> ugly throughout the system, and it definitely seems like we need to give
> byte order and address type an explicit representation.
>
> If you come up with something, I'd love to see it.
>
> David
>
> On Tue, Mar 17, 2009 at 5:02 AM, Marko Kohtala ####@####.####
>
>> I too have been trying to wrap my head around these memory issues.
>>
>> Which is rather difficult as there seem to be things which are not
>> obvious and there are little comments to explain what is the design
>> choice. Does any developer have any feelings the code is better off
>> without such comments?
>>
>> This particular bug seems to be in the gpreadhex.c. It combines two
>> bytes into one word, hence dividing each address in .hex by two to get
>> the address for MemBlock access. However it does not do this for the
>> .hex record 04 value which sets the page. Therefore the data after
>> this records ends up into addresses twice too large in MemBlock. Of
>> course the values in page 0 are not affected.
>>
>> My current understanding is it would be clearer if MemBlock was
>> changed to be byte storage. It would then record separately for each
>> byte whether they have been used or not and these .hex file readers
>> and few other places need not be so complex to support partial words.
>>
>> Instead of scattering the processor class specifics in accessing
>> MemBlock around, I'd rather use some object oriented design and access
>> MemBlock through the processor class. The processor class would then
>> uniformly solve these addressing issues, as well as byte order issues.
>> This design change easily blows up because it could replace also a
>> number of other switch-case and if-else constructs around the code
>> with single call to processor class function.
>>
>> It'd take some thought.
>>
>> Maybe a simple fix to this gpdasm problem could be made before such
>> design changes. I attach a patch. Robert, does this solve the problem?
>>
>> Marko
>>
>> On Tue, Mar 17, 2009 at 9:04 AM, Peter Keller ####@####.#### wrote:
>> > On Wed, Mar 11, 2009 at 09:41:10PM +0000, Robert Pearce wrote:
>> >> Given what you've posted, it appears gpdasm may be misinterpreting the
>> >> "type 04" blocks - the lines of the HEX file that begin ":02000004"
>> >
>> > So, I inspected the readhex() function and followed what it was doing
>> during
>> > the gpdasm, and eventually found that it led to here:
>> >
>> > MemBlock * i_memory_new(MemBlock *m, MemBlock *mbp, unsigned int
>> base_address)
>> > {
>> > unsigned int base;
>> >
>> > /* added by psilord */
>> > printf("i_memory_new raw base_address = 0x%x\n", base_address);
>> >
>> > base = (base_address >> I_MEM_BITS) & 0xFFFF;
>> >
>> > /* added by psilord */
>> > printf("i_memory_new cooked base_address = 0x%x\n", base);
>> >
>> > mbp->memory = (unsigned int *)calloc(MAX_I_MEM, sizeof(unsigned int));
>> > mbp->base = base;
>> >
>> > do {
>> >
>> > if((m->next == NULL) || (m->next->base > base)) {
>> > /* Insert after this block */
>> > mbp->next = m->next;
>> > m->next = mbp;
>> > return mbp;
>> > }
>> >
>> > m = m->next;
>> > } while(m);
>> >
>> > assert(0);
>> >
>> > return NULL;
>> > }
>> >
>> >
>> > Here is some sample output:
>> >
>> > i_memory_new raw base_address = 0x30000000
>> > i_memory_new cooked base_address = 0x6000
>> >
>> > i_memory_new raw base_address = 0xf0000000
>> > i_memory_new cooked base_address = 0xe000
>> >
>> > So, from what I can gather, when readhex is supplying to i_memory_put the
>> > address to which it wants to write, the base address gets miscomputed in
>> the
>> > above function when it is a "new" address in a different memblock.
>> >
>> > Given the portion of code in gpdasm.c:dasm() which shifts left the
>> > base address of the mem block by one bit of the processor is of class
>> > PROC_CLASS_16E, maybe when i_memory_new is called and the state.class
>> > is PROC_CLASS_16E, it should shift right the base address by one more
>> > bit so it doesn't get truncated by the 0xFFFF? I hacked a test of that by
>> > hand hand it seemed to do the right thing. However, the state variable
>> > doesn't seem to be used in gpmemory.c, and I fear if I add that
>> dependancy
>> > in there I'm doing something wrong since it wasn't already in there. ;)
>> >
>> > Any ideas?
>> >
>> > Thanks.
>> >
>> > -pete
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: ####@####.####
>> > For additional commands, e-mail: ####@####.####
>> >
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: ####@####.####
>> For additional commands, e-mail: ####@####.####
>>
>
