linux-kernel - Re: [PATCH v2] iio: adc: max9611: fix module auto-loading

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 1 Jan 2018 09:53:42 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Dmitry Mastykin <mastichi@...il.com>
Cc:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Jacopo Mondi <jacopo+renesas@...ndi.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Rob Herring <robh@...nel.org>, linux-iio@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] iio: adc: max9611: fix module auto-loading

On Sat, 30 Dec 2017 15:43:38 +0300
Dmitry Mastykin <mastichi@...il.com> wrote:

> > On Fri, 29 Dec 2017 17:30:39 +0200
> > Andy Shevchenko <andy.shevchenko@...il.com> wrote:  
> >> Suggested-by: Andy Shevchenko <andy.shevchenko@...il.com>
> >>  
> I was not aware of this tag, that's what I wanted to write, thanks!
> 
> On Fri, Dec 29, 2017 at 8:54 PM, Jonathan Cameron <jic23@...nel.org> wrote:
> > I'm a little confused.  Why does this now make module autoloading
> > work where it previously didn't?
> >  
> As I understand, .probe is not called if module doesn't export i2c id table,
> there is nothing to put into second argument.
> But .probe_new is called, because it's supposed to do of_match_device itself.
Not that I can see.  They are a straight swap in drivers/i2c/i2c-core-base.c
presumably intended to drop the parameter that is unused in the devicetree
and acpi cases.

This isn't an area I know that much about so I've just played follow the
code for a few minutes.

Module autoloading relies on the MODULE_DEVICE_TABLE macro.  This creates
an alias so the scripts/mod/file2alias.c file can work out what MODULE_ALIAS
lines need to be created.  This results in entries being added to section
.modinfo of the module which userspace can then read to work out which
module does what.

Here you already have that for the device tree case so this should already work
with or without this change.

So what circumstance did you have where this module did not autoprobe correctly?

I may well be missing some subtle detail of course having spent only a few
minute looking at this!

Jonathan

> 
> Kind regards,
> Dmitry
> 
> >> > Signed-off-by: Dmitry Mastykin <mastichi@...il.com>
> >> > ---
> >> >  drivers/iio/adc/max9611.c | 5 ++---
> >> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/iio/adc/max9611.c b/drivers/iio/adc/max9611.c
> >> > index b1dd17c..ce0115a 100644
> >> > --- a/drivers/iio/adc/max9611.c
> >> > +++ b/drivers/iio/adc/max9611.c
> >> > @@ -523,8 +523,7 @@ static const struct of_device_id max9611_of_table[] = {
> >> >  };
> >> >
> >> >  MODULE_DEVICE_TABLE(of, max9611_of_table);
> >> > -static int max9611_probe(struct i2c_client *client,
> >> > -                        const struct i2c_device_id *id)
> >> > +static int max9611_probe(struct i2c_client *client)
> >> >  {
> >> >         const char * const shunt_res_prop = "shunt-resistor-micro-ohms";
> >> >         const struct device_node *of_node = client->dev.of_node;
> >> > @@ -576,7 +575,7 @@ static struct i2c_driver max9611_driver = {
> >> >                    .owner = THIS_MODULE,
> >> >                    .of_match_table = max9611_of_table,
> >> >         },
> >> > -       .probe = max9611_probe,
> >> > +       .probe_new = max9611_probe,
> >> >  };
> >> >  module_i2c_driver(max9611_driver);
> >> >
> >> > --
> >> > 2.7.4
> >> >