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 5DC9BC4725D for ; Mon, 22 Jan 2024 04:39:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:To:Subject: CC: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=zjW0bFKs04LFqLy6cXFCxaa19fjsD4wI37JVpYDUFkk=; b=B3jE0aL4FYFlZZ EOl2/VYPpXphQNObPgar8jiJPDPvhiLvC5oMCuU5y6MxZlqk4gKcGhL9FHK6YTyrURFwdQk90n5ZF vsd1iyxYuSwHs6e5pGQkwFsaicwPN4C5rI4eMsxJAC9Kk6n/fUkr1yyzeUkY0+Xp7fjSLNhDRYsNH OYfQkLw+D18pyuO8/d5Y8ymuv7rK/WHXIgzuu33jzXYE/ZM6J3KZlkzYV5JiXv7fgNrdRjfecft6J K4Tw01R8ZJS9cGy1Srp7xEX+Tqe5KfjE0XYfXB2UhEr4Q/NphtidGmRx4qeKQIT+VFQUQwSI2ArBw vUIep+/uQSMLWNeM2WlA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rRm5J-00AXs6-1c; Mon, 22 Jan 2024 04:38:57 +0000 Received: from lelv0143.ext.ti.com ([198.47.23.248]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rRm5E-00AXrX-2f for linux-arm-kernel@lists.infradead.org; Mon, 22 Jan 2024 04:38:56 +0000 Received: from lelv0265.itg.ti.com ([10.180.67.224]) by lelv0143.ext.ti.com (8.15.2/8.15.2) with ESMTP id 40M4cQp3003682; Sun, 21 Jan 2024 22:38:26 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1705898306; bh=Yv2nSf8N9CL9Pd+LiNW9j/JB6/vZI1vHFGPJonKLe/Y=; h=Date:CC:Subject:To:References:From:In-Reply-To; b=DuLqF5t4Xn6B336Glo9YIVrMn1yjHSnjWm/6a/AGr9TLuRLstdDl20s87afXH3AxS ghNL9O9fk0dGF8HJxVl7aY3lki6hZ9DGA+/yKPaqV8O8bAlkn1zda4/v6IMRrDh+3p QrhuIcnmSxCdbozlmKPZuPvzmrHzC7J+4CkRybAQ= Received: from DFLE113.ent.ti.com (dfle113.ent.ti.com [10.64.6.34]) by lelv0265.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 40M4cQAk007632 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Sun, 21 Jan 2024 22:38:26 -0600 Received: from DFLE109.ent.ti.com (10.64.6.30) by DFLE113.ent.ti.com (10.64.6.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23; Sun, 21 Jan 2024 22:38:26 -0600 Received: from lelvsmtp5.itg.ti.com (10.180.75.250) by DFLE109.ent.ti.com (10.64.6.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23 via Frontend Transport; Sun, 21 Jan 2024 22:38:25 -0600 Received: from [172.24.227.9] (uda0492258.dhcp.ti.com [172.24.227.9]) by lelvsmtp5.itg.ti.com (8.15.2/8.15.2) with ESMTP id 40M4cLO5106030; Sun, 21 Jan 2024 22:38:22 -0600 Message-ID: <230c2000-1510-4da2-b5b7-df363cd9a45d@ti.com> Date: Mon, 22 Jan 2024 10:08:21 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird CC: , , , , , , , , , , , Subject: Re: [PATCH v3] PCI: keystone: Fix race condition when initializing PHYs To: Bjorn Helgaas References: <20240119232032.GA192245@bhelgaas> Content-Language: en-US From: Siddharth Vadapalli In-Reply-To: <20240119232032.GA192245@bhelgaas> X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240121_203853_010322_FEC5CD55 X-CRM114-Status: GOOD ( 19.11 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 20/01/24 04:50, Bjorn Helgaas wrote: > On Wed, Jan 10, 2024 at 11:35:24AM +0530, Siddharth Vadapalli wrote: >> Hello Bjorn, >> >> On 10/01/24 02:53, Bjorn Helgaas wrote: ... >> >> Does the following look fine? >> =============================================================================== >> diff --git a/drivers/pci/controller/dwc/pci-keystone.c >> b/drivers/pci/controller/dwc/pci-keystone.c >> index e02236003b46..6e9f9589d26c 100644 >> --- a/drivers/pci/controller/dwc/pci-keystone.c >> +++ b/drivers/pci/controller/dwc/pci-keystone.c >> @@ -962,6 +962,9 @@ static int ks_pcie_enable_phy(struct keystone_pcie *ks_pcie) >> int num_lanes = ks_pcie->num_lanes; >> >> for (i = 0; i < num_lanes; i++) { >> + /* Obtain reference to the phy */ >> + phy_pm_runtime_get_sync(ks_pcie->phy[i]); > > I thought the point was that you needed to guarantee that all PHYs are > powered on and stay that way before initializing any of them. If so, > you would need a separate loop, e.g., > > for (i = 0; i < num_lanes; i++) > phy_pm_runtime_get_sync(ks_pcie->phy[i]); > > for (i = 0; i < num_lanes; i++) { > ret = phy_reset(ks_pcie->phy[i]); > ... > Yes, the above implementation seems better. The strict requirement will be that post initialization of the first PHY (Serdes), it remains powered ON so that it can provide its reference clock to the second PHY (Serdes). Therefore, getting the reference to the PHYs within the loop will work too since the reference is being release only outside the loop. Nevertheless I shall go ahead with the implementation suggested by you since that looks much better and cleaner. >> ret = phy_reset(ks_pcie->phy[i]); >> if (ret < 0) >> goto err_phy; >> @@ -977,12 +980,18 @@ static int ks_pcie_enable_phy(struct keystone_pcie *ks_pcie) >> } >> } >> >> + /* Release reference(s) to the phy(s) */ >> + for (i = 0; i < num_lanes; i++) >> + phy_pm_runtime_put_sync(ks_pcie->phy[i]); >> + >> return 0; >> >> err_phy: >> while (--i >= 0) { >> phy_power_off(ks_pcie->phy[i]); >> phy_exit(ks_pcie->phy[i]); >> + /* Release reference to the phy */ >> + phy_pm_runtime_put_sync(ks_pcie->phy[i]); >> } >> >> return ret; -- Regards, Siddharth. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel