linux-kernel - Re: [PATCH v7 2/2] add new platform driver for PCI RC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 03 Feb 2016 00:27:46 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Bjorn Helgaas <helgaas@...nel.org>
Cc:	Joao Pinto <Joao.Pinto@...opsys.com>, Vineet.Gupta1@...opsys.com,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-snps-arc@...ts.infradead.org, CARLOS.PALMINHA@...opsys.com,
	Alexey.Brodkin@...opsys.com, robh+dt@...nel.org,
	pawel.moll@....com, mark.rutland@....com,
	ijc+devicetree@...lion.org.uk, galak@...eaurora.org
Subject: Re: [PATCH v7 2/2] add new platform driver for PCI RC

On Tuesday 02 February 2016 17:14:49 Bjorn Helgaas wrote:
> On Tue, Feb 02, 2016 at 09:25:25PM +0100, Arnd Bergmann wrote:
> > On Monday 01 February 2016 18:07:45 Joao Pinto wrote:
> > > +static void synopsys_pcie_establish_link(struct pcie_port *pp)
> > > +{
> > > +   int retries = 0;
> > > +
> > > +   /* check if the link is up or not */
> > > +   for (retries = 0; retries < 10; retries++) {
> > > +           if (dw_pcie_link_up(pp)) {
> > > +                   dev_info(pp->dev, "Link up\n");
> > > +                   return;
> > > +           }
> > > +           mdelay(100);
> > > +   }
> > > +}
> > 
> > That mdelay() needs to be an msleep(). You should never waste
> > a whole second worth of CPU time in a driver.
> 
> There are six other copies of this code, and two of them (exynos and
> spear) have the same problem.

Good point. Maybe move one (correct) copy into the main driver file and
have it called by the other drivers?

IIRC there were some problems in drivers that did a similar
function from the config space accessors where you cannot call
msleep(), but this driver is not one of them.

	Arnd