lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 24 Mar 2020 14:32:53 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Marek Vasut <marex@...x.de>
Cc:     netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
        Lukas Wunner <lukas@...ner.de>, Petr Stetiar <ynezz@...e.cz>,
        YueHaibing <yuehaibing@...wei.com>
Subject: Re: [PATCH 08/14] net: ks8851: Use 16-bit read of RXFC register

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ