From patchwork Tue Mar 26 10:05:43 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sekhar Nori X-Patchwork-Id: 2335571 Return-Path: X-Original-To: patchwork-davinci@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from bear.ext.ti.com (bear.ext.ti.com [192.94.94.41]) by patchwork1.kernel.org (Postfix) with ESMTP id 488833FD40 for ; Tue, 26 Mar 2013 10:10:06 +0000 (UTC) Received: from dlelxv30.itg.ti.com ([172.17.2.17]) by bear.ext.ti.com (8.13.7/8.13.7) with ESMTP id r2QA5tJo025361; Tue, 26 Mar 2013 05:05:55 -0500 Received: from DFLE72.ent.ti.com (dfle72.ent.ti.com [128.247.5.109]) by dlelxv30.itg.ti.com (8.13.8/8.13.8) with ESMTP id r2QA5ssK012664; Tue, 26 Mar 2013 05:05:54 -0500 Received: from dlelxv24.itg.ti.com (172.17.1.199) by DFLE72.ent.ti.com (128.247.5.109) with Microsoft SMTP Server id 14.2.342.3; Tue, 26 Mar 2013 05:05:54 -0500 Received: from linux.omap.com (dlelxs01.itg.ti.com [157.170.227.31]) by dlelxv24.itg.ti.com (8.13.8/8.13.8) with ESMTP id r2QA5sPY016599; Tue, 26 Mar 2013 05:05:54 -0500 Received: from linux.omap.com (localhost [127.0.0.1]) by linux.omap.com (Postfix) with ESMTP id A5AD080627; Tue, 26 Mar 2013 04:05:54 -0600 (CST) X-Original-To: davinci-linux-open-source@linux.davincidsp.com Delivered-To: davinci-linux-open-source@linux.davincidsp.com Received: from dbdp20.itg.ti.com (dbdp20.itg.ti.com [172.24.170.38]) by linux.omap.com (Postfix) with ESMTP id BD32880626 for ; Tue, 26 Mar 2013 04:05:52 -0600 (CST) Received: from DBDE70.ent.ti.com (localhost [127.0.0.1]) by dbdp20.itg.ti.com (8.13.8/8.13.8) with ESMTP id r2QA5mlN008517; Tue, 26 Mar 2013 15:35:49 +0530 (IST) Received: from dbdp33.itg.ti.com (172.24.170.252) by dbde70.ent.ti.com (172.24.170.148) with Microsoft SMTP Server id 14.1.323.3; Tue, 26 Mar 2013 15:35:47 +0530 Received: from [172.24.133.11] (smtpvbd.itg.ti.com [172.24.170.250]) by dbdp33.itg.ti.com (8.13.8/8.13.8) with ESMTP id r2QA5hhb032260; Tue, 26 Mar 2013 15:35:44 +0530 Message-ID: <515172F7.3050600@ti.com> Date: Tue, 26 Mar 2013 15:35:43 +0530 From: Sekhar Nori User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130307 Thunderbird/17.0.4 MIME-Version: 1.0 To: Mike Turquette Subject: Re: [PATCH net-next 2/2] net/davinci_emac: use clk_{prepare|unprepare} References: <1364203547-2960-1-git-send-email-nsekhar@ti.com> <1364203547-2960-3-git-send-email-nsekhar@ti.com> <20130325161654.4014.68401@quantum> In-Reply-To: <20130325161654.4014.68401@quantum> CC: "Rafael J. Wysocki" , , , "David S. Miller" X-BeenThere: davinci-linux-open-source@linux.davincidsp.com X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: Errors-To: davinci-linux-open-source-bounces@linux.davincidsp.com On 3/25/2013 9:46 PM, Mike Turquette wrote: > Quoting Sekhar Nori (2013-03-25 02:25:47) >> Use clk_prepare()/clk_unprepare() in the driver since common >> clock framework needs these to be called before clock is enabled. >> >> This is in preparation of common clock framework migration >> for DaVinci. >> >> Cc: Mike Turquette >> Signed-off-by: Sekhar Nori >> --- >> drivers/net/ethernet/ti/davinci_emac.c | 19 +++++++++++++++++-- >> 1 file changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c >> index b1349b5..436296c 100644 >> --- a/drivers/net/ethernet/ti/davinci_emac.c >> +++ b/drivers/net/ethernet/ti/davinci_emac.c >> @@ -350,6 +350,7 @@ struct emac_priv { >> /*platform specific members*/ >> void (*int_enable) (void); >> void (*int_disable) (void); >> + struct clk *clk; >> }; >> >> /* EMAC TX Host Error description strings */ >> @@ -1870,19 +1871,29 @@ static int davinci_emac_probe(struct platform_device *pdev) >> dev_err(&pdev->dev, "failed to get EMAC clock\n"); >> return -EBUSY; >> } >> + >> + rc = clk_prepare(emac_clk); >> + if (rc) { >> + dev_err(&pdev->dev, "emac clock prepare failed.\n"); >> + return rc; >> + } >> + > > Is clk_enable ever called for emac_clk here? I don't see it in the > diff. clk_prepare and clk_enable should usually be paired, even if > simply calling clk_prepare generates the clock signal that your device > needs. clk_enable() is called by pm_runtime framework. Without this patch, the clk_enable() call from pm_clk_resume() will result in a warning when using common clock framework. This issue can actually be fixed by patch below which fixes this in drivers/base/power/clock_ops.c so individual drivers don't have to do this, but we need to careful since will break for callers who intend to use the pm_runtime apis from atomic context. Anyway, its probably much better to fix this in pm_runtime framework so I will work with Rafael to see if I can come up with something acceptable. This patch can then be dropped for now, but 1/2 can still be applied. That one is pretty harmless! Thanks, Sekhar diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c index 9d8fde7..60d389a 100644 --- a/drivers/base/power/clock_ops.c +++ b/drivers/base/power/clock_ops.c @@ -230,7 +230,7 @@ int pm_clk_suspend(struct device *dev) list_for_each_entry_reverse(ce, &psd->clock_list, node) { if (ce->status < PCE_STATUS_ERROR) { if (ce->status == PCE_STATUS_ENABLED) - clk_disable(ce->clk); + clk_disable_unprepare(ce->clk); ce->status = PCE_STATUS_ACQUIRED; } } @@ -259,7 +259,7 @@ int pm_clk_resume(struct device *dev) list_for_each_entry(ce, &psd->clock_list, node) { if (ce->status < PCE_STATUS_ERROR) { - clk_enable(ce->clk); + clk_prepare_enable(ce->clk); ce->status = PCE_STATUS_ENABLED; } }