| Message ID | f4110289-f7e9-471a-bb91-941c85bfb43e@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: core: Improve i2c_new_scanned_device | expand |
On Tue, Jan 07, 2025 at 09:29:07PM +0100, Heiner Kallweit wrote: > Simplify the logic in this function. And as we're no longer limited to > C89, move the iterator variable declaration to the for loop. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Did you have HW to test this patch?
> No functional change is intended. I have hw using i801, which calls > i2c_register_spd() -> i2c_new_scanned_device(). With this patch > tools like decode-dimms still work. Thanks. Still, thinking more about it, I am not comfortable with this patch. Kernel code where the intended exit of a function is not at the end but somewhere in the middle often enough confused me in the past. So, although it is not as concise as your suggestion, I would prefer to leave the code as-is for clarity.
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 4a858b1ae..f3b1106f2 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -2507,12 +2507,10 @@ i2c_new_scanned_device(struct i2c_adapter *adap, unsigned short const *addr_list, int (*probe)(struct i2c_adapter *adap, unsigned short addr)) { - int i; - if (!probe) probe = i2c_default_probe; - for (i = 0; addr_list[i] != I2C_CLIENT_END; i++) { + for (int i = 0; addr_list[i] != I2C_CLIENT_END; i++) { /* Check address validity */ if (i2c_check_7bit_addr_validity_strict(addr_list[i]) < 0) { dev_warn(&adap->dev, "Invalid 7-bit address 0x%02x\n", @@ -2529,17 +2527,15 @@ i2c_new_scanned_device(struct i2c_adapter *adap, } /* Test address responsiveness */ - if (probe(adap, addr_list[i])) - break; + if ((probe(adap, addr_list[i]))) { + info->addr = addr_list[i]; + return i2c_new_client_device(adap, info); + } } - if (addr_list[i] == I2C_CLIENT_END) { - dev_dbg(&adap->dev, "Probing failed, no device found\n"); - return ERR_PTR(-ENODEV); - } + dev_dbg(&adap->dev, "Probing failed, no device found\n"); - info->addr = addr_list[i]; - return i2c_new_client_device(adap, info); + return ERR_PTR(-ENODEV); } EXPORT_SYMBOL_GPL(i2c_new_scanned_device);
Simplify the logic in this function. And as we're no longer limited to C89, move the iterator variable declaration to the for loop. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/i2c/i2c-core-base.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)