From: Aaron Sierra <asierra@xes-inc•com>
To: Scott Wood <scottwood@freescale•com>
Cc: Minghuan Lian <Minghuan.Lian@freescale•com>,
linuxppc-dev@lists•ozlabs.org
Subject: Re: [PATCH] powerpc: fsl_pci: Fix PCI/PCI-X regression
Date: Fri, 22 Aug 2014 12:54:53 -0500 (CDT) [thread overview]
Message-ID: <964109192.130707.1408730093575.JavaMail.zimbra@xes-inc.com> (raw)
In-Reply-To: <1408658506.2395.11.camel@snotra.buserror.net>
----- Original Message -----
> From: "Scott Wood" <scottwood@freescale•com>
> 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" <scottwood@freescale•com>
> > > 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
next prev parent reply other threads:[~2014-08-22 17:57 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1884375274.96749.1408578481814.JavaMail.zimbra@xes-inc.com>
2014-08-20 23:51 ` [PATCH] powerpc: fsl_pci: Fix PCI/PCI-X regression Aaron Sierra
2014-08-21 21:19 ` Scott Wood
2014-08-21 21:54 ` Aaron Sierra
2014-08-21 22:01 ` Scott Wood
2014-08-22 17:54 ` Aaron Sierra [this message]
2014-08-22 18:36 ` Scott Wood
2014-08-22 19:00 ` Aaron Sierra
2014-08-22 19:53 ` Scott Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=964109192.130707.1408730093575.JavaMail.zimbra@xes-inc.com \
--to=asierra@xes-inc$(echo .)com \
--cc=Minghuan.Lian@freescale$(echo .)com \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=scottwood@freescale$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox