* [bug report] twl4030_charger: use runtime_pm to keep usb phy active while charging.
@ 2016-10-11 11:30 Dan Carpenter
2016-10-12 4:39 ` NeilBrown
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2016-10-11 11:30 UTC (permalink / raw)
To: neil; +Cc: linux-pm
Hello NeilBrown,
The patch e57c4a67d712: "twl4030_charger: use runtime_pm to keep usb
phy active while charging." from Jul 30, 2015, leads to the following
static checker warning:
drivers/power/supply/twl4030_charger.c:521 twl4030_charger_enable_usb()
error: 'bci->transceiver' dereferencing possible ERR_PTR()
drivers/power/supply/twl4030_charger.c
468 static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
469 {
470 int ret;
471
472 if (bci->usb_mode == CHARGE_OFF)
473 enable = false;
474 if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We assume that ->transceiver can be NULL.
475
476 twl4030_charger_update_current(bci);
477
478 /* Need to keep phy powered */
479 if (!bci->usb_enabled) {
480 pm_runtime_get_sync(bci->transceiver->dev);
481 bci->usb_enabled = 1;
482 }
483
484 if (bci->usb_mode == CHARGE_AUTO)
485 /* forcing the field BCIAUTOUSB (BOOT_BCI[1]) to 1 */
486 ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOUSB);
487
488 /* forcing USBFASTMCHG(BCIMFSTS4[2]) to 1 */
489 ret = twl4030_clear_set(TWL_MODULE_MAIN_CHARGE, 0,
490 TWL4030_USBFASTMCHG, TWL4030_BCIMFSTS4);
491 if (bci->usb_mode == CHARGE_LINEAR) {
492 twl4030_clear_set_boot_bci(TWL4030_BCIAUTOAC|TWL4030_CVENAC, 0);
493 /* Watch dog key: WOVF acknowledge */
494 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x33,
495 TWL4030_BCIWDKEY);
496 /* 0x24 + EKEY6: off mode */
497 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x2a,
498 TWL4030_BCIMDKEY);
499 /* EKEY2: Linear charge: USB path */
500 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x26,
501 TWL4030_BCIMDKEY);
502 /* WDKEY5: stop watchdog count */
503 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xf3,
504 TWL4030_BCIWDKEY);
505 /* enable MFEN3 access */
506 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x9c,
507 TWL4030_BCIMFKEY);
508 /* ICHGEOCEN - end-of-charge monitor (current < 80mA)
509 * (charging continues)
510 * ICHGLOWEN - current level monitor (charge continues)
511 * don't monitor over-current or heat save
512 */
513 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xf0,
514 TWL4030_BCIMFEN3);
515 }
516 } else {
517 ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOUSB, 0);
518 ret |= twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x2a,
519 TWL4030_BCIMDKEY);
520 if (bci->usb_enabled) {
521 pm_runtime_mark_last_busy(bci->transceiver->dev);
^^^^^^^^^^^^^^^^^^^^^
But we dereference it here. Possibly checking ->usb_enabled prevents
a crash?
522 pm_runtime_put_autosuspend(bci->transceiver->dev);
523 bci->usb_enabled = 0;
524 }
525 bci->usb_cur = 0;
526 }
527
528 return ret;
529 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] twl4030_charger: use runtime_pm to keep usb phy active while charging.
2016-10-11 11:30 [bug report] twl4030_charger: use runtime_pm to keep usb phy active while charging Dan Carpenter
@ 2016-10-12 4:39 ` NeilBrown
0 siblings, 0 replies; 2+ messages in thread
From: NeilBrown @ 2016-10-12 4:39 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-pm
[-- Attachment #1: Type: text/plain, Size: 4819 bytes --]
On Tue, Oct 11 2016, Dan Carpenter wrote:
> Hello NeilBrown,
>
> The patch e57c4a67d712: "twl4030_charger: use runtime_pm to keep usb
> phy active while charging." from Jul 30, 2015, leads to the following
> static checker warning:
>
> drivers/power/supply/twl4030_charger.c:521 twl4030_charger_enable_usb()
> error: 'bci->transceiver' dereferencing possible ERR_PTR()
>
> drivers/power/supply/twl4030_charger.c
> 468 static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
> 469 {
> 470 int ret;
> 471
> 472 if (bci->usb_mode == CHARGE_OFF)
> 473 enable = false;
> 474 if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> We assume that ->transceiver can be NULL.
>
> 475
> 476 twl4030_charger_update_current(bci);
> 477
> 478 /* Need to keep phy powered */
> 479 if (!bci->usb_enabled) {
> 480 pm_runtime_get_sync(bci->transceiver->dev);
> 481 bci->usb_enabled = 1;
This is the only time we set usb_enabled to 1, so bci->transceiver but
be non-NULL when usb_enabled.
> 482 }
> 483
> 484 if (bci->usb_mode == CHARGE_AUTO)
> 485 /* forcing the field BCIAUTOUSB (BOOT_BCI[1]) to 1 */
> 486 ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOUSB);
> 487
> 488 /* forcing USBFASTMCHG(BCIMFSTS4[2]) to 1 */
> 489 ret = twl4030_clear_set(TWL_MODULE_MAIN_CHARGE, 0,
> 490 TWL4030_USBFASTMCHG, TWL4030_BCIMFSTS4);
> 491 if (bci->usb_mode == CHARGE_LINEAR) {
> 492 twl4030_clear_set_boot_bci(TWL4030_BCIAUTOAC|TWL4030_CVENAC, 0);
> 493 /* Watch dog key: WOVF acknowledge */
> 494 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x33,
> 495 TWL4030_BCIWDKEY);
> 496 /* 0x24 + EKEY6: off mode */
> 497 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x2a,
> 498 TWL4030_BCIMDKEY);
> 499 /* EKEY2: Linear charge: USB path */
> 500 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x26,
> 501 TWL4030_BCIMDKEY);
> 502 /* WDKEY5: stop watchdog count */
> 503 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xf3,
> 504 TWL4030_BCIWDKEY);
> 505 /* enable MFEN3 access */
> 506 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x9c,
> 507 TWL4030_BCIMFKEY);
> 508 /* ICHGEOCEN - end-of-charge monitor (current < 80mA)
> 509 * (charging continues)
> 510 * ICHGLOWEN - current level monitor (charge continues)
> 511 * don't monitor over-current or heat save
> 512 */
> 513 ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xf0,
> 514 TWL4030_BCIMFEN3);
> 515 }
> 516 } else {
> 517 ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOUSB, 0);
> 518 ret |= twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x2a,
> 519 TWL4030_BCIMDKEY);
> 520 if (bci->usb_enabled) {
> 521 pm_runtime_mark_last_busy(bci->transceiver->dev);
> ^^^^^^^^^^^^^^^^^^^^^
> But we dereference it here. Possibly checking ->usb_enabled prevents
> a crash?
Yes it does (see above).
Can we get smatch to parse:
#invariant ! (IS_ERR_OR_NULL(bci->transceiver) && bci->usb_enabled)
validate it, and use it to validate other code ? :-)
Thanks,
NeilBrown
>
> 522 pm_runtime_put_autosuspend(bci->transceiver->dev);
> 523 bci->usb_enabled = 0;
> 524 }
> 525 bci->usb_cur = 0;
> 526 }
> 527
> 528 return ret;
> 529 }
>
> regards,
> dan carpenter
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-10-12 4:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-11 11:30 [bug report] twl4030_charger: use runtime_pm to keep usb phy active while charging Dan Carpenter
2016-10-12 4:39 ` NeilBrown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).