Hello Paul, Thanks a lot for the feedback.
On 08/03/2015 01:43 PM, Paul Bolle wrote: > Hi Javier, > > (Mark already applied this patch. Still, I couldn't wrap my head around > it. So maybe you'd still like to answer a question or two, basically to > educate me.) > > On do, 2015-07-30 at 18:18 +0200, Javier Martinez Canillas wrote: >> The I2C core always reports the MODALIAS uevent as "i2c:<client name" >> regardless if the driver was matched using the I2C id_table or the >> of_match_table. > > It's the other way round, I think. > > Both MODULE_DEVICE_TABLE() macros add a set of aliases to this module. > These aliases are used by userspace to load the fan53555 module if it > notices a uevent that contains the proper "MODALIAS=" string. Only after > this module is loaded by userspace will the I2C id_table and the > of_match_table be available to match this driver to the hardware found You are right on that but... > in the machine and, if matching hardware is found, call > fan53555_regulator_probe() to get this module to actually do something. > ...I2C is a little special in that it uses the id_table again to match in i2c_device_probe() and pass a i2c_device_id to the I2C driver's probe function. That is what I meant by matching but maybe I could had been more precise. > That being said, before this patch the fan53555 module contained these > aliases: > alias: of:N*T*Csilergy,syr828* > alias: of:N*T*Csilergy,syr827* > alias: of:N*T*Cfcs,fan53555* > > While this patch ad these two aliases: > alias: i2c:syr82x > alias: i2c:fan53555 > > Now I don't have an "of" or "i2c" capable machine at hand, which makes > it a bit hard to figure out how all of this is supposed to fit together. > But I'm guessing that parsing a device tree blob that contains strings > like > compatible = "silergy,syr828" > > would add strings like > MODALIAS=of:N[...]T[...]Csilergy,syr828 > That would be the correct behavior and is what the RFC patch #27 does. > to the related uevents. (Likewise for the two other aliases.) Doesn't > that happen here? > No, that is exactly the problem. Take a look to i2c_device_uevent() [0], it just does: add_uevent_var(env, "MODALIAS=%s%s", I2C_MODULE_PREFIX, client->name)) So if you have a i2c_device_id table but no MODULE_DEVICE_TABLE(i2c,...), then module autoload won't work. >> So the driver needs to export the I2C table and this >> be built into the module or udev won't have the necessary information >> to auto load the correct module when the device is added. > > s/the correct module/this module/, right? > I don't think it's a big difference in semantics but probably yes. >> --- a/drivers/regulator/fan53555.c >> +++ b/drivers/regulator/fan53555.c > >> +MODULE_DEVICE_TABLE(i2c, fan53555_id); > > As I said above this patch adds two aliases to the fan53555 module: > alias: i2c:syr82x > alias: i2c:fan53555 > > But neither the string "fan53555" nor the string "syr82x" generate > interesting hits in current linux-next. Are these strings perhaps only > used in out of tree device tree source files? > There are a lot of drivers whose platform code has not been upstreamed so I haven't checked which ones are used an which ones are not. But if someone is using this driver and registering a I2C device either via board code or a Device Tree, it will break as soon as someone builds it as a module without this patch. > Thanks, > > > Paul Bolle > [0]: http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L483 Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/