| Message ID | c9ee31c641a8612c49172123cd56407484e699d6.1444243358.git.hns@goldelico.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wednesday 07 October 2015 20:42:39 H. Nikolaus Schaller wrote: > Explicitly call request_module() in the wrapper so that the > bq27x00_battery driver is loded on demand, when compiled as module. > > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > --- > drivers/w1/slaves/w1_bq27000.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/w1/slaves/w1_bq27000.c > b/drivers/w1/slaves/w1_bq27000.c index caafb17..745bd6e 100644 > --- a/drivers/w1/slaves/w1_bq27000.c > +++ b/drivers/w1/slaves/w1_bq27000.c > @@ -49,6 +49,8 @@ static int w1_bq27000_add_slave(struct w1_slave > *sl) int ret; > struct platform_device *pdev; > > + request_module("bq27x00_battery"); /* load as module if needed */ > + > pdev = platform_device_alloc("bq27000-battery", -1); > if (!pdev) { > ret = -ENOMEM; Hi! Function platform_device_alloc would allocate kernel device and it should also load appropriate kernel driver (via platform:bq27000-battery alias). If that does not work, problem is somewhere else. Maybe depmod is not properly generated? Or bq27x00_batter.ko modules does not contain needed alias? Adding request_module() here is not proper solution. Real problem is somewhere else... Anyway there is series of patches for bq27x00_battery which should cleanup some problems, so maybe they also fix your problem.
Am 08.10.2015 um 00:26 schrieb Pali Rohár <pali.rohar@gmail.com>: > On Wednesday 07 October 2015 20:42:39 H. Nikolaus Schaller wrote: >> Explicitly call request_module() in the wrapper so that the >> bq27x00_battery driver is loded on demand, when compiled as module. >> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >> --- >> drivers/w1/slaves/w1_bq27000.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/w1/slaves/w1_bq27000.c >> b/drivers/w1/slaves/w1_bq27000.c index caafb17..745bd6e 100644 >> --- a/drivers/w1/slaves/w1_bq27000.c >> +++ b/drivers/w1/slaves/w1_bq27000.c >> @@ -49,6 +49,8 @@ static int w1_bq27000_add_slave(struct w1_slave >> *sl) int ret; >> struct platform_device *pdev; >> >> + request_module("bq27x00_battery"); /* load as module if needed */ >> + >> pdev = platform_device_alloc("bq27000-battery", -1); >> if (!pdev) { >> ret = -ENOMEM; > > Hi! Function platform_device_alloc would allocate kernel device and it > should also load appropriate kernel driver (via platform:bq27000-battery > alias). If that does not work, problem is somewhere else. Maybe depmod > is not properly generated? Or bq27x00_batter.ko modules does not contain > needed alias? alias exists: #ifdef CONFIG_BATTERY_BQ27X00_PLATFORM MODULE_ALIAS("platform:bq27000-battery"); #endif depmod entry exist: root@letux:~# modprobe -c | fgrep bq27 alias i2c:bq27000_battery bq27x00_battery alias i2c:bq27200 bq27x00_battery alias i2c:bq27425 bq27x00_battery alias i2c:bq27500 bq27x00_battery alias i2c:bq27510 bq27x00_battery alias i2c:bq27742 bq27x00_battery alias platform:bq27000_battery bq27x00_battery alias w1_family_0x01 w1_bq27000 And I have traced all calls to call_modprobe [ 9.969085] call_modprobe w1-family-0x01 [ 10.189727] call_modprobe bq27x00_battery [ 29.879333] call_modprobe net-pf-10 [ 30.530548] call_modprobe netdev-eth0 [ 30.548156] call_modprobe eth0 [ 30.565063] call_modprobe netdev-eth0 [ 30.581420] call_modprobe eth0 [ 31.301300] call_modprobe netdev-eth1 [ 31.316101] call_modprobe eth1 [ 32.008270] call_modprobe usbfunc:ecm [ 35.554748] call_modprobe net-pf-16-proto-9 [ 36.243164] call_modprobe net-pf-31 [ 36.614410] call_modprobe bt-proto-4 [ 73.888122] call_modprobe net-pf-16-proto-9 [ 73.921112] call_modprobe net-pf-16-proto-9 [ 77.153625] call_modprobe net-pf-16-proto-9 [ 77.188262] call_modprobe net-pf-16-proto-9 [ 77.342895] call_modprobe net-pf-16-proto-9 [ 77.389129] call_modprobe net-pf-16-proto-9 [ 77.419006] call_modprobe net-pf-16-proto-9 And there is only the line I have added. I.e. I can't find an automatic attempt to load the platform:bq27000-battery. It also doesn't help to change to platform_device_alloc("bq27000_battery", -1) So I think platform_device_alloc() does only what it tells: it allocates a device but does *not* try to load a kernel module. It only binds to a driver compiled into the kernel. AFAIR the I2C bus driver also calls request_module() at some stage after finding a client (i2c:bq27000_battery). > Adding request_module() here is not proper solution. Real problem is > somewhere else... Anyway there is series of patches for bq27x00_battery > which should cleanup some problems, so maybe they also fix your problem. Is there a tree where I can pull the patches to cross-check if they change something? BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 08 October 2015 09:26:26 H. Nikolaus Schaller wrote: > > Am 08.10.2015 um 00:26 schrieb Pali Rohár <pali.rohar@gmail.com>: > > > On Wednesday 07 October 2015 20:42:39 H. Nikolaus Schaller wrote: > >> Explicitly call request_module() in the wrapper so that the > >> bq27x00_battery driver is loded on demand, when compiled as module. > >> > >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > >> --- > >> drivers/w1/slaves/w1_bq27000.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/w1/slaves/w1_bq27000.c > >> b/drivers/w1/slaves/w1_bq27000.c index caafb17..745bd6e 100644 > >> --- a/drivers/w1/slaves/w1_bq27000.c > >> +++ b/drivers/w1/slaves/w1_bq27000.c > >> @@ -49,6 +49,8 @@ static int w1_bq27000_add_slave(struct w1_slave > >> *sl) int ret; > >> struct platform_device *pdev; > >> > >> + request_module("bq27x00_battery"); /* load as module if needed */ > >> + > >> pdev = platform_device_alloc("bq27000-battery", -1); > >> if (!pdev) { > >> ret = -ENOMEM; > > > > Hi! Function platform_device_alloc would allocate kernel device and it > > should also load appropriate kernel driver (via platform:bq27000-battery > > alias). If that does not work, problem is somewhere else. Maybe depmod > > is not properly generated? Or bq27x00_batter.ko modules does not contain > > needed alias? > At least driver is renaming in upstream kernel, so hardcoding driver name is wrong approach. It will be broken... If we show that request_module is really needed (but I believe not!) then please use alias instead. > alias exists: > > #ifdef CONFIG_BATTERY_BQ27X00_PLATFORM > MODULE_ALIAS("platform:bq27000-battery"); > #endif > > depmod entry exist: > > root@letux:~# modprobe -c | fgrep bq27 > alias i2c:bq27000_battery bq27x00_battery > alias i2c:bq27200 bq27x00_battery > alias i2c:bq27425 bq27x00_battery > alias i2c:bq27500 bq27x00_battery > alias i2c:bq27510 bq27x00_battery > alias i2c:bq27742 bq27x00_battery > alias platform:bq27000_battery bq27x00_battery > alias w1_family_0x01 w1_bq27000 > > And I have traced all calls to call_modprobe > > [ 9.969085] call_modprobe w1-family-0x01 > [ 10.189727] call_modprobe bq27x00_battery > [ 29.879333] call_modprobe net-pf-10 > [ 30.530548] call_modprobe netdev-eth0 > [ 30.548156] call_modprobe eth0 > [ 30.565063] call_modprobe netdev-eth0 > [ 30.581420] call_modprobe eth0 > [ 31.301300] call_modprobe netdev-eth1 > [ 31.316101] call_modprobe eth1 > [ 32.008270] call_modprobe usbfunc:ecm > [ 35.554748] call_modprobe net-pf-16-proto-9 > [ 36.243164] call_modprobe net-pf-31 > [ 36.614410] call_modprobe bt-proto-4 > [ 73.888122] call_modprobe net-pf-16-proto-9 > [ 73.921112] call_modprobe net-pf-16-proto-9 > [ 77.153625] call_modprobe net-pf-16-proto-9 > [ 77.188262] call_modprobe net-pf-16-proto-9 > [ 77.342895] call_modprobe net-pf-16-proto-9 > [ 77.389129] call_modprobe net-pf-16-proto-9 > [ 77.419006] call_modprobe net-pf-16-proto-9 > > And there is only the line I have added. I.e. I can't find an automatic attempt > to load the platform:bq27000-battery. It also doesn't help to change to > platform_device_alloc("bq27000_battery", -1) > > So I think platform_device_alloc() does only what it tells: it allocates a device > but does *not* try to load a kernel module. It only binds to a driver compiled > into the kernel. AFAIR the I2C bus driver also calls request_module() at some > stage after finding a client (i2c:bq27000_battery). > Try to call platform_device_register(). E.g for rx51-battery power supply device it is working fine. Device itself is allocated and registered in arch/arm/mach-omap2/board-rx51-peripherals.c and driver is in drivers/power/rx51_battery.c. You can also try to reuse that code... I see that w1 is quite different. > > Adding request_module() here is not proper solution. Real problem is > > somewhere else... Anyway there is series of patches for bq27x00_battery > > which should cleanup some problems, so maybe they also fix your problem. > > Is there a tree where I can pull the patches to cross-check if they change > something? > Sebastian? Where are those new bq27k patches now? > BR, > Nikolaus >
Hi, Am 08.10.2015 um 09:54 schrieb Pali Rohár <pali.rohar@gmail.com>: > On Thursday 08 October 2015 09:26:26 H. Nikolaus Schaller wrote: >> >> Am 08.10.2015 um 00:26 schrieb Pali Rohár <pali.rohar@gmail.com>: >> >>> On Wednesday 07 October 2015 20:42:39 H. Nikolaus Schaller wrote: >>>> Explicitly call request_module() in the wrapper so that the >>>> bq27x00_battery driver is loded on demand, when compiled as module. >>>> >>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >>>> --- >>>> drivers/w1/slaves/w1_bq27000.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/w1/slaves/w1_bq27000.c >>>> b/drivers/w1/slaves/w1_bq27000.c index caafb17..745bd6e 100644 >>>> --- a/drivers/w1/slaves/w1_bq27000.c >>>> +++ b/drivers/w1/slaves/w1_bq27000.c >>>> @@ -49,6 +49,8 @@ static int w1_bq27000_add_slave(struct w1_slave >>>> *sl) int ret; >>>> struct platform_device *pdev; >>>> >>>> + request_module("bq27x00_battery"); /* load as module if needed */ >>>> + >>>> pdev = platform_device_alloc("bq27000-battery", -1); >>>> if (!pdev) { >>>> ret = -ENOMEM; >>> >>> Hi! Function platform_device_alloc would allocate kernel device and it >>> should also load appropriate kernel driver (via platform:bq27000-battery >>> alias). If that does not work, problem is somewhere else. Maybe depmod >>> is not properly generated? Or bq27x00_batter.ko modules does not contain >>> needed alias? >> > > At least driver is renaming in upstream kernel, so hardcoding driver > name is wrong approach. It will be broken... If we show that > request_module is really needed (but I believe not!) then please use > alias instead. > >> alias exists: >> >> #ifdef CONFIG_BATTERY_BQ27X00_PLATFORM >> MODULE_ALIAS("platform:bq27000-battery"); >> #endif >> >> depmod entry exist: >> >> root@letux:~# modprobe -c | fgrep bq27 >> alias i2c:bq27000_battery bq27x00_battery >> alias i2c:bq27200 bq27x00_battery >> alias i2c:bq27425 bq27x00_battery >> alias i2c:bq27500 bq27x00_battery >> alias i2c:bq27510 bq27x00_battery >> alias i2c:bq27742 bq27x00_battery >> alias platform:bq27000_battery bq27x00_battery >> alias w1_family_0x01 w1_bq27000 >> >> And I have traced all calls to call_modprobe >> >> [ 9.969085] call_modprobe w1-family-0x01 >> [ 10.189727] call_modprobe bq27x00_battery I have tried without request_module("bq27x00_battery") and this call to call_modprobe() disappears. But it still works, i.e. the module appears in lsmod! I am a bit puzzled now... What could have happened: * there was no automatic module load when we developed the patch (a while ago - it was sleeping in our development tree) * when automatic load was improved somewhere else, we did not notice (because a second request_module() does not have a visible side-effects) * so we just though that this is still required and did not check that it isn't any more So as it appears to work, we can simply drop it from the patch set (and our development tree) v2. Sorry for the noise and thanks for helping to find it. >> [ 29.879333] call_modprobe net-pf-10 >> [ 30.530548] call_modprobe netdev-eth0 >> [ 30.548156] call_modprobe eth0 >> [ 30.565063] call_modprobe netdev-eth0 >> [ 30.581420] call_modprobe eth0 >> [ 31.301300] call_modprobe netdev-eth1 >> [ 31.316101] call_modprobe eth1 >> [ 32.008270] call_modprobe usbfunc:ecm >> [ 35.554748] call_modprobe net-pf-16-proto-9 >> [ 36.243164] call_modprobe net-pf-31 >> [ 36.614410] call_modprobe bt-proto-4 >> [ 73.888122] call_modprobe net-pf-16-proto-9 >> [ 73.921112] call_modprobe net-pf-16-proto-9 >> [ 77.153625] call_modprobe net-pf-16-proto-9 >> [ 77.188262] call_modprobe net-pf-16-proto-9 >> [ 77.342895] call_modprobe net-pf-16-proto-9 >> [ 77.389129] call_modprobe net-pf-16-proto-9 >> [ 77.419006] call_modprobe net-pf-16-proto-9 >> >> And there is only the line I have added. I.e. I can't find an automatic attempt >> to load the platform:bq27000-battery. It also doesn't help to change to >> platform_device_alloc("bq27000_battery", -1) >> >> So I think platform_device_alloc() does only what it tells: it allocates a device >> but does *not* try to load a kernel module. It only binds to a driver compiled >> into the kernel. AFAIR the I2C bus driver also calls request_module() at some >> stage after finding a client (i2c:bq27000_battery). >> > > Try to call platform_device_register(). E.g for rx51-battery power > supply device it is working fine. Device itself is allocated and > registered in arch/arm/mach-omap2/board-rx51-peripherals.c and driver is > in drivers/power/rx51_battery.c. You can also try to reuse that code... > I see that w1 is quite different. > >>> Adding request_module() here is not proper solution. Real problem is >>> somewhere else... Anyway there is series of patches for bq27x00_battery >>> which should cleanup some problems, so maybe they also fix your problem. >> >> Is there a tree where I can pull the patches to cross-check if they change >> something? >> > > Sebastian? Where are those new bq27k patches now? > >> BR, >> Nikolaus >> > > -- > Pali Rohár > pali.rohar@gmail.com BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-pm" 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/w1/slaves/w1_bq27000.c b/drivers/w1/slaves/w1_bq27000.c index caafb17..745bd6e 100644 --- a/drivers/w1/slaves/w1_bq27000.c +++ b/drivers/w1/slaves/w1_bq27000.c @@ -49,6 +49,8 @@ static int w1_bq27000_add_slave(struct w1_slave *sl) int ret; struct platform_device *pdev; + request_module("bq27x00_battery"); /* load as module if needed */ + pdev = platform_device_alloc("bq27000-battery", -1); if (!pdev) { ret = -ENOMEM;
Explicitly call request_module() in the wrapper so that the bq27x00_battery driver is loded on demand, when compiled as module. Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> --- drivers/w1/slaves/w1_bq27000.c | 2 ++ 1 file changed, 2 insertions(+)