Re: [PATCH v3] i2c: Add support for NXP PCA984x family.

[Peter Rosin <peda@axentia.se>]

On 2017-12-11 20:14, Peter Rosin wrote:
> On 2017-12-11 17:58, Adrian Fiergolski wrote:
>> +	/* Check manufacturer ID (12 bits) */
>> +	manufacturer_id = ((u16) device_id_raw.block[1] << 4) | (device_id_raw.block[2] >> 4);
> 
> This assignment is still broken...

Ooops, no, this one is fine. Sorry about that.

>> +	if (manufacturer_id != NXP_ID) {
>> +		dev_warn(&client->dev, "PCA9846 family: Manufacturer ID does not match NXP\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	/* Check device ID (9 bits) */
>> +	device_id = ((u16) device_id_raw.block[2] << 5) | (device_id_raw.block[3] >> 3);
> 
> ...and this is also broken.

But this one is broken. You need to mask out the upper half of
...block[2] so that the lower bits of the manufacturer don't
end up above the device id bits.

Cheers,
Peter

>> +	if (device_id != chips[id->driver_data].deviceID) {
>> +		dev_warn(&client->dev, "PCA9846 family: Device ID does not match %s\n", id->name);
>> +		return -ENODEV;
>> +	}