* [PATCH RFC 0/4] Start unification of of_device_id and i2c_device_id
@ 2023-08-14 21:52 Dmitry Torokhov
2023-08-14 21:52 ` [PATCH RFC 1/4] i2c: start migrating to a pointer in i2c_device_id Dmitry Torokhov
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2023-08-14 21:52 UTC (permalink / raw)
To: Wolfram Sang, Andy Shevchenko, Andi Shyti, Biju Das,
Jonathan Cameron, Michael Hennerich, Peter Rosin
Cc: linux-i2c, linux-kernel
i2c_device_id is using unsigned long as driver data, whereas
of_device_id is using a void pointer. Normally drivers use them both as
either a pointer or a scalar, but in some cases driver's authors chose
to treat one as a pointer and another as a scalar that is being used as
an index in array of chip-specific data, i.e. something like this:
data = device_get_match_data(...);
if (!data) {
id = i2c_client_get_device_id(...);
data = &some_array[id->driver_data];
}
This makes it hard to turn device_get_match_data() into truly generic
function working well for ACPI and DT, as well as for legacy cases.
So let's try to switch i2c_device_id to also use a void pointer and
first switch drivers with the patterns above to it, and then the rest of
them.
I did a conversion of a couple of drivers in i2c to illustrate.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Dmitry Torokhov (4):
i2c: start migrating to a pointer in i2c_device_id
i2c: mux: ltc4306: convert to using a pointer in i2c_device_id
i2c: mux: pca954x: convert to using a pointer in i2c_device_id
i2c: slave-eeprom: convert to using a pointer in i2c_device_id
drivers/i2c/i2c-core-base.c | 2 +-
drivers/i2c/i2c-slave-eeprom.c | 99 ++++++++++++++++++++++++-------------
drivers/i2c/muxes/i2c-mux-ltc4306.c | 7 ++-
drivers/i2c/muxes/i2c-mux-pca954x.c | 28 +++++------
include/linux/mod_devicetable.h | 3 +-
5 files changed, 86 insertions(+), 53 deletions(-)
---
base-commit: 21ef7b1e17d039053edaeaf41142423810572741
change-id: 20230814-i2c-id-rework-9a4742e9ee52
Best regards,
--
Dmitry
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH RFC 1/4] i2c: start migrating to a pointer in i2c_device_id
2023-08-14 21:52 [PATCH RFC 0/4] Start unification of of_device_id and i2c_device_id Dmitry Torokhov
@ 2023-08-14 21:52 ` Dmitry Torokhov
2023-08-15 15:55 ` Andy Shevchenko
2023-08-14 21:52 ` [PATCH RFC 2/4] i2c: mux: ltc4306: convert to using " Dmitry Torokhov
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2023-08-14 21:52 UTC (permalink / raw)
To: Wolfram Sang, Andy Shevchenko, Andi Shyti, Biju Das,
Jonathan Cameron, Michael Hennerich, Peter Rosin
Cc: linux-i2c, linux-kernel
The of_device_id structure uses a void pointer to associate additional
driver-private data with device id, most commonly used to refer to a
structure containing additional characteristics of a particular chip.
However i2c_device_id uses an unsigned long. While it can easily be
converted to a pointer, several drivers use it to store a scalar (and it
is possible to use a pointer in of_device_id to store a scalar value as
well). The worst case comes when OF part of a driver uses pointers,
while legacy part is using scalars, causing constructs like:
data = device_get_match_data(...);
if (!data)
data = &some_array[i2c_match_id(...)->data];
...
To avoid this introduce a const void "data" pointer to i2c_device_id as
well, so that we can convert drivers one by one, and drop driver_data
member in the end.
The end goal is to clean up all helpers and make device_get_match_data()
work universally for all ACPI, DT, and legacy variants.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/i2c/i2c-core-base.c | 2 +-
include/linux/mod_devicetable.h | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 60746652fd52..75d12d79d8f1 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -126,7 +126,7 @@ const void *i2c_get_match_data(const struct i2c_client *client)
if (!match)
return NULL;
- data = (const void *)match->driver_data;
+ data = match->data ?: (const void *)match->driver_data;
}
return data;
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index b0678b093cb2..21ac2a96e828 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -477,7 +477,8 @@ struct rpmsg_device_id {
struct i2c_device_id {
char name[I2C_NAME_SIZE];
- kernel_ulong_t driver_data; /* Data private to the driver */
+ kernel_ulong_t driver_data; /* This field is obsolete */
+ const void *data; /* Data private to the driver */
};
/* pci_epf */
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH RFC 2/4] i2c: mux: ltc4306: convert to using a pointer in i2c_device_id
2023-08-14 21:52 [PATCH RFC 0/4] Start unification of of_device_id and i2c_device_id Dmitry Torokhov
2023-08-14 21:52 ` [PATCH RFC 1/4] i2c: start migrating to a pointer in i2c_device_id Dmitry Torokhov
@ 2023-08-14 21:52 ` Dmitry Torokhov
2023-08-15 15:50 ` Andy Shevchenko
2023-08-14 21:52 ` [PATCH RFC 3/4] i2c: mux: pca954x: " Dmitry Torokhov
2023-08-14 21:52 ` [PATCH RFC 4/4] i2c: slave-eeprom: " Dmitry Torokhov
3 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2023-08-14 21:52 UTC (permalink / raw)
To: Wolfram Sang, Andy Shevchenko, Andi Shyti, Biju Das,
Jonathan Cameron, Michael Hennerich, Peter Rosin
Cc: linux-i2c, linux-kernel
Switch the driver to use newly added "data" pointer in i2c_device_id to
streamline DT and legacy flows.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/i2c/muxes/i2c-mux-ltc4306.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-ltc4306.c b/drivers/i2c/muxes/i2c-mux-ltc4306.c
index 637e25506490..17b9ed9ceef3 100644
--- a/drivers/i2c/muxes/i2c-mux-ltc4306.c
+++ b/drivers/i2c/muxes/i2c-mux-ltc4306.c
@@ -192,8 +192,8 @@ static int ltc4306_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
}
static const struct i2c_device_id ltc4306_id[] = {
- { "ltc4305", ltc_4305 },
- { "ltc4306", ltc_4306 },
+ { "ltc4305", .data = &chips[ltc_4305] },
+ { "ltc4306", .data = &chips[ltc_4306] },
{ }
};
MODULE_DEVICE_TABLE(i2c, ltc4306_id);
@@ -217,9 +217,8 @@ static int ltc4306_probe(struct i2c_client *client)
int num, ret;
chip = of_device_get_match_data(&client->dev);
-
if (!chip)
- chip = &chips[i2c_match_id(ltc4306_id, client)->driver_data];
+ chip = i2c_match_id(ltc4306_id, client)->data;
idle_disc = device_property_read_bool(&client->dev,
"i2c-mux-idle-disconnect");
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH RFC 3/4] i2c: mux: pca954x: convert to using a pointer in i2c_device_id
2023-08-14 21:52 [PATCH RFC 0/4] Start unification of of_device_id and i2c_device_id Dmitry Torokhov
2023-08-14 21:52 ` [PATCH RFC 1/4] i2c: start migrating to a pointer in i2c_device_id Dmitry Torokhov
2023-08-14 21:52 ` [PATCH RFC 2/4] i2c: mux: ltc4306: convert to using " Dmitry Torokhov
@ 2023-08-14 21:52 ` Dmitry Torokhov
2023-08-15 15:51 ` Andy Shevchenko
2023-08-14 21:52 ` [PATCH RFC 4/4] i2c: slave-eeprom: " Dmitry Torokhov
3 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2023-08-14 21:52 UTC (permalink / raw)
To: Wolfram Sang, Andy Shevchenko, Andi Shyti, Biju Das,
Jonathan Cameron, Michael Hennerich, Peter Rosin
Cc: linux-i2c, linux-kernel
Switch the driver to use newly added "data" pointer in i2c_device_id to
streamline DT and legacy flows.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 6965bf4c2348..c8540e4ca660 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -177,18 +177,18 @@ static const struct chip_desc chips[] = {
};
static const struct i2c_device_id pca954x_id[] = {
- { "pca9540", pca_9540 },
- { "pca9542", pca_9542 },
- { "pca9543", pca_9543 },
- { "pca9544", pca_9544 },
- { "pca9545", pca_9545 },
- { "pca9546", pca_9546 },
- { "pca9547", pca_9547 },
- { "pca9548", pca_9548 },
- { "pca9846", pca_9846 },
- { "pca9847", pca_9847 },
- { "pca9848", pca_9848 },
- { "pca9849", pca_9849 },
+ { "pca9540", .data = &chips[pca_9540] },
+ { "pca9542", .data = &chips[pca_9542] },
+ { "pca9543", .data = &chips[pca_9543] },
+ { "pca9544", .data = &chips[pca_9544] },
+ { "pca9545", .data = &chips[pca_9545] },
+ { "pca9546", .data = &chips[pca_9546] },
+ { "pca9547", .data = &chips[pca_9547] },
+ { "pca9548", .data = &chips[pca_9548] },
+ { "pca9846", .data = &chips[pca_9846] },
+ { "pca9847", .data = &chips[pca_9847] },
+ { "pca9848", .data = &chips[pca_9848] },
+ { "pca9849", .data = &chips[pca_9849] },
{ }
};
MODULE_DEVICE_TABLE(i2c, pca954x_id);
@@ -206,7 +206,7 @@ static const struct of_device_id pca954x_of_match[] = {
{ .compatible = "nxp,pca9847", .data = &chips[pca_9847] },
{ .compatible = "nxp,pca9848", .data = &chips[pca_9848] },
{ .compatible = "nxp,pca9849", .data = &chips[pca_9849] },
- {}
+ { }
};
MODULE_DEVICE_TABLE(of, pca954x_of_match);
@@ -447,7 +447,7 @@ static int pca954x_probe(struct i2c_client *client)
data->chip = device_get_match_data(dev);
if (!data->chip)
- data->chip = &chips[id->driver_data];
+ data->chip = id->data;
if (data->chip->id.manufacturer_id != I2C_DEVICE_ID_NONE) {
struct i2c_device_identity id;
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH RFC 4/4] i2c: slave-eeprom: convert to using a pointer in i2c_device_id
2023-08-14 21:52 [PATCH RFC 0/4] Start unification of of_device_id and i2c_device_id Dmitry Torokhov
` (2 preceding siblings ...)
2023-08-14 21:52 ` [PATCH RFC 3/4] i2c: mux: pca954x: " Dmitry Torokhov
@ 2023-08-14 21:52 ` Dmitry Torokhov
2023-08-15 15:53 ` Andy Shevchenko
3 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2023-08-14 21:52 UTC (permalink / raw)
To: Wolfram Sang, Andy Shevchenko, Andi Shyti, Biju Das,
Jonathan Cameron, Michael Hennerich, Peter Rosin
Cc: linux-i2c, linux-kernel
Switch the driver to use newly added "data" pointer in i2c_device_id and
introduce struct eeprom_chip to describe chip's characteristics instead
of cramming it all into an unisigned long and then decipher.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/i2c/i2c-slave-eeprom.c | 99 ++++++++++++++++++++++++++++--------------
1 file changed, 66 insertions(+), 33 deletions(-)
diff --git a/drivers/i2c/i2c-slave-eeprom.c b/drivers/i2c/i2c-slave-eeprom.c
index 5946c0d0aef9..1d2340da5a70 100644
--- a/drivers/i2c/i2c-slave-eeprom.c
+++ b/drivers/i2c/i2c-slave-eeprom.c
@@ -16,32 +16,29 @@
* EEPROMs, though. We currently use the 8 bit as a valid address.
*/
-#include <linux/bitfield.h>
#include <linux/firmware.h>
#include <linux/i2c.h>
#include <linux/init.h>
#include <linux/module.h>
-#include <linux/of.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/sysfs.h>
+struct eeprom_chip {
+ u16 address_mask;
+ u8 num_address_bytes;
+ bool read_only;
+};
+
struct eeprom_data {
+ const struct eeprom_chip *chip;
struct bin_attribute bin;
spinlock_t buffer_lock;
u16 buffer_idx;
- u16 address_mask;
- u8 num_address_bytes;
u8 idx_write_cnt;
- bool read_only;
u8 buffer[];
};
-#define I2C_SLAVE_BYTELEN GENMASK(15, 0)
-#define I2C_SLAVE_FLAG_ADDR16 BIT(16)
-#define I2C_SLAVE_FLAG_RO BIT(17)
-#define I2C_SLAVE_DEVICE_MAGIC(_len, _flags) ((_flags) | ((_len) - 1))
-
static int i2c_slave_eeprom_slave_cb(struct i2c_client *client,
enum i2c_slave_event event, u8 *val)
{
@@ -49,17 +46,15 @@ static int i2c_slave_eeprom_slave_cb(struct i2c_client *client,
switch (event) {
case I2C_SLAVE_WRITE_RECEIVED:
- if (eeprom->idx_write_cnt < eeprom->num_address_bytes) {
+ if (eeprom->idx_write_cnt < eeprom->chip->num_address_bytes) {
if (eeprom->idx_write_cnt == 0)
eeprom->buffer_idx = 0;
eeprom->buffer_idx = *val | (eeprom->buffer_idx << 8);
eeprom->idx_write_cnt++;
- } else {
- if (!eeprom->read_only) {
- spin_lock(&eeprom->buffer_lock);
- eeprom->buffer[eeprom->buffer_idx++ & eeprom->address_mask] = *val;
- spin_unlock(&eeprom->buffer_lock);
- }
+ } else if (!eeprom->chip->read_only) {
+ spin_lock(&eeprom->buffer_lock);
+ eeprom->buffer[eeprom->buffer_idx++ & eeprom->chip->address_mask] = *val;
+ spin_unlock(&eeprom->buffer_lock);
}
break;
@@ -69,7 +64,7 @@ static int i2c_slave_eeprom_slave_cb(struct i2c_client *client,
fallthrough;
case I2C_SLAVE_READ_REQUESTED:
spin_lock(&eeprom->buffer_lock);
- *val = eeprom->buffer[eeprom->buffer_idx & eeprom->address_mask];
+ *val = eeprom->buffer[eeprom->buffer_idx & eeprom->chip->address_mask];
spin_unlock(&eeprom->buffer_lock);
/*
* Do not increment buffer_idx here, because we don't know if
@@ -142,19 +137,25 @@ static int i2c_slave_init_eeprom_data(struct eeprom_data *eeprom, struct i2c_cli
static int i2c_slave_eeprom_probe(struct i2c_client *client)
{
- const struct i2c_device_id *id = i2c_client_get_device_id(client);
+ const struct i2c_device_id *id;
+ const struct eeprom_chip *chip;
struct eeprom_data *eeprom;
int ret;
- unsigned int size = FIELD_GET(I2C_SLAVE_BYTELEN, id->driver_data) + 1;
- unsigned int flag_addr16 = FIELD_GET(I2C_SLAVE_FLAG_ADDR16, id->driver_data);
+ unsigned int size;
+
+ id = i2c_client_get_device_id(client);
+ if (!id)
+ return -EINVAL;
+
+ chip = id->data;
+ size = chip->address_mask + 1;
- eeprom = devm_kzalloc(&client->dev, sizeof(struct eeprom_data) + size, GFP_KERNEL);
+ eeprom = devm_kzalloc(&client->dev, struct_size(eeprom, buffer, size),
+ GFP_KERNEL);
if (!eeprom)
return -ENOMEM;
- eeprom->num_address_bytes = flag_addr16 ? 2 : 1;
- eeprom->address_mask = size - 1;
- eeprom->read_only = FIELD_GET(I2C_SLAVE_FLAG_RO, id->driver_data);
+ eeprom->chip = chip;
spin_lock_init(&eeprom->buffer_lock);
i2c_set_clientdata(client, eeprom);
@@ -190,15 +191,47 @@ static void i2c_slave_eeprom_remove(struct i2c_client *client)
sysfs_remove_bin_file(&client->dev.kobj, &eeprom->bin);
}
+static const struct eeprom_chip chip_24c02 = {
+ .address_mask = 2048 / 8 - 1, .num_address_bytes = 1,
+};
+
+static const struct eeprom_chip chip_24c02ro = {
+ .address_mask = 2048 / 8 - 1, .num_address_bytes = 1, .read_only = true,
+};
+
+static const struct eeprom_chip chip_24c32 = {
+ .address_mask = 32768 / 8 - 1, .num_address_bytes = 2,
+};
+
+static const struct eeprom_chip chip_24c32ro = {
+ .address_mask = 32768 / 8 - 1, .num_address_bytes = 2, .read_only = true,
+};
+
+static const struct eeprom_chip chip_24c64 = {
+ .address_mask = 65536 / 8 - 1, .num_address_bytes = 2,
+};
+
+static const struct eeprom_chip chip_24c64ro = {
+ .address_mask = 65536 / 8 - 1, .num_address_bytes = 2, .read_only = true,
+};
+
+static const struct eeprom_chip chip_24c512 = {
+ .address_mask = 524288 / 8 - 1, .num_address_bytes = 2,
+};
+
+static const struct eeprom_chip chip_24c512ro = {
+ .address_mask = 524288 / 8 - 1, .num_address_bytes = 2, .read_only = true,
+};
+
static const struct i2c_device_id i2c_slave_eeprom_id[] = {
- { "slave-24c02", I2C_SLAVE_DEVICE_MAGIC(2048 / 8, 0) },
- { "slave-24c02ro", I2C_SLAVE_DEVICE_MAGIC(2048 / 8, I2C_SLAVE_FLAG_RO) },
- { "slave-24c32", I2C_SLAVE_DEVICE_MAGIC(32768 / 8, I2C_SLAVE_FLAG_ADDR16) },
- { "slave-24c32ro", I2C_SLAVE_DEVICE_MAGIC(32768 / 8, I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
- { "slave-24c64", I2C_SLAVE_DEVICE_MAGIC(65536 / 8, I2C_SLAVE_FLAG_ADDR16) },
- { "slave-24c64ro", I2C_SLAVE_DEVICE_MAGIC(65536 / 8, I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
- { "slave-24c512", I2C_SLAVE_DEVICE_MAGIC(524288 / 8, I2C_SLAVE_FLAG_ADDR16) },
- { "slave-24c512ro", I2C_SLAVE_DEVICE_MAGIC(524288 / 8, I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
+ { "slave-24c02", .data = &chip_24c02 },
+ { "slave-24c02ro", .data = &chip_24c02ro },
+ { "slave-24c32", .data = &chip_24c32 },
+ { "slave-24c32ro", .data = &chip_24c32ro },
+ { "slave-24c64", .data = &chip_24c64 },
+ { "slave-24c64ro", .data = &chip_24c64ro },
+ { "slave-24c512", .data = &chip_24c512 },
+ { "slave-24c512ro", .data = &chip_24c512ro },
{ }
};
MODULE_DEVICE_TABLE(i2c, i2c_slave_eeprom_id);
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 2/4] i2c: mux: ltc4306: convert to using a pointer in i2c_device_id
2023-08-14 21:52 ` [PATCH RFC 2/4] i2c: mux: ltc4306: convert to using " Dmitry Torokhov
@ 2023-08-15 15:50 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2023-08-15 15:50 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Wolfram Sang, Andi Shyti, Biju Das, Jonathan Cameron,
Michael Hennerich, Peter Rosin, linux-i2c, linux-kernel
On Mon, Aug 14, 2023 at 02:52:50PM -0700, Dmitry Torokhov wrote:
> Switch the driver to use newly added "data" pointer in i2c_device_id to
> streamline DT and legacy flows.
...
> chip = of_device_get_match_data(&client->dev);
> -
> if (!chip)
> - chip = &chips[i2c_match_id(ltc4306_id, client)->driver_data];
> + chip = i2c_match_id(ltc4306_id, client)->data;
The whole thing can be converted to i2c_device_get_match_data() with the new
helper from another thread. That said, I don't see value in this half-baked
change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 3/4] i2c: mux: pca954x: convert to using a pointer in i2c_device_id
2023-08-14 21:52 ` [PATCH RFC 3/4] i2c: mux: pca954x: " Dmitry Torokhov
@ 2023-08-15 15:51 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2023-08-15 15:51 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Wolfram Sang, Andi Shyti, Biju Das, Jonathan Cameron,
Michael Hennerich, Peter Rosin, linux-i2c, linux-kernel
On Mon, Aug 14, 2023 at 02:52:51PM -0700, Dmitry Torokhov wrote:
> Switch the driver to use newly added "data" pointer in i2c_device_id to
> streamline DT and legacy flows.
...
> @@ -206,7 +206,7 @@ static const struct of_device_id pca954x_of_match[] = {
> { .compatible = "nxp,pca9847", .data = &chips[pca_9847] },
> { .compatible = "nxp,pca9848", .data = &chips[pca_9848] },
> { .compatible = "nxp,pca9849", .data = &chips[pca_9849] },
> - {}
> + { }
> };
> MODULE_DEVICE_TABLE(of, pca954x_of_match);
Stray change.
...
> data->chip = device_get_match_data(dev);
> if (!data->chip)
> - data->chip = &chips[id->driver_data];
> + data->chip = id->data;
Same, why not to use that helper and if anything, modify it instead.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 4/4] i2c: slave-eeprom: convert to using a pointer in i2c_device_id
2023-08-14 21:52 ` [PATCH RFC 4/4] i2c: slave-eeprom: " Dmitry Torokhov
@ 2023-08-15 15:53 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2023-08-15 15:53 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Wolfram Sang, Andi Shyti, Biju Das, Jonathan Cameron,
Michael Hennerich, Peter Rosin, linux-i2c, linux-kernel
On Mon, Aug 14, 2023 at 02:52:52PM -0700, Dmitry Torokhov wrote:
> Switch the driver to use newly added "data" pointer in i2c_device_id and
> introduce struct eeprom_chip to describe chip's characteristics instead
> of cramming it all into an unisigned long and then decipher.
...
> - const struct i2c_device_id *id = i2c_client_get_device_id(client);
> + const struct i2c_device_id *id;
> + id = i2c_client_get_device_id(client);
> + if (!id)
> + return -EINVAL;
Unneeded piece of change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 1/4] i2c: start migrating to a pointer in i2c_device_id
2023-08-14 21:52 ` [PATCH RFC 1/4] i2c: start migrating to a pointer in i2c_device_id Dmitry Torokhov
@ 2023-08-15 15:55 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2023-08-15 15:55 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Wolfram Sang, Andi Shyti, Biju Das, Jonathan Cameron,
Michael Hennerich, Peter Rosin, linux-i2c, linux-kernel
On Mon, Aug 14, 2023 at 02:52:49PM -0700, Dmitry Torokhov wrote:
> The of_device_id structure uses a void pointer to associate additional
> driver-private data with device id, most commonly used to refer to a
> structure containing additional characteristics of a particular chip.
> However i2c_device_id uses an unsigned long. While it can easily be
> converted to a pointer, several drivers use it to store a scalar (and it
> is possible to use a pointer in of_device_id to store a scalar value as
> well). The worst case comes when OF part of a driver uses pointers,
> while legacy part is using scalars, causing constructs like:
>
> data = device_get_match_data(...);
> if (!data)
> data = &some_array[i2c_match_id(...)->data];
> ...
>
> To avoid this introduce a const void "data" pointer to i2c_device_id as
> well, so that we can convert drivers one by one, and drop driver_data
> member in the end.
>
> The end goal is to clean up all helpers and make device_get_match_data()
> work universally for all ACPI, DT, and legacy variants.
So, we have in the parallel the activity to make buses to have a callback,
why do we need this one? Looks to me as yet another 1000+ churn for not
much value. What the good outcome of this is constification, but maybe
we can find the way on how to prove const to stay over the kernel_ulong_t
transformations for all device ID tables?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-08-15 15:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-14 21:52 [PATCH RFC 0/4] Start unification of of_device_id and i2c_device_id Dmitry Torokhov
2023-08-14 21:52 ` [PATCH RFC 1/4] i2c: start migrating to a pointer in i2c_device_id Dmitry Torokhov
2023-08-15 15:55 ` Andy Shevchenko
2023-08-14 21:52 ` [PATCH RFC 2/4] i2c: mux: ltc4306: convert to using " Dmitry Torokhov
2023-08-15 15:50 ` Andy Shevchenko
2023-08-14 21:52 ` [PATCH RFC 3/4] i2c: mux: pca954x: " Dmitry Torokhov
2023-08-15 15:51 ` Andy Shevchenko
2023-08-14 21:52 ` [PATCH RFC 4/4] i2c: slave-eeprom: " Dmitry Torokhov
2023-08-15 15:53 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).