qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Michael Tokarev <mjt@tls.msk.ru>
To: Peter Crosthwaite <peter.crosthwaite@xilinx.com>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	qemu-trivial <qemu-trivial@nongnu.org>
Cc: Edgar Iglesias <edgar.iglesias@xilinx.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH net v1 0/4] Cadence GEM patches
Date: Sat, 24 May 2014 11:17:52 +0400	[thread overview]
Message-ID: <538047A0.2090704@msgid.tls.msk.ru> (raw)
In-Reply-To: <CAEgOgz46UcJiGC6H==O1XZFkvZh4Wa+2kBy8DDwAfKG6SUmp+g@mail.gmail.com>

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".

>>>   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?  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?

>>>   net: cadence_gem: Fix top comment

I applied this one.

>>>   net: cadence_gem: Comment spelling sweep

This look okay, except that it clashes a bit with the first.

/mjt

  reply	other threads:[~2014-05-24  7:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-07  9:15 [Qemu-devel] [PATCH net v1 0/4] Cadence GEM patches Peter Crosthwaite
2014-05-07  9:16 ` [Qemu-devel] [PATCH net v1 1/4] net: cadence_gem: Fix Tx descriptor update Peter Crosthwaite
2014-05-24  6:38   ` [Qemu-devel] [net, v1, " Michael Tokarev
2014-05-24  6:48     ` Michael Tokarev
2014-05-07  9:16 ` [Qemu-devel] [PATCH net v1 2/4] net: cadence_gem: Add Tx descriptor fetch printf Peter Crosthwaite
2014-05-07  9:17 ` [Qemu-devel] [PATCH net v1 3/4] net: cadence_gem: Fix top comment Peter Crosthwaite
2014-05-07  9:18 ` [Qemu-devel] [PATCH net v1 4/4] net: cadence_gem: Comment spelling sweep Peter Crosthwaite
2014-05-14 13:17 ` [Qemu-devel] [PATCH net v1 0/4] Cadence GEM patches Peter Crosthwaite
2014-05-23 23:06   ` Peter Crosthwaite
2014-05-24  7:17     ` Michael Tokarev [this message]
2014-05-26  7:44       ` [Qemu-devel] [Qemu-trivial] " Peter Crosthwaite

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=538047A0.2090704@msgid.tls.msk.ru \
    --to=mjt@tls.msk.ru \
    --cc=edgar.iglesias@xilinx.com \
    --cc=peter.crosthwaite@xilinx.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).