Re: [PATCH 4/4] drivers/pci/hotplug: Support surprise hotplug
From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2016-09-21 16:57:09
Also in:
linuxppc-dev
Hi Gavin, You don't need my ack for any of these, and I assume you'll merge them through the powerpc tree. Minor comments below, feel free to ignore them. On Wed, Sep 21, 2016 at 10:15:30PM +1000, Gavin Shan wrote:
quoted hunk ↗ jump to hunk
...@@ -536,9 +565,16 @@ static struct pnv_php_slot *pnv_php_alloc_slot(struct device_node *dn) if (unlikely(!php_slot)) return NULL; + php_slot->event = kzalloc(sizeof(struct pnv_php_event), GFP_KERNEL); + if (unlikely(!php_slot->event)) { + kfree(php_slot); + return NULL; + }
Since you *always* allocate the event when allocating the php_slot, making the event a member of php_slot (instead of keeping a pointer to it) would simplify your memory management a bit. It seems to be the style in this file to use "unlikely" liberally, but I really doubt there's any performance consideration in this code. To me it adds more clutter than usefulness.
+static irqreturn_t pnv_php_interrupt(int irq, void *data)
+{
+ struct pnv_php_slot *php_slot = data;
+ struct pci_dev *pchild, *pdev = php_slot->pdev;
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ struct pnv_php_event *event;
+ u16 sts, lsts;
+ u8 presence;
+ bool added;
+ unsigned long flags;
+ int ret;
+
+ pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &sts);
+ sts &= (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC);
+ pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, sts);I didn't realize that this is some sort of hybrid of native PCIe hotplug and PowerNV-specific stuff. Wonder if there's any opportunity to combine with or leverage pciehp. That seems pretty blue-sky though, since there's so much PowerNV special sauce here. Bjorn