[PATCH] i2c: core: Improve i2c_new_scanned_device

[PATCH] i2c: core: Improve i2c_new_scanned_device

[Heiner Kallweit]

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(-)

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);
 
-- 
2.47.1

Re: [PATCH] i2c: core: Improve i2c_new_scanned_device

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 306 bytes --]

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?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: core: Improve i2c_new_scanned_device

[Heiner Kallweit]

On 09.01.2025 11:27, Wolfram Sang wrote:
> 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.

Re: [PATCH] i2c: core: Improve i2c_new_scanned_device

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 495 bytes --]


> 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.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]