From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Finn Thain <fthain@telegraphics.com.au>,
Jason Wang <jasowang@redhat.com>,
qemu-devel@nongnu.org
Cc: Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>,
Herve Poussineau <hpoussin@reactos.org>,
Laurent Vivier <laurent@vivier.eu>,
qemu-stable@nongnu.org
Subject: Re: [PATCH 01/10] dp8393x: Mask EOL bit from descriptor addresses
Date: Sat, 14 Dec 2019 14:35:34 +0100 [thread overview]
Message-ID: <7f28e2ae-e3d1-d4b8-f753-c418b72293c6@redhat.com> (raw)
In-Reply-To: <7d220205700c43b15d6ae6cefd6520a97c763709.1576286757.git.fthain@telegraphics.com.au>
Hi Finn,
On 12/14/19 2:25 AM, Finn Thain wrote:
> The LSB of descriptor address registers is used as an EOL flag.
> It has to be masked when those registers are to be used as actual
> addresses for copying memory around. But when the registers are
> to be updated the EOL bit should not be masked.
>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
> hw/net/dp8393x.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 3d991af163..164311c055 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -197,7 +197,7 @@ static uint32_t dp8393x_crba(dp8393xState *s)
>
> static uint32_t dp8393x_crda(dp8393xState *s)
> {
> - return (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA];
> + return (s->regs[SONIC_URDA] << 16) | (s->regs[SONIC_CRDA] & 0xfffe);
> }
>
> static uint32_t dp8393x_rbwc(dp8393xState *s)
> @@ -217,7 +217,7 @@ static uint32_t dp8393x_tsa(dp8393xState *s)
>
> static uint32_t dp8393x_ttda(dp8393xState *s)
> {
> - return (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA];
> + return (s->regs[SONIC_UTDA] << 16) | (s->regs[SONIC_TTDA] & 0xfffe);
> }
>
> static uint32_t dp8393x_wt(dp8393xState *s)
> @@ -506,8 +506,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
> sizeof(uint16_t) *
> (4 + 3 * s->regs[SONIC_TFC]) * width,
> MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0);
> - s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1;
> - if (dp8393x_get(s, width, 0) & 0x1) {
> + s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0);
> + if (s->regs[SONIC_CTDA] & 0x1) {
Can you add a definition for the EOL bit and use it, instead of these
magic 0x1/0xfffe values? That way the meaning will be obvious for future
reviewers.
Thanks,
Phil.
> /* EOL detected */
> break;
> }
>
next prev parent reply other threads:[~2019-12-14 21:38 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-14 1:25 [PATCH 00/10] Fixes for DP8393X SONIC device emulation Finn Thain
2019-12-14 1:25 ` [PATCH 08/10] dp8393x: Implement packet size limit and RBAE interrupt Finn Thain
2019-12-14 1:25 ` [PATCH 09/10] dp8393x: Don't stop reception upon RBE interrupt assertion Finn Thain
2019-12-14 1:25 ` [PATCH 06/10] dp8393x: Clear RRRA command register bit only when appropriate Finn Thain
2019-12-14 13:31 ` Philippe Mathieu-Daudé
2019-12-14 1:25 ` [PATCH 02/10] dp8393x: Clean up endianness hacks Finn Thain
2019-12-14 1:25 ` [PATCH 05/10] dp8393x: Update LLFA register Finn Thain
2019-12-14 1:25 ` [PATCH 10/10] dp8393x: Don't clobber packet checksum Finn Thain
2019-12-14 13:21 ` Philippe Mathieu-Daudé
2019-12-14 1:25 ` [PATCH 07/10] dp8393x: Implement TBWC0 and TBWC1 registers to restore buffer state Finn Thain
2019-12-14 1:25 ` [PATCH 01/10] dp8393x: Mask EOL bit from descriptor addresses Finn Thain
2019-12-14 13:35 ` Philippe Mathieu-Daudé [this message]
2019-12-14 23:21 ` Finn Thain
2019-12-14 1:25 ` [PATCH 04/10] dp8393x: Don't advance RX descriptor twice Finn Thain
2019-12-14 1:25 ` [PATCH 03/10] dp8393x: Have dp8393x_receive() return the packet size Finn Thain
2019-12-14 13:26 ` Philippe Mathieu-Daudé
2019-12-14 1:43 ` [PATCH 00/10] Fixes for DP8393X SONIC device emulation no-reply
2019-12-14 2:52 ` Finn Thain
2019-12-14 13:38 ` Philippe Mathieu-Daudé
2019-12-14 13:45 ` Eric Blake
2019-12-14 17:17 ` Aleksandar Markovic
2019-12-14 23:16 ` Finn Thain
2019-12-14 23:32 ` Aleksandar Markovic
2019-12-14 23:35 ` Aleksandar Markovic
2019-12-20 4:24 ` Finn Thain
2019-12-23 17:17 ` Philippe Mathieu-Daudé
2019-12-24 0:12 ` NetBSD/arc on MIPS Magnum, was " Finn Thain
2019-12-24 4:33 ` Finn Thain
2019-12-24 6:53 ` Hervé Poussineau
2020-01-06 22:15 ` Finn Thain
2019-12-16 0:36 ` Finn Thain
2019-12-20 4:21 ` Finn Thain
2019-12-20 11:38 ` Aleksandar Markovic
2019-12-20 12:03 ` Aleksandar Markovic
2019-12-20 12:54 ` Laurent Vivier
2019-12-20 23:22 ` Finn Thain
2019-12-21 12:03 ` Aleksandar Markovic
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=7f28e2ae-e3d1-d4b8-f753-c418b72293c6@redhat.com \
--to=philmd@redhat.com \
--cc=aleksandar.rikalo@rt-rk.com \
--cc=fthain@telegraphics.com.au \
--cc=hpoussin@reactos.org \
--cc=jasowang@redhat.com \
--cc=laurent@vivier.eu \
--cc=qemu-devel@nongnu.org \
--cc=qemu-stable@nongnu.org \
/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).