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
| ||
|
Date: Fri, 14 Jun 2013 09:55:50 +0200 From: Rojhalat Ibrahim <imr@...chenk.de> To: Scott Wood <scottwood@...escale.com> Cc: Michael Guntsche <michael.guntsche@...loops.com>, linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org Subject: Re: [BUG] PCI related panic on powerpc based board with 3.10-rcX On Thursday 13 June 2013 11:49:17 Scott Wood wrote: > On 06/13/2013 02:21:24 AM, Rojhalat Ibrahim wrote: > > On Wednesday 12 June 2013 16:50:26 Scott Wood wrote: > > > On 06/12/2013 03:19:30 AM, Rojhalat Ibrahim wrote: > > > > On Tuesday 11 June 2013 12:28:59 Scott Wood wrote: > > > > > Yes, I figured it was non-PCIe because the code change that you > > > > said > > > > > > > helped was on the non-PCIe branch of the if/else. Generally > > > > it's > > > > > > good > > > > > > > > > to explicitly mention the chip you're using, though. > > > > > > > > > > fsl_setup_indirect_pci should be renamed to > > > > fsl_setup_indirect_pcie. > > > > > > > Your patch above should be applied, and fsl_setup_indirect_pcie > > > > > > > > should > > > > > > > > > be moved into the booke/86xx ifdef to avoid an unused function > > > > > > > > warning. > > > > > > > > > -Scott > > > > > > > > How about this patch? It uses setup_indirect_pci for the PCI case > > > > in > > > > > > mpc83xx_add_bridge. Additionally it adds a check in > > > > fsl_setup_indirect_pci > > > > to only use the modified read function in case of PCIe. > > > > > > If we're adding the check to fsl_setup_indirect_pci, there's no > > > > need to > > > > > change the 83xx call back to setup_indirect_pci. I see that 85xx is > > > also callirng fsl_setup_indirect_pci for both; it'd be good to be > > > consistent. > > > > > > In any case, can you send a proper patch with a signoff and commit > > > message? > > > > > > -Scott > > > > Where is it called for 85xx? As far as I can tell > > fsl_setup_indirect_pci is > > called exactly once in fsl_add_bridge and nowhere else (after > > applying the > > proposed patch). > > fsl_add_bridge() is where it's called for 85xx. > > > For 83xx the decision between PCI and PCIe has already been made at > > the point where the setup function is called. So IMO it doesn't make > > sense > > to call fsl_setup_indirect_pci and do the check again. Moreover PCIe > > on 83xx > > uses a completely different set of functions. > > My concern is consistency. E.g. if 85xx is using > fsl_setup_indirect_pci for both, but 83xx isn't, then a developer using > 83xx could end up breaking 85xx by introducing another PCIe dependency > in fsl_setup_indirect_pci. Or an 85xx developer could put something > non-PCIe-related in fsl_setup_indirect_pci that 83xx would benefit from. > > Alternatively, you could call it fsl_setup_indirect_pcie, and move the > PCIe check into fsl_add_bridge(). > > -Scott Ok. I'll post a v2 of the patch. Rojhalat -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists