lkml.org 
[lkml]   [2018]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] PCI: pciehp: Report degraded links via link bandwidth notification
Date
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

\
 
 \ /
  Last update: 2018-11-29 20:02    [W:0.088 / U:3.092 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site