| Message ID | 1448046582-20231-1-git-send-email-javier@osg.samsung.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Nov 20, 2015 at 04:09:42PM -0300, Javier Martinez Canillas wrote: > The Atmel maxtouch DT binding documents that the compatible string for > the device is "atmel,maxtouch" and the I2C core always reports a module > alias of the form i2c:alias where alias is the compatible string model: > > $ grep MODALIAS /sys/devices/platform/12e00000.i2c/i2c-8/8-004b/uevent > MODALIAS=i2c:maxtouch Do we know why we are not reporting modalias as "of:..." so that autoloading using the of table works properly? It sounds like the of matching and modalias is quite a mess at the moment. Thanks.
Hello Dmitry, On 11/20/2015 04:15 PM, Dmitry Torokhov wrote: > On Fri, Nov 20, 2015 at 04:09:42PM -0300, Javier Martinez Canillas wrote: >> The Atmel maxtouch DT binding documents that the compatible string for >> the device is "atmel,maxtouch" and the I2C core always reports a module >> alias of the form i2c:alias where alias is the compatible string model: >> >> $ grep MODALIAS /sys/devices/platform/12e00000.i2c/i2c-8/8-004b/uevent >> MODALIAS=i2c:maxtouch > > Do we know why we are not reporting modalias as "of:..." so that > autoloading using the of table works properly? > Yes we do but is not that easy to fix without risking breaking autload in a lot of drivers that are relying on the current behavior. I posted a patch series to fix the OF and I2C tables for all the drivers that could find in mainline and in the cover letter I explain the problem in detail: https://lkml.org/lkml/2015/7/30/519 But is not complete because the .driver_data in i2c_device_id is an kernel_ulong_t while the .data in of_device_id is a const void * so some casting will be needed to add an OF table in some drivers that still lack one and I wanted to discuss that a little bit to see if the I2C core could return a common data field in both cases. > It sounds like the of matching and modalias is quite a mess at the > moment. > Agreed, I2C is specially complicated because not only OF drivers need a I2C table for module autoloading but also the core uses the I2C table to match devices and pass a struct i2c_device_id to their probe function. Now this requirement has become more relaxed since Lee's series mentioned in my cover letter were merged but still most I2C drivers use an I2C table for this legacy reason. So there isn't too much gain in cleaning up the modalias reporting if most drivers still need an I2C table anyways. > Thanks. > Best regards,
On 11/20/2015 04:32 PM, Javier Martinez Canillas wrote: [snip] > > But is not complete because the .driver_data in i2c_device_id is an > kernel_ulong_t while the .data in of_device_id is a const void * so > some casting will be needed to add an OF table in some drivers that Some casting in the tables *and* some logic to get the .data from the OF table entries, so something like the following will be needed: static int foo_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { struct of_device_id *match; kernel_ulong_t data; if (i2c->dev.of_node) { match = of_match_node(of_match, i2c->dev.of_node); if (!match) return -EINVAL; data = (kernel_ulong_t)match->data; } else { data = id->driver_data; } ... } while currently I2C drivers just rely on the model part of the compatible string to match with the entry in the I2C device ID table and the core always passing the correct .driver_data to the probe function. So as you can see, converting all the drivers to not rely on the I2C table requires some refactoring before proper OF modalias reporting can be used. Best regards,
Hello Dmitry, On 11/20/2015 04:46 PM, Javier Martinez Canillas wrote: > On 11/20/2015 04:32 PM, Javier Martinez Canillas wrote: > > [snip] > >> >> But is not complete because the .driver_data in i2c_device_id is an >> kernel_ulong_t while the .data in of_device_id is a const void * so >> some casting will be needed to add an OF table in some drivers that > > Some casting in the tables *and* some logic to get the .data from the > OF table entries, so something like the following will be needed: > > static int foo_i2c_probe(struct i2c_client *i2c, > const struct i2c_device_id *id) > { > struct of_device_id *match; > kernel_ulong_t data; > > if (i2c->dev.of_node) { > match = of_match_node(of_match, i2c->dev.of_node); > if (!match) > return -EINVAL; > > data = (kernel_ulong_t)match->data; > } else { > data = id->driver_data; > } > ... > } > > while currently I2C drivers just rely on the model part of the compatible > string to match with the entry in the I2C device ID table and the core > always passing the correct .driver_data to the probe function. > > So as you can see, converting all the drivers to not rely on the I2C table > requires some refactoring before proper OF modalias reporting can be used. > Any comments about this? I'm planning to address all this at some point but for now I think we would need $SUBJECT to have module autoloading working on this driver when the device is registered via OF. > Best regards, > Best regards,
Hello Dmitry, On 11/20/2015 04:15 PM, Dmitry Torokhov wrote: > On Fri, Nov 20, 2015 at 04:09:42PM -0300, Javier Martinez Canillas wrote: >> The Atmel maxtouch DT binding documents that the compatible string for >> the device is "atmel,maxtouch" and the I2C core always reports a module >> alias of the form i2c:alias where alias is the compatible string model: >> >> $ grep MODALIAS /sys/devices/platform/12e00000.i2c/i2c-8/8-004b/uevent >> MODALIAS=i2c:maxtouch > > Do we know why we are not reporting modalias as "of:..." so that > autoloading using the of table works properly? > > It sounds like the of matching and modalias is quite a mess at the > moment. > I already explained the issues with OF modalias and why I believe $SUBJECT is the least bad solution for now until the I2C subsystem is changed to properly report OF modalias. Please let me know if you are considering applying this or if module autoloading will be broken for this driver in the meantime. > Thanks. > Best regards,
Hi Javier, On Thu, Dec 10, 2015 at 08:07:47PM -0300, Javier Martinez Canillas wrote: > Hello Dmitry, > > On 11/20/2015 04:15 PM, Dmitry Torokhov wrote: > > On Fri, Nov 20, 2015 at 04:09:42PM -0300, Javier Martinez Canillas wrote: > >> The Atmel maxtouch DT binding documents that the compatible string for > >> the device is "atmel,maxtouch" and the I2C core always reports a module > >> alias of the form i2c:alias where alias is the compatible string model: > >> > >> $ grep MODALIAS /sys/devices/platform/12e00000.i2c/i2c-8/8-004b/uevent > >> MODALIAS=i2c:maxtouch > > > > Do we know why we are not reporting modalias as "of:..." so that > > autoloading using the of table works properly? > > > > It sounds like the of matching and modalias is quite a mess at the > > moment. > > > > I already explained the issues with OF modalias and why I believe > $SUBJECT is the least bad solution for now until the I2C subsystem > is changed to properly report OF modalias. > > Please let me know if you are considering applying this or if > module autoloading will be broken for this driver in the meantime. My bad, I was going to apply it several times but then my attention would wander to some other patchset and I'd forget. Applied now.
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index c5622058c22b..b6e53b97244c 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -2701,6 +2701,7 @@ static const struct i2c_device_id mxt_id[] = { { "qt602240_ts", 0 }, { "atmel_mxt_ts", 0 }, { "atmel_mxt_tp", 0 }, + { "maxtouch", 0 }, { "mXT224", 0 }, { } };
The Atmel maxtouch DT binding documents that the compatible string for the device is "atmel,maxtouch" and the I2C core always reports a module alias of the form i2c:alias where alias is the compatible string model: $ grep MODALIAS /sys/devices/platform/12e00000.i2c/i2c-8/8-004b/uevent MODALIAS=i2c:maxtouch But there isn't maxtouch entry in the I2C device ID table so when the i2c:maxtouch MODALIAS uevent is reported, kmod is not able to match the alias with a module to load: $ modinfo atmel_mxt_ts | grep alias alias: of:N*T*Catmel,maxtouch alias: i2c:mXT224 alias: i2c:atmel_mxt_tp alias: i2c:atmel_mxt_ts alias: i2c:qt602240_ts So add the maxtouch entry to the I2C device ID table to allow the module to be autoloaded when the device is registered via OF. Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> --- Changes in v2: - Add the commands to the message that were removed by git commit since I used '#' as a commmand prompt and were taken as comments. Sorry for the noise. drivers/input/touchscreen/atmel_mxt_ts.c | 1 + 1 file changed, 1 insertion(+)