From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6391488130164195328 X-Received: by 10.223.162.155 with SMTP id s27mr423820wra.7.1488203734238; Mon, 27 Feb 2017 05:55:34 -0800 (PST) X-BeenThere: outreachy-kernel@googlegroups.com Received: by 10.46.80.22 with SMTP id e22ls442206ljb.12.gmail; Mon, 27 Feb 2017 05:55:32 -0800 (PST) X-Received: by 10.25.145.17 with SMTP id t17mr1714571lfd.23.1488203732952; Mon, 27 Feb 2017 05:55:32 -0800 (PST) Return-Path: Received: from mail3-relais-sop.national.inria.fr (mail3-relais-sop.national.inria.fr. [192.134.164.104]) by gmr-mx.google.com with ESMTPS id f7si812309wmg.3.2017.02.27.05.55.32 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 27 Feb 2017 05:55:32 -0800 (PST) Received-SPF: neutral (google.com: 192.134.164.104 is neither permitted nor denied by domain of julia.lawall@lip6.fr) client-ip=192.134.164.104; Authentication-Results: gmr-mx.google.com; spf=neutral (google.com: 192.134.164.104 is neither permitted nor denied by domain of julia.lawall@lip6.fr) smtp.mailfrom=julia.lawall@lip6.fr X-IronPort-AV: E=Sophos;i="5.35,214,1484002800"; d="scan'208";a="214869502" Received: from vaio-julia.rsr.lip6.fr ([132.227.76.33]) by mail3-relais-sop.national.inria.fr with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Feb 2017 14:55:32 +0100 Date: Mon, 27 Feb 2017 14:55:23 +0100 (CET) From: Julia Lawall X-X-Sender: jll@hadrien To: Aishwarya Pant cc: Lidza Louina , Mark Hounschell , Greg Kroah-Hartman , outreachy-kernel@googlegroups.com Subject: Re: [Outreachy kernel] [PATCH] staging: dgnc: replace udelay with usleep_range In-Reply-To: <20170227114637.GA2158@aishwarya> Message-ID: References: <20170226183806.GA3498@aishwarya> <20170227114637.GA2158@aishwarya> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Mon, 27 Feb 2017, Aishwarya Pant wrote: > On Sun, Feb 26, 2017 at 08:02:56PM +0100, Julia Lawall wrote: > > > > > > On Mon, 27 Feb 2017, Aishwarya Pant wrote: > > > > > Adding first patch for checkpatch warning on dgnc_cls.c - > > > > What do you mean by "Adding first patch"? > > > > Actually, commit messages should be written in the imperative (as you have > > for the subject line), and should describe what you do and why. > > > > > CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt > > > > Also commit messages should be limited to around 70 characters or so per > > line. > > > > How did you choose 20 as the upper bound? > > > > julia > > > > From what I've read of `udelay` and `usleep_range` from the > documentation, this is what I understand: > > udelay(t) initiates a busy-wait loop. This means it's taking up CPU > execution cycles to do work. Useful for atomic transactions as we'd like > to prevent other processes from doing work. Because it depends on CPU > frequency, the delay is sometimes more than t microseconds. > > To sleep for precisely t microseconds without depending on CPU > frequency, we utilise hrtimers (high resolution timers), which are > implemented as callbacks. No CPU cycles are spent, a notification > (interrupt) is received when delay period is over. > It is possible to use usleep_range(t, t) for precision, but if we use > this pattern throughout too many interrupts will be triggered - a big > overhead. If the range is sufficiently large, one optimisation the > kernel can make is to coalesce wakeups for efficiency. We should use > usleep_range as usleep_range(t, t + delta). A safe choice for delta is t > , so we have - usleep_range(t, 2*t) > (Another recommendation is that t lie between 10 and 20 microseconds, so > the choice of delta seems safe). > > In the end, usleep_range is precise within a defined window of > (LOWER_LIMIT, UPPER_LIMIT) > > I have a couple of questions about usleep_range - > > -- What are the other wake-ups that we can coalesce (any example)? > > -- "The larger a range you supply, the greater a chance that you will > not trigger an interrupt." Our goal is to trigger an interrupt, so that > we can end delay. How is end delay handled then? Thanks for the very nice explanation. You could add to your commit message that you chose the upper limit based on the suggestions in XXX. I don't know the answer to the other questions, but maybe someone else can help out. julia > > -- > Cheers > Aishwarya Pant > > > > > > > Signed-off-by: Aishwarya Pant > > > --- > > > drivers/staging/dgnc/dgnc_cls.c | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c > > > index c20ffdd..2cf146e 100644 > > > --- a/drivers/staging/dgnc/dgnc_cls.c > > > +++ b/drivers/staging/dgnc/dgnc_cls.c > > > @@ -16,7 +16,7 @@ > > > #include > > > #include /* For jiffies, task states */ > > > #include /* For tasklet and interrupt structs/defines */ > > > -#include /* For udelay */ > > > +#include /* For usleep_range */ > > > #include /* For read[bwl]/write[bwl] */ > > > #include /* For struct async_serial */ > > > #include /* For the various UART offsets */ > > > @@ -409,7 +409,7 @@ static void cls_assert_modem_signals(struct channel_t *ch) > > > writeb(out, &ch->ch_cls_uart->mcr); > > > > > > /* Give time for the UART to actually drop the signals */ > > > - udelay(10); > > > + usleep_range(10, 20); > > > } > > > > > > static void cls_copy_data_from_queue_to_uart(struct channel_t *ch) > > > @@ -631,7 +631,7 @@ static void cls_flush_uart_read(struct channel_t *ch) > > > * Presumably, this is a bug in this UART. > > > */ > > > > > > - udelay(10); > > > + usleep_range(10, 20); > > > } > > > > > > /* > > > @@ -1096,7 +1096,7 @@ static void cls_uart_init(struct channel_t *ch) > > > > > > writeb(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT, > > > &ch->ch_cls_uart->isr_fcr); > > > - udelay(10); > > > + usleep_range(10, 20); > > > > > > ch->ch_flags |= (CH_FIFO_ENABLED | CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM); > > > > > > -- > > > 2.7.4 > > > > > > -- > > > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > > > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > > > To post to this group, send email to outreachy-kernel@googlegroups.com. > > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170226183806.GA3498%40aishwarya. > > > For more options, visit https://groups.google.com/d/optout. > > > >