Re: [bug report] i2c: imx: Make sure to unregister adapter on remove()

[Dan Carpenter <dan.carpenter@oracle.com>]

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