On Sat, May 24, 2014 at 5:17 PM, Michael Tokarev <m...@tls.msk.ru> wrote:
> 24.05.2014 03:06, Peter Crosthwaite wrote:
>> Ping^2!
>>
>> I'll try trivial queue too :)
>
> Actually this looks like trivial material.
>
> I'll comment in one place, for all.
>
>
>>>> Peter Crosthwaite (4):
>>>> net: cadence_gem: Fix Tx descriptor update
>
> This appears to be a bugfix, but with an interesting "incomplete"
> update:
>
> ...
> + unsigned desc_first[2];
> ...
> cpu_physical_memory_read(s->tx_desc_addr,
> - (uint8_t *)&desc[0], sizeof(desc));
> + (uint8_t *)&desc_first[0],
> sizeof(desc));
>
> sizeof(desc_first), not sizeof(desc). Also can drop extra
> "readdressing".
>
Fixed.
>>>> net: cadence_gem: Add Tx descriptor fetch printf
>
> + DB_PRINT("read descriptor 0x%x\n", (unsigned)packet_desc_addr);
>
> packet_desc_addr is hwaddr, which is uint64_t, here it is being cast
> to unsigned, Is it right?
Probably not.
Other code in this file does the same,
> but it still does not mean it's right. Maybe it is because it uses
> only the lower 32 bits of it, or that the high bits just aren't
> useful for debugging?
>
Well they are not useful because the IP is only 32-bit but converting
to HWADDR_PRIx because its probably the right thing to do anyway.
>>>> net: cadence_gem: Fix top comment
>
> I applied this one.
>
Thanks.
Spinning V2. Thanks for review.
Regards,
Peter
>>>> net: cadence_gem: Comment spelling sweep
>
> This look okay, except that it clashes a bit with the first.
>
> /mjt
>
