Re: [PATCH] PCI: pciehp: Report degraded links via link bandwidth notification

[Alex_Gagniuc]

On 11/29/2018 10:06 AM, Mika Westerberg wrote:
>> @@ -515,7 +515,8 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id)
>>   	struct controller *ctrl = (struct controller *)dev_id;
>>   	struct pci_dev *pdev = ctrl_dev(ctrl);
>>   	struct device *parent = pdev->dev.parent;
>> -	u16 status, events;
>> +	struct pci_dev *endpoint;
>> +	u16 status, events, link_status;
> 
> Looks better if you write them in opposite order (reverse xmas-tree):
> 
> 	u16 status, events, link_status;
> 	struct pci_dev *endpoint;
> 

I don't decorate in November :p

>> +	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &link_status);
>> +
> 
> Unnecessary empty line.

However Bjorn wants it, though I don't like the crowded look with this 
line removed.

>> +	if (link_status & PCI_EXP_LNKSTA_LBMS) {
>> +		if (pdev->subordinate && pdev->subordinate->self)
>> +			endpoint = pdev->subordinate->self;
> 
> Hmm, I thought pdev->subordinate->self == pdev, no?

That makes no sense, but I think you're right. I'm trying to get to the 
other end of the PCIe link. Is there a simple way to do that? (other 
than convoluted logic that all leads to the same mistake)

>>   static void pcie_enable_notification(struct controller *ctrl)
>>   {
>>   	u16 cmd, mask;
>> @@ -713,6 +743,9 @@ static void pcie_enable_notification(struct controller *ctrl)
>>   	pcie_write_cmd_nowait(ctrl, cmd, mask);
>>   	ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
>>   		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, cmd);
>> +
>> +	if (pcie_link_bandwidth_notification_supported(ctrl))
>> +		pcie_enable_link_bandwidth_notification(ctrl);
> 
> Do we ever need to disable it?

I can't think of a case where that would be needed.

Alex