From: Andrew Lunn <andrew@lunn.ch>
To: Marek Vasut <marex@denx.de>
Cc: netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
Lukas Wunner <lukas@wunner.de>, Petr Stetiar <ynezz@true.cz>,
YueHaibing <yuehaibing@huawei.com>
Subject: Re: [PATCH 08/14] net: ks8851: Use 16-bit read of RXFC register
Date: Tue, 24 Mar 2020 14:32:53 +0100 [thread overview]
Message-ID: <20200324133253.GX3819@lunn.ch> (raw)
In-Reply-To: <8079699e-8235-c800-44a8-022ade8140f1@denx.de>
On Tue, Mar 24, 2020 at 01:50:53PM +0100, Marek Vasut wrote:
> On 3/24/20 2:50 AM, Andrew Lunn wrote:
> >> @@ -470,7 +455,7 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)
> >> unsigned rxstat;
> >> u8 *rxpkt;
> >>
> >> - rxfc = ks8851_rdreg8(ks, KS_RXFC);
> >> + rxfc = (ks8851_rdreg16(ks, KS_RXFCTR) >> 8) & 0xff;
> >
> > The datasheet says:
> >
> > 2. When software driver reads back Receive Frame Count (RXFCTR)
> > Register; the KSZ8851 will update both Receive Frame Header Status and
> > Byte Count Registers (RXFHSR/RXFHBCR)
> >
> > Are you sure there is no side affect here?
>
> Yes, look at the RXFC register 0x9c itself. It's a 16bit register, 0x9c
> is the LSByte and 0x9d is the MSByte.
>
> What happened here before was readout of register 0x9d, MSByte of RXFC,
> which triggers the update of RXFHSR/RXFHBCR. What happens now is the
> readout of the whole RXFC as 16bit value, which also triggers the update.
Hi Marek
It would be nice to indicate in the commit message that things like
this have been considered. As a reviewer, these are the sort of
questions which goes through my mind. If there is a comment it has
been considered, i get the answer to my questions without having to
ask.
Andrew
next prev parent reply other threads:[~2020-03-24 13:32 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-23 23:42 [PATCH 00/14] net: ks8851: Unify KS8851 SPI and MLL drivers Marek Vasut
2020-03-23 23:42 ` [PATCH 01/14] net: ks8851: Factor out spi->dev in probe()/remove() Marek Vasut
2020-03-24 1:15 ` Andrew Lunn
2020-03-24 6:46 ` Lukas Wunner
2020-03-23 23:42 ` [PATCH 02/14] net: ks8851: Replace dev_err() with netdev_err() in IRQ handler Marek Vasut
2020-03-24 1:16 ` Andrew Lunn
2020-03-23 23:42 ` [PATCH 03/14] net: ks8851: Pass device pointer into ks8851_init_mac() Marek Vasut
2020-03-24 1:06 ` Andrew Lunn
2020-03-24 7:08 ` Lukas Wunner
2020-03-23 23:42 ` [PATCH 04/14] net: ks8851: Use devm_alloc_etherdev() Marek Vasut
2020-03-24 1:19 ` Andrew Lunn
2020-03-23 23:42 ` [PATCH 05/14] net: ks8851: Use dev_{get,set}_drvdata() Marek Vasut
2020-03-24 1:22 ` Andrew Lunn
2020-03-23 23:42 ` [PATCH 06/14] net: ks8851: Remove ks8851_rdreg32() Marek Vasut
2020-03-24 1:30 ` Andrew Lunn
2020-03-24 12:34 ` Marek Vasut
2020-03-24 7:22 ` Lukas Wunner
2020-03-24 12:37 ` Marek Vasut
2020-03-23 23:42 ` [PATCH 07/14] net: ks8851: Use 16-bit writes to program MAC address Marek Vasut
2020-03-24 1:40 ` Andrew Lunn
2020-03-24 7:17 ` Michal Kubecek
2020-03-24 8:13 ` Lukas Wunner
2020-03-24 12:25 ` Andrew Lunn
2020-03-24 12:36 ` Lukas Wunner
2020-03-24 13:09 ` Marek Vasut
2020-03-24 13:31 ` Marek Vasut
2020-03-24 14:47 ` Lukas Wunner
2020-03-24 14:53 ` Marek Vasut
2020-03-23 23:42 ` [PATCH 08/14] net: ks8851: Use 16-bit read of RXFC register Marek Vasut
2020-03-24 1:50 ` Andrew Lunn
2020-03-24 12:50 ` Marek Vasut
2020-03-24 13:32 ` Andrew Lunn [this message]
2020-03-24 10:41 ` Lukas Wunner
2020-03-24 12:42 ` Marek Vasut
2020-03-23 23:42 ` [PATCH 09/14] net: ks8851: Split out SPI specific entries in struct ks8851_net Marek Vasut
2020-03-24 1:55 ` Andrew Lunn
2020-03-24 10:49 ` Lukas Wunner
2020-03-24 12:27 ` Andrew Lunn
2020-03-23 23:42 ` [PATCH 10/14] net: ks8851: Split out SPI specific code from probe() and remove() Marek Vasut
2020-03-24 1:58 ` Andrew Lunn
2020-03-23 23:43 ` [PATCH 11/14] net: ks8851: Implement register and FIFO accessor callbacks Marek Vasut
2020-03-24 13:45 ` Lukas Wunner
2020-03-24 14:10 ` Marek Vasut
2020-03-24 14:29 ` Lukas Wunner
2020-03-24 14:44 ` Marek Vasut
2020-03-29 14:22 ` Marek Vasut
2020-03-23 23:43 ` [PATCH 12/14] net: ks8851: Separate SPI operations into separate file Marek Vasut
2020-03-23 23:43 ` [PATCH 13/14] net: ks8851: Implement Parallel bus operations Marek Vasut
2020-03-24 8:16 ` kbuild test robot
2020-03-24 8:16 ` kbuild test robot
2020-03-23 23:43 ` [PATCH 14/14] net: ks8851: Remove ks8851_mll.c Marek Vasut
2020-03-24 14:08 ` Lukas Wunner
2020-03-24 14:12 ` Marek Vasut
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=20200324133253.GX3819@lunn.ch \
--to=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=lukas@wunner.de \
--cc=marex@denx.de \
--cc=netdev@vger.kernel.org \
--cc=ynezz@true.cz \
--cc=yuehaibing@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.