* [bug report] i2c: imx: Make sure to unregister adapter on remove()
@ 2022-08-25 13:28 Dan Carpenter
2022-09-12 13:24 ` Uwe Kleine-König
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2022-08-25 13:28 UTC (permalink / raw)
To: u.kleine-koenig; +Cc: NXP Linux Team, linux-i2c
Hello Uwe Kleine-König,
The patch d98bdd3a5b50: "i2c: imx: Make sure to unregister adapter on
remove()" from Jul 20, 2022, leads to the following Smatch static
checker warning:
drivers/i2c/busses/i2c-imx.c:1586 i2c_imx_remove()
warn: pm_runtime_get_sync() also returns 1 on success
drivers/i2c/busses/i2c-imx.c
1570 static int i2c_imx_remove(struct platform_device *pdev)
1571 {
1572 struct imx_i2c_struct *i2c_imx = platform_get_drvdata(pdev);
1573 int irq, ret;
1574
1575 ret = pm_runtime_get_sync(&pdev->dev);
1576
1577 hrtimer_cancel(&i2c_imx->slave_timer);
1578
1579 /* remove adapter */
1580 dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n");
1581 i2c_del_adapter(&i2c_imx->adapter);
1582
1583 if (i2c_imx->dma)
1584 i2c_imx_dma_free(i2c_imx);
1585
--> 1586 if (ret == 0) {
Probably this should be ret >= 0?
1587 /* setup chip registers to defaults */
1588 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR);
1589 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR);
1590 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2CR);
1591 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR);
1592 clk_disable(i2c_imx->clk);
1593 }
1594
1595 clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb);
1596 irq = platform_get_irq(pdev, 0);
1597 if (irq >= 0)
1598 free_irq(irq, i2c_imx);
1599
1600 clk_unprepare(i2c_imx->clk);
1601
1602 pm_runtime_put_noidle(&pdev->dev);
1603 pm_runtime_disable(&pdev->dev);
1604
1605 return 0;
1606 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] i2c: imx: Make sure to unregister adapter on remove() 2022-08-25 13:28 [bug report] i2c: imx: Make sure to unregister adapter on remove() Dan Carpenter @ 2022-09-12 13:24 ` Uwe Kleine-König 2022-09-12 13:29 ` Dan Carpenter 0 siblings, 1 reply; 3+ messages in thread From: Uwe Kleine-König @ 2022-09-12 13:24 UTC (permalink / raw) To: Dan Carpenter; +Cc: NXP Linux Team, linux-i2c, kernel [-- Attachment #1: Type: text/plain, Size: 2564 bytes --] Hello Dan, On Thu, Aug 25, 2022 at 04:28:19PM +0300, Dan Carpenter wrote: > The patch d98bdd3a5b50: "i2c: imx: Make sure to unregister adapter on > remove()" from Jul 20, 2022, leads to the following Smatch static > checker warning: > > drivers/i2c/busses/i2c-imx.c:1586 i2c_imx_remove() > warn: pm_runtime_get_sync() also returns 1 on success > > drivers/i2c/busses/i2c-imx.c > 1570 static int i2c_imx_remove(struct platform_device *pdev) > 1571 { > 1572 struct imx_i2c_struct *i2c_imx = platform_get_drvdata(pdev); > 1573 int irq, ret; > 1574 > 1575 ret = pm_runtime_get_sync(&pdev->dev); > 1576 > 1577 hrtimer_cancel(&i2c_imx->slave_timer); > 1578 > 1579 /* remove adapter */ > 1580 dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n"); > 1581 i2c_del_adapter(&i2c_imx->adapter); > 1582 > 1583 if (i2c_imx->dma) > 1584 i2c_imx_dma_free(i2c_imx); > 1585 > --> 1586 if (ret == 0) { > > Probably this should be ret >= 0? > > 1587 /* setup chip registers to defaults */ > 1588 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR); > 1589 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR); > 1590 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2CR); > 1591 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR); > 1592 clk_disable(i2c_imx->clk); > 1593 } > 1594 > 1595 clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb); > 1596 irq = platform_get_irq(pdev, 0); > 1597 if (irq >= 0) > 1598 free_irq(irq, i2c_imx); > 1599 > 1600 clk_unprepare(i2c_imx->clk); > 1601 > 1602 pm_runtime_put_noidle(&pdev->dev); > 1603 pm_runtime_disable(&pdev->dev); > 1604 > 1605 return 0; > 1606 } I don't know how automatic you send these reports, but I wonder why you Cc:d the NXP Linux Team, but not Oleksij (i.e. the maintainer of the driver, who also Acked the blamed commit) and the Pengutronix Kernel team (which is included in the driver's MAINTAINER entry). Apart from that, I just sent a patch for that issue, thanks for your report. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] i2c: imx: Make sure to unregister adapter on remove() 2022-09-12 13:24 ` Uwe Kleine-König @ 2022-09-12 13:29 ` Dan Carpenter 0 siblings, 0 replies; 3+ messages in thread From: Dan Carpenter @ 2022-09-12 13:29 UTC (permalink / raw) To: Uwe Kleine-König; +Cc: NXP Linux Team, linux-i2c, kernel On Mon, Sep 12, 2022 at 03:24:28PM +0200, Uwe Kleine-König wrote: > Hello Dan, > > On Thu, Aug 25, 2022 at 04:28:19PM +0300, Dan Carpenter wrote: > > The patch d98bdd3a5b50: "i2c: imx: Make sure to unregister adapter on > > remove()" from Jul 20, 2022, leads to the following Smatch static > > checker warning: > > > > drivers/i2c/busses/i2c-imx.c:1586 i2c_imx_remove() > > warn: pm_runtime_get_sync() also returns 1 on success > > > > drivers/i2c/busses/i2c-imx.c > > 1570 static int i2c_imx_remove(struct platform_device *pdev) > > 1571 { > > 1572 struct imx_i2c_struct *i2c_imx = platform_get_drvdata(pdev); > > 1573 int irq, ret; > > 1574 > > 1575 ret = pm_runtime_get_sync(&pdev->dev); > > 1576 > > 1577 hrtimer_cancel(&i2c_imx->slave_timer); > > 1578 > > 1579 /* remove adapter */ > > 1580 dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n"); > > 1581 i2c_del_adapter(&i2c_imx->adapter); > > 1582 > > 1583 if (i2c_imx->dma) > > 1584 i2c_imx_dma_free(i2c_imx); > > 1585 > > --> 1586 if (ret == 0) { > > > > Probably this should be ret >= 0? > > > > 1587 /* setup chip registers to defaults */ > > 1588 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR); > > 1589 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR); > > 1590 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2CR); > > 1591 imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR); > > 1592 clk_disable(i2c_imx->clk); > > 1593 } > > 1594 > > 1595 clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb); > > 1596 irq = platform_get_irq(pdev, 0); > > 1597 if (irq >= 0) > > 1598 free_irq(irq, i2c_imx); > > 1599 > > 1600 clk_unprepare(i2c_imx->clk); > > 1601 > > 1602 pm_runtime_put_noidle(&pdev->dev); > > 1603 pm_runtime_disable(&pdev->dev); > > 1604 > > 1605 return 0; > > 1606 } > > I don't know how automatic you send these reports, but I wonder why you > Cc:d the NXP Linux Team, but not Oleksij (i.e. the maintainer of the > driver, who also Acked the blamed commit) and the Pengutronix Kernel > team (which is included in the driver's MAINTAINER entry). I try to only send it to the author and a lore.kernel.org list. I normally CC vendor lists like linux-imx@nxp.com as well because it's hard to know what those are. They might be the main devel mailing list? Anyway, if they aren't saved on lore, they're kind of useless. > > Apart from that, I just sent a patch for that issue, thanks for your > report. Thanks! regards, dan carpenter ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-12 13:29 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-08-25 13:28 [bug report] i2c: imx: Make sure to unregister adapter on remove() Dan Carpenter 2022-09-12 13:24 ` Uwe Kleine-König 2022-09-12 13:29 ` Dan Carpenter
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).