From patchwork Wed Nov 16 20:33:18 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grygorii Strashko X-Patchwork-Id: 9432607 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 184236021C for ; Wed, 16 Nov 2016 20:33:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 09B9128854 for ; Wed, 16 Nov 2016 20:33:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F1F3E290C5; Wed, 16 Nov 2016 20:33:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 722B328854 for ; Wed, 16 Nov 2016 20:33:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753942AbcKPUdW (ORCPT ); Wed, 16 Nov 2016 15:33:22 -0500 Received: from fllnx210.ext.ti.com ([198.47.19.17]:21971 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752679AbcKPUdV (ORCPT ); Wed, 16 Nov 2016 15:33:21 -0500 Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by fllnx210.ext.ti.com (8.15.1/8.15.1) with ESMTP id uAGKXJB5032110; Wed, 16 Nov 2016 14:33:19 -0600 Received: from DLEE70.ent.ti.com (dlee70.ent.ti.com [157.170.170.113]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id uAGKXIOt003981; Wed, 16 Nov 2016 14:33:18 -0600 Received: from [128.247.83.173] (128.247.83.173) by DLEE70.ent.ti.com (157.170.170.113) with Microsoft SMTP Server id 14.3.294.0; Wed, 16 Nov 2016 14:33:18 -0600 Subject: Re: [PATCH net 1/7] net: ethernet: ti: cpsw: fix bad register access in probe error path To: Johan Hovold , Mugunthan V N References: <1479306916-27673-1-git-send-email-johan@kernel.org> <1479306916-27673-2-git-send-email-johan@kernel.org> CC: , , From: Grygorii Strashko Message-ID: <664c1f10-dd31-8566-852f-ee5ca0b5c46e@ti.com> Date: Wed, 16 Nov 2016 14:33:18 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1479306916-27673-2-git-send-email-johan@kernel.org> X-Originating-IP: [128.247.83.173] Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 11/16/2016 08:35 AM, Johan Hovold wrote: > Make sure to resume the platform device to enable clocks before > accessing the CPSW registers in the probe error path (e.g. for deferred > probe). > > Unhandled fault: external abort on non-linefetch (0x1008) at 0xd0872d08 > ... > [] (cpsw_ale_control_set) from [] (cpsw_ale_destroy+0x2c/0x44) > [] (cpsw_ale_destroy) from [] (cpsw_probe+0xbd0/0x10c4) > [] (cpsw_probe) from [] (platform_drv_probe+0x5c/0xc0) > > Note that in the unlikely event of a runtime-resume failure, we'll leak > the ale struct. > > Fixes: df828598a755 ("netdev: driver: ethernet: Add TI CPSW driver") > Signed-off-by: Johan Hovold > --- > drivers/net/ethernet/ti/cpsw.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c > index c6cff3d2ff05..5bc5e6189661 100644 > --- a/drivers/net/ethernet/ti/cpsw.c > +++ b/drivers/net/ethernet/ti/cpsw.c > @@ -2818,7 +2818,12 @@ static int cpsw_probe(struct platform_device *pdev) > return 0; > > clean_ale_ret: > - cpsw_ale_destroy(cpsw->ale); > + if (pm_runtime_get_sync(&pdev->dev) < 0) { > + pm_runtime_put_noidle(&pdev->dev); > + } else { > + cpsw_ale_destroy(cpsw->ale); > + pm_runtime_put_sync(&pdev->dev); > + } > clean_dma_ret: > cpdma_ctlr_destroy(cpsw->dma); > clean_runtime_disable_ret: > I think, wouldn't it be logically more simple to just keep CPSW PM runtime enabled during probe? Like in below diff (not tested): diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index 0548e56..deaac1b 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -2657,13 +2657,12 @@ static int cpsw_probe(struct platform_device *pdev) goto clean_runtime_disable_ret; } cpsw->version = readl(&cpsw->regs->id_ver); - pm_runtime_put_sync(&pdev->dev); res = platform_get_resource(pdev, IORESOURCE_MEM, 1); cpsw->wr_regs = devm_ioremap_resource(&pdev->dev, res); if (IS_ERR(cpsw->wr_regs)) { ret = PTR_ERR(cpsw->wr_regs); - goto clean_runtime_disable_ret; + goto clean_runtime_put_ret; } memset(&dma_params, 0, sizeof(dma_params)); @@ -2700,7 +2699,7 @@ static int cpsw_probe(struct platform_device *pdev) default: dev_err(priv->dev, "unknown version 0x%08x\n", cpsw->version); ret = -ENODEV; - goto clean_runtime_disable_ret; + goto clean_runtime_put_ret; } for (i = 0; i < cpsw->data.slaves; i++) { struct cpsw_slave *slave = &cpsw->slaves[i]; @@ -2729,7 +2728,7 @@ static int cpsw_probe(struct platform_device *pdev) if (!cpsw->dma) { dev_err(priv->dev, "error initializing dma\n"); ret = -ENOMEM; - goto clean_runtime_disable_ret; + goto clean_runtime_put_ret; } cpsw->txch[0] = cpdma_chan_create(cpsw->dma, 0, cpsw_tx_handler, 0); @@ -2831,12 +2830,16 @@ static int cpsw_probe(struct platform_device *pdev) } } + pm_runtime_put(&pdev->dev); + return 0; clean_ale_ret: cpsw_ale_destroy(cpsw->ale); clean_dma_ret: cpdma_ctlr_destroy(cpsw->dma); +clean_runtime_put_ret: + pm_runtime_put_sync(&pdev->dev); clean_runtime_disable_ret: pm_runtime_disable(&pdev->dev); clean_ndev_ret: