From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753523AbbHCOaE (ORCPT ); Mon, 3 Aug 2015 10:30:04 -0400 Received: from lists.s-osg.org ([54.187.51.154]:55764 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752407AbbHCOaB (ORCPT ); Mon, 3 Aug 2015 10:30:01 -0400 Subject: Re: [PATCH 15/27] regulator: fan53555: Export I2C module alias information To: Paul Bolle References: <1438273132-20926-1-git-send-email-javier@osg.samsung.com> <1438273132-20926-16-git-send-email-javier@osg.samsung.com> <1438602218.3726.45.camel@tiscali.nl> Cc: Liam Girdwood , Mark Brown , linux-kernel@vger.kernel.org From: Javier Martinez Canillas X-Enigmail-Draft-Status: N1110 Message-ID: <55BF7ADF.3070801@osg.samsung.com> Date: Mon, 3 Aug 2015 16:29:51 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 MIME-Version: 1.0 In-Reply-To: <1438602218.3726.45.camel@tiscali.nl> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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:> 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