diff mbox series

[V4,07/19] net: ks8851: Remove ks8851_rdreg32()

Message ID 20200414182029.183594-8-marex@denx.de
State Deferred
Delegated to: David Miller
Headers show
Series net: ks8851: Unify KS8851 SPI and MLL drivers | expand

Commit Message

Marek Vasut April 14, 2020, 6:20 p.m. UTC
The ks8851_rdreg32() is used only in one place, to read two registers
using a single read. To make it easier to support 16-bit accesses via
parallel bus later on, replace this single read with two 16-bit reads
from each of the registers and drop the ks8851_rdreg32() altogether.

If this has noticeable performance impact on the SPI variant of KS8851,
then we should consider using regmap to abstract the SPI and parallel
bus options and in case of SPI, permit regmap to merge register reads
of neighboring registers into single, longer, read.

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
---
 drivers/net/ethernet/micrel/ks8851.c | 25 ++-----------------------
 1 file changed, 2 insertions(+), 23 deletions(-)

Comments

Lukas Wunner April 20, 2020, 2:07 p.m. UTC | #1
On Tue, Apr 14, 2020 at 08:20:17PM +0200, Marek Vasut wrote:
> The ks8851_rdreg32() is used only in one place, to read two registers
> using a single read. To make it easier to support 16-bit accesses via
> parallel bus later on, replace this single read with two 16-bit reads
> from each of the registers and drop the ks8851_rdreg32() altogether.
> 
> If this has noticeable performance impact on the SPI variant of KS8851,
> then we should consider using regmap to abstract the SPI and parallel
> bus options and in case of SPI, permit regmap to merge register reads
> of neighboring registers into single, longer, read.

Bisection has shown this patch to be the biggest cause of the performance
regression introduced by this series:  Latency increases by about 9 usec.

Removal of the 8-bit register access in patch [9/19] further increases
latency by about 2 usec.

In other words, performing two 16-bit SPI transfers to read a 32-bit
register has a more significant impact than reading an 8-bit register
as 16-bits.  Nevertheless both should be performed with their native
size if the driver is used in SPI mode.

Therefore please fold the patch below into the next version of your
series.  The ks8851_mll portion is compile-tested only.

Thanks,

Lukas

-- >8 --

From: Lukas Wunner <lukas@wunner.de>
Subject: [PATCH] net: ks8851: Reinstate 8-bit and 32-bit register access

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/net/ethernet/micrel/ks8851.h        |  4 +++
 drivers/net/ethernet/micrel/ks8851_common.c | 34 +++++++++++++++++--
 drivers/net/ethernet/micrel/ks8851_par.c    | 31 ++++++++++++++++++
 drivers/net/ethernet/micrel/ks8851_spi.c    | 36 +++++++++++++++++++++
 4 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.h b/drivers/net/ethernet/micrel/ks8851.h
index e218e8b7d364..757c49dbf0be 100644
--- a/drivers/net/ethernet/micrel/ks8851.h
+++ b/drivers/net/ethernet/micrel/ks8851.h
@@ -399,10 +399,14 @@ struct ks8851_net {
 					unsigned long *flags);
 	void			(*unlock)(struct ks8851_net *ks,
 					  unsigned long *flags);
+	unsigned int		(*rdreg8)(struct ks8851_net *ks,
+					  unsigned int reg);
 	unsigned int		(*rdreg16)(struct ks8851_net *ks,
 					   unsigned int reg);
 	void			(*wrreg16)(struct ks8851_net *ks,
 					   unsigned int reg, unsigned int val);
+	unsigned int		(*rdreg32)(struct ks8851_net *ks,
+					   unsigned int reg);
 	void			(*rdfifo)(struct ks8851_net *ks, u8 *buff,
 					  unsigned int len);
 	void			(*wrfifo)(struct ks8851_net *ks,
diff --git a/drivers/net/ethernet/micrel/ks8851_common.c b/drivers/net/ethernet/micrel/ks8851_common.c
index 27a351ad691b..a2bc85ae241f 100644
--- a/drivers/net/ethernet/micrel/ks8851_common.c
+++ b/drivers/net/ethernet/micrel/ks8851_common.c
@@ -53,6 +53,19 @@ static void ks8851_unlock(struct ks8851_net *ks, unsigned long *flags)
 	ks->unlock(ks, flags);
 }
 
+/**
+ * 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 int ks8851_rdreg8(struct ks8851_net *ks,
+				  unsigned int reg)
+{
+	return ks->rdreg8(ks, reg);
+}
+
 /**
  * ks8851_wrreg16 - write 16bit register value to chip
  * @ks: The chip state
@@ -80,6 +93,19 @@ static unsigned int ks8851_rdreg16(struct ks8851_net *ks,
 	return ks->rdreg16(ks, reg);
 }
 
+/**
+ * ks8851_rdreg32 - read 32 bit register from device
+ * @ks: The chip information
+ * @reg: The register address
+ *
+ * Read a 32bit register from the chip, returning the result
+ */
+static unsigned int ks8851_rdreg32(struct ks8851_net *ks,
+				   unsigned int reg)
+{
+	return ks->rdreg32(ks, reg);
+}
+
 /**
  * ks8851_soft_reset - issue one of the soft reset to the device
  * @ks: The device state.
@@ -257,9 +283,10 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)
 	unsigned rxfc;
 	unsigned rxlen;
 	unsigned rxstat;
+	u32 rxh;
 	u8 *rxpkt;
 
-	rxfc = (ks8851_rdreg16(ks, KS_RXFCTR) >> 8) & 0xff;
+	rxfc = ks8851_rdreg8(ks, KS_RXFC);
 
 	netif_dbg(ks, rx_status, ks->netdev,
 		  "%s: %d packets\n", __func__, rxfc);
@@ -275,8 +302,9 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)
 	 */
 
 	for (; rxfc != 0; rxfc--) {
-		rxstat = ks8851_rdreg16(ks, KS_RXFHSR);
-		rxlen = ks8851_rdreg16(ks, KS_RXFHBCR) & RXFHBCR_CNT_MASK;
+		rxh = ks8851_rdreg32(ks, KS_RXFHSR);
+		rxstat = rxh & 0xffff;
+		rxlen = (rxh >> 16) & 0xfff;
 
 		netif_dbg(ks, rx_status, ks->netdev,
 			  "rx: stat 0x%04x, len 0x%04x\n", rxstat, rxlen);
diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c
index a470e40b3817..3a5d42ecdba7 100644
--- a/drivers/net/ethernet/micrel/ks8851_par.c
+++ b/drivers/net/ethernet/micrel/ks8851_par.c
@@ -162,6 +162,35 @@ static unsigned int ks8851_rdreg16_par(struct ks8851_net *ks, unsigned int reg)
 	return ioread16(ksp->hw_addr);
 }
 
+/**
+ * ks8851_rdreg8_par - read 8 bit register from chip
+ * @ks: The chip information
+ * @reg: The register address
+ *
+ * Read a 8bit register from the chip, returning the result
+ */
+static unsigned int ks8851_rdreg8_par(struct ks8851_net *ks, unsigned int reg)
+{
+	return le16_to_cpu(ks8851_rdreg16_par(ks, reg)) & 0xff;
+}
+
+/**
+ * ks8851_rdreg32_par - read 32 bit register from chip
+ * @ks: The chip information
+ * @reg: The register address
+ *
+ * Read a 32bit register from the chip, returning the result
+ */
+static unsigned int ks8851_rdreg32_par(struct ks8851_net *ks, unsigned int reg)
+{
+	u16 rx[2];
+
+	rx[0] = ks8851_rdreg16_par(ks, reg);
+	rx[1] = ks8851_rdreg16_par(ks, reg + 2);
+
+	return le32_to_cpup((u32 *)rx);
+}
+
 /**
  * ks8851_rdfifo_par - read data from the receive fifo
  * @ks: The device state.
@@ -284,8 +313,10 @@ static int ks8851_probe_par(struct platform_device *pdev)
 
 	ks->lock = ks8851_lock_par;
 	ks->unlock = ks8851_unlock_par;
+	ks->rdreg8 = ks8851_rdreg8_par;
 	ks->rdreg16 = ks8851_rdreg16_par;
 	ks->wrreg16 = ks8851_wrreg16_par;
+	ks->rdreg32 = ks8851_rdreg32_par;
 	ks->rdfifo = ks8851_rdfifo_par;
 	ks->wrfifo = ks8851_wrfifo_par;
 	ks->start_xmit = ks8851_start_xmit_par;
diff --git a/drivers/net/ethernet/micrel/ks8851_spi.c b/drivers/net/ethernet/micrel/ks8851_spi.c
index 8d497cb440ff..0b909c27075e 100644
--- a/drivers/net/ethernet/micrel/ks8851_spi.c
+++ b/drivers/net/ethernet/micrel/ks8851_spi.c
@@ -184,6 +184,21 @@ static void ks8851_rdreg(struct ks8851_net *ks, unsigned int op,
 		memcpy(rxb, trx + 2, rxl);
 }
 
+/**
+ * ks8851_rdreg8_spi - 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 int ks8851_rdreg8_spi(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_spi - read 16 bit register from device via SPI
  * @ks: The chip information
@@ -199,6 +214,25 @@ static unsigned int ks8851_rdreg16_spi(struct ks8851_net *ks, unsigned int reg)
 	return le16_to_cpu(rx);
 }
 
+/**
+ * ks8851_rdreg32_spi - read 32 bit register from device
+ * @ks: The chip information
+ * @reg: The register address
+ *
+ * Read a 32bit register from the chip.
+ *
+ * Note, this read requires the address be aligned to 4 bytes.
+*/
+static unsigned ks8851_rdreg32_spi(struct ks8851_net *ks, unsigned reg)
+{
+	__le32 rx = 0;
+
+	WARN_ON(reg & 3);
+
+	ks8851_rdreg(ks, MK_OP(0xf, reg), (u8 *)&rx, 4);
+	return le32_to_cpu(rx);
+}
+
 /**
  * ks8851_rdfifo_spi - read data from the receive fifo via SPI
  * @ks: The device state.
@@ -414,8 +448,10 @@ static int ks8851_probe_spi(struct spi_device *spi)
 
 	ks->lock = ks8851_lock_spi;
 	ks->unlock = ks8851_unlock_spi;
+	ks->rdreg8 = ks8851_rdreg8_spi;
 	ks->rdreg16 = ks8851_rdreg16_spi;
 	ks->wrreg16 = ks8851_wrreg16_spi;
+	ks->rdreg32 = ks8851_rdreg32_spi;
 	ks->rdfifo = ks8851_rdfifo_spi;
 	ks->wrfifo = ks8851_wrfifo_spi;
 	ks->start_xmit = ks8851_start_xmit_spi;
Marek Vasut April 20, 2020, 2:12 p.m. UTC | #2
On 4/20/20 4:07 PM, Lukas Wunner wrote:
> On Tue, Apr 14, 2020 at 08:20:17PM +0200, Marek Vasut wrote:
>> The ks8851_rdreg32() is used only in one place, to read two registers
>> using a single read. To make it easier to support 16-bit accesses via
>> parallel bus later on, replace this single read with two 16-bit reads
>> from each of the registers and drop the ks8851_rdreg32() altogether.
>>
>> If this has noticeable performance impact on the SPI variant of KS8851,
>> then we should consider using regmap to abstract the SPI and parallel
>> bus options and in case of SPI, permit regmap to merge register reads
>> of neighboring registers into single, longer, read.
> 
> Bisection has shown this patch to be the biggest cause of the performance
> regression introduced by this series:  Latency increases by about 9 usec.

Just for completeness, did you perform this bisect on current linux-next
without any patches except this series OR your patched rpi downstream
vendor tree Linux 4.19 with preempt-rt patch applied ?

[...]
Lukas Wunner April 20, 2020, 2:20 p.m. UTC | #3
On Mon, Apr 20, 2020 at 04:12:59PM +0200, Marek Vasut wrote:
> On 4/20/20 4:07 PM, Lukas Wunner wrote:
> > On Tue, Apr 14, 2020 at 08:20:17PM +0200, Marek Vasut wrote:
> >> The ks8851_rdreg32() is used only in one place, to read two registers
> >> using a single read. To make it easier to support 16-bit accesses via
> >> parallel bus later on, replace this single read with two 16-bit reads
> >> from each of the registers and drop the ks8851_rdreg32() altogether.
> >>
> >> If this has noticeable performance impact on the SPI variant of KS8851,
> >> then we should consider using regmap to abstract the SPI and parallel
> >> bus options and in case of SPI, permit regmap to merge register reads
> >> of neighboring registers into single, longer, read.
> > 
> > Bisection has shown this patch to be the biggest cause of the performance
> > regression introduced by this series:  Latency increases by about 9 usec.
> 
> Just for completeness, did you perform this bisect on current linux-next
> without any patches except this series OR your patched rpi downstream
> vendor tree Linux 4.19 with preempt-rt patch applied ?

The latter because latency without CONFIG_PREEMPT_RT_FULL=y is too imprecise
to really see the difference and that's the configuration we care about.

Thanks,

Lukas
Marek Vasut April 20, 2020, 2:24 p.m. UTC | #4
On 4/20/20 4:20 PM, Lukas Wunner wrote:
> On Mon, Apr 20, 2020 at 04:12:59PM +0200, Marek Vasut wrote:
>> On 4/20/20 4:07 PM, Lukas Wunner wrote:
>>> On Tue, Apr 14, 2020 at 08:20:17PM +0200, Marek Vasut wrote:
>>>> The ks8851_rdreg32() is used only in one place, to read two registers
>>>> using a single read. To make it easier to support 16-bit accesses via
>>>> parallel bus later on, replace this single read with two 16-bit reads
>>>> from each of the registers and drop the ks8851_rdreg32() altogether.
>>>>
>>>> If this has noticeable performance impact on the SPI variant of KS8851,
>>>> then we should consider using regmap to abstract the SPI and parallel
>>>> bus options and in case of SPI, permit regmap to merge register reads
>>>> of neighboring registers into single, longer, read.
>>>
>>> Bisection has shown this patch to be the biggest cause of the performance
>>> regression introduced by this series:  Latency increases by about 9 usec.
>>
>> Just for completeness, did you perform this bisect on current linux-next
>> without any patches except this series OR your patched rpi downstream
>> vendor tree Linux 4.19 with preempt-rt patch applied ?
> 
> The latter because latency without CONFIG_PREEMPT_RT_FULL=y is too imprecise
> to really see the difference and that's the configuration we care about.

Why am I not able to see the same on the RPi3 then ?
How can I replicate this observation ?
Lukas Wunner April 20, 2020, 2:44 p.m. UTC | #5
On Mon, Apr 20, 2020 at 04:24:05PM +0200, Marek Vasut wrote:
> On 4/20/20 4:20 PM, Lukas Wunner wrote:
> > On Mon, Apr 20, 2020 at 04:12:59PM +0200, Marek Vasut wrote:
> >> On 4/20/20 4:07 PM, Lukas Wunner wrote:
> >>> On Tue, Apr 14, 2020 at 08:20:17PM +0200, Marek Vasut wrote:
> >>>> The ks8851_rdreg32() is used only in one place, to read two registers
> >>>> using a single read. To make it easier to support 16-bit accesses via
> >>>> parallel bus later on, replace this single read with two 16-bit reads
> >>>> from each of the registers and drop the ks8851_rdreg32() altogether.
> >>>>
> >>>> If this has noticeable performance impact on the SPI variant of KS8851,
> >>>> then we should consider using regmap to abstract the SPI and parallel
> >>>> bus options and in case of SPI, permit regmap to merge register reads
> >>>> of neighboring registers into single, longer, read.
> >>>
> >>> Bisection has shown this patch to be the biggest cause of the performance
> >>> regression introduced by this series:  Latency increases by about 9 usec.
> >>
> >> Just for completeness, did you perform this bisect on current linux-next
> >> without any patches except this series OR your patched rpi downstream
> >> vendor tree Linux 4.19 with preempt-rt patch applied ?
> > 
> > The latter because latency without CONFIG_PREEMPT_RT_FULL=y is too imprecise
> > to really see the difference and that's the configuration we care about.
> 
> Why am I not able to see the same on the RPi3 then ?
> How can I replicate this observation ?

Compile this branch with CONFIG_PREEMPT_RT_FULL=y:

https://github.com/l1k/linux/commits/revpi-4.19-marek-v4

Alternatively, download this file:

http://wunner.de/ks8851-marekv4.tar

Install the "raspberrypi-kernel" deb-package included in the tarball on a
stock Raspbian image and copy one of the included ks8851.ko to:
/lib/modules/4.19.95-rt38-v7+/kernel/drivers/net/ethernet/micrel

Thanks,

Lukas
Marek Vasut April 20, 2020, 3:38 p.m. UTC | #6
On 4/20/20 4:44 PM, Lukas Wunner wrote:
> On Mon, Apr 20, 2020 at 04:24:05PM +0200, Marek Vasut wrote:
>> On 4/20/20 4:20 PM, Lukas Wunner wrote:
>>> On Mon, Apr 20, 2020 at 04:12:59PM +0200, Marek Vasut wrote:
>>>> On 4/20/20 4:07 PM, Lukas Wunner wrote:
>>>>> On Tue, Apr 14, 2020 at 08:20:17PM +0200, Marek Vasut wrote:
>>>>>> The ks8851_rdreg32() is used only in one place, to read two registers
>>>>>> using a single read. To make it easier to support 16-bit accesses via
>>>>>> parallel bus later on, replace this single read with two 16-bit reads
>>>>>> from each of the registers and drop the ks8851_rdreg32() altogether.
>>>>>>
>>>>>> If this has noticeable performance impact on the SPI variant of KS8851,
>>>>>> then we should consider using regmap to abstract the SPI and parallel
>>>>>> bus options and in case of SPI, permit regmap to merge register reads
>>>>>> of neighboring registers into single, longer, read.
>>>>>
>>>>> Bisection has shown this patch to be the biggest cause of the performance
>>>>> regression introduced by this series:  Latency increases by about 9 usec.
>>>>
>>>> Just for completeness, did you perform this bisect on current linux-next
>>>> without any patches except this series OR your patched rpi downstream
>>>> vendor tree Linux 4.19 with preempt-rt patch applied ?
>>>
>>> The latter because latency without CONFIG_PREEMPT_RT_FULL=y is too imprecise
>>> to really see the difference and that's the configuration we care about.
>>
>> Why am I not able to see the same on the RPi3 then ?
>> How can I replicate this observation ?
> 
> Compile this branch with CONFIG_PREEMPT_RT_FULL=y:
> 
> https://github.com/l1k/linux/commits/revpi-4.19-marek-v4
> 
> Alternatively, download this file:
> 
> http://wunner.de/ks8851-marekv4.tar
> 
> Install the "raspberrypi-kernel" deb-package included in the tarball on a
> stock Raspbian image and copy one of the included ks8851.ko to:
> /lib/modules/4.19.95-rt38-v7+/kernel/drivers/net/ethernet/micrel

Why don't you rather try to replicate this problem in linux-next?
Andrew Lunn April 20, 2020, 3:50 p.m. UTC | #7
On Mon, Apr 20, 2020 at 05:38:16PM +0200, Marek Vasut wrote:
> On 4/20/20 4:44 PM, Lukas Wunner wrote:
> > On Mon, Apr 20, 2020 at 04:24:05PM +0200, Marek Vasut wrote:
> >> On 4/20/20 4:20 PM, Lukas Wunner wrote:
> >>> On Mon, Apr 20, 2020 at 04:12:59PM +0200, Marek Vasut wrote:
> >>>> On 4/20/20 4:07 PM, Lukas Wunner wrote:
> >>>>> On Tue, Apr 14, 2020 at 08:20:17PM +0200, Marek Vasut wrote:
> >>>>>> The ks8851_rdreg32() is used only in one place, to read two registers
> >>>>>> using a single read. To make it easier to support 16-bit accesses via
> >>>>>> parallel bus later on, replace this single read with two 16-bit reads
> >>>>>> from each of the registers and drop the ks8851_rdreg32() altogether.
> >>>>>>
> >>>>>> If this has noticeable performance impact on the SPI variant of KS8851,
> >>>>>> then we should consider using regmap to abstract the SPI and parallel
> >>>>>> bus options and in case of SPI, permit regmap to merge register reads
> >>>>>> of neighboring registers into single, longer, read.
> >>>>>
> >>>>> Bisection has shown this patch to be the biggest cause of the performance
> >>>>> regression introduced by this series:  Latency increases by about 9 usec.
> >>>>
> >>>> Just for completeness, did you perform this bisect on current linux-next
> >>>> without any patches except this series OR your patched rpi downstream
> >>>> vendor tree Linux 4.19 with preempt-rt patch applied ?
> >>>
> >>> The latter because latency without CONFIG_PREEMPT_RT_FULL=y is too imprecise
> >>> to really see the difference and that's the configuration we care about.
> >>
> >> Why am I not able to see the same on the RPi3 then ?
> >> How can I replicate this observation ?
> > 
> > Compile this branch with CONFIG_PREEMPT_RT_FULL=y:
> > 
> > https://github.com/l1k/linux/commits/revpi-4.19-marek-v4
> > 
> > Alternatively, download this file:
> > 
> > http://wunner.de/ks8851-marekv4.tar
> > 
> > Install the "raspberrypi-kernel" deb-package included in the tarball on a
> > stock Raspbian image and copy one of the included ks8851.ko to:
> > /lib/modules/4.19.95-rt38-v7+/kernel/drivers/net/ethernet/micrel
> 
> Why don't you rather try to replicate this problem in linux-next?

Hi Lukas

4.19 is dead in terms of development work. It is now over 18 months
old. All mainline development work is done against either the last
-rc1 kernel, or a subsystems specific 'for-next' branch.

Please test Marek patches against net-next. If there are performance
regressions with net-next, they should be addressed, but mainline does
not care about some random long time dead tree.

    Andrew
Marek Vasut May 7, 2020, 2:22 p.m. UTC | #8
On 4/20/20 5:50 PM, Andrew Lunn wrote:
> On Mon, Apr 20, 2020 at 05:38:16PM +0200, Marek Vasut wrote:
>> On 4/20/20 4:44 PM, Lukas Wunner wrote:
>>> On Mon, Apr 20, 2020 at 04:24:05PM +0200, Marek Vasut wrote:
>>>> On 4/20/20 4:20 PM, Lukas Wunner wrote:
>>>>> On Mon, Apr 20, 2020 at 04:12:59PM +0200, Marek Vasut wrote:
>>>>>> On 4/20/20 4:07 PM, Lukas Wunner wrote:
>>>>>>> On Tue, Apr 14, 2020 at 08:20:17PM +0200, Marek Vasut wrote:
>>>>>>>> The ks8851_rdreg32() is used only in one place, to read two registers
>>>>>>>> using a single read. To make it easier to support 16-bit accesses via
>>>>>>>> parallel bus later on, replace this single read with two 16-bit reads
>>>>>>>> from each of the registers and drop the ks8851_rdreg32() altogether.
>>>>>>>>
>>>>>>>> If this has noticeable performance impact on the SPI variant of KS8851,
>>>>>>>> then we should consider using regmap to abstract the SPI and parallel
>>>>>>>> bus options and in case of SPI, permit regmap to merge register reads
>>>>>>>> of neighboring registers into single, longer, read.
>>>>>>>
>>>>>>> Bisection has shown this patch to be the biggest cause of the performance
>>>>>>> regression introduced by this series:  Latency increases by about 9 usec.
>>>>>>
>>>>>> Just for completeness, did you perform this bisect on current linux-next
>>>>>> without any patches except this series OR your patched rpi downstream
>>>>>> vendor tree Linux 4.19 with preempt-rt patch applied ?
>>>>>
>>>>> The latter because latency without CONFIG_PREEMPT_RT_FULL=y is too imprecise
>>>>> to really see the difference and that's the configuration we care about.
>>>>
>>>> Why am I not able to see the same on the RPi3 then ?
>>>> How can I replicate this observation ?
>>>
>>> Compile this branch with CONFIG_PREEMPT_RT_FULL=y:
>>>
>>> https://github.com/l1k/linux/commits/revpi-4.19-marek-v4
>>>
>>> Alternatively, download this file:
>>>
>>> http://wunner.de/ks8851-marekv4.tar
>>>
>>> Install the "raspberrypi-kernel" deb-package included in the tarball on a
>>> stock Raspbian image and copy one of the included ks8851.ko to:
>>> /lib/modules/4.19.95-rt38-v7+/kernel/drivers/net/ethernet/micrel
>>
>> Why don't you rather try to replicate this problem in linux-next?
> 
> Hi Lukas
> 
> 4.19 is dead in terms of development work. It is now over 18 months
> old. All mainline development work is done against either the last
> -rc1 kernel, or a subsystems specific 'for-next' branch.
> 
> Please test Marek patches against net-next. If there are performance
> regressions with net-next, they should be addressed, but mainline does
> not care about some random long time dead tree.

Are there any news on this one ?
diff mbox series

Patch

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index c2f381a7b3f3..8f1cc05dc3c8 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -296,25 +296,6 @@  static unsigned ks8851_rdreg16(struct ks8851_net *ks, unsigned reg)
 	return le16_to_cpu(rx);
 }
 
-/**
- * ks8851_rdreg32 - read 32 bit register from device
- * @ks: The chip information
- * @reg: The register address
- *
- * Read a 32bit register from the chip.
- *
- * Note, this read requires the address be aligned to 4 bytes.
-*/
-static unsigned ks8851_rdreg32(struct ks8851_net *ks, unsigned reg)
-{
-	__le32 rx = 0;
-
-	WARN_ON(reg & 3);
-
-	ks8851_rdreg(ks, MK_OP(0xf, reg), (u8 *)&rx, 4);
-	return le32_to_cpu(rx);
-}
-
 /**
  * ks8851_soft_reset - issue one of the soft reset to the device
  * @ks: The device state.
@@ -508,7 +489,6 @@  static void ks8851_rx_pkts(struct ks8851_net *ks)
 	unsigned rxfc;
 	unsigned rxlen;
 	unsigned rxstat;
-	u32 rxh;
 	u8 *rxpkt;
 
 	rxfc = ks8851_rdreg8(ks, KS_RXFC);
@@ -527,9 +507,8 @@  static void ks8851_rx_pkts(struct ks8851_net *ks)
 	 */
 
 	for (; rxfc != 0; rxfc--) {
-		rxh = ks8851_rdreg32(ks, KS_RXFHSR);
-		rxstat = rxh & 0xffff;
-		rxlen = (rxh >> 16) & 0xfff;
+		rxstat = ks8851_rdreg16(ks, KS_RXFHSR);
+		rxlen = ks8851_rdreg16(ks, KS_RXFHBCR) & RXFHBCR_CNT_MASK;
 
 		netif_dbg(ks, rx_status, ks->netdev,
 			  "rx: stat 0x%04x, len 0x%04x\n", rxstat, rxlen);