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

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

[Dmitry Mastykin]

The simplest fix is to move to ->probe_new() since the driver is not
used out of DT/ACPI enumeration.

Signed-off-by: Dmitry Mastykin <mastichi@gmail.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

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

[Andy Shevchenko]

On Fri, Dec 29, 2017 at 3:59 PM, Dmitry Mastykin <mastichi@gmail.com> wrote:
> The simplest fix is to move to ->probe_new() since the driver is not
> used out of DT/ACPI enumeration.
>

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>

I suppose, and

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thanks!

> Signed-off-by: Dmitry Mastykin <mastichi@gmail.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
>



-- 
With Best Regards,
Andy Shevchenko

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

[Jonathan Cameron]

On Fri, 29 Dec 2017 17:30:39 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Fri, Dec 29, 2017 at 3:59 PM, Dmitry Mastykin <mastichi@gmail.com> wrote:
> > The simplest fix is to move to ->probe_new() since the driver is not
> > used out of DT/ACPI enumeration.
> >  
> 
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> I suppose, and
> 
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
I'm a little confused.  Why does this now make module autoloading
work where it previously didn't?

Jonathan

> 
> Thanks!
> 
> > Signed-off-by: Dmitry Mastykin <mastichi@gmail.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
> >  
> 
> 
> 

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

[Dmitry Mastykin]

> On Fri, 29 Dec 2017 17:30:39 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.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@kernel.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.

Kind regards,
Dmitry

>> > Signed-off-by: Dmitry Mastykin <mastichi@gmail.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
>> >

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

[Jonathan Cameron]

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

> > On Fri, 29 Dec 2017 17:30:39 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:  
> >> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.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@kernel.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@gmail.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
> >> >  

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

[Dmitry Mastykin]

On Mon, Jan 1, 2018 at 12:53 PM, Jonathan Cameron <jic23@kernel.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.

Kind regards,
Dmitry

>> >> > Signed-off-by: Dmitry Mastykin <mastichi@gmail.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
>> >> >
>

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

[Jonathan Cameron]

On Wed, 3 Jan 2018 15:41:13 +0300
Dmitry Mastykin <mastichi@gmail.com> wrote:

> On Mon, Jan 1, 2018 at 12:53 PM, Jonathan Cameron <jic23@kernel.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@gmail.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
> >> >> >  
> >  

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

[Dmitry Mastykin]

On Sat, Jan 6, 2018 at 4:14 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> Are you using devicetree or some other method to instantiate the device?
> i.e. How does the kernel know it exists?
>
Yes, I'm using device-tree (in a way like in bindings txt). I have
also max1137 connected, which is loaded and probed well.
Thank you!
Dmitry

>> >> >> > Signed-off-by: Dmitry Mastykin <mastichi@gmail.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
>> >> >> >
>> >
>

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

[Jonathan Cameron]

On Sun, 7 Jan 2018 00:37:44 +0300
Dmitry Mastykin <mastichi@gmail.com> wrote:

> On Sat, Jan 6, 2018 at 4:14 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> > Are you using devicetree or some other method to instantiate the device?
> > i.e. How does the kernel know it exists?
> >  
> Yes, I'm using device-tree (in a way like in bindings txt). I have
> also max1137 connected, which is loaded and probed well.
> Thank you!

If you wouldn't mind, could you post the snippet of the tree for this
device.  I'm still not sure why it isn't probing, but it could be
that the manufacturer isn't specified, but just the part number?

Jonathan

> Dmitry
> 
> >> >> >> > Signed-off-by: Dmitry Mastykin <mastichi@gmail.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
> >> >> >> >  
> >> >  
> >  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Dmitry Mastykin]

On Sun, Jan 7, 2018 at 7:08 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> If you wouldn't mind, could you post the snippet of the tree for this
> device.  I'm still not sure why it isn't probing, but it could be
> that the manufacturer isn't specified, but just the part number?
>
&i2c0 {
    pinctrl-names = "default";
    pinctrl-0 = <&i2c0_pins_default>;
    status = "okay";
    cadc0: max9611_0_70@70 {
        status = "okay";
        compatible = "maxim,max9611";
        reg = <0x70>;

        shunt-resistor-micro-ohms = <500000>;
    };
};

Thank you!
Dmitry

>> >> >> >> > Signed-off-by: Dmitry Mastykin <mastichi@gmail.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
>> >> >> >> >
>> >> >
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

[Dmitry Mastykin]

Hello Johnatan,
May be the issue is the same as Javier Martinez Canillas wrote:
"What is not correct is to require OF-only drivers to have an I2C
device ID table just as a workaround to have their modules
auto-loading working."
Please see: http://lkml.iu.edu/hypermail/linux/kernel/1712.0/01779.html
So the problem is not in driver, but in auto-loading procedure.
Thank you!

Kind regards,
Dmitry
>>> >> >> >> > Signed-off-by: Dmitry Mastykin <mastichi@gmail.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
>>> >> >> >> >
>>> >> >
>>> >
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

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

[Javier Martinez Canillas]

Hello Jonathan,

On Mon, Jan 1, 2018 at 10:53 AM, Jonathan Cameron <jic23@kernel.org> wrote:

[snip]

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

Your understanding is correct. This change has nothing to do with
module autoloading.

The .probe_new callback is only used to avoid requiring an I2C device
ID table since the old .probe callback has a struct i2c_device_id as a
parameter and so requires a table even in OF (or ACPI) only drivers.

> Jonathan

Best regards,
Javier

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

[Javier Martinez Canillas]

Hello Dmitry,

On Wed, Jan 10, 2018 at 4:51 AM, Dmitry Mastykin <mastichi@gmail.com> wrote:
> Hello Johnatan,
> May be the issue is the same as Javier Martinez Canillas wrote:
> "What is not correct is to require OF-only drivers to have an I2C
> device ID table just as a workaround to have their modules
> auto-loading working."
> Please see: http://lkml.iu.edu/hypermail/linux/kernel/1712.0/01779.html
> So the problem is not in driver, but in auto-loading procedure.

I've posted [0] a fix for the I2C core some time ago. Could you please
give a try to that and maybe provide your {Tested,Reviewed}-by tag?

[0]: https://patchwork.kernel.org/patch/10089425/

Best regards,
Javier

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

[Jonathan Cameron]

On Wed, 10 Jan 2018 12:01:07 +0100
Javier Martinez Canillas <javier@dowhile0.org> wrote:

> Hello Jonathan,
> 
> On Mon, Jan 1, 2018 at 10:53 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> 
> [snip]
> 
> >
> > I may well be missing some subtle detail of course having spent only a few
> > minute looking at this!
> >  
> 
> Your understanding is correct. This change has nothing to do with
> module autoloading.
> 
> The .probe_new callback is only used to avoid requiring an I2C device
> ID table since the old .probe callback has a struct i2c_device_id as a
> parameter and so requires a table even in OF (or ACPI) only drivers.

OK, then let us revisit this patch once we have an answer on your one
fixing the autoprobing more generally.  Until we then we don't know
whether we need to add the i2c table in or not whether this suffices.

Jonathan

> 
> > Jonathan  
> 
> Best regards,
> Javier
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Javier Martinez Canillas]

On Sun, Jan 14, 2018 at 11:51 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> On Wed, 10 Jan 2018 12:01:07 +0100
> Javier Martinez Canillas <javier@dowhile0.org> wrote:
>
>> Hello Jonathan,
>>
>> On Mon, Jan 1, 2018 at 10:53 AM, Jonathan Cameron <jic23@kernel.org> wrote:
>>
>> [snip]
>>
>> >
>> > I may well be missing some subtle detail of course having spent only a few
>> > minute looking at this!
>> >
>>
>> Your understanding is correct. This change has nothing to do with
>> module autoloading.
>>
>> The .probe_new callback is only used to avoid requiring an I2C device
>> ID table since the old .probe callback has a struct i2c_device_id as a
>> parameter and so requires a table even in OF (or ACPI) only drivers.
>
> OK, then let us revisit this patch once we have an answer on your one
> fixing the autoprobing more generally.  Until we then we don't know
> whether we need to add the i2c table in or not whether this suffices.
>

That's correct. If the driver is for a device that's OF-only (e.g:
will never be used from a non-DT platform) then I don't think we
should add a I2C table.

Now, if Wolfram nacks my patch then we should add it as a workaround
to have auto-loading working.

> Jonathan
>

Best regards,
Javier