Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1399478

Re: [PATCH] cocci: Provide script to find i2c_tables missing exports

Show all headers | View raw

Hello Kieran,

Thanks for the patch, it would be great to automate this to avoid new
drivers to introduce new issues!

On 05/10/2016 11:07 AM, Kieran Bingham wrote:
> Adds MODULE_DEVICE_TABLE(i2c, ...) to correctly export tables in i2c drivers
> ---
> Ahem, My appologies, I wrongly sent the patch I was working on for OF tables,
>

No worries and in fact we need to identify both (I2C && OF tables not exported).

There are currently 3 issues that have to be fixed on I2C drivers w.r.t to device
and driver match and module auto-loading:

1) Drivers with I2C table but not exported (match will work but autoload no).

2) Drivers with OF table but not exported (match will work but autoload no).

3) Drivers with an I2C table but with no OF table even when the I2C devices
   are registered via OF. Match and autoload works because the core always
   reports a MODALIAS=i2c:* for both but once we change it to reports the
   corret modalias, things will break since now will be MODALIAS=of:N*fT*C
   and the module won't have a modalias of that form.

1) and 2) are somehow easy and your semantic patches can be used to find
and fix them.

Now 3) is more tricky since is hard to say if a DT is relying on the fact
that the I2C core strips the manufacture from the compatible string and
just matches using the model part.

I think the best we can do is to check if there are such a users in the
in-tree DTS or documented compatible in the DT bindings documents. But if
someone is using with a DT that's not in mainline, then things will break. 

So if you can think of a way to automatize 3) using coccinelle, that will
be great. My coccinelle knowledge is near to non-existent but one issue I
see is that coccinelle context seems to be restricted to just the current
file so it can't check in external files like DTS or DT bindings docs.

> Of course this is the correct patch for adding the i2c_device_id exports,
> which I think is what you are currently looking at
> --
> Kieran
> 

[snip]

> +
> +// A1: Export it!
> +
> +@ add_mod_dev_table depends on !i2c_dev_table @
> +identifier dev_id.arr;
> +@@
> +struct i2c_device_id arr[] = { ... };
> ++ MODULE_DEVICE_TABLE(i2c, arr);
> 

A problem with this semantic patch is that is going to give a lot of
false positives. Not always a MODULE_DEVICE_TABLE(i2c,...) is needed
after a i2c_device_id array, drivers that can't be build as a module
(i.e: whose Kconfig is not tristate) or board files are two examples.

One way I think we can minimize this is by checking if the driver is
including the module.h header file or not. This still will give some
false positives, since many drivers that can't be build as module
wrongly include module.h but in any case that's something that needs
to be fixed in those drivers.

So I think you need something like the following change to squash with
your patch:

diff --git a/scripts/coccinelle/i2c/i2c_table_missing_export.cocci b/scripts/coccinelle/i2c/i2c_table_missing_export.cocci
index 58c06856e4d4..df98279e326d 100644
--- a/scripts/coccinelle/i2c/i2c_table_missing_export.cocci
+++ b/scripts/coccinelle/i2c/i2c_table_missing_export.cocci
@@ -4,9 +4,16 @@
 // Usage:
 // spatch --sp-file scripts/coccinelle/i2c/i2c_table_missing_export.cocci . --in-place
 
+// C0 : Check if module.h is included
+
+@ includes_module @
+@@
+
+# include <linux/module.h>
+
 // C1 : Identify the i2c_device_id array
 
-@ dev_id @
+@ dev_id depends on includes_module @
 identifier arr;
 @@
 struct i2c_device_id arr[] = { ... };

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCHv5 0/8] 2c: Relax mandatory I2C ID table passing Kieran Bingham <kieran@ksquared.org.uk> - 2016-05-04 17:20 +0200
  [PATCHv5 6/8] i2c: Provide a temporary .probe_new() call-back type Kieran Bingham <kieran@ksquared.org.uk> - 2016-05-04 17:20 +0200
  [PATCHv5 1/8] i2c: Add pointer dereference protection to i2c_match_id() Kieran Bingham <kieran@ksquared.org.uk> - 2016-05-04 17:20 +0200
    Re: [PATCHv5 1/8] i2c: Add pointer dereference protection to  i2c_match_id() Javier Martinez Canillas <javier@osg.samsung.com> - 2016-05-10 07:00 +0200
  [PATCHv5 3/8] i2c: Match using traditional OF methods, then by vendor-less compatible strings Kieran Bingham <kieran@ksquared.org.uk> - 2016-05-04 17:20 +0200
  [PATCHv5 7/8] mfd: 88pm860x: Move over to new I2C device .probe() call Kieran Bingham <kieran@ksquared.org.uk> - 2016-05-04 17:20 +0200
    Re: [PATCHv5 7/8] mfd: 88pm860x: Move over to new I2C device .probe()  call Javier Martinez Canillas <javier@osg.samsung.com> - 2016-05-10 07:10 +0200
  [PATCHv5 8/8] mfd: as3722: Rid driver of superfluous I2C device ID structure Kieran Bingham <kieran@ksquared.org.uk> - 2016-05-04 17:20 +0200
    Re: [PATCHv5 8/8] mfd: as3722: Rid driver of superfluous I2C device  ID structure Javier Martinez Canillas <javier@osg.samsung.com> - 2016-05-10 07:30 +0200
      Re: [PATCHv5 8/8] mfd: as3722: Rid driver of superfluous I2C device  ID structure Lee Jones <lee.jones@linaro.org> - 2016-05-10 09:40 +0200
        Re: [PATCHv5 8/8] mfd: as3722: Rid driver of superfluous I2C device  ID structure Javier Martinez Canillas <javier@osg.samsung.com> - 2016-05-10 15:30 +0200
          Re: [PATCHv5 8/8] mfd: as3722: Rid driver of superfluous I2C device  ID structure Lee Jones <lee.jones@linaro.org> - 2016-05-10 16:10 +0200
          [PATCH] cocci: Find i2c drivers with an of_device table that isn't exported Kieran Bingham <kieran@ksquared.org.uk> - 2016-05-10 16:40 +0200
          [PATCH] cocci: Provide script to find i2c_tables missing exports Kieran Bingham <kieran@ksquared.org.uk> - 2016-05-10 17:10 +0200
            Re: [PATCH] cocci: Provide script to find i2c_tables missing exports Javier Martinez Canillas <javier@osg.samsung.com> - 2016-05-11 22:10 +0200
  Re: [PATCHv5 0/8] 2c: Relax mandatory I2C ID table passing Lee Jones <lee.jones@linaro.org> - 2016-05-09 11:20 +0200
    Re: [PATCHv5 0/8] 2c: Relax mandatory I2C ID table passing Javier Martinez Canillas <javier@osg.samsung.com> - 2016-05-09 15:30 +0200
  Re: [PATCHv5 0/8] 2c: Relax mandatory I2C ID table passing Javier Martinez Canillas <javier@osg.samsung.com> - 2016-05-10 07:40 +0200
    Re: [PATCHv5 0/8] 2c: Relax mandatory I2C ID table passing Kieran Bingham <kieran@ksquared.org.uk> - 2016-05-10 09:50 +0200

csiph-web