Message ID | 1514556006-23293-1-git-send-email-mastichi@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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 >
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 > > > > > -- 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
> 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 >> > -- 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
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 > >> > -- 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
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 >> >> > > -- 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
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 > >> >> > > > -- 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
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 >> >> >> > >> > > -- 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
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 -- 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
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 > -- 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
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 >> -- 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
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 -- 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
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 -- 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
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 -- 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
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 -- 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
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);
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(-)