From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752590AbdJTSTV (ORCPT ); Fri, 20 Oct 2017 14:19:21 -0400 Received: from mail.kernel.org ([198.145.29.99]:59430 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752274AbdJTSTU (ORCPT ); Fri, 20 Oct 2017 14:19:20 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B51052187C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=helgaas@kernel.org Date: Fri, 20 Oct 2017 13:19:17 -0500 From: Bjorn Helgaas To: Pankaj Dubey Cc: David Laight , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Kishon Vijay Abraham I , Bjorn Helgaas , Jingoo Han , Joao Pinto Subject: Re: [PATCH] PCI: dwc: designware: don't sleep in atomic context Message-ID: <20171020181917.GL6332@bhelgaas-glaptop.roam.corp.google.com> References: <1507794889-11148-1-git-send-email-pankaj.dubey@samsung.com> <063D6719AE5E284EB5DD2968C1650D6DD00924BC@AcuExch.aculab.com> <70cd34bd-01db-cd93-ddf7-d82114ccc61a@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <70cd34bd-01db-cd93-ddf7-d82114ccc61a@samsung.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 13, 2017 at 09:10:38AM +0530, Pankaj Dubey wrote: > > > On 10/12/2017 04:09 PM, David Laight wrote: > >From: Pankaj Dubey > >>Sent: 12 October 2017 08:55 > >>In pcie-designware.c many places we are calling "usleep_range" which > >>are in atomic context. This patch fixes these potential BUGs and > >>replaces "usleep_range" with mdelay calls. > >> > >>Signed-off-by: Pankaj Dubey > >>--- > >> drivers/pci/dwc/pcie-designware.c | 8 ++++---- > >> drivers/pci/dwc/pcie-designware.h | 3 +-- > >> 2 files changed, 5 insertions(+), 6 deletions(-) > >> > >>diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c > >>index 88abddd..35d19b9 100644 > >>--- a/drivers/pci/dwc/pcie-designware.c > >>+++ b/drivers/pci/dwc/pcie-designware.c > >>@@ -138,7 +138,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index, > >> if (val & PCIE_ATU_ENABLE) > >> return; > >> > >>- usleep_range(LINK_WAIT_IATU_MIN, LINK_WAIT_IATU_MAX); > >>+ mdelay(LINK_WAIT_IATU_MIN); > >> } > >Spinning for 9ms (possibly 10 times) isn't really a good idea. > > Yes. It may not be a good idea, however in our experiment it never > hit maximum retry count. I just converted usleep_range to mdelay > keeping min time limitation as it is, though I am not sure, how do > we arrived on these numbers in original code, may be Joao Pinto from > Synopsys have some idea, I will try to do few experiment and try to > find out what is sufficient minimum time in our hardware for these > mdelay. Just based on the preceding comment, it looks like the wait is essential because subsequent config and I/O accesses won't work correctly until the ATU enable takes effect. If we timeout here, I suspect it's because something is seriously wrong in the hardware, so I doubt there's any point in trying to minimize the timeout period. If something is that broken, it doesn't matter whether we wait 9ms or 900ms. Maybe the message should be more strident or maybe we should even return failure so the caller can do something, e.g., fail an access, instead of just printing an error and continuing on. I'm also looking for an ack from Joao and/or Jingoo. Bjorn