2023-07-26 03:22:45

by Hayes Wang

Subject: [PATCH net-next v2 1/2] r8152: adjust generic_ocp_write function

Reduce the control transfer if all bytes of first or the last DWORD are
written.

The original method is to split the control transfer into three parts
(the first DWORD, middle continuous data, and the last DWORD). However,
they could be combined if whole bytes of the first DWORD or last DWORD
are written. That is, the first DWORD or the last DWORD could be combined
with the middle continuous data, if the byte_en is 0xff.

Signed-off-by: Hayes Wang <[email protected]>
---
drivers/net/usb/r8152.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0738baa5b82e..f6578a99dbac 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1314,16 +1314,24 @@ static int generic_ocp_write(struct r8152 *tp, u16 index, u16 byteen,
byteen_end = byteen & BYTE_EN_END_MASK;

byen = byteen_start | (byteen_start << 4);
- ret = set_registers(tp, index, type | byen, 4, data);
- if (ret < 0)
- goto error1;

- index += 4;
- data += 4;
- size -= 4;
+ /* Split the first DWORD if the byte_en is not 0xff */
+ if (byen != BYTE_EN_DWORD) {
+ ret = set_registers(tp, index, type | byen, 4, data);
+ if (ret < 0)
+ goto error1;

- if (size) {
+ index += 4;
+ data += 4;
size -= 4;
+ }
+
+ if (size) {
+ byen = byteen_end | (byteen_end >> 4);
+
+ /* Split the last DWORD if the byte_en is not 0xff */
+ if (byen != BYTE_EN_DWORD)
+ size -= 4;

while (size) {
if (size > limit) {
@@ -1350,10 +1358,9 @@ static int generic_ocp_write(struct r8152 *tp, u16 index, u16 byteen,
}
}

- byen = byteen_end | (byteen_end >> 4);
- ret = set_registers(tp, index, type | byen, 4, data);
- if (ret < 0)
- goto error1;
+ /* Set the last DWORD */
+ if (byen != BYTE_EN_DWORD)
+ ret = set_registers(tp, index, type | byen, 4, data);
}

error1:
--
2.41.0

2023-07-26 08:55:57

by Andrew Lunn

Subject: Re: [PATCH net-next v2 1/2] r8152: adjust generic_ocp_write function

On Wed, Jul 26, 2023 at 11:08:07AM +0800, Hayes Wang wrote:
> Reduce the control transfer if all bytes of first or the last DWORD are
> written.
>
> The original method is to split the control transfer into three parts
> (the first DWORD, middle continuous data, and the last DWORD). However,
> they could be combined if whole bytes of the first DWORD or last DWORD
> are written. That is, the first DWORD or the last DWORD could be combined
> with the middle continuous data, if the byte_en is 0xff.

How often is byte_en 0xff? Do you have some benchmark numbers to show
it is worth the complexity?

Andrew

2023-07-26 11:00:04

by Hayes Wang

Subject: RE: [PATCH net-next v2 1/2] r8152: adjust generic_ocp_write function

Andrew Lunn <[email protected]>
> Sent: Wednesday, July 26, 2023 4:37 PM
[...]
> How often is byte_en 0xff? Do you have some benchmark numbers to show
> it is worth the complexity?

It is usually used for writing firmware.
The firmware contains several blocks of continuous registers.

I think it is worth, even this only saves several numbers of control transfer.
This patch could replace 3 control transfers with 1 control transfer.
If you could do it with one step, why do you use 3 steps?
Besides, a control transfer is a complex process.
I think this could reduce the loading of both software and hardware.

Best Regards,
Hayes