diff mbox series

[RFC,2/2] i2c: mux: pca954x: verify the device id of the pca984x chips

Message ID 20180122113657.32094-3-peda@axentia.se
State Changes Requested
Headers show
Series check I2C device id for pca984x chips | expand

Commit Message

Peter Rosin Jan. 22, 2018, 11:36 a.m. UTC 
Make sure to not disallow the chips on adapters that are not capable
of reading the device id.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 43 +++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

Comments

Wolfram Sang March 5, 2018, 3:53 p.m. UTC | #1 
> @@ -97,59 +98,83 @@ static const struct chip_desc chips[] = {
>  		.nchans = 2,
>  		.enable = 0x4,
>  		.muxtype = pca954x_ismux,
> +		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },

Can't we just leave this empty and add a NULL pointer check below when
testing for the device id?
Peter Rosin March 5, 2018, 3:55 p.m. UTC | #2 
On 2018-03-05 16:53, Wolfram Sang wrote:
> 
>> @@ -97,59 +98,83 @@ static const struct chip_desc chips[] = {
>>  		.nchans = 2,
>>  		.enable = 0x4,
>>  		.muxtype = pca954x_ismux,
>> +		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
> 
> Can't we just leave this empty and add a NULL pointer check below when
> testing for the device id?

Nope, it's not a pointer. I didn't think it should be, but maybe it
should? The drawback is that you have to create an extra variable
for each of the chips that do need an id, and I didn't fancy that...

Cheers,
Peter
Wolfram Sang March 5, 2018, 4:29 p.m. UTC | #3 
On Mon, Mar 05, 2018 at 04:55:56PM +0100, Peter Rosin wrote:
> On 2018-03-05 16:53, Wolfram Sang wrote:
> > 
> >> @@ -97,59 +98,83 @@ static const struct chip_desc chips[] = {
> >>  		.nchans = 2,
> >>  		.enable = 0x4,
> >>  		.muxtype = pca954x_ismux,
> >> +		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
> > 
> > Can't we just leave this empty and add a NULL pointer check below when
> > testing for the device id?
> 
> Nope, it's not a pointer. I didn't think it should be, but maybe it
> should?

Nope, you are right. Thanks for the heads up!
diff mbox series

Patch

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index fbb84c7ef282..ff0c7a144fbb 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -77,6 +77,7 @@  struct chip_desc {
 		pca954x_ismux = 0,
 		pca954x_isswi
 	} muxtype;
+	struct i2c_device_identity id;
 };
 
 struct pca954x {
@@ -97,59 +98,83 @@  static const struct chip_desc chips[] = {
 		.nchans = 2,
 		.enable = 0x4,
 		.muxtype = pca954x_ismux,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
 	},
 	[pca_9542] = {
 		.nchans = 2,
 		.enable = 0x4,
 		.has_irq = 1,
 		.muxtype = pca954x_ismux,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
 	},
 	[pca_9543] = {
 		.nchans = 2,
 		.has_irq = 1,
 		.muxtype = pca954x_isswi,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
 	},
 	[pca_9544] = {
 		.nchans = 4,
 		.enable = 0x4,
 		.has_irq = 1,
 		.muxtype = pca954x_ismux,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
 	},
 	[pca_9545] = {
 		.nchans = 4,
 		.has_irq = 1,
 		.muxtype = pca954x_isswi,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
 	},
 	[pca_9546] = {
 		.nchans = 4,
 		.muxtype = pca954x_isswi,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
 	},
 	[pca_9547] = {
 		.nchans = 8,
 		.enable = 0x8,
 		.muxtype = pca954x_ismux,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
 	},
 	[pca_9548] = {
 		.nchans = 8,
 		.muxtype = pca954x_isswi,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
 	},
 	[pca_9846] = {
 		.nchans = 4,
 		.muxtype = pca954x_isswi,
+		.id = {
+			.manufacturer_id = I2C_DEVICE_ID_NXP_SEMICONDUCTORS,
+			.part_id = 0x10b,
+		},
 	},
 	[pca_9847] = {
 		.nchans = 8,
 		.enable = 0x8,
 		.muxtype = pca954x_ismux,
+		.id = {
+			.manufacturer_id = I2C_DEVICE_ID_NXP_SEMICONDUCTORS,
+			.part_id = 0x108,
+		},
 	},
 	[pca_9848] = {
 		.nchans = 8,
 		.muxtype = pca954x_isswi,
+		.id = {
+			.manufacturer_id = I2C_DEVICE_ID_NXP_SEMICONDUCTORS,
+			.part_id = 0x10a,
+		},
 	},
 	[pca_9849] = {
 		.nchans = 4,
 		.enable = 0x4,
 		.muxtype = pca954x_ismux,
+		.id = {
+			.manufacturer_id = I2C_DEVICE_ID_NXP_SEMICONDUCTORS,
+			.part_id = 0x109,
+		},
 	},
 };
 
@@ -384,6 +409,24 @@  static int pca954x_probe(struct i2c_client *client,
 	else
 		data->chip = &chips[id->driver_data];
 
+	if (data->chip->id.manufacturer_id != I2C_DEVICE_ID_NONE) {
+		struct i2c_device_identity id;
+
+		ret = i2c_get_device_id(client, &id);
+		if (ret && ret != -EOPNOTSUPP)
+			return ret;
+
+		if (!ret &&
+		    (id.manufacturer_id != data->chip->id.manufacturer_id ||
+		     id.part_id != data->chip->id.part_id)) {
+			dev_warn(&client->dev,
+				 "unexpected device id %03x-%03x-%x\n",
+				 id.manufacturer_id, id.part_id,
+				 id.die_revision);
+			return -ENODEV;
+		}
+	}
+
 	data->last_chan = 0;		   /* force the first selection */
 
 	idle_disconnect_dt = of_node &&