> > >> > Currently in my patch, the pciehp_use_link_events=1 is used to > > >> > only > > >> enable link state change notifications, but in case pcie_isr() > > >> finds "DLL changed" bit set in "Slot status", it processes the link > > >> state change. I'll make the single line change to pcie_isr() to > > >> process the event only if pciehp_use_link_events=1. Let me know if > > >> this sounds alright. > > >> > > >> If we can avoid it, I'd prefer not to add the > "pciehp_use_link_events" > > >> module parameter. What would break if we just *always* used link > > >> events? > > > > > > Not really "break", but I'd think some changes in behavior shall be > visible: > > > > > > 1) When a card is pulled out (thus causing the link to go down), it > shall always be disabled - whether or not it supports "surprise removal" > (in the capabilities). I think this would be the right thing to do in > this situation. > > > > Yep, this sounds like it would be OK. > > > > > 2) Even other scenarios where the link may go down - e.g. device > reset using an external reset pin, or a firmware initiated reset etc, > shall cause the device to be immediately removed and re-added again when > the link comes back up). This may also be desired. > > > > This sounds OK, too. There's already some fancy dancing for similar > > scenarios in pciehp_reset_slot(); maybe that would have to be > > extended. Alex might have input here. > > So long as reset initiated through pci-core doesn't cause a hotplug I > think I'm ok with it. I saw code was already being added around slot > reset to mask link changes, just like we do with presence detection. Thanks Alex for taking a look at my patch. Yes, I have taken care in the pciehp_reset_slot(). > Thanks, > > Alex > > > > 3) If the link comes up automatically as soon as a card is plugged > in (i.e. without the need to turn on power or do anything else from SW), > the card shall be immediately added - even if surprise capability is not > present. I'm not sure if this is desired. That's because the current > behavior is that for slots that do not support surprise events, the user > has to initiate the hot-add using the attention button. Even after that, > he gets a 5 second time to cancel the operation in case he wishes. This > will no longer be the case, if use of link events is always enabled. > > > > This one sounds like a problem. But I think we could manage this, > > e.g., by noticing the link-up interrupt, but waiting for the 5-second > > timeout if an attention button is present. I think that one aspect might still not be right. The spec seems to indicate that the hot-plug should be *initiated* by the press of attention button, and after attention button press user has a time of 5 seconds. So I think if we want to remove that module parameter and also retain attention button behavior, our options might be: * If attention button is present, do not use link state events all together. (i.e. user has to use attention button always to initiate enabling or disabling device. This'd be compliant with current behavior and expectations I guess). In all the remaining cases, the link events can be used just fine. * If attention button is present, process LINK-DOWN events, but not LINK-UP events. Any devices whose link suddenly goes down, will be disabled (sounds quite right). But when Link comes back up, they'll not be enabled (although this may not sound right, but this complies with the current behavior and the semantics of attention button). Or else, we can leave the module parameter in place. BTW, what are the odds of coming across a slot where the link comes up automatically without the need for SW to do anything (despite having an attention button there), but "surprise" events are not supported :-)? Just wondering if we are discussing a realistic scenario. If we don't see this as realistic, then we can safely drop the module parameter IMHO. Thanks, Rajat > > > > > I think it would be fair to say that if use of link events is always > supported, it would be roughly equivalent to a slot supporting surprise > events. (Surprise events work on "presence detect" pin, and this will > work on the link state changes). > > > > > > Thanks, > > > > > > Rajat > > > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info > at http://vger.kernel.org/majordomo-info.html > ��.n��������+%������w��{.n�����{���"�)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
