On 03.05.2018 14:13, Harini Katakam wrote: > Hi Claudiu, > > On Thu, May 3, 2018 at 3:39 PM, Claudiu Beznea > <claudiu.bez...@microchip.com> wrote: >> >> >> On 22.03.2018 15:51, harinikatakamli...@gmail.com wrote: >>> From: Harini Katakam <hari...@xilinx.com> > <snip> >> I would use a "goto" instruction, e.g.: >> value = -ETIMEDOUT; >> goto out; >> > > Will do > >> >> Below, in macb_open() you have a return err; case: >> err = macb_alloc_consistent(bp); >> if (err) { >> netdev_err(dev, "Unable to allocate DMA memory (error %d)\n", >> err); >> return err; >> } >> >> You have to undo pm_runtime_get_sync() with pm_runtime_put_sync() or >> something >> similar to decrement dev->power.usage_count. > > Will do > > <snip> >>> @@ -4104,11 +4145,16 @@ static int macb_remove(struct platform_device *pdev) >>> mdiobus_free(bp->mii_bus); >>> >>> unregister_netdev(dev); >>> - clk_disable_unprepare(bp->tx_clk); >>> - clk_disable_unprepare(bp->hclk); >>> - clk_disable_unprepare(bp->pclk); >>> - clk_disable_unprepare(bp->rx_clk); >>> - clk_disable_unprepare(bp->tsu_clk); >>> + pm_runtime_disable(&pdev->dev); >>> + pm_runtime_dont_use_autosuspend(&pdev->dev); >>> + if (!pm_runtime_suspended(&pdev->dev)) { >>> + clk_disable_unprepare(bp->tx_clk); >>> + clk_disable_unprepare(bp->hclk); >>> + clk_disable_unprepare(bp->pclk); >>> + clk_disable_unprepare(bp->rx_clk); >>> + clk_disable_unprepare(bp->tsu_clk); >>> + pm_runtime_set_suspended(&pdev->dev); >> >> This is driver remove function. Shouldn't clocks be removed? > > clk_disable_unprepare IS being done here. > The check for !pm_runtime_suspended is just to make sure the > clocks are not already removed (in runtime_suspend).
Could this happen? Looking over code, starting with platform_driver_unregister() it looks to me that the runtime resume is called just before driver remove is called. platform_driver_unregister() -> driver_unregister() -> bus_remove_driver() -> driver_detach() -> device_release_driver_internal() -> __device_release_driver() { // ... pm_runtime_get_sync(dev); // runtime resume pm_runtime_clean_up_links(dev); // ... pm_runtime_put_sync(dev); if (dev->bus && dev->bus->remove) dev->bus->remove(dev); else if (drv->remove) drv->remove(dev); // ... } Thank you, Claudiu > > Regards, > Harini >