linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Subject: Re: [git pull] PCI fixes
Date: Mon, 5 Dec 2011 11:22:56 -0800	[thread overview]
Message-ID: <20111205112256.68e59434@jbarnes-desktop> (raw)
In-Reply-To: <CA+55aFyaiJBYmgVCLfPA7q3hqtPHYh9wwsmVTCewbBoxMsDE8A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]

On Wed, 23 Nov 2011 15:02:01 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, Nov 23, 2011 at 2:44 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >
> > Nothing too exciting this time, mostly some minor fixes for things
> > normal people don't really hit.  Happy Thanksgiving.
> 
> Ugh. This looks bogus:
> 
> > Kenji Kaneshige (2):
> >      PCI: pciehp: wait 1000 ms before Link Training check
> 
> Look at that patch more closely. After the patch, the code looks like this:
> 
> 
>          if (ctrl->link_active_reporting)
>                  pcie_wait_link_active(ctrl);
>          else
>                  msleep(1000);
> 
> +	/*
> +	 * Need to wait for 1000 ms after Data Link Layer Link Active
> +	 * (DLLLA) bit reads 1b before sending configuration request.
> +	 * We need it before checking Link Training (LT) bit becuase
> +	 * LT is still set even after DLLLA bit is set on some platform.
> +	 */
> +	msleep(1000);
> 
> and I'm pretty sure you should remove the "else msleep(1000)" there.
> Doing the 1s wait *twice* seems entirely bogus, even if you are
> missing link_active_reporting. No?
> 
> I pulled it since I can't test it, but it really smells fishy to me.

Sure looks like it... Kenji-san, you went back and forth on this one a
little, can you confirm (and preferably test)?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2011-12-05 19:22 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-23 22:44 [git pull] PCI fixes Jesse Barnes
2011-11-23 23:02 ` Linus Torvalds
2011-12-05 19:22   ` Jesse Barnes [this message]
2011-12-06  8:08     ` Kenji Kaneshige
2011-12-06 16:14       ` Linus Torvalds
2011-12-06 22:36         ` Yinghai Lu
2011-12-07  8:18           ` Kenji Kaneshige
2011-12-07 19:20             ` Yinghai Lu
2011-12-07  7:58         ` Kenji Kaneshige
  -- strict thread matches above, loose matches on Subject: below --
2011-12-17 17:29 Jesse Barnes
2011-12-18  2:33 ` Yinghai Lu
2011-12-18  5:52   ` Jesse Barnes
2011-12-18 22:14     ` Linus Torvalds
2012-01-06 20:46       ` Jesse Barnes
2012-01-07  1:14         ` Yinghai Lu
2012-02-17 17:24 Jesse Barnes
2012-03-05 21:49 Jesse Barnes

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=20111205112256.68e59434@jbarnes-desktop \
    --to=jbarnes@virtuousgeek.org \
    --cc=kaneshige.kenji@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=torvalds@linux-foundation.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;
as well as URLs for NNTP newsgroup(s).