* [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
* [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 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
* 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 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 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
* 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
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.