| Message ID | 20230726030808.9093-418-nic_swsd@realtek.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 57df0fb9d511f91202114813e90128d65c0589f0 |
| Headers | show |
| Series | r8152: reduce control transfer | expand |
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
Andrew Lunn <andrew@lunn.ch> > 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
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:
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 <hayeswang@realtek.com> --- drivers/net/usb/r8152.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)