From: Bjorn Helgaas <helgaas@kernel.org> To: "Pali Rohár" <pali@kernel.org> Cc: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>, "Andrew Murray" <amurray@thegoodpenguin.co.uk>, "Bjorn Helgaas" <bhelgaas@google.com>, "Marek Behún" <marek.behun@nic.cz>, "Remi Pommarel" <repk@triplefau.lt>, "Tomasz Maciej Nowak" <tmn505@gmail.com>, Xogium <contact@xogium.me>, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] PCI: aardvark: Don't touch PCIe registers if no card connected Date: Wed, 1 Jul 2020 16:34:42 -0500 [thread overview] Message-ID: <20200701213442.GA3662456@bjorn-Precision-5520> (raw) In-Reply-To: <20200701082044.4494-1-pali@kernel.org> On Wed, Jul 01, 2020 at 10:20:44AM +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. > > This is just an optimization to prevent accessing PCIe registers when card > is disconnected. Trying to access PCIe registers of disconnected card does > not cause any crash, kernel just needs to wait for a timeout. So if card > disappear immediately after checking for PCIe link (before accessing PCIe > registers), it does not cause any problems. Thanks, this is good. I'd really like a short comment in the code as well, because this sort of link-up check tends to get copied to new drivers where it shouldn't be used, e.g., something like this: /* * If the link goes down after we check for link-up, nothing bad * happens but the config access times out. */ > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > Changes in V2: > * Update commit message, mention that this is optimization > --- > 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; > + > return true; > } > > -- > 2.20.1 >
WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <helgaas@kernel.org> To: "Pali Rohár" <pali@kernel.org> Cc: "Tomasz Maciej Nowak" <tmn505@gmail.com>, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>, linux-pci@vger.kernel.org, Xogium <contact@xogium.me>, linux-kernel@vger.kernel.org, "Marek Behún" <marek.behun@nic.cz>, "Remi Pommarel" <repk@triplefau.lt>, "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>, "Bjorn Helgaas" <bhelgaas@google.com>, linux-arm-kernel@lists.infradead.org, "Andrew Murray" <amurray@thegoodpenguin.co.uk> Subject: Re: [PATCH v2] PCI: aardvark: Don't touch PCIe registers if no card connected Date: Wed, 1 Jul 2020 16:34:42 -0500 [thread overview] Message-ID: <20200701213442.GA3662456@bjorn-Precision-5520> (raw) In-Reply-To: <20200701082044.4494-1-pali@kernel.org> On Wed, Jul 01, 2020 at 10:20:44AM +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. > > This is just an optimization to prevent accessing PCIe registers when card > is disconnected. Trying to access PCIe registers of disconnected card does > not cause any crash, kernel just needs to wait for a timeout. So if card > disappear immediately after checking for PCIe link (before accessing PCIe > registers), it does not cause any problems. Thanks, this is good. I'd really like a short comment in the code as well, because this sort of link-up check tends to get copied to new drivers where it shouldn't be used, e.g., something like this: /* * If the link goes down after we check for link-up, nothing bad * happens but the config access times out. */ > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > Changes in V2: > * Update commit message, mention that this is optimization > --- > 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; > + > return true; > } > > -- > 2.20.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-07-01 21:34 UTC|newest] Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-28 14:31 [PATCH] PCI: aardvark: Don't touch PCIe registers if no card connected Pali Rohár 2020-05-28 14:31 ` Pali Rohár 2020-05-28 16:26 ` Bjorn Helgaas 2020-05-28 16:26 ` Bjorn Helgaas 2020-05-28 16:38 ` Pali Rohár 2020-05-28 16:38 ` Pali Rohár 2020-05-28 16:49 ` Bjorn Helgaas 2020-05-28 16:49 ` Bjorn Helgaas 2020-05-29 8:30 ` Pali Rohár 2020-05-29 8:30 ` Pali Rohár 2020-06-30 12:31 ` Pali Rohár 2020-06-30 12:31 ` Pali Rohár 2020-06-30 13:51 ` Bjorn Helgaas 2020-06-30 13:51 ` Bjorn Helgaas 2020-06-30 14:04 ` Pali Rohár 2020-06-30 14:04 ` Pali Rohár 2020-06-30 14:58 ` Bjorn Helgaas 2020-06-30 14:58 ` Bjorn Helgaas 2020-07-01 8:08 ` Pali Rohár 2020-07-01 8:08 ` Pali Rohár 2020-07-01 8:20 ` [PATCH v2] " Pali Rohár 2020-07-01 8:20 ` Pali Rohár 2020-07-01 21:34 ` Bjorn Helgaas [this message] 2020-07-01 21:34 ` Bjorn Helgaas 2020-07-02 8:23 ` Pali Rohár 2020-07-02 8:23 ` Pali Rohár 2020-07-02 8:30 ` [PATCH v3] " Pali Rohár 2020-07-02 8:30 ` Pali Rohár 2020-07-09 11:35 ` Lorenzo Pieralisi 2020-07-09 11:35 ` Lorenzo Pieralisi 2020-07-09 12:22 ` Pali Rohár 2020-07-09 12:22 ` Pali Rohár 2020-07-09 14:47 ` Lorenzo Pieralisi 2020-07-09 14:47 ` Lorenzo Pieralisi 2020-07-09 15:09 ` Pali Rohár 2020-07-09 15:09 ` Pali Rohár 2020-07-10 9:18 ` Lorenzo Pieralisi 2020-07-10 9:18 ` Lorenzo Pieralisi 2020-07-10 15:44 ` Pali Rohár 2020-07-10 15:44 ` Pali Rohár 2020-07-10 16:08 ` Bjorn Helgaas 2020-07-10 16:08 ` Bjorn Helgaas 2020-07-10 19:30 ` Pali Rohár 2020-07-10 19:30 ` Pali Rohár 2020-07-10 20:08 ` Bjorn Helgaas 2020-07-10 20:08 ` Bjorn Helgaas 2020-07-13 8:27 ` Pali Rohár 2020-07-13 8:27 ` Pali Rohár 2020-07-13 11:23 ` Lorenzo Pieralisi 2020-07-13 11:23 ` Lorenzo Pieralisi 2020-07-13 14:50 ` Pali Rohár 2020-07-13 14:50 ` Pali Rohár 2020-07-13 16:41 ` Lorenzo Pieralisi 2020-07-13 16:41 ` Lorenzo Pieralisi 2020-07-14 7:38 ` Pali Rohár 2020-07-14 7:38 ` Pali Rohár 2020-07-15 12:17 ` Pali Rohár 2020-07-15 12:17 ` Pali Rohár 2020-07-15 16:21 ` Lorenzo Pieralisi 2020-07-15 16:21 ` Lorenzo Pieralisi 2020-07-21 8:57 ` Pali Rohár 2020-07-21 8:57 ` Pali Rohár 2020-07-21 10:48 ` Lorenzo Pieralisi 2020-07-21 10:48 ` Lorenzo Pieralisi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200701213442.GA3662456@bjorn-Precision-5520 \ --to=helgaas@kernel.org \ --cc=amurray@thegoodpenguin.co.uk \ --cc=bhelgaas@google.com \ --cc=contact@xogium.me \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=marek.behun@nic.cz \ --cc=pali@kernel.org \ --cc=repk@triplefau.lt \ --cc=thomas.petazzoni@bootlin.com \ --cc=tmn505@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.