Optimize the i2c_dw_read() function by reading the message flags only
once per message, rather than for every byte.

The message index is only modified by the outer loop, so reading the
flags in the inner loop was always getting the same value.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 drivers/i2c/busses/i2c-designware-master.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 41e9b5ecad20..ec4fc2708d03 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -586,11 +586,12 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 	unsigned int rx_valid;
 
 	for (; dev->msg_read_idx < dev->msgs_num; dev->msg_read_idx++) {
+		u32 flags = msgs[dev->msg_read_idx].flags;
 		unsigned int tmp;
 		u32 len;
 		u8 *buf;
 
-		if (!(msgs[dev->msg_read_idx].flags & I2C_M_RD))
+		if (!(flags & I2C_M_RD))
 			continue;
 
 		if (!(dev->status & STATUS_READ_IN_PROGRESS)) {
@@ -604,8 +605,6 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 		regmap_read(dev->map, DW_IC_RXFLR, &rx_valid);
 
 		for (; len > 0 && rx_valid > 0; len--, rx_valid--) {
-			u32 flags = msgs[dev->msg_read_idx].flags;
-
 			regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
 			tmp &= DW_IC_DATA_CMD_DAT;
 			/* Ensure length byte is a valid value */

-- 
2.51.1

On Fri, Oct 31, 2025 at 03:35:40PM +0100, Benoît Monin wrote:
> Optimize the i2c_dw_read() function by reading the message flags only
> once per message, rather than for every byte.
> 
> The message index is only modified by the outer loop, so reading the
> flags in the inner loop was always getting the same value.

Does it affect the binary (compiled) file?

-- 
With Best Regards,
Andy Shevchenko

On 31/10/2025 15:48, Andy Shevchenko wrote:
> On Fri, Oct 31, 2025 at 03:35:40PM +0100, Benoît Monin wrote:
>> Optimize the i2c_dw_read() function by reading the message flags only
>> once per message, rather than for every byte.
>>
>> The message index is only modified by the outer loop, so reading the
>> flags in the inner loop was always getting the same value.
> 
> Does it affect the binary (compiled) file?


It does not really matter that much, because new code is more readable -
'flags' depend on the outer (first) loop and they are used there, so
that's where variable should be defined.

Best regards,
Krzysztof

On Thu, Nov 06, 2025 at 11:50:36AM +0100, Krzysztof Kozlowski wrote:
> On 31/10/2025 15:48, Andy Shevchenko wrote:
> > On Fri, Oct 31, 2025 at 03:35:40PM +0100, Benoît Monin wrote:
> >> Optimize the i2c_dw_read() function by reading the message flags only
> >> once per message, rather than for every byte.
> >>
> >> The message index is only modified by the outer loop, so reading the
> >> flags in the inner loop was always getting the same value.
> > 
> > Does it affect the binary (compiled) file?
> 
> It does not really matter that much, because new code is more readable -
> 'flags' depend on the outer (first) loop and they are used there, so
> that's where variable should be defined.

Did I oppose that?

-- 
With Best Regards,
Andy Shevchenko

Hi Andy,

On Friday, 31 October 2025 at 15:48:50 CET, Andy Shevchenko wrote:
> On Fri, Oct 31, 2025 at 03:35:40PM +0100, Benoît Monin wrote:
> > Optimize the i2c_dw_read() function by reading the message flags only
> > once per message, rather than for every byte.
> > 
> > The message index is only modified by the outer loop, so reading the
> > flags in the inner loop was always getting the same value.
> 
> Does it affect the binary (compiled) file?
> 
Yes it does. On the mips64 system I am testing this on, built with gcc11,
i2c_dw_process_transfer() which inline i2c_dw_read() get 16 bytes shorter.
 
Looking at the disassembled code, the flags was read in the inner loop prior
to the patch.

Best regards,
-- 
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com