From: <Alex_Gagniuc@Dellteam.com>
To: <lukas@wunner.de>, <mr.nuke.me@gmail.com>
Cc: <bhelgaas@google.com>, <Austin.Bolen@dell.com>,
<keith.busch@intel.com>, <Shyam.Iyer@dell.com>,
<okaya@kernel.org>, <linux-pci@vger.kernel.org>,
<gustavo@embeddedor.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH RFC v2 2/4] PCI: pciehp: Do not turn off slot if presence comes up after link
Date: Fri, 22 Feb 2019 19:56:28 +0000 [thread overview]
Message-ID: <dfeadc6798984cce965e2202e90fe0b0@ausx13mps321.AMER.DELL.COM> (raw)
In-Reply-To: 20190221073634.snvazxlzgvjhilaz@wunner.de
On 2/21/19 1:36 AM, Lukas Wunner wrote:
> On Tue, Feb 19, 2019 at 07:20:28PM -0600, Alexandru Gagniuc wrote:
>> mutex_lock(&ctrl->state_lock);
>> + present = pciehp_card_present(ctrl);
>> + link_active = pciehp_check_link_active(ctrl);
>> switch (ctrl->state) {
>
> These two assignments appear to be superfluous as you're also performing
> them in pciehp_check_link_active().
Not sure. Between the first check, and this check, you can have several
seconds elapse depending on whether the driver's .probe()/remove() is
invoked. Whatever you got at the beginning would be stale. If you had a
picture dictionary and looked up 'bad idea', it would have a picture of
the above code with the second check removed.
I've got all the other review comments addressed in my local branch. I'm
waiting on Lord Helgass' decision on which solution is better.
Alex
next prev parent reply other threads:[~2019-02-22 19:56 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-20 1:20 [PATCH RFC v2 0/4] PCI: pciehp: Do not turn off slot if presence comes up after link Alexandru Gagniuc
2019-02-20 1:20 ` [PATCH RFC v2 1/4] PCI: hotplug: Add support for disabling in-band presence Alexandru Gagniuc
2019-02-21 7:19 ` Lukas Wunner
2019-02-21 18:05 ` Alex_Gagniuc
2019-02-20 1:20 ` [PATCH RFC v2 2/4] PCI: pciehp: Do not turn off slot if presence comes up after link Alexandru Gagniuc
2019-02-21 7:36 ` Lukas Wunner
2019-02-22 19:56 ` Alex_Gagniuc [this message]
2019-02-23 6:49 ` Lukas Wunner
2019-02-24 22:27 ` Alex_Gagniuc
2019-02-20 1:20 ` [PATCH RFC v2 3/4] PCI: hotplug: Wait for PDS when in-band presence is disabled Alexandru Gagniuc
2019-02-20 1:20 ` [PATCH RFC v2 4/4] PCI: hotplug: Add quirk For Dell nvme pcie switches Alexandru Gagniuc
2019-02-21 7:56 ` Lukas Wunner
2019-02-21 18:35 ` Alex_Gagniuc
2019-02-22 1:20 ` Joe Perches
2019-02-22 2:04 ` Oliver
2019-02-22 19:19 ` Alex_Gagniuc
2019-02-21 15:38 ` [PATCH RFC v2 0/4] PCI: pciehp: Do not turn off slot if presence comes up after link Lukas Wunner
2019-02-21 18:17 ` Alex_Gagniuc
2019-04-19 0:01 ` Bjorn Helgaas
2019-04-19 15:22 ` [PATCH v3 " Alexandru Gagniuc
2019-04-19 15:22 ` [PATCH v3 1/4] PCI: hotplug: Add support for disabling in-band presence Alexandru Gagniuc
2019-04-19 15:22 ` [PATCH v3 2/4] PCI: pciehp: Do not turn off slot if presence comes up after link Alexandru Gagniuc
2019-04-19 15:22 ` [PATCH v3 3/4] PCI: hotplug: Wait for PDS when in-band presence is disabled Alexandru Gagniuc
2019-04-19 15:22 ` [PATCH v3 4/4] PCI: hotplug: Add quirk For Dell nvme pcie switches Alexandru Gagniuc
2019-04-19 21:00 ` Alan J. Wylie
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=dfeadc6798984cce965e2202e90fe0b0@ausx13mps321.AMER.DELL.COM \
--to=alex_gagniuc@dellteam.com \
--cc=Austin.Bolen@dell.com \
--cc=Shyam.Iyer@dell.com \
--cc=bhelgaas@google.com \
--cc=gustavo@embeddedor.com \
--cc=keith.busch@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=mr.nuke.me@gmail.com \
--cc=okaya@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;
as well as URLs for NNTP newsgroup(s).