Re: [PATCH 1/3] hwmon: lenovo-ec-sensors: Fix EC signature check logic in probe

[Mark Pearson]

Hi Guenter,

On Wed, May 13, 2026, at 9:37 PM, Guenter Roeck wrote:
> On 5/13/26 18:14, Kean wrote:
>> The probe function reads a 4-byte signature ("MCHP") from the EC to
>> verify it is a Microchip EC before binding the driver. The condition
>> uses && (AND) to check if each byte differs from the expected value:
>> 
>>    if ((byte0 != 'M') && (byte1 != 'C') && (byte2 != 'H') && (byte3 != 'P'))
>> 
>> This rejects the device only if ALL four bytes are wrong simultaneously.
>> A partially matching signature (e.g. "MXXX") would pass the check and
>> cause the driver to bind to a non-Microchip EC, as long as at least one
>> byte matches the expected value.
>> 
>> Change && to || so the driver is rejected when ANY byte does not match
>> the expected "MCHP" signature.
>> 
>> Signed-off-by: Kean <rh_king@xxxxxxx>
>> 
>> Reviewed-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
>
> As with the other patches: How do I know that this review really happened ?

I confirm I did it.
For the series:
Reviewed-by: Mark Pearson <mpearson@lenovo@xxxxxxxxx>

Mark

>
> Guenter
>
>> ---
>>   drivers/hwmon/lenovo-ec-sensors.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/hwmon/lenovo-ec-sensors.c b/drivers/hwmon/lenovo-ec-sensors.c
>> index 8681bbf6665b..a32b1f2c6a3a 100644
>> --- a/drivers/hwmon/lenovo-ec-sensors.c
>> +++ b/drivers/hwmon/lenovo-ec-sensors.c
>> @@ -537,9 +537,9 @@ static int lenovo_ec_probe(struct platform_device *pdev)
>>   	outw_p(MCHP_SING_IDX, MCHP_EMI0_EC_ADDRESS);
>>   	mutex_unlock(&ec_data->mec_mutex);
>>   
>> -	if ((inb_p(MCHP_EMI0_EC_DATA_BYTE0) != 'M') &&
>> -	    (inb_p(MCHP_EMI0_EC_DATA_BYTE1) != 'C') &&
>> -	    (inb_p(MCHP_EMI0_EC_DATA_BYTE2) != 'H') &&
>> +	if ((inb_p(MCHP_EMI0_EC_DATA_BYTE0) != 'M') ||
>> +	    (inb_p(MCHP_EMI0_EC_DATA_BYTE1) != 'C') ||
>> +	    (inb_p(MCHP_EMI0_EC_DATA_BYTE2) != 'H') ||
>>   	    (inb_p(MCHP_EMI0_EC_DATA_BYTE3) != 'P')) {
>>   		release_region(IO_REGION_START, IO_REGION_LENGTH);
>>   		return -ENODEV;