From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from xes-mad.com (xes-mad.com [216.165.139.218]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 00D451A0054 for ; Sat, 23 Aug 2014 03:57:00 +1000 (EST) Date: Fri, 22 Aug 2014 12:54:53 -0500 (CDT) From: Aaron Sierra To: Scott Wood Message-ID: <964109192.130707.1408730093575.JavaMail.zimbra@xes-inc.com> In-Reply-To: <1408658506.2395.11.camel@snotra.buserror.net> References: <1335064415.96829.1408578704825.JavaMail.zimbra@xes-inc.com> <1408655996.2395.7.camel@snotra.buserror.net> <529271512.72955.1408658076028.JavaMail.zimbra@xes-inc.com> <1408658506.2395.11.camel@snotra.buserror.net> Subject: Re: [PATCH] powerpc: fsl_pci: Fix PCI/PCI-X regression MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Cc: Minghuan Lian , linuxppc-dev@lists.ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , ----- Original Message ----- > From: "Scott Wood" > Sent: Thursday, August 21, 2014 5:01:46 PM > > On Thu, 2014-08-21 at 16:54 -0500, Aaron Sierra wrote: > > ----- Original Message ----- > > > From: "Scott Wood" > > > Sent: Thursday, August 21, 2014 4:19:56 PM > > > > > > On Wed, 2014-08-20 at 18:51 -0500, Aaron Sierra wrote: > > > > @@ -520,9 +520,22 @@ int fsl_add_bridge(struct platform_device *pdev, > > > > int > > > > is_primary) > > > > goto no_bridge; > > > > > > > > } else { > > > > - /* For PCI read PROG to identify controller mode */ > > > > - early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif); > > > > - if ((progif & 1) == 1) > > > > + u16 master; > > > > + > > > > + /* > > > > + * If the controller is PCI-X, then Host mode refers to a > > > > + * bridge that drives the PCI-X initialization pattern to > > > > + * indicate bus operating mode/frequency to devices on the bus. > > > > + * Some hardware (specifically PrPMC modules) are Agents, since > > > > + * the mezzanine carrier is responsible for driving the > > > > + * pattern, but they still may perform bus enumeration. > > > > + * > > > > + * Allow the bridge to be used for enumeration, if hardware > > > > + * strapping (Host mode) or firmware (Agent mode) has enabled > > > > + * bus mastering. > > > > + */ > > > > + early_read_config_word(hose, 0, 0, PCI_COMMAND, &master); > > > > + if (!(master & PCI_COMMAND_MASTER)) > > > > goto no_bridge; > > > > } > > > > > > Why wouldn't a normal PCI agent be able to bus master? > > > > > > -Scott > > > > > > > Short answer: > > > > Simply because the hardware strapping for Host/Agent determines the > > default state of the Bus Master bit in the Command register. Without > > that bit being set, an Agent won't be able to send the PCI cycles > > necessary to enumerate the bus. > > But what if the host has already set that bit before Linux boots? That's a very good point. I think that concern can be addressed by looking for another telltale sign of enumeration, whether an address has been assigned to the bridge's BAR 0 (PCSRBAR). > > Long answer: > > > > I think there was an assumption in the patch that introduced the > > regression that Host and Agent in conventional PCI and PCI-X are more > > equivalent to PCIe Root Complex and Endpoint than they really are. > > > > I think the purpose of Minghuan's patch was to not attempt to enumerate > > the bus with a bridge that simply cannot do it. A PCIe Endpoint cannot > > enumerate the bus and a PCI/PCI-X device that cannot master cycles on the > > bus will not be able to enumerate the bus either. > > No, the point is also to not enumerate the bus in cases where we > shouldn't. We don't want to step on the host's toes. Agreed. > > The hardware strapping for Host/Agent mode determines the default state > > of the Bus Master bit in the Command register. > > > > In our more specialized, but still industry standardized, environment our > > firmware must detect whether our board should be the bus's sole enumerator > > and set the Bus Master bit accordingly. > > > > My comment in the code still mentions Host and Agent mode, simply because > > the code it is replacing based its decision on the mode. > > I understand why you need to do this -- I just don't think this is a > reliable way of detecting that you're in that situation. How about a > kernel command line setting? I'd like to avoid requiring a kernel command-line option for this. Instead, I propose checking for Agent mode like Minghuan's patch, but prohibiting enumeration only if Bus Master is not enabled OR the address portion of PCSRBAR is non-zero (indicating another device has enumerated the bus). Pseudo code: if (agent && (!master || enumerated)) goto no_bridge; -Aaron