Re: [PATCH] spi: fix erroneous logic inversion in spi_match_id() usage

[Heiner Kallweit <hkallweit1@xxxxxxxxx>]

On 01.12.2021 16:41, Mark Brown wrote:
> On Tue, Nov 30, 2021 at 09:13:35PM +0100, Heiner Kallweit wrote:
> 
>> We want to continue in case of a match. Fix the erroneously inverted
>> logic.
> 
> We do?  Why?  I can't tell from this changelog what the problem is or
> why the patch would fix it.
> 

That's the relevant part of 3f07657506df. Before this change we hit the
continue path if spi_id->name[0] != NULL, means if a match was found.
spi_match_id() returns NULL if no match was found. The commit message
of 3f07657506df doesn't mention an intention to change the logic.

@@ -474,12 +473,8 @@ int __spi_register_driver(struct module *owner, struct spi_driver *sdrv)
 			if (sdrv->id_table) {
 				const struct spi_device_id *spi_id;
 
-				for (spi_id = sdrv->id_table; spi_id->name[0];
-				     spi_id++)
-					if (strcmp(spi_id->name, of_name) == 0)
-						break;
-
-				if (spi_id->name[0])
+				spi_id = spi_match_id(sdrv->id_table, of_name);
+				if (!spi_id)
 					continue;
 			} else {

>> @@ -471,10 +471,7 @@ int __spi_register_driver(struct module *owner, struct spi_driver *sdrv)
>>  				of_name = of_id->compatible;
>>  
>>  			if (sdrv->id_table) {
>> -				const struct spi_device_id *spi_id;
>> -
>> -				spi_id = spi_match_id(sdrv->id_table, of_name);
>> -				if (!spi_id)
>> +				if (spi_match_id(sdrv->id_table, of_name))
>>  					continue;
>>  			} else {
>>  				if (strcmp(sdrv->driver.name, of_name) == 0)
> 
> This appears to correspond to the current code anyway?
> 
No. Now we (again) continue if spi_match_id() != NULL.