On Fri, Jul 8, 2011 at 3:08 PM, Andreas Fritiofson
<andreas.fritiof...@gmail.com> wrote:
>
>
> On Fri, Jul 8, 2011 at 1:26 PM, Drasko DRASKOVIC
> <drasko.drasko...@gmail.com> wrote:
>>
>> On Fri, Jul 8, 2011 at 1:19 PM, Andreas Fritiofson
>> <andreas.fritiof...@gmail.com> wrote:
>> > I looked briefly at the memory read functions in mips32_dmaacc.c and
>> > mips32_pracc.c and it looks like the type usage is a bit confused. The
>> > difference between the *_read_mem{32,16,8} functions should only be what
>> > kind of access is made *on the target*.
>> How do we determine that ?
>>
>> I thought that it is kept in the "size" parameter, and
>> mips32_pracc_read_mem is doing exactly this :
>>
>> int mips32_pracc_read_mem(struct mips_ejtag *ejtag_info, uint32_t
>> addr, int size, int count, void *buf)
>> {
>>        switch (size)
>>        {
>>                case 1:
>>                        return mips32_pracc_read_mem8(ejtag_info, addr,
>> count, (uint8_t*)buf);
>>                case 2:
>>                        return mips32_pracc_read_mem16(ejtag_info, addr,
>> count, (uint16_t*)buf);
>>                case 4:
>>                        if (count == 1)
>>                                return mips32_pracc_read_u32(ejtag_info,
>> addr, (uint32_t*)buf);
>>                        else
>>                                return mips32_pracc_read_mem32(ejtag_info,
>> addr, count, (uint32_t*)buf);
>>        }
>>
>>        return ERROR_OK;
>> }
>>
>> > Host data buffer type should be
>> > identical, preferrably void*, with no alignment requirement, and count
>> > should be in number of bytes.
>>
>> Problem is not in the mips32_pracc.c, thought, but when you come back
>> to mips_m4k_read_memory(), in which buf is uint8_t*.
>
> But already here buf is cast to uint{8,16,32}_t* (from void* so no warning
> but equally risky) which shouldn't be needed because all read_mem* functions
> could accept a void* buffer and write the data to arbitrary alignment. Main
> point is that the alignment requirement (buffer data type) on the host
> should not be coupled to the access size used on the target.

I do not know all the OpenOCD internals, but I guess that "size"
represent data access size you are mentioning.
like, if count is 5 and size is 4 that would mean " I want 5  ints".

If my presumption is true, then I agree with you, and looking at the
PrAcc code that we can safely use void for these buffer pointers.
They are just addresses where read data will be put.

However, data will be read (accessed) in the "size" - if it is 4 we
will use MIPS32_LW which loads word from memory, and if it is 2, we
will use MIPS32_LHU - load halfword unsigned.

So, from this point everything seems to be OK, except from this
unnecessary casting void* to uintX_t*.

Problem is however in  mips_m4k_read_memory(), where you have explicit
casting t32 = *(uint32_t*)&buffer[i];

> And also the
> data returned in the host buffer should be identical, regardless of which
> target access size is chosen (this requirement probably gives the answer to
> whether endian swapping should be done here or not). Does this make sense?
Yes, and it is (IMHO).

> Of course you may not be able to do an access with size 2 or 4 to an
> unaligned *target address* but that has nothing to do with host buffer
> alignment.
>
> If the memory functions only handle byte addressed generic memory blocks,
> there are no endian issues (here). They pop up first when higher level code
> tries to interpret the memory contents as multi-byte entities (instructions,
> addresses, ...) in which case that code must be aware of the target
> endianness. Again, I may be confused here.

I've got lost here...

BR,
Drasko
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to