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 770ADC52D6F for ; Wed, 21 Aug 2024 09:10:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:Message-ID:References:In-Reply-To:Subject:Cc:To:From:Date: MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=KnuXtuSFEhMmZPIHCVemmAS+lD9TQ4aP2hCr8AgCkXc=; b=4AHYzZ/BsowGDRoe0EJSiQFj71 uiyF0DRwkngKOMsxuO5i2+i7BU943p9W7RFyRwgv7nT2BGMHJbrS5g+XEobsZWSWi2y2SYmjUAx1S y5gwU3FWcK6kkaGJrK30FZz1xFi1LeIsSqGIn82ZsEWIoNP2XmaKAiDxi8GHuVjvUaItA772ObCBt waNiwvqAUfLISO40fMgoD6TvA2xeCVhjEp/NmgaIAxHj+6ZEFtSF2Cg0vRFno774/LIEKb4yRtYaa g4rOJilZnEfVal0iFzC5piz8edxcdAs/MjK4SSKnV3BpI6mZZPkbx5DFA9aMdvaJGUdOpJMVshT+0 ICJLap9w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sghLh-00000008CfR-0GJ0; Wed, 21 Aug 2024 09:09:49 +0000 Received: from mail.manjaro.org ([2a01:4f8:c0c:51f3::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sghKz-00000008Cb1-2OFN; Wed, 21 Aug 2024 09:09:07 +0000 MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=manjaro.org; s=2021; t=1724231343; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KnuXtuSFEhMmZPIHCVemmAS+lD9TQ4aP2hCr8AgCkXc=; b=a0rF/+1LfI97JuyVlhGIwDzyth9/l0OMMZUt81srp2rWt4dkkM6bd0ZaWWTYfGrJc5A1tv pUfu43vT6KSTOexVuMJLUaGKuoUQ1Q9eqgjcYEZazt2HeiiGsbE+K8aUJvnn4m3xIl2+oi KI1ZS+5sPyt6+fCGBjHyZNmFZ5tiUSHBQ95UHBEl/Hl1rR/LbgPP8kRNBCyDmGcuA5ehkX oUarZQQCxjfAZ/bOSUW1sjEmlprH0DPmERR9xcyLMLloiw5BYzaKLqH1evyeJNtUB09YLT ydNkBfQH8JW+upU5iAAuO6o8rxXuJQwx4S1uTATV8yPltOWvH95thM5+UpdGFg== Date: Wed, 21 Aug 2024 11:09:03 +0200 From: Dragan Simic To: =?UTF-8?Q?Heiko_St=C3=BCbner?= Cc: linux-rockchip@lists.infradead.org, linux-phy@lists.infradead.org, vkoul@kernel.org, kishon@kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 3/3] phy: phy-rockchip-inno-usb2: Improve error handling while probing In-Reply-To: <12869965.VsHLxoZxqI@diego> References: <866445027a4f41165c872f494b04c218b6e67b09.1724225528.git.dsimic@manjaro.org> <12869965.VsHLxoZxqI@diego> Message-ID: <486bddb6aad14d05a3fb2d876d0d9d0d@manjaro.org> X-Sender: dsimic@manjaro.org Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Authentication-Results: ORIGINATING; auth=pass smtp.auth=dsimic@manjaro.org smtp.mailfrom=dsimic@manjaro.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240821_020905_802169_94C3685A X-CRM114-Status: GOOD ( 14.35 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2024-08-21 10:44, Heiko Stübner wrote: > Am Mittwoch, 21. August 2024, 09:37:55 CEST schrieb Dragan Simic: >> Improve error handling in the probe path by using function >> dev_err_probe() >> where appropriate, and by no longer using it rather pointlessly in one >> place >> that actually produces a single, hardcoded error code. >> >> Signed-off-by: Dragan Simic > >> @@ -1375,8 +1372,10 @@ static int rockchip_usb2phy_probe(struct >> platform_device *pdev) >> rphy->irq = platform_get_irq_optional(pdev, 0); >> platform_set_drvdata(pdev, rphy); >> >> - if (!phy_cfgs) >> - return dev_err_probe(dev, -EINVAL, "phy configs are not >> assigned!\n"); >> + if (!phy_cfgs) { >> + dev_err(dev, "phy configs are not assigned\n"); >> + return -EINVAL; >> + } >> >> ret = rockchip_usb2phy_extcon_register(rphy); >> if (ret) > > I really don't understand the rationale here. Using dev_err_probe here > is just fine and with that change you just introduce more lines of code > for exactly the same functionality? As we know, dev_err_probe() decides how to log the received error message based on the error code it receives, but in this case the error code is hardcoded as -EINVAL. Thus, in this case it isn't about keeping the LoC count a bit lower, but about using dev_err() where the resulting outcome of error logging is aleady known, and where logging the error code actually isn't helpful, because it's hardcoded and the logged error message already tells everything about the error condition. In other words, it's about being as precise as possible when deciding between dev_err() and dev_err_probe(), in both directions. I hope it makes sense.