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 3C923C52D7C for ; Tue, 13 Aug 2024 17:08:20 +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:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=j7dE1uz+PlBpAsbXxct/lp6oijbCvFNYuF6DJYvQMZ0=; b=mucsP+eqmYXaSLXYuBhhUKBs05 Hf2hONxPELHD5Ihxb6V55s+hdJoSXwNflFbVffYy8b51LBDZPN24Ehqb8LDjT8J2z35qEXkQOWk3e PDMUnbJ6QEjfpZpof5EWlBu/yJNxHo0iLnv3v59aInpOkFq8zMvn9rRhkf0RzSXK4dw5zpzEUgtgR PUa9odgSiLa7qKYA9BsXo76VOytdFtAC2yZD9A3EZVFvLlTFX/239tONakbfb7d9X5TgiNWVENGAA BEZyv0Ud5ezAdn9PO2r4KSOs6sWoUPngZ/79ESGCfrHqZ5XZ2r0Jv5Ycd73z9E/68mjWzFq1ZEH8m PNKKh74g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sdv09-00000004T0x-3P7u; Tue, 13 Aug 2024 17:08:05 +0000 Received: from mail-vk1-xa35.google.com ([2607:f8b0:4864:20::a35]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sduzW-00000004Sox-23F7 for linux-arm-kernel@lists.infradead.org; Tue, 13 Aug 2024 17:07:28 +0000 Received: by mail-vk1-xa35.google.com with SMTP id 71dfb90a1353d-4f524fa193aso42272e0c.0 for ; Tue, 13 Aug 2024 10:07:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; t=1723568845; x=1724173645; darn=lists.infradead.org; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to; bh=j7dE1uz+PlBpAsbXxct/lp6oijbCvFNYuF6DJYvQMZ0=; b=dZQ2mdvAjPGv4ozRJR0DWlto2J2Ecz1zcC44R43/9Inp4L6Axsu0YKHjeYag7Mpp1n 6vJaOSpKT4HW2ivcoYmc8fYypwAqwReb+a6Z2wbC1zpyDoZm1FGID8FO4cBh+jh6JDtE dpWkiPRqzc2aF/NwInohGn+PIJfFj1BipOzRc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723568845; x=1724173645; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=j7dE1uz+PlBpAsbXxct/lp6oijbCvFNYuF6DJYvQMZ0=; b=ucIdSoiuaMdpIhmlwA3vHylSGOD90sLM25P6ygwzq+V+hbIZpK92y2k64OFxCQl7R0 Fea3R9FiRmjldaUR1AiFsxvo4OjqB/qUJ7CI0Bo5NqHExT03yS8fV60BVyLAt7yDUftG P+OCjQHzjrdRW9kYkEURCHPCtgrsOnO3u/pQ545eJDlrt3pWRn6avzu5zwOhHPhg1GEO I7Bo3UiYrCqQvjFX9RlM4FC5hdvcRmWCX7Lt+Z/eDeTzw7Ldky6R/XB3PhpMXDO4unSE lTCYnpqI3ApUH0KVZuZs3PtO9v9Zcq8MqGHMVnutfrNxSAQ/YglUWA5shDo5jX432wiu K4AA== X-Forwarded-Encrypted: i=1; AJvYcCVsm/y9MUKJ8qndRq8i82sj20BelfrNmSj+W1gmhP04mX8uJ70pMGgvvI5v88qN98LMsrLLfUsxgJnoUSrKq3BfsxWGJTcQz6Xgg1mLxacMI/sbpUc= X-Gm-Message-State: AOJu0YxU6uNkcY/VtV1077DOvl268oQ1YXcwxIwode+Kk7Q13ACiJOJb eqHudAtArBHZYAciGRXT13kfpRvS1MbRobqnoeiBgrnWCrzgzV+bnO6Z9/Xcm530AeCYFx3PMio = X-Google-Smtp-Source: AGHT+IEm+cPjZoAGDMlpqNjutrZD6aLSO7tcCEgr6LpNF3ViHJ7PEFGjqtQm7IfGb1prdhqITbW/ug== X-Received: by 2002:a05:6a20:2444:b0:1c6:b364:dbdd with SMTP id adf61e73a8af0-1c8da1b653fmr5630850637.8.1723568776311; Tue, 13 Aug 2024 10:06:16 -0700 (PDT) Received: from [10.28.17.173] ([192.19.144.250]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7c6979ed3c7sm1732968a12.30.2024.08.13.10.06.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Aug 2024 10:06:15 -0700 (PDT) Message-ID: <8ed1aa9d-5ff5-465b-b91f-b32cc25cac34@broadcom.com> Date: Tue, 13 Aug 2024 13:06:12 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH v5 03/12] PCI: brcmstb: Use common error handling code in brcm_pcie_probe() To: Stanimir Varbanov , linux-pci@vger.kernel.org, Nicolas Saenz Julienne , Bjorn Helgaas , Lorenzo Pieralisi , Cyril Brulebois , Manivannan Sadhasivam , Krzysztof Kozlowski , bcm-kernel-feedback-list@broadcom.com, jim2101024@gmail.com Cc: Florian Fainelli , Lorenzo Pieralisi , =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= , Rob Herring , Philipp Zabel , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" , open list References: <20240731222831.14895-1-james.quinlan@broadcom.com> <20240731222831.14895-4-james.quinlan@broadcom.com> <9e3b0b10-7605-4203-a4c0-351b51b2e63b@suse.de> Content-Language: en-US From: James Quinlan Autocrypt: addr=james.quinlan@broadcom.com; keydata= xsBNBFa+BXgBCADrHC4AsC/G3fOZKB754tCYPhOHR3G/vtDmc1O2ugnIIR3uRjzNNRFLUaC+ BrnULBNhYfCKjH8f1TM1wCtNf6ag0bkd1Vj+IbI+f4ri9hMk/y2vDlHeC7dbOtTEa6on6Bxn r88ZH68lt66LSWEciIn+HMFRFKieXwYGqWyc4reakWanRvlAgB8R5K02uk9O9fZKL7uFyolD 7WR4/qeHTMUjyLJQBZJyaMj++VjHfyXj3DNQjFyW1cjIxiLZOk9JkMyeWOZ+axP/Aoe6UvWl Pg7UpxkAwCNHigmqMrZDft6e5ORXdRT163en07xDbzeMr/+DQyvTgpYst2CANb1y4lCFABEB AAHNKEppbSBRdWlubGFuIDxqYW1lcy5xdWlubGFuQGJyb2FkY29tLmNvbT7CwO8EEAEIAJkF AmNo/6MgFAAAAAAAFgABa2V5LXVzYWdlLW1hc2tAcGdwLmNvbYwwFIAAAAAAIAAHcHJlZmVy cmVkLWVtYWlsLWVuY29kaW5nQHBncC5jb21wZ3BtaW1lCAsJCAcDAgEKAhkBBReAAAAAGRhs ZGFwOi8va2V5cy5icm9hZGNvbS5uZXQFGwMAAAADFgIBBR4BAAAABBUICQoACgkQ3G9aYyHP Y/47xgf/TV+WKO0Hv3z+FgSEtOihmwyPPMispJbgJ50QH8O2dymaURX+v0jiCjPyQ273E4yn w5Z9x8fUMJtmzIrBgsxdvnhcnBbXUXZ7SZLL81CkiOl/dzEoKJOp60A7H+lR1Ce0ysT+tzng qkezi06YBhzD094bRUZ7pYZIgdk6lG+sMsbTNztg1OJKs54WJHtcFFV5WAUUNUb6WoaKOowk dVtWK/Dyw0ivka9TE//PdB1lLDGsC7fzbCevvptGGlNM/cSAbC258qnPu7XAii56yXH/+WrQ gL6WzcRtPnAlaAOz0jSqqOfNStoVCchTRFSe0an8bBm5Q/OVyiTZtII0GXq11c7ATQRWvgV4 AQgA7rnljIJvW5f5Zl6JN/zJn5qBAa9PPf27InGeQTRiL7SsNvi+yx1YZJL8leHw67IUyd4g 7XXIQ7Qog83TM05dzIjqV5JJ2vOnCGZDCu39UVcF45oCmyB0F5tRlWdQ3/JtSdVY55zhOdNe 6yr0qHVrgDI64J5M3n2xbQcyWS5/vhFCRgBNTDsohqn/4LzHOxRX8v9LUgSIEISKxphvKGP5 9aSst67cMTRuode3j1p+VTG4vPyN5xws2Wyv8pJMDmn4xy1M4Up48jCJRNCxswxnG9Yr2Wwz p77WvLx0yfMYo/ednfpBAAaNPqzQyTnUKUw0mUGHph9+tYjzKMx/UnJpzQARAQABwsGBBBgB AgErBQJWvgV5BRsMAAAAwF0gBBkBCAAGBQJWvgV4AAoJEOa8+mKcd3+LLC4IAKIxCqH1yUnf +ta4Wy+aZchAwVTWBPPSL1xJoVgFnIW1rt0TkITbqSPgGAayEjUvUv5eSjWrWwq4rbqDfSBN 2VfAomYgiCI99N/d6M97eBe3e4sAugZ1XDk1TatetRusWUFxLrmzPhkq2SMMoPZXqUFTBXf0 uHtLHZ2L0yE40zLILOrApkuaS15RVvxKmruqzsJk60K/LJaPdy1e4fPGyO2bHekT9m1UQw9g sN9w4mhm6hTeLkKDeNp/Gok5FajlEr5NR8w+yGHPtPdM6kzKgVvv1wjrbPbTbdbF1qmTmWJX tl3C+9ah7aDYRbvFIcRFxm86G5E26ws4bYrNj7c9B34ACgkQ3G9aYyHPY/7g8QgAn9yOx90V zuD0cEyfU69NPGoGs8QNw/V+W0S/nvxaDKZEA/jCqDk3vbb9CRMmuyd1s8eSttHD4RrnUros OT7+L6/4EnYGuE0Dr6N9aOIIajbtKN7nqWI3vNg5+O4qO5eb/n+pa2Zg4260l34p6d1T1EWy PqNP1eFNUMF2Tozk4haiOvnOOSw/U6QY8zIklF1N/NomnlmD5z063WrOnmonCJ+j9YDaucWm XFBxUJewmGLGnXHlR+lvHUjHLIRxNzHgpJDocGrwwZ+FDaUJQTTayQ9ZgzRLd+/9+XRtFGF7 HANaeMFDm07Hev5eqDLLgTADdb85prURmV59Rrgg8FgBWw== In-Reply-To: <9e3b0b10-7605-4203-a4c0-351b51b2e63b@suse.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240813_100726_541294_94733F87 X-CRM114-Status: GOOD ( 23.63 ) 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 8/13/24 12:45, Stanimir Varbanov wrote: > Hi Jim, > > On 8/1/24 01:28, Jim Quinlan wrote: >> o Move the clk_prepare_enable() below the resource allocations. >> o Move the clk_prepare_enable() out of __brcm_pcie_remove() but >> add it to the end of brcm_pcie_remove(). >> o Add a jump target (clk_disable_unprepare) so that a bit of exception >> handling can be better reused at the end of this function implementation. >> o Use dev_err_probe() where it makes sense.PASSWORD > Those dev_err_probe produce these errors on RPi5: Hi Stan, Sorry, I clearly  missed that.  My perception of the dev_err_probe() semantics were incorrect -- I thought it didn't print anything at all  if the "ret" param was zero, which would allow it to be used as an "if" conditional, as I am doing. Obviously not the case. Will fix. BTW I do not yet possess a CM5 so my testing is done on a CM4, 7712, and other STB chips. Regards and thanks, Jim Quinlan Broadcom STB/CM > > rpi5:~ # dmesg -l err > [ 1.004960] brcm-pcie 1000110000.pcie: error 0000000000000000: could > not assert reset 'swinit' > [ 1.013812] brcm-pcie 1000110000.pcie: error 0000000000000000: could > not de-assert reset 'swinit' after asserting > [ 1.024222] brcm-pcie 1000110000.pcie: error 0000000000000000: failed > to deassert 'rescal' > [ 1.533839] brcm-pcie 1000110000.pcie: link down > [ 1.627564] brcm-pcie 1000120000.pcie: error 0000000000000000: could > not assert reset 'swinit' > [ 1.636415] brcm-pcie 1000120000.pcie: error 0000000000000000: could > not de-assert reset 'swinit' after asserting > [ 1.646829] brcm-pcie 1000120000.pcie: error 0000000000000000: failed > to deassert 'rescal' > > ... as you can see there is no error at all. > > ~Stan > >> Signed-off-by: Jim Quinlan >> --- >> drivers/pci/controller/pcie-brcmstb.c | 34 ++++++++++++--------------- >> 1 file changed, 15 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c >> index c08683febdd4..7595e7009192 100644 >> --- a/drivers/pci/controller/pcie-brcmstb.c >> +++ b/drivers/pci/controller/pcie-brcmstb.c >> @@ -1473,7 +1473,6 @@ static void __brcm_pcie_remove(struct brcm_pcie *pcie) >> dev_err(pcie->dev, "Could not stop phy\n"); >> if (reset_control_rearm(pcie->rescal)) >> dev_err(pcie->dev, "Could not rearm rescal reset\n"); >> - clk_disable_unprepare(pcie->clk); >> } >> >> static void brcm_pcie_remove(struct platform_device *pdev) >> @@ -1484,6 +1483,7 @@ static void brcm_pcie_remove(struct platform_device *pdev) >> pci_stop_root_bus(bridge->bus); >> pci_remove_root_bus(bridge->bus); >> __brcm_pcie_remove(pcie); >> + clk_disable_unprepare(pcie->clk); >> } >> >> static const int pcie_offsets[] = { >> @@ -1613,31 +1613,26 @@ static int brcm_pcie_probe(struct platform_device *pdev) >> >> pcie->ssc = of_property_read_bool(np, "brcm,enable-ssc"); >> >> - ret = clk_prepare_enable(pcie->clk); >> - if (ret) { >> - dev_err(&pdev->dev, "could not enable clock\n"); >> - return ret; >> - } >> pcie->rescal = devm_reset_control_get_optional_shared(&pdev->dev, "rescal"); >> - if (IS_ERR(pcie->rescal)) { >> - clk_disable_unprepare(pcie->clk); >> + if (IS_ERR(pcie->rescal)) >> return PTR_ERR(pcie->rescal); >> - } >> + >> pcie->perst_reset = devm_reset_control_get_optional_exclusive(&pdev->dev, "perst"); >> - if (IS_ERR(pcie->perst_reset)) { >> - clk_disable_unprepare(pcie->clk); >> + if (IS_ERR(pcie->perst_reset)) >> return PTR_ERR(pcie->perst_reset); >> - } >> >> - ret = reset_control_reset(pcie->rescal); >> + ret = clk_prepare_enable(pcie->clk); >> if (ret) >> - dev_err(&pdev->dev, "failed to deassert 'rescal'\n"); >> + return dev_err_probe(&pdev->dev, ret, "could not enable clock\n"); >> + >> + ret = reset_control_reset(pcie->rescal); >> + if (dev_err_probe(&pdev->dev, ret, "failed to deassert 'rescal'\n")) >> + goto clk_disable_unprepare; >> >> ret = brcm_phy_start(pcie); >> if (ret) { >> reset_control_rearm(pcie->rescal); >> - clk_disable_unprepare(pcie->clk); >> - return ret; >> + goto clk_disable_unprepare; >> } >> >> ret = brcm_pcie_setup(pcie); >> @@ -1654,10 +1649,8 @@ static int brcm_pcie_probe(struct platform_device *pdev) >> msi_np = of_parse_phandle(pcie->np, "msi-parent", 0); >> if (pci_msi_enabled() && msi_np == pcie->np) { >> ret = brcm_pcie_enable_msi(pcie); >> - if (ret) { >> - dev_err(pcie->dev, "probe of internal MSI failed"); >> + if (dev_err_probe(pcie->dev, ret, "probe of internal MSI failed")) >> goto fail; >> - } >> } >> >> bridge->ops = pcie->type == BCM7425 ? &brcm7425_pcie_ops : &brcm_pcie_ops; >> @@ -1678,6 +1671,9 @@ static int brcm_pcie_probe(struct platform_device *pdev) >> >> fail: >> __brcm_pcie_remove(pcie); >> +clk_disable_unprepare: >> + clk_disable_unprepare(pcie->clk); >> + >> return ret; >> } >>