lkml.org 
[lkml]   [2019]   [Feb]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] PCI: pciehp: Do not turn off slot if presence comes up after link
On Tue, Feb 12, 2019 at 11:57:55PM +0000, Alex_Gagniuc@Dellteam.com wrote:
> The recommendation is to set the "in-band PD disable" bit, and it's
> possible that platform firmware may have set it at boot time

Ok, then I'd suggest to check in pcie_init() whether the feature is
supported and enable it if so. And store that fact in a flag, either
in struct pci_dev or struct controller.


> (*) A bit hypothetical: There is no hardware yet implementing the ECN.

Hm, this contradicts Austin Bolen's e-mail of Jan 25 that "Yes, this
platform disables in-band presence" (when asked whether your host
controller already adheres to the ECN).


> I'm
> not sure that there is a spec-justifiable reason to not access a device
> whose DLL is up, but PD isn't.

Austin asked in an e-mail of Jan 24 whether "the hot-inserted device's
config space [is] accessed immediately after waiting this 20 + 100 ms
delay", which sounded to me like you'd prefer the device not to be
accessed until PDS is 1. This can be achieved by polling PDS in
pcie_wait_for_link() if in-band presence is disabled, which is why
I suggested it.


> > Be mindful however that pcie_wait_for_link() is also called from the
> > DPC driver. Keith should be able to judge whether a change to that
> > function breaks DPC.
>
> That's why I went for ammending pciehp_handle_presence_or_link_change().
> Smaller bug surface ;). What I'm thinking at this point is, keep the
> patch as is, but, also check for in-band PD disable before aborting the
> shutdown. Old behavior stays the same.

I'm worried that amending pciehp_handle_presence_or_link_change() makes
the event handling logic more difficult to understand. Polling PDS in
pcie_wait_for_link() or disabling either PDC or DLLSC if in-band presence
is disabled seems simpler to reason about.


> in-band PD disable (what's a good acronym for that, BTW?)

I don't know, maybe inband_presence_disabled?

Thanks,

Lukas

\
 
 \ /
  Last update: 2019-02-13 09:46    [W:0.073 / U:4.364 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site