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 X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EFDC1C433E0 for ; Tue, 30 Jun 2020 14:04:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CA889206EB for ; Tue, 30 Jun 2020 14:04:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1593525864; bh=SqVQK1p3HM5Lh8t8VwYGyeSHYFj1rCTiTIyGwW608xs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=wmwsawpqvnlYMfGabO97yIEjo4Qwq+Gl6EtOGvxezFFt1HX3Wb/NKRZGRODyTMQkB FqY5u+z1NhCp+aaaLxwGKekDR+UYEuICynj4H3g5gciC6ufGY+SvKPxsPb0BoEEIxE vUtgvYF7Xivuk9hek51n3+a2msffCLOf+GooFXOY= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728686AbgF3OEY (ORCPT ); Tue, 30 Jun 2020 10:04:24 -0400 Received: from mail.kernel.org ([198.145.29.99]:49740 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727796AbgF3OEY (ORCPT ); Tue, 30 Jun 2020 10:04:24 -0400 Received: from pali.im (pali.im [31.31.79.79]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 07375206BE; Tue, 30 Jun 2020 14:04:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1593525863; bh=SqVQK1p3HM5Lh8t8VwYGyeSHYFj1rCTiTIyGwW608xs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=VC9amz8/vWzYBN4hRZC3Fmfd1OnVrNc3D+bbz2VFBnUOAutjh7xGXV5DKIW1hs9x/ HNYKmJFG5bIzc5eV2VqXVanjn29mwmp5zBt4ERRU0LYhgEJ3p1I8SJ4FHD7aa2ob6H 74dXi4rrFfcqNx5SAki+O5xSv50INVKkNsGBGn1o= Received: by pali.im (Postfix) id C08AA81A; Tue, 30 Jun 2020 16:04:20 +0200 (CEST) Date: Tue, 30 Jun 2020 16:04:20 +0200 From: Pali =?utf-8?B?Um9ow6Fy?= To: Bjorn Helgaas Cc: Thomas Petazzoni , Lorenzo Pieralisi , Andrew Murray , Bjorn Helgaas , Marek =?utf-8?B?QmVow7pu?= , Remi Pommarel , Tomasz Maciej Nowak , Xogium , linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] PCI: aardvark: Don't touch PCIe registers if no card connected Message-ID: <20200630140420.7qkuq7wturmaafzf@pali> References: <20200528163809.54f5ldvphrjg3zg3@pali> <20200630135127.GA3414186@bjorn-Precision-5520> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20200630135127.GA3414186@bjorn-Precision-5520> User-Agent: NeoMutt/20180716 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Tuesday 30 June 2020 08:51:27 Bjorn Helgaas wrote: > On Thu, May 28, 2020 at 06:38:09PM +0200, Pali Rohár wrote: > > On Thursday 28 May 2020 11:26:04 Bjorn Helgaas wrote: > > > On Thu, May 28, 2020 at 04:31:41PM +0200, Pali Rohár wrote: > > > > When there is no PCIe card connected and advk_pcie_rd_conf() or > > > > advk_pcie_wr_conf() is called for PCI bus which doesn't belong to emulated > > > > root bridge, the aardvark driver throws the following error message: > > > > > > > > advk-pcie d0070000.pcie: config read/write timed out > > > > > > > > Obviously accessing PCIe registers of disconnected card is not possible. > > > > > > > > Extend check in advk_pcie_valid_device() function for validating > > > > availability of PCIe bus. If PCIe link is down, then the device is marked > > > > as Not Found and the driver does not try to access these registers. > > > > > > > > Signed-off-by: Pali Rohár > > > > --- > > > > drivers/pci/controller/pci-aardvark.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c > > > > index 90ff291c24f0..53a4cfd7d377 100644 > > > > --- a/drivers/pci/controller/pci-aardvark.c > > > > +++ b/drivers/pci/controller/pci-aardvark.c > > > > @@ -644,6 +644,9 @@ static bool advk_pcie_valid_device(struct advk_pcie *pcie, struct pci_bus *bus, > > > > if ((bus->number == pcie->root_bus_nr) && PCI_SLOT(devfn) != 0) > > > > return false; > > > > > > > > + if (bus->number != pcie->root_bus_nr && !advk_pcie_link_up(pcie)) > > > > + return false; > > > > > > I don't think this is the right fix. This makes it racy because the > > > link may go down after we call advk_pcie_valid_device() but before we > > > perform the config read. > > > > Yes, it is racy, but I do not think it cause problems. Trying to read > > PCIe registers when device is not connected cause just those timeouts, > > printing error message and increased delay in advk_pcie_wait_pio() due > > to polling loop. This patch reduce unnecessary access to PCIe registers > > when advk_pcie_wait_pio() polling just fail. > > What happens when the device is removed after advk_pcie_link_up() > returns true, but before we actually do the config access? Do you mean to remove device physically at runtime? I was told that our board would crash or issue reset. Removing device from mini PCIe slot without power off is not supported. Anyway, currently we are trying to read from device registers even when no device is connected. So when advk_pcie_link_up() returns true and after that device is not connected (somehow board and kernel would be still alive) I guess that it would behave as without applying this patch. So kernel starts reading from register and would wait until timeout expires. As device is not connected there would be no answer, so kernel print error message to dmesg (same as in commit message) and returns error that read failed.