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 0B595CD13CF for ; Mon, 2 Sep 2024 19:19:40 +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:In-Reply-To:Content-Type: MIME-Version:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=77RBoZJaLdoK+X6+VwQVcRKGS6So6TwrYtr5FndinKQ=; b=mn7aqxHuiVsyHQ mQSKJu6RNMv4I8Gya5/uCj0Ze8oQwgDsT3/lSOXC7MxBYVmgYqRF83/jYTlIi34aXD2nwgDul/qMR zB+uJ4vRF/319ClnwMP8LGucPoaXzATOCOEC02NZNHdOlnD413pZ+v9OBPQze3MUb/MrmMzsdQ4Xe ywer/xRqgSP4S0v+zTycdVYVUlpE5KbH/uI6qgpPBs88Lojd0tnn7jGWTVZuulstR9r7Mk5oLEK6B MQQ+phvMfxkG78fkWFvW6QHORnw7A8YKEZ1g/Qhef9QtcxSd0Vd9THJDsPL+ydCpxtxmy0hFunuLK 2zqULGsW86FHAYiyKBUg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1slCaI-0000000FOPB-1qLg; Mon, 02 Sep 2024 19:19:30 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1slCZL-0000000FO94-3dMa; Mon, 02 Sep 2024 19:18:33 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 619065C5931; Mon, 2 Sep 2024 19:18:26 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 37A85C4CEC2; Mon, 2 Sep 2024 19:18:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1725304709; bh=lVD6NdQtOSiq4Qr2J9qExnjFEtwYqUvBaZrGo41wt28=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=i/2GE23dWychP1K6x47bYp0CIj0J+5yO9gjCNex4s0iGlgq5ZeBXROW72YJ/rOjM7 zLhWOJQ/vNZ1Ql3QnpyFofuZ3sDhUUDR6RxW/Vh5Nd0MG7knAO8LlpQP/E3aRc9rVE oO2qnpYTd2p8PJuKJjlG34GRb+ddOyZYuftrNsD45IiUX8t0sosMLVss3RhCAy4DbK DLjmQHgwjx0PMoISULpHa8vW0aics6L3GfXbiWPlx/paw3bI8/BoWT90gU5VxHcB04 LWBg/Nx0RG2wuXvjf6wqg3wQkypZp0kVz4qUg0mBr6d5uwou8/m/5KMQZ24eR87GHG lIFksc4Hb9QGg== Date: Mon, 2 Sep 2024 14:18:27 -0500 From: Bjorn Helgaas To: Jim Quinlan Cc: linux-pci@vger.kernel.org, Nicolas Saenz Julienne , Bjorn Helgaas , Lorenzo Pieralisi , Cyril Brulebois , Stanimir Varbanov , Manivannan Sadhasivam , Krzysztof Kozlowski , bcm-kernel-feedback-list@broadcom.com, jim2101024@gmail.com, Florian Fainelli , Lorenzo Pieralisi , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Rob Herring , Philipp Zabel , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" , open list Subject: Re: [PATCH v6 05/13] PCI: brcmstb: Use bridge reset if available Message-ID: <20240902191827.GA224376@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240815225731.40276-6-james.quinlan@broadcom.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240902_121831_972395_12F49A7D X-CRM114-Status: GOOD ( 15.31 ) 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 Thu, Aug 15, 2024 at 06:57:18PM -0400, Jim Quinlan wrote: > The 7712 SOC has a bridge reset which can be described in the device tree. > Use it if present. Otherwise, continue to use the legacy method to reset > the bridge. > static void brcm_pcie_bridge_sw_init_set_generic(struct brcm_pcie *pcie, u32 val) > { > - u32 tmp, mask = RGR1_SW_INIT_1_INIT_GENERIC_MASK; > - u32 shift = RGR1_SW_INIT_1_INIT_GENERIC_SHIFT; > + if (val) > + reset_control_assert(pcie->bridge_reset); > + else > + reset_control_deassert(pcie->bridge_reset); > > - tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > - tmp = (tmp & ~mask) | ((val << shift) & mask); > - writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > + if (!pcie->bridge_reset) { > + u32 tmp, mask = RGR1_SW_INIT_1_INIT_GENERIC_MASK; > + u32 shift = RGR1_SW_INIT_1_INIT_GENERIC_SHIFT; > + > + tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > + tmp = (tmp & ~mask) | ((val << shift) & mask); > + writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > + } This pattern looks goofy: reset_control_assert(pcie->bridge_reset); if (!pcie->bridge_reset) { ... If we're going to test pcie->bridge_reset at all, it should be first so it's obvious what's going on and the reader doesn't have to go verify that reset_control_assert() ignores and returns success for a NULL pointer: if (pcie->bridge_reset) { if (val) reset_control_assert(pcie->bridge_reset); else reset_control_deassert(pcie->bridge_reset); return; } u32 tmp, mask = RGR1_SW_INIT_1_INIT_GENERIC_MASK; ... Krzysztof, can you amend this on the branch? It will also make the eventual return checking and error message simpler because we won't have to initialize "ret" first, and we can "return 0" directly for the legacy case. Bjorn