mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Johan Hovold <johan@kernel.org>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: "Konrad Dybcio" <konrad.dybcio@linaro.org>,
	"Alexander Lobakin" <aleksander.lobakin@intel.com>,
	"Jingoo Han" <jingoohan1@gmail.com>,
	"Gustavo Pimentel" <gustavo.pimentel@synopsys.com>,
	"Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Rob Herring" <robh@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Marijn Suijten" <marijn.suijten@somainline.org>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Johan Hovold" <johan+linaro@kernel.org>
Subject: Re: [PATCH] PCI: dwc: Use the correct sleep function in wait_for_link
Date: Wed, 21 Feb 2024 13:40:27 +0100	[thread overview]
Message-ID: <ZdXvO6q8IQWQlGk5@hovoldconsulting.com> (raw)
In-Reply-To: <20240220230048.GA1511259@bhelgaas>

On Tue, Feb 20, 2024 at 05:00:48PM -0600, Bjorn Helgaas wrote:
> On Tue, Feb 20, 2024 at 09:23:24AM +0100, Johan Hovold wrote:
> > On Thu, Feb 15, 2024 at 06:46:55PM +0100, Konrad Dybcio wrote:
> > > On 15.02.2024 18:02, Bjorn Helgaas wrote:
> > > > On Thu, Feb 15, 2024 at 02:35:13PM +0100, Alexander Lobakin wrote:

> > > >> Just use fsleep(LINK_WAIT_USLEEP_MAX) and let the kernel decide which
> > > >> function to pick.
> > > 
> > > IMO, fsleep only makes sense when the argument is variable.. This way, we
> > > can save on bothering the compiler or adding an unnecessary branch
> > 
> > I fully agree. Using fsleep() with a constant just looks sloppy (e.g.
> > with that hardcoded usleep range) and hides what is really going on for
> > no good reason.
> 
> Why does it look sloppy?  I'd be surprised if using a constant led to
> more executable code, given that fsleep() is inline.  I'm all for
> having the compiler choose the right thing instead of having to look
> up the guidelines myself.

It's not about the generated code, but about hiding what's really going
from kernel developers that should be aware of such things. The fact
that you end up with an usleep range of 20 to 40 ms if you want to sleep
for 20 ms is not very nice either.

Except possibly for a few cases where you'd otherwise end up open-coding
fsleep() I don't think there's any reason to use it.

Johan

  reply	other threads:[~2024-02-21 12:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-15 10:39 Konrad Dybcio
2024-02-15 13:35 ` Alexander Lobakin
2024-02-15 17:02   ` Bjorn Helgaas
2024-02-15 17:46     ` Konrad Dybcio
2024-02-20  8:23       ` Johan Hovold
2024-02-20 23:00         ` Bjorn Helgaas
2024-02-21 12:40           ` Johan Hovold [this message]
2024-02-15 14:17 ` Serge Semin
2024-02-15 17:47   ` Konrad Dybcio
2024-02-15 14:51 ` Kuppuswamy Sathyanarayanan
2024-02-15 17:48   ` Konrad Dybcio

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=ZdXvO6q8IQWQlGk5@hovoldconsulting.com \
    --to=johan@kernel.org \
    --cc=aleksander.lobakin@intel.com \
    --cc=bhelgaas@google.com \
    --cc=gustavo.pimentel@synopsys.com \
    --cc=helgaas@kernel.org \
    --cc=jingoohan1@gmail.com \
    --cc=johan+linaro@kernel.org \
    --cc=konrad.dybcio@linaro.org \
    --cc=kw@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=marijn.suijten@somainline.org \
    --cc=robh@kernel.org \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®