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 X-Spam-Level: X-Spam-Status: No, score=-16.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9E476C4320A for ; Fri, 27 Aug 2021 14:02:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 845CB60F35 for ; Fri, 27 Aug 2021 14:02:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241586AbhH0OD3 (ORCPT ); Fri, 27 Aug 2021 10:03:29 -0400 Received: from mail.kernel.org ([198.145.29.99]:45146 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231675AbhH0OD2 (ORCPT ); Fri, 27 Aug 2021 10:03:28 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id D25C760F25; Fri, 27 Aug 2021 14:02:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1630072959; bh=9KVL338uZU2WstNlPctg6BYMhKgoDAyusOTsEn/ouuM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=CASqYK4WvoeOw7f6syajt3xV0idVa1b4L0oQ4JtnOFWpZqjTSaZmZcx+zbzhe+qB9 ixjl3hCW1sWlfD6NbuUgZXHNiy6pnMM/SG//ANdPM1FAI+ij2zCUbsgoMcdhVqLGjL k5BWgJNi7B988nnPscyVug5z4gppxLKvKNgA/PMtj8F+BWll7nu2WuILM6yTfHP9P0 H7ZO5kRrUvstQTPNbDlxFc7SYtUcGz6/YiiRGuPXrjotg//y94Bne/NtZ058zAhBna xBOhU/xYrd5sGhFEG695Fez4ULsInXSzQejMBC/CQJwOWPQjQ8B3HvCqZm2cv7Dplq QBpUkFpsQ63ww== Date: Fri, 27 Aug 2021 07:02:38 -0700 From: Jakub Kicinski To: zhaoxiao Cc: davem@davemloft.net, mcoquelin.stm32@gmail.com, peppe.cavallaro@st.com, alexandre.torgue@foss.st.com, joabreu@synopsys.com, netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] stmmac: dwmac-loongson: change the pr_info() to dev_err() in loongson_dwmac_probe() Message-ID: <20210827070238.7586fb11@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> In-Reply-To: <20210827085550.13519-1-zhaoxiao@uniontech.com> References: <20210827085550.13519-1-zhaoxiao@uniontech.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 27 Aug 2021 16:55:50 +0800 zhaoxiao wrote: > - Change the pr_info/dev_info to dev_err. > - Add the dev_err to improve readability. > > Signed-off-by: zhaoxiao > --- > .../net/ethernet/stmicro/stmmac/dwmac-loongson.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > index 4c9a37dd0d3f..495c94e7929f 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > @@ -54,20 +54,21 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > bool mdio = false; > > np = dev_of_node(&pdev->dev); > - > if (!np) { > - pr_info("dwmac_loongson_pci: No OF node\n"); > + dev_err(&pdev->dev, "dwmac_loongson_pci: No OF node\n"); > return -ENODEV; > } > > if (!of_device_is_compatible(np, "loongson, pci-gmac")) { > - pr_info("dwmac_loongson_pci: Incompatible OF node\n"); > + dev_err(&pdev->dev, "dwmac_loongson_pci: Incompatible OF node\n"); > return -ENODEV; > } > > plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); > - if (!plat) > + if (!plat) { > + dev_err(&pdev->dev, "memory allocation failed\n"); Please don't add error messages after allocation failures. OOM will produce a lot of kernel messages and a stack trace already. > return -ENOMEM; > + } > > if (plat->mdio_node) { > dev_err(&pdev->dev, "Found MDIO subnode\n"); > @@ -109,8 +110,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > plat->bus_id = pci_dev_id(pdev); > > phy_mode = device_get_phy_mode(&pdev->dev); > - if (phy_mode < 0) > + if (phy_mode < 0) { > dev_err(&pdev->dev, "phy_mode not found\n"); > + return phy_mode; You're adding a return here, it should be a separate patch with its own justification. > + } > > plat->phy_interface = phy_mode; > plat->interface = PHY_INTERFACE_MODE_GMII; > @@ -130,7 +133,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > > res.wol_irq = of_irq_get_byname(np, "eth_wake_irq"); > if (res.wol_irq < 0) { > - dev_info(&pdev->dev, "IRQ eth_wake_irq not found, using macirq\n"); > + dev_err(&pdev->dev, "IRQ eth_wake_irq not found, using macirq\n"); Why upgrade to an error, isn't wol_irq optional? > res.wol_irq = res.irq; > } > 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 X-Spam-Level: X-Spam-Status: No, score=-14.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E68EC4320E for ; Fri, 27 Aug 2021 14:05:41 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 11E3D60E90 for ; Fri, 27 Aug 2021 14:05:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 11E3D60E90 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=imtEyeg2AS44Lzb/kDVis+rHOuVIu+yH93dYX0B65Ak=; b=pGwRzOfXKCVQTG /kVaxdwwpBD25XrDHiNuzwKTdlmyc1qnq0h5tQRnohKnIvtC4ODYCa3A6lL9ZV8fi1rT4ako288Qv M9jRS2GM/sPdFDrKrwBvS5HTrcf4Ydvb0j0Fx0I/HIazejAeuhpR5EedCpmS9Fj5YMczYxAkbduMy wwqkOI/1zNXLPcMulijxlJBZlvPdV5oGSQ2UeY1s6emZ+WrpyPC8kbZb6BXh6K1dzLpPKhk+qj7m0 EQTF6ESYoUPtUrTU7IK+cB/AZAhjCy+RembdSLN8JgFeQeMReKcbvG90a/EG+k4WMR1656jXYudsI +k7V+2720InrfSfp3Rpw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mJcRP-00CXfx-ET; Fri, 27 Aug 2021 14:02:43 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mJcRM-00CXfc-66 for linux-arm-kernel@lists.infradead.org; Fri, 27 Aug 2021 14:02:41 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id D25C760F25; Fri, 27 Aug 2021 14:02:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1630072959; bh=9KVL338uZU2WstNlPctg6BYMhKgoDAyusOTsEn/ouuM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=CASqYK4WvoeOw7f6syajt3xV0idVa1b4L0oQ4JtnOFWpZqjTSaZmZcx+zbzhe+qB9 ixjl3hCW1sWlfD6NbuUgZXHNiy6pnMM/SG//ANdPM1FAI+ij2zCUbsgoMcdhVqLGjL k5BWgJNi7B988nnPscyVug5z4gppxLKvKNgA/PMtj8F+BWll7nu2WuILM6yTfHP9P0 H7ZO5kRrUvstQTPNbDlxFc7SYtUcGz6/YiiRGuPXrjotg//y94Bne/NtZ058zAhBna xBOhU/xYrd5sGhFEG695Fez4ULsInXSzQejMBC/CQJwOWPQjQ8B3HvCqZm2cv7Dplq QBpUkFpsQ63ww== Date: Fri, 27 Aug 2021 07:02:38 -0700 From: Jakub Kicinski To: zhaoxiao Cc: davem@davemloft.net, mcoquelin.stm32@gmail.com, peppe.cavallaro@st.com, alexandre.torgue@foss.st.com, joabreu@synopsys.com, netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] stmmac: dwmac-loongson: change the pr_info() to dev_err() in loongson_dwmac_probe() Message-ID: <20210827070238.7586fb11@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> In-Reply-To: <20210827085550.13519-1-zhaoxiao@uniontech.com> References: <20210827085550.13519-1-zhaoxiao@uniontech.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210827_070240_321670_033F3DA8 X-CRM114-Status: GOOD ( 18.88 ) 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 On Fri, 27 Aug 2021 16:55:50 +0800 zhaoxiao wrote: > - Change the pr_info/dev_info to dev_err. > - Add the dev_err to improve readability. > > Signed-off-by: zhaoxiao > --- > .../net/ethernet/stmicro/stmmac/dwmac-loongson.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > index 4c9a37dd0d3f..495c94e7929f 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > @@ -54,20 +54,21 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > bool mdio = false; > > np = dev_of_node(&pdev->dev); > - > if (!np) { > - pr_info("dwmac_loongson_pci: No OF node\n"); > + dev_err(&pdev->dev, "dwmac_loongson_pci: No OF node\n"); > return -ENODEV; > } > > if (!of_device_is_compatible(np, "loongson, pci-gmac")) { > - pr_info("dwmac_loongson_pci: Incompatible OF node\n"); > + dev_err(&pdev->dev, "dwmac_loongson_pci: Incompatible OF node\n"); > return -ENODEV; > } > > plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); > - if (!plat) > + if (!plat) { > + dev_err(&pdev->dev, "memory allocation failed\n"); Please don't add error messages after allocation failures. OOM will produce a lot of kernel messages and a stack trace already. > return -ENOMEM; > + } > > if (plat->mdio_node) { > dev_err(&pdev->dev, "Found MDIO subnode\n"); > @@ -109,8 +110,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > plat->bus_id = pci_dev_id(pdev); > > phy_mode = device_get_phy_mode(&pdev->dev); > - if (phy_mode < 0) > + if (phy_mode < 0) { > dev_err(&pdev->dev, "phy_mode not found\n"); > + return phy_mode; You're adding a return here, it should be a separate patch with its own justification. > + } > > plat->phy_interface = phy_mode; > plat->interface = PHY_INTERFACE_MODE_GMII; > @@ -130,7 +133,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > > res.wol_irq = of_irq_get_byname(np, "eth_wake_irq"); > if (res.wol_irq < 0) { > - dev_info(&pdev->dev, "IRQ eth_wake_irq not found, using macirq\n"); > + dev_err(&pdev->dev, "IRQ eth_wake_irq not found, using macirq\n"); Why upgrade to an error, isn't wol_irq optional? > res.wol_irq = res.irq; > } > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel