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]

Message-ID: <20180106131434.550e8e42@archlinux>
Date:   Sat, 6 Jan 2018 13:14:34 +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 Wed, 3 Jan 2018 15:41:13 +0300
Dmitry Mastykin <mastichi@...il.com> wrote:

> On Mon, Jan 1, 2018 at 12:53 PM, Jonathan Cameron <jic23@...nel.org> wrote:
> 
> > 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  
> 
> Hi Johnatan,
> Thank you for the answer, I think you're right.
> I use the driver as module and one my colleague as built-in.
> With current driver module auto-load doesn't work, but built-in does
> auto-probe well.
> In patch v1 I added MODULE_DEVICE_TABLE(i2c,... and auto-load started to work.
> Then in patch v2 I removed the table and switched to probe_new.
> I think v2 worked only because I forgot to do depmod after replacing .ko-file.
> I will have chance to try it again after 2 weeks.
> May be better way for me is to build in the driver, because it seems
> strange to add the table
> to driver whereas people get rid of this table in other drivers.

Are you using devicetree or some other method to instantiate the device?
i.e. How does the kernel know it exists?
> 
> 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
> >> >> >  
> >