Message ID | 20230814-i2c-id-rework-v1-1-3e5bc71c49ee@gmail.com |
---|---|
State | RFC |
Headers | show |
Series | Start unification of of_device_id and i2c_device_id | expand |
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?
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 */
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(-)