From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by ash.osuosl.org (Postfix) with ESMTP id 24A491BF41B for ; Wed, 7 Nov 2018 13:11:46 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 2186D22786 for ; Wed, 7 Nov 2018 13:11:46 +0000 (UTC) Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id AX-Erg6+BUYZ for ; Wed, 7 Nov 2018 13:11:45 +0000 (UTC) Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by silver.osuosl.org (Postfix) with ESMTPS id 5B0E82209C for ; Wed, 7 Nov 2018 13:11:45 +0000 (UTC) Received: by mail-wr1-f67.google.com with SMTP id k15-v6so14361731wre.12 for ; Wed, 07 Nov 2018 05:11:45 -0800 (PST) Date: Wed, 7 Nov 2018 14:11:40 +0100 From: Sergio Paracuellos Subject: Re: [PATCH v6 33/33] staging: mt7621-pci: replace 'mdelay()' with 'msleep()' Message-ID: <20181107131140.GA22887@foobar> References: <1541328599-18396-1-git-send-email-sergio.paracuellos@gmail.com> <1541328599-18396-34-git-send-email-sergio.paracuellos@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: List-Id: Linux Driver Project Developer List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" To: Frans Klaver Cc: neil@brown.name, Greg KH , driverdev-devel@linuxdriverproject.org On Mon, Nov 05, 2018 at 09:09:39AM +0100, Frans Klaver wrote: > On Sun, Nov 4, 2018 at 11:51 AM Sergio Paracuellos > wrote: > > > > Function 'mt7621_pcie_init_ports' is never called in atomic context. > > It calls mdelay() to busily wait, which is not necessary. mdelay() > > can be replaced with msleep(). > > > > Signed-off-by: Sergio Paracuellos > > --- > > drivers/staging/mt7621-pci/pci-mt7621.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c > > index fb9aa6b..14cec23 100644 > > --- a/drivers/staging/mt7621-pci/pci-mt7621.c > > +++ b/drivers/staging/mt7621-pci/pci-mt7621.c > > @@ -632,7 +632,7 @@ static void mt7621_pcie_init_ports(struct mt7621_pcie *pcie) > > rt_sysc_m32(PCIE_CLK_GEN_EN, PCIE_CLK_GEN_DIS, RALINK_PCIE_CLK_GEN); > > rt_sysc_m32(PCIE_CLK_GEN1_DIS, PCIE_CLK_GEN1_EN, RALINK_PCIE_CLK_GEN1); > > rt_sysc_m32(PCIE_CLK_GEN_DIS, PCIE_CLK_GEN_EN, RALINK_PCIE_CLK_GEN); > > - mdelay(50); > > + msleep(50); > > rt_sysc_m32(RALINK_PCIE_RST, 0, RALINK_RSTCTRL); Hi Frans, > > From a software point of view it makes sense to remove the busy-wait. > From a hardware perspective it may still make sense to keep it. Can > you guarantee that the PCIE_RST is done at the correct time. I haven't > looked thoroughly at the datasheet yet, but I can imagine the level > has to shift within a certain time frame? Or doesn't that make too > much of a difference? Perhaps mention that in your commit message? Thanks for the feedback. I agree with you that from a hardware point of view keep this could make sense. The fact is that after reading the datasheet for me is not clear at all that in this concrete case it make sense. I cannot also test this because I don't have the hardware to be able to do (all the patches, as I said are only compile-tested). This is also a reason to keep this patch as the last in this series to just skip it if this fails when is tested. Best regards, Sergio Paracuellos _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel