linux-kernel - Re: [PATCH v8 19/30] powerpc/pci: Use pci_scan_host

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1427321631.2575.3.camel@axtens.net>
Date:	Thu, 26 Mar 2015 09:13:51 +1100
From:	Daniel Axtens <dja@...ens.net>
To:	Yijing Wang <wangyijing@...wei.com>
Cc:	Liviu Dudau <liviu@...au.co.uk>,
	Rusty Russell <rusty@...tcorp.com.au>,
	linux-ia64@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
	Marc Zyngier <marc.zyngier@....com>, linux-pci@...r.kernel.org,
	x86@...nel.org, linux-kernel@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	linuxppc-dev@...ts.ozlabs.org, linux-m68k@...ts.linux-m68k.org,
	Tony Luck <tony.luck@...el.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	linux-alpha@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
	Russell King <linux@....linux.org.uk>,
	Thomas Gleixner <tglx@...utronix.de>,
	Guan Xuetao <gxt@...c.pku.edu.cn>,
	Yinghai Lu <yinghai@...nel.org>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v8 19/30] powerpc/pci: Use pci_scan_host_bridge() for
 simplicity

Hi Yijing,

I wasn't quite sure I understood your comments, so I was trying to apply
your patch series and test it, but patch 3 doesn't apply cleanly to
4.0-rc5 or master. Can you respin the series?

Thanks,
Daniel


> Hi Daniel, thanks for your review and comments. We want to make a generic pci_host_bridge,
> which would hold the common host information, for example, pci domain is common info for
> pci host bridge, this series saved domain in pci_host_bridge, then we no need to
> extract out domain by pci_bus->sysdata by platform specific pci_domain_nr().
> Also we store the sysdata in pci_host_bridge, and pci_bus_to_host() is the platform
> interface, I think use the common interface would be better.
> 
> >> +
> >> +	/* Get probe mode and perform scan */
> >> +	if (hose->dn && ppc_md.pci_probe_mode)
> >> +		mode = ppc_md.pci_probe_mode(bus);
> >> +
> >> +	pr_debug("    probe mode: %d\n", mode);
> >> +	if (mode == PCI_PROBE_DEVTREE)
> >> +		of_scan_bus(hose->dn, bus);
> >> +
> >> +	if (mode == PCI_PROBE_NORMAL) {
> >> +		pci_bus_update_busn_res_end(bus, 255);
> >> +		hose->last_busno = pci_scan_child_bus(bus);
> >> +		pci_bus_update_busn_res_end(bus, hose->last_busno);
> >> +	}
> >> +
> >> +	return pci_bus_child_max_busnr(bus);
> >> +}
> >> +
> > I'm having trouble convincing myself that this patch covers every
> > variation within our PCI implementations. In particular, there's a
> > stanza in of_scan_pci_bridge in kernel/pci_of_scan.c that's almost
> > identical to this function. Does that implementation need to be cleaned
> > up and replaced with this function too?
> > 
> 
> This is a pci_host_bridge_ops hook function, which would be called in
> PCI core, and after applied this series, we only need to call pci_scan_host_bridge()
> to scan pci devices, and this function is also extracted from the pcibios_scan_phb(),
> it's not the redundant code.
> 
> > 
> >> @@ -1641,9 +1655,9 @@ void pcibios_scan_phb(struct pci_controller *hose)
> >>  		ppc_md.pcibios_fixup_phb(hose);
> >>  
> >>  	/* Configure PCI Express settings */
> >> -	if (bus && !pci_has_flag(PCI_PROBE_ONLY)) {
> >> +	if (host->bus && !pci_has_flag(PCI_PROBE_ONLY)) {
> >>  		struct pci_bus *child;
> >> -		list_for_each_entry(child, &bus->children, node)
> >> +		list_for_each_entry(child, &host->bus->children, node)
> >>  			pcie_bus_configure_settings(child);
> >>  	}
> >>  }
> > Two things: Firstly, the function uses hose throughout, not host.
> > Secondly, you're not deleting the bus variable: what's the purpose of
> > this change?
> 
> host is the common pci_host_bridge which is created by PCI core for pci host bridge driver,
> the hose is the platform data used in powerpc. The purpose of the patch/series is to simplify
> pci enumeration interface, and try to reduce the weak functions which were used to setup pci bus/devices
> during PCI enumeration.
> 
> > 
> > Regards,
> > Daniel
> > 
> 
> 


Download attachment "signature.asc" of type "application/pgp-signature" (861 bytes)