mbox series

[0/3] PCI: pciehp: Do not turn off slot if presence comes up after link

Message ID 20191001211419.11245-1-stuart.w.hayes@gmail.com (mailing list archive)
Headers show
Series PCI: pciehp: Do not turn off slot if presence comes up after link | expand

Message

stuart hayes Oct. 1, 2019, 9:14 p.m. UTC
In older PCIe specs, PDS (presence detect) would come up when the
"in-band" presence detect pin connected, and would be up before DLLLA
(link active).

In PCIe 4.0 (as an ECN) and in PCIe 5.0, there is a new bit to show if
in-band presence detection can be disabled for the slot, and another bit
that disables it--and a recommendation that it should be disabled if it
can be. In addition, certain OEMs disable in-band presence detection
without implementing these bits.

This means it is possible to get a "card present" interrupt after the
link is up and the driver is loaded.  This causes an erroneous removal
of the device driver, followed by an immediate re-probing.

This patch set defines these new bits, uses them to disable in-band
presence detection if it can be, waits for PDS to go up if in-band
presence detection is disabled, and adds a DMI table that will let us
know if we should assume in-band presence is disabled on a system.

This patch set is based on a patch set [1] submitted many months ago by
Alexandru Gagniuc, who is no longer working on it.

[1] https://patchwork.kernel.org/cover/10909167/
    [v3,0/4] PCI: pciehp: Do not turn off slot if presence comes up after link

Stuart Hayes (3):
  PCI: pciehp: Add support for disabling in-band presence
  PCI: pciehp: Wait for PDS when in-band presence is disabled
  PCI: pciehp: Add dmi table for systems with in-band presence disabled

 drivers/pci/hotplug/pciehp.h     |  1 +
 drivers/pci/hotplug/pciehp_hpc.c | 45 +++++++++++++++++++++++++++++++-
 include/uapi/linux/pci_regs.h    |  2 ++
 3 files changed, 47 insertions(+), 1 deletion(-)

Comments

Lukas Wunner Oct. 2, 2019, 4:13 a.m. UTC | #1
On Tue, Oct 01, 2019 at 05:14:16PM -0400, Stuart Hayes wrote:
> This patch set is based on a patch set [1] submitted many months ago by
> Alexandru Gagniuc, who is no longer working on it.
> 
> [1] https://patchwork.kernel.org/cover/10909167/
>     [v3,0/4] PCI: pciehp: Do not turn off slot if presence comes up after link

If I'm not mistaken, these two are identical to Alex' patches, right?

  PCI: pciehp: Add support for disabling in-band presence
  PCI: pciehp: Wait for PDS when in-band presence is disabled

I'm not sure if it's appropriate to change the author and
omit Alex' Signed-off-by.

Otherwise I have no objections against this series.

Thanks,

Lukas
stuart hayes Oct. 2, 2019, 4:31 a.m. UTC | #2
On Tue, Oct 1, 2019 at 11:13 PM Lukas Wunner <lukas@wunner.de> wrote:
>
> On Tue, Oct 01, 2019 at 05:14:16PM -0400, Stuart Hayes wrote:
> > This patch set is based on a patch set [1] submitted many months ago by
> > Alexandru Gagniuc, who is no longer working on it.
> >
> > [1] https://patchwork.kernel.org/cover/10909167/
> >     [v3,0/4] PCI: pciehp: Do not turn off slot if presence comes up after link
>
> If I'm not mistaken, these two are identical to Alex' patches, right?
>
>   PCI: pciehp: Add support for disabling in-band presence
>   PCI: pciehp: Wait for PDS when in-band presence is disabled
>
> I'm not sure if it's appropriate to change the author and
> omit Alex' Signed-off-by.
>
> Otherwise I have no objections against this series.
>
> Thanks,
>
> Lukas

Thanks!  The first patch is identical to the one Alex submitted, and
the second is nearly so... they both basically his work.  I wasn't
sure what proper etiquette was--I was thinking the signed-off-by was
taking responsibility that the patch was ok (functional, not
copyrighted by someone else, etc) rather than giving credit, but he
definitely deserves credit for them.  I'm happy to add a signed-off-by
for Alex on the first two and resubmit if he doesn't object.
Alex G. Oct. 2, 2019, 10:13 p.m. UTC | #3
On 10/1/19 11:13 PM, Lukas Wunner wrote:
> On Tue, Oct 01, 2019 at 05:14:16PM -0400, Stuart Hayes wrote:
>> This patch set is based on a patch set [1] submitted many months ago by
>> Alexandru Gagniuc, who is no longer working on it.
>>
>> [1] https://patchwork.kernel.org/cover/10909167/
>>      [v3,0/4] PCI: pciehp: Do not turn off slot if presence comes up after link
> 
> If I'm not mistaken, these two are identical to Alex' patches, right?
> 
>    PCI: pciehp: Add support for disabling in-band presence
>    PCI: pciehp: Wait for PDS when in-band presence is disabled
> 
> I'm not sure if it's appropriate to change the author and
> omit Alex' Signed-off-by.

Legally Dell owns the patches. I have no objection on my end.

Alex

> Otherwise I have no objections against this series.
> 
> Thanks,
> 
> Lukas
>
Lukas Wunner Oct. 3, 2019, 3:33 a.m. UTC | #4
On Wed, Oct 02, 2019 at 05:13:58PM -0500, Alex G. wrote:
> On 10/1/19 11:13 PM, Lukas Wunner wrote:
> > On Tue, Oct 01, 2019 at 05:14:16PM -0400, Stuart Hayes wrote:
> > > This patch set is based on a patch set [1] submitted many months ago by
> > > Alexandru Gagniuc, who is no longer working on it.
> > 
> > I'm not sure if it's appropriate to change the author and
> > omit Alex' Signed-off-by.
> 
> Legally Dell owns the patches. I have no objection on my end.

From a kernel community POV, I don't think it matters (in this case)
who legally owns the copyright to the contributed code.  It's just that
we go to great lengths to provide proper attribution even for small
contributions (e.g. Tested-by).

The benefit to the community is that we know who to cc if that portion
of the code is changed again and someone knowledgable should take a look.

The benefit to contributors is that they can change jobs and their past
contributions are still visible in the git history and attributed to
their names.  By contrast, if you've worked on closed source code and
changed jobs, that work isn't visible to future employers or even yourself,
and it may happen that someone else takes credit for your past work
without you even knowing about it or being able to stop that.
(I've seen it before.)

In this case, there should be a S-o-b line for Alex preceding that
for Stuart, and the author of the commit should be Alex unless a
significant portion of the patch was changed.

Thanks,

Lukas