* [RFC PATCH 0/2] check I2C device id for pca984x chips
@ 2018-01-22 11:36 Peter Rosin
2018-01-22 11:36 ` [RFC PATCH 1/2] i2c: add i2c_get_device_id() to get the standard i2c device id Peter Rosin
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Peter Rosin @ 2018-01-22 11:36 UTC (permalink / raw)
To: Adrian Fiergolski; +Cc: Peter Rosin, Wolfram Sang, linux-i2c, linux-kernel
Hi!
This series tries to check the I2C device id, but instead of open
coding the check in the pca954x driver, I have a new function in
the core doing the work.
The code is only compile-tested, hence the RFC, and I would really
like a Tested-by: tag from Adrian who presumably have one of these
chips.
Also, I'm not sure if I should list all manufacturers that I know
about in the header, or if I should settle for the one that is
actually used and leave the others to be added by whomever needs
them...
Cheers,
peda
Peter Rosin (2):
i2c: add i2c_get_device_id() to get the standard i2c device id
i2c: mux: pca954x: verify the device id of the pca984x chips
drivers/i2c/i2c-core-base.c | 33 ++++++++++++++++++++++++++++
drivers/i2c/muxes/i2c-mux-pca954x.c | 43 +++++++++++++++++++++++++++++++++++++
include/linux/i2c.h | 30 ++++++++++++++++++++++++++
3 files changed, 106 insertions(+)
--
2.11.0
^ permalink raw reply [flat|nested] 16+ messages in thread* [RFC PATCH 1/2] i2c: add i2c_get_device_id() to get the standard i2c device id 2018-01-22 11:36 [RFC PATCH 0/2] check I2C device id for pca984x chips Peter Rosin @ 2018-01-22 11:36 ` Peter Rosin 2018-03-04 21:47 ` Peter Rosin 2018-03-05 15:51 ` Wolfram Sang 2018-01-22 11:36 ` [RFC PATCH 2/2] i2c: mux: pca954x: verify the device id of the pca984x chips Peter Rosin 2018-01-26 16:33 ` Adrian Fiergolski 2 siblings, 2 replies; 16+ messages in thread From: Peter Rosin @ 2018-01-22 11:36 UTC (permalink / raw) To: Adrian Fiergolski; +Cc: Peter Rosin, Wolfram Sang, linux-i2c, linux-kernel Can be used during probe to double check that the probed device is what is expected. Loosely based on code from Adrian Fiergolski <adrian.fiergolski@cern.ch>. Signed-off-by: Peter Rosin <peda@axentia.se> --- drivers/i2c/i2c-core-base.c | 33 +++++++++++++++++++++++++++++++++ include/linux/i2c.h | 30 ++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 5a00bf443d06..f496b917529d 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -58,6 +58,8 @@ #define I2C_ADDR_7BITS_MAX 0x77 #define I2C_ADDR_7BITS_COUNT (I2C_ADDR_7BITS_MAX + 1) +#define I2C_ADDR_DEVICE_ID 0x7c + /* * core_lock protects i2c_adapter_idr, and guarantees that device detection, * deletion of detected devices, and attach_adapter calls are serialized @@ -1968,6 +1970,37 @@ int i2c_transfer_buffer_flags(const struct i2c_client *client, char *buf, } EXPORT_SYMBOL(i2c_transfer_buffer_flags); +/** + * i2c_get_device_id - get manufacturer, part id and die revision of a device + * @client: The device to query + * @id: The queried information + * + * Returns negative errno on error, zero on success. + */ +int i2c_get_device_id(const struct i2c_client *client, + struct i2c_device_identity *id) +{ + struct i2c_adapter *adap = client->adapter; + union i2c_smbus_data raw_id; + int ret; + + if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_READ_I2C_BLOCK)) + return -EOPNOTSUPP; + + raw_id.block[0] = 3; + ret = i2c_smbus_xfer(adap, I2C_ADDR_DEVICE_ID, client->flags, + I2C_SMBUS_READ, client->addr << 1, + I2C_SMBUS_I2C_BLOCK_DATA, &raw_id); + if (ret) + return ret; + + id->manufacturer_id = (raw_id.block[1] << 4) | (raw_id.block[2] >> 4); + id->part_id = ((raw_id.block[2] & 0xf) << 5) | (raw_id.block[3] >> 3); + id->die_revision = raw_id.block[3] & 0x7; + return 0; +} +EXPORT_SYMBOL_GPL(i2c_get_device_id); + /* ---------------------------------------------------- * the i2c address scanning function * Will not work for 10-bit addresses! diff --git a/include/linux/i2c.h b/include/linux/i2c.h index 419a38e7c315..44ad14e016b5 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -47,6 +47,7 @@ struct i2c_algorithm; struct i2c_adapter; struct i2c_client; struct i2c_driver; +struct i2c_device_identity; union i2c_smbus_data; struct i2c_board_info; enum i2c_slave_event; @@ -186,8 +187,37 @@ extern s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client, extern s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client, u8 command, u8 length, u8 *values); +int i2c_get_device_id(const struct i2c_client *client, + struct i2c_device_identity *id); #endif /* I2C */ +/** + * struct i2c_device_identity - i2c client device identification + * @manufacturer_id: 0 - 4095, database maintained by NXP + * @part_id: 0 - 511, according to manufacturer + * @die_revision: 0 - 7, according to manufacturer + */ +struct i2c_device_identity { + u16 manufacturer_id; +#define I2C_DEVICE_ID_NXP_SEMICONDUCTORS 0 +#define I2C_DEVICE_ID_NXP_SEMICONDUCTORS_1 1 +#define I2C_DEVICE_ID_NXP_SEMICONDUCTORS_2 2 +#define I2C_DEVICE_ID_NXP_SEMICONDUCTORS_3 3 +#define I2C_DEVICE_ID_RAMTRON_INTERNATIONAL 4 +#define I2C_DEVICE_ID_ANALOG_DEVICES 5 +#define I2C_DEVICE_ID_STMICROELECTRONICS 6 +#define I2C_DEVICE_ID_ON_SEMICONDUCTOR 7 +#define I2C_DEVICE_ID_SPRINTEK_CORPORATION 8 +#define I2C_DEVICE_ID_ESPROS_PHOTONICS_AG 9 +#define I2C_DEVICE_ID_FUJITSU_SEMICONDUCTOR 10 +#define I2C_DEVICE_ID_FLIR 11 +#define I2C_DEVICE_ID_O2MICRO 12 +#define I2C_DEVICE_ID_ATMEL 13 +#define I2C_DEVICE_ID_NONE 0xffff + u16 part_id; + u8 die_revision; +}; + enum i2c_alert_protocol { I2C_PROTOCOL_SMBUS_ALERT, I2C_PROTOCOL_SMBUS_HOST_NOTIFY, -- 2.11.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/2] i2c: add i2c_get_device_id() to get the standard i2c device id 2018-01-22 11:36 ` [RFC PATCH 1/2] i2c: add i2c_get_device_id() to get the standard i2c device id Peter Rosin @ 2018-03-04 21:47 ` Peter Rosin 2018-03-05 15:51 ` Wolfram Sang 1 sibling, 0 replies; 16+ messages in thread From: Peter Rosin @ 2018-03-04 21:47 UTC (permalink / raw) To: Wolfram Sang; +Cc: Adrian Fiergolski, linux-i2c, linux-kernel Hi Wolfram, Did you have an opinion on this patch? In case you like it, do you which to take it (don't forget to add a Tested-by-tag from Adrian in that case), or should I take it and you then get it in a pull request? Cheers, Peter On 2018-01-22 12:36, Peter Rosin wrote: > Can be used during probe to double check that the probed device is > what is expected. > > Loosely based on code from Adrian Fiergolski <adrian.fiergolski@cern.ch>. > > Signed-off-by: Peter Rosin <peda@axentia.se> > --- > drivers/i2c/i2c-core-base.c | 33 +++++++++++++++++++++++++++++++++ > include/linux/i2c.h | 30 ++++++++++++++++++++++++++++++ > 2 files changed, 63 insertions(+) > > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c > index 5a00bf443d06..f496b917529d 100644 > --- a/drivers/i2c/i2c-core-base.c > +++ b/drivers/i2c/i2c-core-base.c > @@ -58,6 +58,8 @@ > #define I2C_ADDR_7BITS_MAX 0x77 > #define I2C_ADDR_7BITS_COUNT (I2C_ADDR_7BITS_MAX + 1) > > +#define I2C_ADDR_DEVICE_ID 0x7c > + > /* > * core_lock protects i2c_adapter_idr, and guarantees that device detection, > * deletion of detected devices, and attach_adapter calls are serialized > @@ -1968,6 +1970,37 @@ int i2c_transfer_buffer_flags(const struct i2c_client *client, char *buf, > } > EXPORT_SYMBOL(i2c_transfer_buffer_flags); > > +/** > + * i2c_get_device_id - get manufacturer, part id and die revision of a device > + * @client: The device to query > + * @id: The queried information > + * > + * Returns negative errno on error, zero on success. > + */ > +int i2c_get_device_id(const struct i2c_client *client, > + struct i2c_device_identity *id) > +{ > + struct i2c_adapter *adap = client->adapter; > + union i2c_smbus_data raw_id; > + int ret; > + > + if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_READ_I2C_BLOCK)) > + return -EOPNOTSUPP; > + > + raw_id.block[0] = 3; > + ret = i2c_smbus_xfer(adap, I2C_ADDR_DEVICE_ID, client->flags, > + I2C_SMBUS_READ, client->addr << 1, > + I2C_SMBUS_I2C_BLOCK_DATA, &raw_id); > + if (ret) > + return ret; > + > + id->manufacturer_id = (raw_id.block[1] << 4) | (raw_id.block[2] >> 4); > + id->part_id = ((raw_id.block[2] & 0xf) << 5) | (raw_id.block[3] >> 3); > + id->die_revision = raw_id.block[3] & 0x7; > + return 0; > +} > +EXPORT_SYMBOL_GPL(i2c_get_device_id); > + > /* ---------------------------------------------------- > * the i2c address scanning function > * Will not work for 10-bit addresses! > diff --git a/include/linux/i2c.h b/include/linux/i2c.h > index 419a38e7c315..44ad14e016b5 100644 > --- a/include/linux/i2c.h > +++ b/include/linux/i2c.h > @@ -47,6 +47,7 @@ struct i2c_algorithm; > struct i2c_adapter; > struct i2c_client; > struct i2c_driver; > +struct i2c_device_identity; > union i2c_smbus_data; > struct i2c_board_info; > enum i2c_slave_event; > @@ -186,8 +187,37 @@ extern s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client, > extern s32 > i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client, > u8 command, u8 length, u8 *values); > +int i2c_get_device_id(const struct i2c_client *client, > + struct i2c_device_identity *id); > #endif /* I2C */ > > +/** > + * struct i2c_device_identity - i2c client device identification > + * @manufacturer_id: 0 - 4095, database maintained by NXP > + * @part_id: 0 - 511, according to manufacturer > + * @die_revision: 0 - 7, according to manufacturer > + */ > +struct i2c_device_identity { > + u16 manufacturer_id; > +#define I2C_DEVICE_ID_NXP_SEMICONDUCTORS 0 > +#define I2C_DEVICE_ID_NXP_SEMICONDUCTORS_1 1 > +#define I2C_DEVICE_ID_NXP_SEMICONDUCTORS_2 2 > +#define I2C_DEVICE_ID_NXP_SEMICONDUCTORS_3 3 > +#define I2C_DEVICE_ID_RAMTRON_INTERNATIONAL 4 > +#define I2C_DEVICE_ID_ANALOG_DEVICES 5 > +#define I2C_DEVICE_ID_STMICROELECTRONICS 6 > +#define I2C_DEVICE_ID_ON_SEMICONDUCTOR 7 > +#define I2C_DEVICE_ID_SPRINTEK_CORPORATION 8 > +#define I2C_DEVICE_ID_ESPROS_PHOTONICS_AG 9 > +#define I2C_DEVICE_ID_FUJITSU_SEMICONDUCTOR 10 > +#define I2C_DEVICE_ID_FLIR 11 > +#define I2C_DEVICE_ID_O2MICRO 12 > +#define I2C_DEVICE_ID_ATMEL 13 > +#define I2C_DEVICE_ID_NONE 0xffff > + u16 part_id; > + u8 die_revision; > +}; > + > enum i2c_alert_protocol { > I2C_PROTOCOL_SMBUS_ALERT, > I2C_PROTOCOL_SMBUS_HOST_NOTIFY, > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/2] i2c: add i2c_get_device_id() to get the standard i2c device id 2018-01-22 11:36 ` [RFC PATCH 1/2] i2c: add i2c_get_device_id() to get the standard i2c device id Peter Rosin 2018-03-04 21:47 ` Peter Rosin @ 2018-03-05 15:51 ` Wolfram Sang 2018-03-05 16:06 ` Peter Rosin 1 sibling, 1 reply; 16+ messages in thread From: Wolfram Sang @ 2018-03-05 15:51 UTC (permalink / raw) To: Peter Rosin; +Cc: Adrian Fiergolski, linux-i2c, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1232 bytes --] On Mon, Jan 22, 2018 at 12:36:56PM +0100, Peter Rosin wrote: > Can be used during probe to double check that the probed device is > what is expected. > > Loosely based on code from Adrian Fiergolski <adrian.fiergolski@cern.ch>. > > Signed-off-by: Peter Rosin <peda@axentia.se> In general, nice! I wanted to have such a function in the core but never had a device to test it with. So, much appreciated. Looks mostly good, except... > + ret = i2c_smbus_xfer(adap, I2C_ADDR_DEVICE_ID, client->flags, We shouldn't pass the flag of the clients (like PEC) here. I'd think it could be plain 0 but please double-check. > +/** > + * struct i2c_device_identity - i2c client device identification > + * @manufacturer_id: 0 - 4095, database maintained by NXP > + * @part_id: 0 - 511, according to manufacturer > + * @die_revision: 0 - 7, according to manufacturer > + */ All is nicely documented, very good! About the upstreaming procedure: Could you just make a seperate pull-request out of this feature? I'll pull that in to have it in my tree and you can still collect patches in your usual for-next branch. When the above is fixed you can add my: Reviewed-by: Wolfram Sang <wsa@the-dreams.de> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/2] i2c: add i2c_get_device_id() to get the standard i2c device id 2018-03-05 15:51 ` Wolfram Sang @ 2018-03-05 16:06 ` Peter Rosin 2018-03-05 16:27 ` Wolfram Sang 0 siblings, 1 reply; 16+ messages in thread From: Peter Rosin @ 2018-03-05 16:06 UTC (permalink / raw) To: Wolfram Sang; +Cc: Adrian Fiergolski, linux-i2c, linux-kernel On 2018-03-05 16:51, Wolfram Sang wrote: > On Mon, Jan 22, 2018 at 12:36:56PM +0100, Peter Rosin wrote: >> Can be used during probe to double check that the probed device is >> what is expected. >> >> Loosely based on code from Adrian Fiergolski <adrian.fiergolski@cern.ch>. >> >> Signed-off-by: Peter Rosin <peda@axentia.se> > > In general, nice! I wanted to have such a function in the core but never > had a device to test it with. So, much appreciated. > > Looks mostly good, except... > >> + ret = i2c_smbus_xfer(adap, I2C_ADDR_DEVICE_ID, client->flags, > > We shouldn't pass the flag of the clients (like PEC) here. I'd think it > could be plain 0 but please double-check. Right, ..._TEN should definitely be cleared. ..._SLAVE will probably be cleared anyway. ..._HOST_NOTIFY, ..._WAKE and ..._SCCB I don't know about? Are there others? The bits aren't that densely populated which makes me worry that I'm missing something... Cheers, Peter >> +/** >> + * struct i2c_device_identity - i2c client device identification >> + * @manufacturer_id: 0 - 4095, database maintained by NXP >> + * @part_id: 0 - 511, according to manufacturer >> + * @die_revision: 0 - 7, according to manufacturer >> + */ > > All is nicely documented, very good! > > About the upstreaming procedure: Could you just make a seperate > pull-request out of this feature? I'll pull that in to have it in my > tree and you can still collect patches in your usual for-next branch. > > When the above is fixed you can add my: > > Reviewed-by: Wolfram Sang <wsa@the-dreams.de> > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/2] i2c: add i2c_get_device_id() to get the standard i2c device id 2018-03-05 16:06 ` Peter Rosin @ 2018-03-05 16:27 ` Wolfram Sang 0 siblings, 0 replies; 16+ messages in thread From: Wolfram Sang @ 2018-03-05 16:27 UTC (permalink / raw) To: Peter Rosin; +Cc: Adrian Fiergolski, linux-i2c, linux-kernel [-- Attachment #1: Type: text/plain, Size: 336 bytes --] > ..._TEN should definitely be cleared. > ..._SLAVE will probably be cleared anyway. > ..._HOST_NOTIFY, ..._WAKE and ..._SCCB I don't know about? Not relevant for device_id reading. > Are there others? The bits aren't that densely populated which makes > me worry that I'm missing something... Historic reasons, there are no more. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH 2/2] i2c: mux: pca954x: verify the device id of the pca984x chips 2018-01-22 11:36 [RFC PATCH 0/2] check I2C device id for pca984x chips Peter Rosin 2018-01-22 11:36 ` [RFC PATCH 1/2] i2c: add i2c_get_device_id() to get the standard i2c device id Peter Rosin @ 2018-01-22 11:36 ` Peter Rosin 2018-03-05 15:53 ` Wolfram Sang 2018-01-26 16:33 ` Adrian Fiergolski 2 siblings, 1 reply; 16+ messages in thread From: Peter Rosin @ 2018-01-22 11:36 UTC (permalink / raw) To: Adrian Fiergolski; +Cc: Peter Rosin, Wolfram Sang, linux-i2c, linux-kernel 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(+) 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 && -- 2.11.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 2/2] i2c: mux: pca954x: verify the device id of the pca984x chips 2018-01-22 11:36 ` [RFC PATCH 2/2] i2c: mux: pca954x: verify the device id of the pca984x chips Peter Rosin @ 2018-03-05 15:53 ` Wolfram Sang 2018-03-05 15:55 ` Peter Rosin 0 siblings, 1 reply; 16+ messages in thread From: Wolfram Sang @ 2018-03-05 15:53 UTC (permalink / raw) To: Peter Rosin; +Cc: Adrian Fiergolski, linux-i2c, linux-kernel [-- Attachment #1: Type: text/plain, Size: 288 bytes --] > @@ -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? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 2/2] i2c: mux: pca954x: verify the device id of the pca984x chips 2018-03-05 15:53 ` Wolfram Sang @ 2018-03-05 15:55 ` Peter Rosin 2018-03-05 16:29 ` Wolfram Sang 0 siblings, 1 reply; 16+ messages in thread From: Peter Rosin @ 2018-03-05 15:55 UTC (permalink / raw) To: Wolfram Sang; +Cc: Adrian Fiergolski, linux-i2c, linux-kernel 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 2/2] i2c: mux: pca954x: verify the device id of the pca984x chips 2018-03-05 15:55 ` Peter Rosin @ 2018-03-05 16:29 ` Wolfram Sang 0 siblings, 0 replies; 16+ messages in thread From: Wolfram Sang @ 2018-03-05 16:29 UTC (permalink / raw) To: Peter Rosin; +Cc: Adrian Fiergolski, linux-i2c, linux-kernel [-- Attachment #1: Type: text/plain, Size: 570 bytes --] 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! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/2] check I2C device id for pca984x chips 2018-01-22 11:36 [RFC PATCH 0/2] check I2C device id for pca984x chips Peter Rosin @ 2018-01-26 16:33 ` Adrian Fiergolski 2018-01-22 11:36 ` [RFC PATCH 2/2] i2c: mux: pca954x: verify the device id of the pca984x chips Peter Rosin 2018-01-26 16:33 ` Adrian Fiergolski 2 siblings, 0 replies; 16+ messages in thread From: Adrian Fiergolski @ 2018-01-26 16:33 UTC (permalink / raw) To: Peter Rosin; +Cc: Wolfram Sang, linux-i2c, linux-kernel Hi Peter, Sorry for the late reply. Yes, it's true I have one of the chip. However, my yocto based build system depends on https://github.com/Xilinx/linux-xlnx and it's in version 4.9.0-xilinx-v2017.3. Apparently, there were some bigger changes in i2c core between this version and upstream, thus your patches don't apply. Next week I will try to align only me i2c subdirectory with upstream. Provided it compiles, I will try then to apply and confirm your patches. Regards, Adrian On 22.01.2018 at 12:36, Peter Rosin wrote: > Hi! > > This series tries to check the I2C device id, but instead of open > coding the check in the pca954x driver, I have a new function in > the core doing the work. > > The code is only compile-tested, hence the RFC, and I would really > like a Tested-by: tag from Adrian who presumably have one of these > chips. > > Also, I'm not sure if I should list all manufacturers that I know > about in the header, or if I should settle for the one that is > actually used and leave the others to be added by whomever needs > them... > > Cheers, > peda > > Peter Rosin (2): > i2c: add i2c_get_device_id() to get the standard i2c device id > i2c: mux: pca954x: verify the device id of the pca984x chips > > drivers/i2c/i2c-core-base.c | 33 ++++++++++++++++++++++++++++ > drivers/i2c/muxes/i2c-mux-pca954x.c | 43 +++++++++++++++++++++++++++++++++++++ > include/linux/i2c.h | 30 ++++++++++++++++++++++++++ > 3 files changed, 106 insertions(+) > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/2] check I2C device id for pca984x chips @ 2018-01-26 16:33 ` Adrian Fiergolski 0 siblings, 0 replies; 16+ messages in thread From: Adrian Fiergolski @ 2018-01-26 16:33 UTC (permalink / raw) To: Peter Rosin; +Cc: Wolfram Sang, linux-i2c, linux-kernel Hi Peter, Sorry for the late reply. Yes, it's true I have one of the chip. However, my yocto based build system depends on https://github.com/Xilinx/linux-xlnx and it's in version 4.9.0-xilinx-v2017.3. Apparently, there were some bigger changes in i2c core between this version and upstream, thus your patches don't apply. Next week I will try to align only me i2c subdirectory with upstream. Provided it compiles, I will try then to apply and confirm your patches. Regards, Adrian On 22.01.2018 at 12:36, Peter Rosin wrote: > Hi! > > This series tries to check the I2C device id, but instead of open > coding the check in the pca954x driver, I have a new function in > the core doing the work. > > The code is only compile-tested, hence the RFC, and I would really > like a Tested-by: tag from Adrian who presumably have one of these > chips. > > Also, I'm not sure if I should list all manufacturers that I know > about in the header, or if I should settle for the one that is > actually used and leave the others to be added by whomever needs > them... > > Cheers, > peda > > Peter Rosin (2): > i2c: add i2c_get_device_id() to get the standard i2c device id > i2c: mux: pca954x: verify the device id of the pca984x chips > > drivers/i2c/i2c-core-base.c | 33 ++++++++++++++++++++++++++++ > drivers/i2c/muxes/i2c-mux-pca954x.c | 43 +++++++++++++++++++++++++++++++++++++ > include/linux/i2c.h | 30 ++++++++++++++++++++++++++ > 3 files changed, 106 insertions(+) > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/2] check I2C device id for pca984x chips 2018-01-26 16:33 ` Adrian Fiergolski (?) @ 2018-01-27 8:37 ` Peter Rosin 2018-01-29 17:38 ` Adrian Fiergolski -1 siblings, 1 reply; 16+ messages in thread From: Peter Rosin @ 2018-01-27 8:37 UTC (permalink / raw) To: Adrian Fiergolski; +Cc: Wolfram Sang, linux-i2c, linux-kernel On 2018-01-26 17:33, Adrian Fiergolski wrote: > Hi Peter, > > Sorry for the late reply. No problem. > Yes, it's true I have one of the chip. However, my yocto based build system > depends on https://github.com/Xilinx/linux-xlnx and it's in version > 4.9.0-xilinx-v2017.3. > Apparently, there were some bigger changes in i2c core between this > version and > upstream, thus your patches don't apply. I think the core changes fail to apply mostly because of the file renaming that has been going on, and that it should be fairly trivial to adapt. But I don't know for certain... > Next week I will try to align only me i2c subdirectory with upstream. > Provided it compiles, I will > try then to apply and confirm your patches. I'm looking forward to feedback, thanks! Cheers, Peter > Regards, > Adrian > > On 22.01.2018 at 12:36, Peter Rosin wrote: >> Hi! >> >> This series tries to check the I2C device id, but instead of open >> coding the check in the pca954x driver, I have a new function in >> the core doing the work. >> >> The code is only compile-tested, hence the RFC, and I would really >> like a Tested-by: tag from Adrian who presumably have one of these >> chips. >> >> Also, I'm not sure if I should list all manufacturers that I know >> about in the header, or if I should settle for the one that is >> actually used and leave the others to be added by whomever needs >> them... >> >> Cheers, >> peda >> >> Peter Rosin (2): >> i2c: add i2c_get_device_id() to get the standard i2c device id >> i2c: mux: pca954x: verify the device id of the pca984x chips >> >> drivers/i2c/i2c-core-base.c | 33 ++++++++++++++++++++++++++++ >> drivers/i2c/muxes/i2c-mux-pca954x.c | 43 +++++++++++++++++++++++++++++++++++++ >> include/linux/i2c.h | 30 ++++++++++++++++++++++++++ >> 3 files changed, 106 insertions(+) >> > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/2] check I2C device id for pca984x chips 2018-01-27 8:37 ` Peter Rosin @ 2018-01-29 17:38 ` Adrian Fiergolski 0 siblings, 0 replies; 16+ messages in thread From: Adrian Fiergolski @ 2018-01-29 17:38 UTC (permalink / raw) To: Peter Rosin; +Cc: Wolfram Sang, linux-i2c, linux-kernel On 27.01.2018 at 09:37, Peter Rosin wrote: > On 2018-01-26 17:33, Adrian Fiergolski wrote: >> Hi Peter, >> >> Sorry for the late reply. > No problem. > >> Yes, it's true I have one of the chip. However, my yocto based build system >> depends on https://github.com/Xilinx/linux-xlnx and it's in version >> 4.9.0-xilinx-v2017.3. >> Apparently, there were some bigger changes in i2c core between this >> version and >> upstream, thus your patches don't apply. > I think the core changes fail to apply mostly because of the file renaming > that has been going on, and that it should be fairly trivial to adapt. > But I don't know for certain... > >> Next week I will try to align only me i2c subdirectory with upstream. >> Provided it compiles, I will >> try then to apply and confirm your patches. > I'm looking forward to feedback, thanks! > > Cheers, > Peter > >> Regards, >> Adrian >> >> On 22.01.2018 at 12:36, Peter Rosin wrote: >>> Hi! >>> >>> This series tries to check the I2C device id, but instead of open >>> coding the check in the pca954x driver, I have a new function in >>> the core doing the work. >>> >>> The code is only compile-tested, hence the RFC, and I would really >>> like a Tested-by: tag from Adrian who presumably have one of these >>> chips. >>> >>> Also, I'm not sure if I should list all manufacturers that I know >>> about in the header, or if I should settle for the one that is >>> actually used and leave the others to be added by whomever needs >>> them... >>> >>> Cheers, >>> peda >>> >>> Peter Rosin (2): >>> i2c: add i2c_get_device_id() to get the standard i2c device id >>> i2c: mux: pca954x: verify the device id of the pca984x chips >>> >>> drivers/i2c/i2c-core-base.c | 33 ++++++++++++++++++++++++++++ >>> drivers/i2c/muxes/i2c-mux-pca954x.c | 43 +++++++++++++++++++++++++++++++++++++ >>> include/linux/i2c.h | 30 ++++++++++++++++++++++++++ >>> 3 files changed, 106 insertions(+) >>> Hi Peter, I have tested your patch with the pca9846 device and I confirm it works. Moreover, after short debugging, I can confirm that all read ids (manufacture, part and die) seem to be correct. Moreover, in case of misconfiguration, the probe function return a proper message and fails as expected. Regards, Adrian ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/2] check I2C device id for pca984x chips @ 2018-01-29 17:38 ` Adrian Fiergolski 0 siblings, 0 replies; 16+ messages in thread From: Adrian Fiergolski @ 2018-01-29 17:38 UTC (permalink / raw) To: Peter Rosin; +Cc: Wolfram Sang, linux-i2c, linux-kernel On 27.01.2018 at 09:37, Peter Rosin wrote: > On 2018-01-26 17:33, Adrian Fiergolski wrote: >> Hi Peter, >> >> Sorry for the late reply. > No problem. > >> Yes, it's true I have one of the chip. However, my yocto based build system >> depends on https://github.com/Xilinx/linux-xlnx and it's in version >> 4.9.0-xilinx-v2017.3. >> Apparently, there were some bigger changes in i2c core between this >> version and >> upstream, thus your patches don't apply. > I think the core changes fail to apply mostly because of the file renaming > that has been going on, and that it should be fairly trivial to adapt. > But I don't know for certain... > >> Next week I will try to align only me i2c subdirectory with upstream. >> Provided it compiles, I will >> try then to apply and confirm your patches. > I'm looking forward to feedback, thanks! > > Cheers, > Peter > >> Regards, >> Adrian >> >> On 22.01.2018 at 12:36, Peter Rosin wrote: >>> Hi! >>> >>> This series tries to check the I2C device id, but instead of open >>> coding the check in the pca954x driver, I have a new function in >>> the core doing the work. >>> >>> The code is only compile-tested, hence the RFC, and I would really >>> like a Tested-by: tag from Adrian who presumably have one of these >>> chips. >>> >>> Also, I'm not sure if I should list all manufacturers that I know >>> about in the header, or if I should settle for the one that is >>> actually used and leave the others to be added by whomever needs >>> them... >>> >>> Cheers, >>> peda >>> >>> Peter Rosin (2): >>> i2c: add i2c_get_device_id() to get the standard i2c device id >>> i2c: mux: pca954x: verify the device id of the pca984x chips >>> >>> drivers/i2c/i2c-core-base.c | 33 ++++++++++++++++++++++++++++ >>> drivers/i2c/muxes/i2c-mux-pca954x.c | 43 +++++++++++++++++++++++++++++++++++++ >>> include/linux/i2c.h | 30 ++++++++++++++++++++++++++ >>> 3 files changed, 106 insertions(+) >>> Hi Peter, I have tested your patch with the pca9846 device and I confirm it works. Moreover, after short debugging, I can confirm that all read ids (manufacture, part and die) seem to be correct. Moreover, in case of misconfiguration, the probe function return a proper message and fails as expected. Regards, Adrian ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/2] check I2C device id for pca984x chips 2018-01-29 17:38 ` Adrian Fiergolski (?) @ 2018-02-01 14:41 ` Peter Rosin -1 siblings, 0 replies; 16+ messages in thread From: Peter Rosin @ 2018-02-01 14:41 UTC (permalink / raw) To: Adrian Fiergolski; +Cc: Wolfram Sang, linux-i2c, linux-kernel On 2018-01-29 18:38, Adrian Fiergolski wrote: >>> On 22.01.2018 at 12:36, Peter Rosin wrote: >>>> This series tries to check the I2C device id, but instead of open >>>> coding the check in the pca954x driver, I have a new function in >>>> the core doing the work. >>>> >>>> The code is only compile-tested, hence the RFC, and I would really >>>> like a Tested-by: tag from Adrian who presumably have one of these >>>> chips. >>>> >>>> Also, I'm not sure if I should list all manufacturers that I know >>>> about in the header, or if I should settle for the one that is >>>> actually used and leave the others to be added by whomever needs >>>> them... >>>> >>>> Cheers, >>>> peda >>>> >>>> Peter Rosin (2): >>>> i2c: add i2c_get_device_id() to get the standard i2c device id >>>> i2c: mux: pca954x: verify the device id of the pca984x chips >>>> >>>> drivers/i2c/i2c-core-base.c | 33 ++++++++++++++++++++++++++++ >>>> drivers/i2c/muxes/i2c-mux-pca954x.c | 43 +++++++++++++++++++++++++++++++++++++ >>>> include/linux/i2c.h | 30 ++++++++++++++++++++++++++ >>>> 3 files changed, 106 insertions(+) >>>> > Hi Peter, > > I have tested your patch with the pca9846 device and I confirm it works. > Moreover, after short debugging, I can confirm that all read ids > (manufacture, part and die) seem to be correct. Moreover, in case of > misconfiguration, the probe function return a proper message and fails > as expected. Excellent, thanks! I'll ping Wolfram later on for any input on the core changes (if needed), but let's wait until the 4.16 merge-window closes and things settle down a bit. This will land in 4.17 at the earliest. Cheers, Peter ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-03-05 16:29 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-01-22 11:36 [RFC PATCH 0/2] check I2C device id for pca984x chips Peter Rosin 2018-01-22 11:36 ` [RFC PATCH 1/2] i2c: add i2c_get_device_id() to get the standard i2c device id Peter Rosin 2018-03-04 21:47 ` Peter Rosin 2018-03-05 15:51 ` Wolfram Sang 2018-03-05 16:06 ` Peter Rosin 2018-03-05 16:27 ` Wolfram Sang 2018-01-22 11:36 ` [RFC PATCH 2/2] i2c: mux: pca954x: verify the device id of the pca984x chips Peter Rosin 2018-03-05 15:53 ` Wolfram Sang 2018-03-05 15:55 ` Peter Rosin 2018-03-05 16:29 ` Wolfram Sang 2018-01-26 16:33 ` [RFC PATCH 0/2] check I2C device id for " Adrian Fiergolski 2018-01-26 16:33 ` Adrian Fiergolski 2018-01-27 8:37 ` Peter Rosin 2018-01-29 17:38 ` Adrian Fiergolski 2018-01-29 17:38 ` Adrian Fiergolski 2018-02-01 14:41 ` Peter Rosin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.