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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 49D68C433EF for ; Wed, 18 May 2022 16:50:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240534AbiERQuW (ORCPT ); Wed, 18 May 2022 12:50:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41872 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240519AbiERQuV (ORCPT ); Wed, 18 May 2022 12:50:21 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A4D831FE1EB; Wed, 18 May 2022 09:50:20 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 1E2AC616EF; Wed, 18 May 2022 16:50:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3F6A9C385A5; Wed, 18 May 2022 16:50:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1652892619; bh=EKfo8rCNyIy8YeDE1LpF8Seb9BHiYKO7eUMI8MfUgBs=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=XaXPeOe1KI51PdJFOGclVzYI2pVIPi3mRdJtkiw+bz1rxLTBiJKOT5iCJwBDjQ7e4 08/7S6ec+vl1O1EAzMjFu1QJFY9hHfeug3613KdpEXc5IGIUXa4hpwxO2jiUCTlx8j M5zS1Ij2juPbnLSZ3Gth+Exc4FG6TLVy39jYUfYgpNFWcz0RNFBRuJlG2PluOR4Gcq TzihJWT8NUSt5VXcm4U8ZQ4xLpHx1ecS8XBt7KGJzO8lI9Plc6VvFoQ36sqJRe/yEa T25CbOxQyCPcdPfD71VUvTc9ZcdTEqp1Kz+OGbeeqUsbOWq64z0plg8UtsCMxBclrI uBXOXHofb08hg== Date: Wed, 18 May 2022 11:50:17 -0500 From: Bjorn Helgaas To: Manivannan Sadhasivam Cc: Bjorn Helgaas , Lorenzo Pieralisi , Keith Busch , Christoph Hellwig , linux-nvme@lists.infradead.org, linux-pci@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, Stanimir Varbanov , Bjorn Andersson , Jens Axboe , Veerabhadrarao Badiganti , quic_krichai@quicinc.com, Nitin Rawat , Vidya Sagar , Sagi Grimberg , Prasad Malisetty , Andy Gross , Rob Herring , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Rajat Jain , "Saheed O. Bolarinwa" , Rama Krishna , Stephen Boyd , Dmitry Baryshkov , Kalle Valo Subject: Re: [PATCH 2/3] PCI: dwc: qcom: Set suspend_poweroff flag for SC7280 Message-ID: <20220518165017.GA1145045@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220518035211.GA4791@thinkpad> Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Wed, May 18, 2022 at 09:22:11AM +0530, Manivannan Sadhasivam wrote: > On Tue, May 17, 2022 at 12:18:57PM -0500, Bjorn Helgaas wrote: > > [+cc Prasad, Andy, Rob, Krzysztof, Rajat, Saheed, Rama, Stephen, > > Dmitry, Kalle for connection to https://lore.kernel.org/lkml/CAE-0n53ho2DX2rqQMvvKAuDCfsWW62TceTaNPzv5Mn_NQ-U6dA@mail.gmail.com/T/] > > > > Subject line convention for this file is "PCI: qcom:" (not "PCI: dwc: > > qcom:"). > > > > Find this from "git log --oneline drivers/pci/controller/dwc/pcie-qcom.c". > > > > On Tue, May 17, 2022 at 08:41:34PM +0530, Manivannan Sadhasivam wrote: > > > On Mon, May 16, 2022 at 03:19:50PM -0500, Bjorn Helgaas wrote: > > > > On Fri, May 13, 2022 at 04:30:26PM +0530, Manivannan Sadhasivam wrote: > > > > > For aggressive power saving on SC7280 SoCs, the power for the > > > > > PCI devices will be taken off during system suspend. Hence, > > > > > notify the same to the PCI device drivers using > > > > > "suspend_poweroff" flag so that the drivers can prepare the PCI > > > > > devices to handle the poweroff and recover them during resume. > > > > > > > > No doubt "power ... will be taken off during system suspend" is > > > > true, but this isn't very informative. Is this a property of > > > > SC7280? A choice made by the SC7280 driver? Why is this not > > > > applicable to other systems? > > > > > > The SC7280's RPMh firmware is cutting off the PCIe power domain > > > during system suspend. And as I explained in previous patch, the RC > > > driver itself may put the devices in D3cold conditionally on this > > > platform. The reason is to save power as this chipset is being used > > > in Chromebooks. > > > > It looks like this should be squashed into the patch you mentioned: > > https://lore.kernel.org/lkml/CAE-0n53ho2DX2rqQMvvKAuDCfsWW62TceTaNPzv5Mn_NQ-U6dA@mail.gmail.com/T/ > > > > If Prasad's patch is applied without this, devices will be powered > > off, but nvme will not be prepared for it. Apparently something would > > be broken in that case? > > > > Yes, but Prasad's patch is not yet reviewed so likely not get merged until > further respins. Ok. Please work with Prasad to squash these as needed so there are no regressions along the way. > > Also, I think this patch should be reordered so the nvme driver is > > prepared for suspend_poweroff before the qcom driver starts setting > > it. Otherwise there's a window where qcom sets suspend_poweroff and > > powers off devices, but nvme doesn't know about it, and I assume > > something will be broken in that case? > > As per my understanding, patches in a series should not have build dependency > but they may depend on each other for functionality. Yes. But if qcom starts powering off devices when nvme isn't expecting it, it sounds like nvme will regress until the patch that adds nvme support. That temporary regression is what I want to avoid. Bjorn