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  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:   Sun, 17 May 2020 22:02:09 +0200
From:   Marek Vasut <marex@...x.de>
To:     Andrew Lunn <andrew@...n.ch>
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 V6 09/20] net: ks8851: Use 16-bit read of RXFC register

On 5/17/20 9:44 PM, Andrew Lunn wrote:
> On Sun, May 17, 2020 at 02:33:43AM +0200, Marek Vasut wrote:
>> The RXFC register is the only one being read using 8-bit accessors.
>> To make it easier to support the 16-bit accesses used by the parallel
>> bus variant of KS8851, use 16-bit accessor to read RXFC register as
>> well as neighboring RXFCTR register.
>>
>> Remove ks8851_rdreg8() as it is not used anywhere anymore.
>>
>> There should be no functional change.
>>
>> Signed-off-by: Marek Vasut <marex@...x.de>
>> Cc: David S. Miller <davem@...emloft.net>
>> Cc: Lukas Wunner <lukas@...ner.de>
>> Cc: Petr Stetiar <ynezz@...e.cz>
>> Cc: YueHaibing <yuehaibing@...wei.com>
>> ---
>> V2: No change
>> V3: No change
>> V4: Drop the NOTE from the comment, the performance is OK
>> V5: No change
>> V6: No change
>> ---
>>  drivers/net/ethernet/micrel/ks8851.c | 17 +----------------
>>  1 file changed, 1 insertion(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
>> index 1b81340e811f..e2e75041e931 100644
>> --- a/drivers/net/ethernet/micrel/ks8851.c
>> +++ b/drivers/net/ethernet/micrel/ks8851.c
>> @@ -236,21 +236,6 @@ static void ks8851_rdreg(struct ks8851_net *ks, unsigned op,
>>  		memcpy(rxb, trx + 2, rxl);
>>  }
>>  
>> -/**
>> - * ks8851_rdreg8 - read 8 bit register from device
>> - * @ks: The chip information
>> - * @reg: The register address
>> - *
>> - * Read a 8bit register from the chip, returning the result
>> -*/
>> -static unsigned ks8851_rdreg8(struct ks8851_net *ks, unsigned reg)
>> -{
>> -	u8 rxb[1];
>> -
>> -	ks8851_rdreg(ks, MK_OP(1 << (reg & 3), reg), rxb, 1);
>> -	return rxb[0];
>> -}
>> -
>>  /**
>>   * ks8851_rdreg16 - read 16 bit register from device
>>   * @ks: The chip information
>> @@ -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;
>>  
>>  	netif_dbg(ks, rx_status, ks->netdev,
>>  		  "%s: %d packets\n", __func__, rxfc);
> 
> Hi Marek
> 
> This is the patch which i think it causing most problems. So why not
> add accessors ks8851_rd_rxfc_spi() and ks8851_rd_rxfc_par(), each
> doing which is optimal for each?

Because it makes no difference when I do iperf tests on current
linux-next. I think I mentioned this before a few times, but the real
performance improvement here would be gained if this whole driver was
converted to regmap and then the regmap core could merge the SPI
transfers as needed. But that is something for another series.

Powered by blists - more mailing lists