From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 785A6212F09 for ; Thu, 7 Nov 2024 15:27:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730993229; cv=none; b=c549PLf4A4SJFVAJ0W0UL9aXJ5m9Bm1hmEQxrAxFQQYKeUWtyVWaVr5D+63onAjVn/rWOjkvcNfRGTgQSQmY1+CeSMy2u48SBtQa+gPdxbYjH+6ZY93/lNHP+x0xdl2MshDmSvW1CIifjT195Z5A66vnndHV2PAkHCBHMDBNPPI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730993229; c=relaxed/simple; bh=dXituCL6gut0fSFPiXLwjg5t14g6zYP1Ea17MJMPgz4=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Disposition:In-Reply-To; b=Whtn9Jrj691J1Zhb4I3QKyIQt/Lez9aHxlO/7McsDrAyN2fNSlb28NUyo3iCtfbbbMYnouucPQN4mrGX3a9+hVqCUqW87VNlN2m9fRU2/ElBdAzUNTfYC7nHV4WwwJi9RH7DHhRFYaj9D9llmNnxq8Vvn8xWFqVvXzPQjQA3tZ4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=S6ufIbl/; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="S6ufIbl/" Received: by smtp.kernel.org (Postfix) with ESMTPSA id EE479C4CECC; Thu, 7 Nov 2024 15:27:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1730993228; bh=dXituCL6gut0fSFPiXLwjg5t14g6zYP1Ea17MJMPgz4=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=S6ufIbl/aTbis6ExmtWtZVMDUBioE4lSpL6WbJuTOaYbUyel0lIwRHhzHK+1UNv3L Yifn3xFWnNahmjrVpeboMBpfkOSxMLpVY7uLWnvPdkm7vcVuP4mbFuL9o4Th/gKVth fn4Xi+LwYky+wQPaLhbx6WzNJC4Iu5/yGWxJTvCdsnBS8pWEjR0UADUhM4ONDCtewM yQ7QA7QE78xdMb1uaQIqG1X55kHNlJrrlkixTgRbvb0rRWk4uIGbyj+Url35VYjENi efIheSQVwoDwwsND0x3GWvESf0eL1hnYjPit/K6tVqU2Bz3SK93zmbT/rtuMu2x4T4 SSzN7tpQk0tJQ== Date: Thu, 7 Nov 2024 09:27:05 -0600 From: Bjorn Helgaas To: Lorenzo Bianconi Cc: Ryder Lee , Jianjun Wang , Lorenzo Pieralisi , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Manivannan Sadhasivam , Rob Herring , Bjorn Helgaas , Philipp Zabel , Matthias Brugger , AngeloGioacchino Del Regno , linux-pci@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 3/3] PCI: mediatek-gen3: Move reset/assert callbacks in .power_up() Message-ID: <20241107152705.GA1614612@bhelgaas> Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241107-pcie-en7581-fixes-v1-3-af0c872323c7@kernel.org> On Thu, Nov 07, 2024 at 02:50:55PM +0100, Lorenzo Bianconi wrote: > In order to make the code more readable, move phy and mac reset lines > assert/de-assert configuration in .power_up callback > (mtk_pcie_en7581_power_up/mtk_pcie_power_up). > > Signed-off-by: Lorenzo Bianconi > --- > drivers/pci/controller/pcie-mediatek-gen3.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c > index 8c8c733a145634cdbfefd339f4a692f25a6e24de..c0127d0fb4f059b9f9e816360130e183e8f0e990 100644 > --- a/drivers/pci/controller/pcie-mediatek-gen3.c > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c > @@ -867,6 +867,13 @@ static int mtk_pcie_en7581_power_up(struct mtk_gen3_pcie *pcie) > int err; > u32 val; > > + /* > + * The controller may have been left out of reset by the bootloader > + * so make sure that we get a clean start by asserting resets here. > + */ > + reset_control_bulk_assert(pcie->soc->phy_resets.num_resets, > + pcie->phy_resets); > + reset_control_assert(pcie->mac_reset); Add blank line here. > /* > * Wait for the time needed to complete the bulk assert in > * mtk_pcie_setup for EN7581 SoC. > @@ -941,6 +948,15 @@ static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie) > struct device *dev = pcie->dev; > int err; > > + /* > + * The controller may have been left out of reset by the bootloader > + * so make sure that we get a clean start by asserting resets here. > + */ > + reset_control_bulk_assert(pcie->soc->phy_resets.num_resets, > + pcie->phy_resets); > + reset_control_assert(pcie->mac_reset); > + usleep_range(10, 20); Unrelated to this patch, but since you're moving it, do you know what this delay is for? Can we add a #define and a spec citation for it? Is there a requirement that the PHY and MAC reset ordering be different for EN7581 vs other chips? EN7581: assert PHY reset assert MAC reset power on PHY deassert PHY reset deassert MAC reset others: assert PHY reset assert MAC reset deassert PHY reset power on PHY deassert MAC reset Is there one order that would work for both? > /* PHY power on and enable pipe clock */ > err = reset_control_bulk_deassert(pcie->soc->phy_resets.num_resets, pcie->phy_resets); > if (err) { > @@ -1013,14 +1029,6 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie) > * counter since the bulk is shared. > */ > reset_control_bulk_deassert(pcie->soc->phy_resets.num_resets, pcie->phy_resets); > - /* > - * The controller may have been left out of reset by the bootloader > - * so make sure that we get a clean start by asserting resets here. > - */ > - reset_control_bulk_assert(pcie->soc->phy_resets.num_resets, pcie->phy_resets); > - > - reset_control_assert(pcie->mac_reset); > - usleep_range(10, 20); > > /* Don't touch the hardware registers before power up */ > err = pcie->soc->power_up(pcie); > > -- > 2.47.0 >