Re: [PATCH v3 10/13] hw/misc/auxbus: Replace i2c_send_recv() by i2c_recv() & i2c_send()

[Philippe Mathieu-Daudé]

On 6/16/21 8:46 PM, Richard Henderson wrote:
> On 6/16/21 9:14 AM, Philippe Mathieu-Daudé wrote:
>> @@ -161,12 +157,11 @@ AUXReply aux_request(AUXBus *bus, AUXCommand
>> cmd, uint32_t address,
>>           }
>>             ret = AUX_I2C_ACK;
>> -        while (len > 0) {
>> -            if (i2c_send_recv(i2c_bus, data++, true) < 0) {
>> +        for (i = 0; i < len; i++) {
>> +            if (i2c_send(i2c_bus, data[i]) < 0) {
>>                   ret = AUX_I2C_NACK;
>>                   break;
>>               }
>> -            len--;
>>           }
> 
> This form of updating ret is better than...
> 
>> @@ -200,14 +195,13 @@ AUXReply aux_request(AUXBus *bus, AUXCommand
>> cmd, uint32_t address,
>>             bus->last_transaction = cmd;
>>           bus->last_i2c_address = address;
>> -        while (len > 0) {
>> -            if (i2c_send_recv(i2c_bus, data++, true) < 0) {
>> +        for (i = 0; i < len; i++) {
>> +            if (i2c_send(i2c_bus, data[i]) < 0) {
>>                   i2c_end_transfer(i2c_bus);
>>                   break;
>>               }
>> -            len--;
>>           }
>> -        if (len == 0) {
>> +        if (i == len) {
>>               ret = AUX_I2C_ACK;
>>           }
> 
> ... this one.

I totally agree :) I was a bit ashamed for posting that, I thought
Zoltan would prefer less changes so used this form.
Will update on respin.

> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!