From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752199AbbKJJqZ (ORCPT ); Tue, 10 Nov 2015 04:46:25 -0500 Received: from mout.kundenserver.de ([212.227.126.187]:55693 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751889AbbKJJqV (ORCPT ); Tue, 10 Nov 2015 04:46:21 -0500 From: Arnd Bergmann To: Lucas Stach Cc: linux-arm-kernel@lists.infradead.org, Sanjeev Sharma , linux-pci@vger.kernel.org, Richard.Zhu@freescale.com, linux-kernel@vger.kernel.org, David Mueller , bhelgaas@google.com Subject: Re: [PATCH] PCI: imx6:don't sleep in atomic context Date: Tue, 10 Nov 2015 10:45:04 +0100 Message-ID: <6415545.xzR4A6SaUL@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <1447148110.3142.6.camel@pengutronix.de> References: <1447066080-5859-1-git-send-email-sanjeev_sharma@mentor.com> <4800389.dqcmlJY3HC@wuerfel> <1447148110.3142.6.camel@pengutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:rA9FXThdgExWOfuMY1YTUT1X5lO5+cXgd0CIS99Rmhd3/mEPyYP jJlmQJ960x9QwsmTXVPmNWC8GU+mWJ4No/+CPWMQTC/cI+JS5PBfH/YnPDPH4qnb54h2CtM cUK5cOt/SdjA8nVLNN4z4YGakaonAzCyNbQkg4HPYqXM+UA8TBuUishN72cnKWMZj96MSIo an7dq7bmqQ693BTqdEgpg== X-UI-Out-Filterresults: notjunk:1;V01:K0:im7DW4iE2ds=:6pVYFJ6OImxWNQCfUWt8x7 KM9ZO90kUm6SKC70MB4eT19Ap3uFkV3WoNJmsIjQlOu05zYdKD+4/GiyfB4PLos07qYaGcXHj 1shWCkWlTDPzbw8EigSLiKxGljyr9VIBNL8MVmtay3QNdQYcF22P5LpG5Nw02KGsyLMBGJBnP 5iJxXtU0rT2WSVNIeL6+HrgDgdLy61PxSRtrzTYb0J/hydohSETBh4ILujyDOyGGVSvd9+UN1 cj5tEmUm8Et6dmW5N2lAgTa6H/AqKtO2BrrGEjAD81fExbw18VUSSsUnh5xViKGi7Zyq5yyVH ndUmjw/I6YTFzxCmAL7QyiILryuBqXEQUd2m+Eiy5C5zYR26Vg67PB1UTcP9uVwGSvnWEq7eL yDrcbo/4bKPBR7SIJ0inWodlka7o2dTWi3hRrSSQKTpFtdlaD+Q7wLKbKG9wVKc3fjfYyHBTY gGFOTQgHq3rri8/Qc8qt1Iy4+XW7Os43lO8FQ4qHrtzw9BZIlui0jySC+F3UdQbWan79HKzlE G7GIbciXqAVMqLqqmtG62TCsdZ/1nQiCWPNHryxU2fQVBwkk0i/zndnyINWXr4qpSerLhh/7j 48GMwPDmezncAxTl8zkaPAPmlaPjUmFgpJb8NNvIxZ162Aa8+nS/QQ4xahi0D1fpFfOfG8SUz Sk/H1OViZ+Pm+nwQKyfc6s01N3tIQhubCBQSCH/0j5V0xoJEPLQPPKsz0fDgOMO/ENezIrZV7 u/FsVtLRb2fttZPw Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 10 November 2015 10:35:10 Lucas Stach wrote: > Am Dienstag, den 10.11.2015, 10:28 +0100 schrieb Arnd Bergmann: > > On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote: > > > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > > > > index 233a196..9769b13 100644 > > > > --- a/drivers/pci/host/pci-imx6.c > > > > +++ b/drivers/pci/host/pci-imx6.c > > > > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp) > > > > * Wait a little bit, then re-check if the link finished > > > > * the training. > > > > */ > > > > - usleep_range(1000, 2000); > > > > + mdelay(1000); > > > > > > A mdelay(1000) is a whole different timescale than a usleep(1000). If > > > this patch works for you with mdelay(1) or maybe mdelay(2) I would be > > > fine with it. > > > > mdelay(1) is still a really long time to block the CPU for, on potentially > > every config space access. > > > > Everybody else just returns the link status here, which seems to be > > the better alternative. If you need to delay the startup, better have > > a msleep(1) loop in the initial probe function where you are allowed to > > sleep. > > > Yes, it's somewhere on my TODO list to rework the link-up handling here, > but as there are quite a few timing and ordering implications in that > code, this needs a good thought and a good deal of testing. So I'm > inclined to ACK the current patch to get rid of the obvious bug and sort > things out properly in a follow on patchset. Maybe use that patch with some modifications then: * add a comment to explain that this is currently called from possibly atomic context through pci_config_{read,write} and that the link state handling never belonged here. * instead of looping five times for up to 2ms each, loop 100 times around a udelay(20) to hopefully be done earlier. I was going to suggest using time_before(timeout, jiffies) as the condition to wait for, but that doesn't work if called with interrupts disabled. Arnd