[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <704ab07c-73b2-459e-9cb0-c9e49938454e@linux.ibm.com>
Date: Tue, 14 May 2024 17:35:48 +0530
From: krishna kumar <krishnak@...ux.ibm.com>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: nathanl@...ux.ibm.com, aneesh.kumar@...nel.org,
linux-kernel@...r.kernel.org, npiggin@...il.com, gbatra@...ux.ibm.com,
bhelgaas@...gle.com, tpearson@...torengineering.com, oohall@...il.com,
linux-pci@...r.kernel.org, brking@...ux.vnet.ibm.com,
mahesh.salgaonkar@...ibm.com, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH 2/2] arch/powerpc: hotplug driver bridge support
Thanks Andy for review. I have incorporated your comments and will be
sending the patch soon.
On 5/11/24 11:12, Andy Shevchenko wrote:
> Thu, May 09, 2024 at 05:35:54PM +0530, Krishna Kumar kirjoitti:
>> There is an issue with the hotplug operation when it's done on the
>> bridge/switch slot. The bridge-port and devices behind the bridge, which
>> become offline by hot-unplug operation, don't get hot-plugged/enabled by
>> doing hot-plug operation on that slot. Only the first port of the bridge
>> gets enabled and the remaining port/devices remain unplugged. The hot
>> plug/unplug operation is done by the hotplug driver
>> (drivers/pci/hotplug/pnv_php.c).
>>
>> Root Cause Analysis: This behavior is due to missing code for the DPC
>> switch/bridge. The existing driver depends on pci_hp_add_devices()
>> function for device enablement. This function calls pci_scan_slot() on
>> only one device-node/port of the bridge, not on all the siblings'
>> device-node/port.
>>
>> The missing code needs to be added which will find all the sibling
>> device-nodes/bridge-ports and will run explicit pci_scan_slot() on
>> those. A new function has been added for this purpose which gets
>> invoked from pci_hp_add_devices(). This new function
>> pci_traverse_sibling_nodes_and_scan_slot() gets all the sibling
>> bridge-ports by traversal and explicitly invokes pci_scan_slot on them.
>>
>>
> One blank line is enough here.
I have incorporated this.
>
>> Signed-off-by: Krishna Kumar <krishnak@...ux.ibm.com>
> ...
>
>> +void *pci_traverse_sibling_nodes_and_scan_slot(struct device_node *start, struct pci_bus *bus)
>> +{
>> + struct device_node *dn;
>> + struct device_node *parent;
>> + int slotno;
>> +
>> + const __be32 *classp1;
>> + u32 class1 = 0;
>> + classp1 = of_get_property(start->child, "class-code", NULL);
>> + if (classp1)
>> + class1 = of_read_number(classp1, 1);
> What's wrong with of_property_read_u32()?
I have incorporated this.
>
>
>> + /* Call of pci_scan_slot for non-bridge/EP case */
>> + if (!((class1 >> 8) == PCI_CLASS_BRIDGE_PCI)) {
>> + slotno = PCI_SLOT(PCI_DN(start->child)->devfn);
>> + pci_scan_slot(bus, PCI_DEVFN(slotno, 0));
>> + return NULL;
>> + }
>> +
>> + /* Iterate all siblings */
>> + parent = start;
>> + for_each_child_of_node(parent, dn) {
>> + const __be32 *classp;
>> + u32 class = 0;
>> +
>> + classp = of_get_property(dn, "class-code", NULL);
>> + if (classp)
>> + class = of_read_number(classp, 1);
> Ditto.
>
>> + /* Call of pci_scan_slot on each sibling-nodes/bridge-ports */
>> + if ((class >> 8) == PCI_CLASS_BRIDGE_PCI) {
>> + slotno = PCI_SLOT(PCI_DN(dn)->devfn);
>> + pci_scan_slot(bus, PCI_DEVFN(slotno, 0));
>> + }
>> + }
>> +
>> + return NULL;
>> +}
Best Regards,
Krishna
Powered by blists - more mailing lists