On 8/23/21 6:41 PM, Raj, Ashok wrote:
> On Mon, Aug 23, 2021 at 12:49:19PM -0600, Derrick, Jonathan wrote:
>> From: James Puthukattukaran <james.puthukattukaran@xxxxxxxxxx>
>>
>> When a specific x8 CEM card is bifurcated into x4x4 mode, and the
>> upstream ports both support hotplugging on each respective x4 device, a
>> slot management system for the CEM card requires both x4 devices to be
>> sysfs removed from the OS before it can safely turn-off physical power.
>
> sysfs removed from the OS seems a bit confusing.. Do you mean removed via
> sysfs by echo 0 > power?
Yes it seems to be related to slot power and not just virtual removal
>
>> The implications are that Slot Control will display Powered Off status
>> for the device where the device is actually powered until both ports
>> have powered off.
>>
>> When power is removed from the first half, real power and link remains
>> active while waiting for the second half to have power removed. When
>> power is then removed from the second half, the first half starts
>> shutdown sequence and will trigger a DLLSC event. This is misinterpreted
>> as an enabling event and causes the first half to be re-enabled.
>>
>> The spurious enable can be resolved by ignoring link status change
>> events when no link is active when in the off state. This patch adds a
>> quirk for the card.
>>
>> Acked-by: Jon Derrick <jonathan.derrick@xxxxxxxxx>
>> Signed-off-by: James Puthukattukaran <james.puthukattukaran@xxxxxxxxxx>
>> ---
>> v1->v2: Device-specific quirk
>>
>> drivers/pci/hotplug/pciehp_ctrl.c | 7 +++++++
>> drivers/pci/quirks.c | 30 ++++++++++++++++++++++++++++++
>> include/linux/pci.h | 1 +
>> 3 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
>> index 529c34808440..db41f78bfac8 100644
>> --- a/drivers/pci/hotplug/pciehp_ctrl.c
>> +++ b/drivers/pci/hotplug/pciehp_ctrl.c
>> @@ -225,6 +225,7 @@ void pciehp_handle_disable_request(struct controller *ctrl)
>> void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events)
>> {
>> int present, link_active;
>> + struct pci_dev *pdev = ctrl->pcie->port;
>>
>> /*
>> * If the slot is on and presence or link has changed, turn it off.
>> @@ -265,6 +266,12 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events)
>> cancel_delayed_work(&ctrl->button_work);
>> fallthrough;
>> case OFF_STATE:
>> + if (pdev->shared_pcc_and_link_slot &&
>> + (events & PCI_EXP_SLTSTA_DLLSC) && !link_active) {
>
> Is it !link_active? Based on the explanation, until we turn off both slots
> you would see link is still active correct?
My understanding is the internal link is active but link_active shows negative
status while the one side is faking being 'powered off'.
Then if a DLLSC occurs while the slot is already OFF_STATE and !link_active, I
believe we can assume it was previously !link_active as well and it's an event
from the other side.
>
>> + mutex_unlock(&ctrl->state_lock);
>> + break;
>> + }
>> +
