From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 50721C433EF for ; Fri, 8 Jul 2022 09:48:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:CC:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=BQPgYJLqJAMgx9VQHlbjK/uVriZFjIGN7WfEuUs2oT8=; b=b1+pdYAQXKMXz0 7vdhyHVBmnkgCKa6S43Sc1ATS841e7fFdcWxnCvFJQ7IY5kj+JO5telK33rmyHdcH1QcV5C2DfDyK x9q3mbZDQyaBX6Z7ih9HLfmsU198jYURusu03Uq5e2IbuCirsKnFwlwm+oL7fiy2XGNwusicNHlVa u4uuLh4oITViJzC7mfzaO5L9b7C6O9Ke9Yrhm+/Ilcis8uvtHWb+nNu3IpgAitH63dMjGclsMvp3f aP0SGE7ROQDIf6FtBbxJduy3exKSyqj0MT68XAxkYF5zJfVWydgFOkoM60ZZYHJfkWtMDVMQDqLnc 47nWQLK/a8U+/f25ecdg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o9kZV-0030DI-In; Fri, 08 Jul 2022 09:46:49 +0000 Received: from mailgw01.mediatek.com ([216.200.240.184]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o9kZQ-0030BS-IJ; Fri, 08 Jul 2022 09:46:46 +0000 X-UUID: e0ec93b4339e4921a5838ee123afd265-20220708 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=pkoIdRVMX8SxE0Snd0446EtprJYy7cw6qIVy+3b/MfA=; b=TZyriPEBKLzCfe4mel3CQ3YH6xHFn4652tT04T/+xP4CewqpR0TUL1qR3O5QOu0vuIbeVmoQItaH67Pra3/DPbkOiutpgzRCi+ahnfsJJCjNe6Tw+ZZdTe/X1hzXzUef5Wqojnvw7RBcggC3anv/CecU3whMMYZdWOMKaz1+LOE=; X-CID-P-RULE: Release_Ham X-CID-O-INFO: VERSION:1.1.8,REQID:5fbcd5d5-64ce-4c96-a8a2-1e43b1a9d757,OB:0,LO B:0,IP:0,URL:0,TC:0,Content:0,EDM:0,RT:0,SF:0,FILE:0,RULE:Release_Ham,ACTI ON:release,TS:0 X-CID-META: VersionHash:0f94e32,CLOUDID:0ad2f7d6-5d6d-4eaf-a635-828a3ee48b7c,C OID:IGNORED,Recheck:0,SF:nil,TC:nil,Content:0,EDM:-3,IP:nil,URL:0,File:nil ,QS:nil,BEC:nil,COL:0 X-UUID: e0ec93b4339e4921a5838ee123afd265-20220708 Received: from mtkmbs11n1.mediatek.inc [(172.21.101.185)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLSv1.2 ECDHE-RSA-AES256-GCM-SHA384 256/256) with ESMTP id 1107116232; Fri, 08 Jul 2022 02:46:33 -0700 Received: from mtkcas10.mediatek.inc (172.21.101.39) by mtkmbs10n1.mediatek.inc (172.21.101.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.2.792.15; Fri, 8 Jul 2022 17:46:18 +0800 Received: from mhfsdcap04 (10.17.3.154) by mtkcas10.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Fri, 8 Jul 2022 17:46:17 +0800 Message-ID: <410b8c62ea399b51c11021c4838bd6a62d542703.camel@mediatek.com> Subject: Re: [PATCH net v3] stmmac: dwmac-mediatek: fix clock issue From: Biao Huang To: Matthias Brugger , David Miller CC: Giuseppe Cavallaro , Alexandre Torgue , Jose Abreu , "Eric Dumazet" , Jakub Kicinski , "Paolo Abeni" , Maxime Coquelin , AngeloGioacchino Del Regno , , , , , , Date: Fri, 8 Jul 2022 17:46:17 +0800 In-Reply-To: <14bf5e6b-4230-fffc-4134-c3015cf4d262@gmail.com> References: <20220708083937.27334-1-biao.huang@mediatek.com> <20220708083937.27334-2-biao.huang@mediatek.com> <14bf5e6b-4230-fffc-4134-c3015cf4d262@gmail.com> X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220708_024644_650991_4BC2FDE5 X-CRM114-Status: GOOD ( 28.23 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Dear Mattias, Thanks for your comments. On Fri, 2022-07-08 at 11:22 +0200, Matthias Brugger wrote: > > On 08/07/2022 10:39, Biao Huang wrote: > > Since clocks are handled in mediatek_dwmac_clks_config(), > > remove the clocks configuration in init()/exit(), and > > invoke mediatek_dwmac_clks_config instead. > > > > This issue is found in suspend/resume test. > > > > Commit message is rather confusing. Basically you are moving the > clock enable > into probe instead of init and remove it from exit. That means, > clocks get > enabled earlier and don't get disabled if the module gets unloaded. > That doesn't > sound correct, I think we would at least need to disable the clocks > in remove > function. there is pm_runtime support in driver, and clocks will be disabled/enabled in stmmac_runtime_suspend/stmmac_runtime_resume. stmmac_dvr_probe() invoke pm_runtime_put(device) at the end, and disable clocks, but no clock enable at the beginning. so vendor's probe entry should enable clocks to ensure normal behavior. As to clocks in remove function, we did not test it We should implement a vendor specified remove function who will take care of clocks rather than invoke stmmac_pltfr_remove directly. Anyway, we miss the clock handling case in remove function, and will test it and feed back. > > I suppose that suspend calls exit and that there was a problem when > we disable > the clocks there. Is this a HW issue that has no other possible fix? Not a HW issue. suspend/resume will disable/enable clocks by invoking stmmac_pltfr_noirq_suspend/stmmac_pltfr_noirq_resume --> pm_runtime_force_suspend/pm_runtime_force_resume--> mediatek_dwmac_clks_config, so old clock handling in init/exit is no longer a proper choice. Best Regards! Biao > > Regards, > Matthias > > > Fixes: 3186bdad97d5 ("stmmac: dwmac-mediatek: add platform level > > clocks management") > > Signed-off-by: Biao Huang > > --- > > .../ethernet/stmicro/stmmac/dwmac-mediatek.c | 36 +++++--------- > > ----- > > 1 file changed, 9 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c > > index 6ff88df58767..e86f3e125cb4 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c > > @@ -576,32 +576,7 @@ static int mediatek_dwmac_init(struct > > platform_device *pdev, void *priv) > > } > > } > > > > - ret = clk_bulk_prepare_enable(variant->num_clks, plat->clks); > > - if (ret) { > > - dev_err(plat->dev, "failed to enable clks, err = %d\n", > > ret); > > - return ret; > > - } > > - > > - ret = clk_prepare_enable(plat->rmii_internal_clk); > > - if (ret) { > > - dev_err(plat->dev, "failed to enable rmii internal clk, > > err = %d\n", ret); > > - goto err_clk; > > - } > > - > > return 0; > > - > > -err_clk: > > - clk_bulk_disable_unprepare(variant->num_clks, plat->clks); > > - return ret; > > -} > > - > > -static void mediatek_dwmac_exit(struct platform_device *pdev, void > > *priv) > > -{ > > - struct mediatek_dwmac_plat_data *plat = priv; > > - const struct mediatek_dwmac_variant *variant = plat->variant; > > - > > - clk_disable_unprepare(plat->rmii_internal_clk); > > - clk_bulk_disable_unprepare(variant->num_clks, plat->clks); > > } > > > > static int mediatek_dwmac_clks_config(void *priv, bool enabled) > > @@ -643,7 +618,6 @@ static int mediatek_dwmac_common_data(struct > > platform_device *pdev, > > plat->addr64 = priv_plat->variant->dma_bit_mask; > > plat->bsp_priv = priv_plat; > > plat->init = mediatek_dwmac_init; > > - plat->exit = mediatek_dwmac_exit; > > plat->clks_config = mediatek_dwmac_clks_config; > > if (priv_plat->variant->dwmac_fix_mac_speed) > > plat->fix_mac_speed = priv_plat->variant- > > >dwmac_fix_mac_speed; > > @@ -712,13 +686,21 @@ static int mediatek_dwmac_probe(struct > > platform_device *pdev) > > mediatek_dwmac_common_data(pdev, plat_dat, priv_plat); > > mediatek_dwmac_init(pdev, priv_plat); > > > > + ret = mediatek_dwmac_clks_config(priv_plat, true); > > + if (ret) > > + return ret; > > + > > ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res); > > if (ret) { > > stmmac_remove_config_dt(pdev, plat_dat); > > - return ret; > > + goto err_drv_probe; > > } > > > > return 0; > > + > > +err_drv_probe: > > + mediatek_dwmac_clks_config(priv_plat, false); > > + return ret; > > } > > > > static const struct of_device_id mediatek_dwmac_match[] = { _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel