| Message ID | 20200517003354.233373-10-marex@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: ks8851: Unify KS8851 SPI and MLL drivers | expand |
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@denx.de> > Cc: David S. Miller <davem@davemloft.net> > Cc: Lukas Wunner <lukas@wunner.de> > Cc: Petr Stetiar <ynezz@true.cz> > Cc: YueHaibing <yuehaibing@huawei.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? Andrew
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@denx.de> >> Cc: David S. Miller <davem@davemloft.net> >> Cc: Lukas Wunner <lukas@wunner.de> >> Cc: Petr Stetiar <ynezz@true.cz> >> Cc: YueHaibing <yuehaibing@huawei.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.
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);
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@denx.de> Cc: David S. Miller <davem@davemloft.net> Cc: Lukas Wunner <lukas@wunner.de> Cc: Petr Stetiar <ynezz@true.cz> Cc: YueHaibing <yuehaibing@huawei.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(-)