Re: [PATCHv4 1/2] ppc64: perform proper max_bus_speed detection
On 05/03/2013 03:40 AM, Tony Breeds wrote: > On Thu, May 02, 2013 at 12:21:37PM -0300, Kleber Sacilotto de Souza wrote: > >> Hi Tony, >> >> It seems Lucas' change is a bit incomplete and is not handling the reference >> counter to >> the device_node correctly. Is the following change what you had in mind? > > Ahh Sorry I expected there would be a for_each_parent_of_node macro. > I did a quick grep and it seems that's not very common, so open coding > it should be fine. > >> >> dn = pcibios_get_phb_of_node(bus); >> if (!dn) >> return 0; >> >> for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) { >> pcie_link_speed_stats = (const uint32_t *) of_get_property(pdn, >> "ibm,pcie-link-speed-stats", NULL); >> if (pcie_link_speed_stats) >> break; >> } >> >> of_node_put(pdn); > > I think you need the of_node_put() in the body of the loop, otherwise > aren't you leaking refcounts? of_get_next_parent() takes care of that. It does of_node_put() on the current node after doing of_node_get() on the parent. Thanks, -- Kleber Sacilotto de Souza IBM Linux Technology Center ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCHv4 1/2] ppc64: perform proper max_bus_speed detection
On Thu, May 02, 2013 at 12:21:37PM -0300, Kleber Sacilotto de Souza wrote: > Hi Tony, > > It seems Lucas' change is a bit incomplete and is not handling the reference > counter to > the device_node correctly. Is the following change what you had in mind? Ahh Sorry I expected there would be a for_each_parent_of_node macro. I did a quick grep and it seems that's not very common, so open coding it should be fine. > > dn = pcibios_get_phb_of_node(bus); > if (!dn) > return 0; > > for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) { > pcie_link_speed_stats = (const uint32_t *) of_get_property(pdn, > "ibm,pcie-link-speed-stats", NULL); > if (pcie_link_speed_stats) > break; > } > > of_node_put(pdn); I think you need the of_node_put() in the body of the loop, otherwise aren't you leaking refcounts? Yours Tony pgpwRUenn1A7K.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCHv4 1/2] ppc64: perform proper max_bus_speed detection
On 04/25/2013 02:34 PM, Lucas Kannebley Tavares wrote: > On 04/24/2013 08:48 PM, Tony Breeds wrote: >>> diff --git a/arch/powerpc/platforms/pseries/pci.c >>> b/arch/powerpc/platforms/pseries/pci.c >>> index 0b580f4..7f9c956 100644 >>> --- a/arch/powerpc/platforms/pseries/pci.c >>> +++ b/arch/powerpc/platforms/pseries/pci.c >>> @@ -108,3 +108,54 @@ static void fixup_winbond_82c105(struct pci_dev* >>> dev) >>> } >>> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, >>> PCI_DEVICE_ID_WINBOND_82C105, >>>fixup_winbond_82c105); >>> + >>> +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge) >>> +{ >>> +struct device_node *dn, *pdn; >>> +struct pci_bus *bus; >>> +const uint32_t *pcie_link_speed_stats; >>> + >>> +bus = bridge->bus; >>> + >>> +dn = pcibios_get_phb_of_node(bus); >>> +if (!dn) >>> +return 0; >>> + >>> +for (pdn = dn; pdn != NULL; pdn = pdn->parent) { >>> +pcie_link_speed_stats = (const uint32_t *) of_get_property(dn, >>> +"ibm,pcie-link-speed-stats", NULL); >>> +if (pcie_link_speed_stats) >>> +break; >>> +} >> >> Please use the helpers in include/linux/of.h rather than open coding >> this. >> >> Yours Tony > > > Hi Tony, > > > This is what I can find as an equivalent code: > > for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) { > pcie_link_speed_stats = (const uint32_t *) > of_get_property(dn, > "ibm,pcie-link-speed-stats", NULL); > if (pcie_link_speed_stats) > break; > } > > is this your suggestion, or was it another approach that will have the > same result? > > Thanks, > Hi Tony, It seems Lucas' change is a bit incomplete and is not handling the reference counter to the device_node correctly. Is the following change what you had in mind? dn = pcibios_get_phb_of_node(bus); if (!dn) return 0; for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) { pcie_link_speed_stats = (const uint32_t *) of_get_property(pdn, "ibm,pcie-link-speed-stats", NULL); if (pcie_link_speed_stats) break; } of_node_put(pdn); Thanks, -- Kleber Sacilotto de Souza IBM Linux Technology Center ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCHv4 1/2] ppc64: perform proper max_bus_speed detection
On 04/24/2013 08:48 PM, Tony Breeds wrote: On Wed, Apr 24, 2013 at 07:54:49PM -0300, luca...@linux.vnet.ibm.com wrote: From: Lucas Kannebley Tavares On pseries machines the detection for max_bus_speed should be done through an OpenFirmware property. This patch adds a function to perform this detection and a hook to perform dynamic adding of the function only for pseries. This is done by overwriting the weak pcibios_root_bridge_prepare function which is called by pci_create_root_bus(). Signed-off-by: Lucas Kannebley Tavares --- arch/powerpc/include/asm/machdep.h | 2 ++ arch/powerpc/kernel/pci-common.c | 8 + arch/powerpc/platforms/pseries/pci.c | 51 arch/powerpc/platforms/pseries/pseries.h | 4 +++ arch/powerpc/platforms/pseries/setup.c | 2 ++ 5 files changed, 67 insertions(+) diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h index 3d6b410..8f558bf 100644 --- a/arch/powerpc/include/asm/machdep.h +++ b/arch/powerpc/include/asm/machdep.h @@ -107,6 +107,8 @@ struct machdep_calls { void(*pcibios_fixup)(void); int (*pci_probe_mode)(struct pci_bus *); void(*pci_irq_fixup)(struct pci_dev *dev); + int (*pcibios_root_bridge_prepare)(struct pci_host_bridge + *bridge); /* To setup PHBs when using automatic OF platform driver for PCI */ int (*pci_setup_phb)(struct pci_controller *host); diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index fa12ae4..80986cf 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -844,6 +844,14 @@ int pci_proc_domain(struct pci_bus *bus) return 1; } +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{ + if (ppc_md.pcibios_root_bridge_prepare) + return ppc_md.pcibios_root_bridge_prepare(bridge); + + return 0; +} + /* This header fixup will do the resource fixup for all devices as they are * probed, but not for bridge ranges */ diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c index 0b580f4..7f9c956 100644 --- a/arch/powerpc/platforms/pseries/pci.c +++ b/arch/powerpc/platforms/pseries/pci.c @@ -108,3 +108,54 @@ static void fixup_winbond_82c105(struct pci_dev* dev) } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105, fixup_winbond_82c105); + +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge) +{ + struct device_node *dn, *pdn; + struct pci_bus *bus; + const uint32_t *pcie_link_speed_stats; + + bus = bridge->bus; + + dn = pcibios_get_phb_of_node(bus); + if (!dn) + return 0; + + for (pdn = dn; pdn != NULL; pdn = pdn->parent) { + pcie_link_speed_stats = (const uint32_t *) of_get_property(dn, + "ibm,pcie-link-speed-stats", NULL); + if (pcie_link_speed_stats) + break; + } Please use the helpers in include/linux/of.h rather than open coding this. Yours Tony Hi Tony, This is what I can find as an equivalent code: for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) { pcie_link_speed_stats = (const uint32_t *) of_get_property(dn, "ibm,pcie-link-speed-stats", NULL); if (pcie_link_speed_stats) break; } is this your suggestion, or was it another approach that will have the same result? Thanks, -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCHv4 1/2] ppc64: perform proper max_bus_speed detection
On Wed, Apr 24, 2013 at 07:54:49PM -0300, luca...@linux.vnet.ibm.com wrote: > From: Lucas Kannebley Tavares > > On pseries machines the detection for max_bus_speed should be done > through an OpenFirmware property. This patch adds a function to perform > this detection and a hook to perform dynamic adding of the function only for > pseries. This is done by overwriting the weak > pcibios_root_bridge_prepare function which is called by pci_create_root_bus(). > > Signed-off-by: Lucas Kannebley Tavares > --- > arch/powerpc/include/asm/machdep.h | 2 ++ > arch/powerpc/kernel/pci-common.c | 8 + > arch/powerpc/platforms/pseries/pci.c | 51 > > arch/powerpc/platforms/pseries/pseries.h | 4 +++ > arch/powerpc/platforms/pseries/setup.c | 2 ++ > 5 files changed, 67 insertions(+) > > diff --git a/arch/powerpc/include/asm/machdep.h > b/arch/powerpc/include/asm/machdep.h > index 3d6b410..8f558bf 100644 > --- a/arch/powerpc/include/asm/machdep.h > +++ b/arch/powerpc/include/asm/machdep.h > @@ -107,6 +107,8 @@ struct machdep_calls { > void(*pcibios_fixup)(void); > int (*pci_probe_mode)(struct pci_bus *); > void(*pci_irq_fixup)(struct pci_dev *dev); > + int (*pcibios_root_bridge_prepare)(struct pci_host_bridge > + *bridge); > > /* To setup PHBs when using automatic OF platform driver for PCI */ > int (*pci_setup_phb)(struct pci_controller *host); > diff --git a/arch/powerpc/kernel/pci-common.c > b/arch/powerpc/kernel/pci-common.c > index fa12ae4..80986cf 100644 > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -844,6 +844,14 @@ int pci_proc_domain(struct pci_bus *bus) > return 1; > } > > +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) > +{ > + if (ppc_md.pcibios_root_bridge_prepare) > + return ppc_md.pcibios_root_bridge_prepare(bridge); > + > + return 0; > +} > + > /* This header fixup will do the resource fixup for all devices as they are > * probed, but not for bridge ranges > */ > diff --git a/arch/powerpc/platforms/pseries/pci.c > b/arch/powerpc/platforms/pseries/pci.c > index 0b580f4..7f9c956 100644 > --- a/arch/powerpc/platforms/pseries/pci.c > +++ b/arch/powerpc/platforms/pseries/pci.c > @@ -108,3 +108,54 @@ static void fixup_winbond_82c105(struct pci_dev* dev) > } > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105, >fixup_winbond_82c105); > + > +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge) > +{ > + struct device_node *dn, *pdn; > + struct pci_bus *bus; > + const uint32_t *pcie_link_speed_stats; > + > + bus = bridge->bus; > + > + dn = pcibios_get_phb_of_node(bus); > + if (!dn) > + return 0; > + > + for (pdn = dn; pdn != NULL; pdn = pdn->parent) { > + pcie_link_speed_stats = (const uint32_t *) of_get_property(dn, > + "ibm,pcie-link-speed-stats", NULL); > + if (pcie_link_speed_stats) > + break; > + } Please use the helpers in include/linux/of.h rather than open coding this. Yours Tony pgpTiexFyqHWG.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCHv4 1/2] ppc64: perform proper max_bus_speed detection
From: Lucas Kannebley Tavares On pseries machines the detection for max_bus_speed should be done through an OpenFirmware property. This patch adds a function to perform this detection and a hook to perform dynamic adding of the function only for pseries. This is done by overwriting the weak pcibios_root_bridge_prepare function which is called by pci_create_root_bus(). Signed-off-by: Lucas Kannebley Tavares --- arch/powerpc/include/asm/machdep.h | 2 ++ arch/powerpc/kernel/pci-common.c | 8 + arch/powerpc/platforms/pseries/pci.c | 51 arch/powerpc/platforms/pseries/pseries.h | 4 +++ arch/powerpc/platforms/pseries/setup.c | 2 ++ 5 files changed, 67 insertions(+) diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h index 3d6b410..8f558bf 100644 --- a/arch/powerpc/include/asm/machdep.h +++ b/arch/powerpc/include/asm/machdep.h @@ -107,6 +107,8 @@ struct machdep_calls { void(*pcibios_fixup)(void); int (*pci_probe_mode)(struct pci_bus *); void(*pci_irq_fixup)(struct pci_dev *dev); + int (*pcibios_root_bridge_prepare)(struct pci_host_bridge + *bridge); /* To setup PHBs when using automatic OF platform driver for PCI */ int (*pci_setup_phb)(struct pci_controller *host); diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index fa12ae4..80986cf 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -844,6 +844,14 @@ int pci_proc_domain(struct pci_bus *bus) return 1; } +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{ + if (ppc_md.pcibios_root_bridge_prepare) + return ppc_md.pcibios_root_bridge_prepare(bridge); + + return 0; +} + /* This header fixup will do the resource fixup for all devices as they are * probed, but not for bridge ranges */ diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c index 0b580f4..7f9c956 100644 --- a/arch/powerpc/platforms/pseries/pci.c +++ b/arch/powerpc/platforms/pseries/pci.c @@ -108,3 +108,54 @@ static void fixup_winbond_82c105(struct pci_dev* dev) } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105, fixup_winbond_82c105); + +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge) +{ + struct device_node *dn, *pdn; + struct pci_bus *bus; + const uint32_t *pcie_link_speed_stats; + + bus = bridge->bus; + + dn = pcibios_get_phb_of_node(bus); + if (!dn) + return 0; + + for (pdn = dn; pdn != NULL; pdn = pdn->parent) { + pcie_link_speed_stats = (const uint32_t *) of_get_property(dn, + "ibm,pcie-link-speed-stats", NULL); + if (pcie_link_speed_stats) + break; + } + + if (!pcie_link_speed_stats) { + pr_err("no ibm,pcie-link-speed-stats property\n"); + return 0; + } + + switch (pcie_link_speed_stats[0]) { + case 0x01: + bus->max_bus_speed = PCIE_SPEED_2_5GT; + break; + case 0x02: + bus->max_bus_speed = PCIE_SPEED_5_0GT; + break; + default: + bus->max_bus_speed = PCI_SPEED_UNKNOWN; + break; + } + + switch (pcie_link_speed_stats[1]) { + case 0x01: + bus->cur_bus_speed = PCIE_SPEED_2_5GT; + break; + case 0x02: + bus->cur_bus_speed = PCIE_SPEED_5_0GT; + break; + default: + bus->cur_bus_speed = PCI_SPEED_UNKNOWN; + break; + } + + return 0; +} diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h index 9a3dda0..b79393d 100644 --- a/arch/powerpc/platforms/pseries/pseries.h +++ b/arch/powerpc/platforms/pseries/pseries.h @@ -60,4 +60,8 @@ extern int dlpar_detach_node(struct device_node *); /* Snooze Delay, pseries_idle */ DECLARE_PER_CPU(long, smt_snooze_delay); +/* PCI root bridge prepare function override for pseries */ +struct pci_host_bridge; +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge); + #endif /* _PSERIES_PSERIES_H */ diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 8bcc9ca..bf34cc9 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -466,6 +466,8 @@ static void __init pSeries_setup_arch(void) else ppc_md.enable_pmcs = power4_enable_pmcs; + ppc_md.pcibios_root_bridge_prepare = pseries_root_bridge_prepare; + if (firmware_has_feature(FW_FEATURE_SET_MODE)) { long rc; if ((rc = pSeries_enable_reloc_on_exc()) !=