public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] PCI: imx6: fix downstream bus scanning
Date: Tue, 23 May 2017 10:07:29 +0100	[thread overview]
Message-ID: <20170523090729.GB18204@red-moon> (raw)
In-Reply-To: <20170522221346.GG19733@bhelgaas-glaptop.roam.corp.google.com>

On Mon, May 22, 2017 at 05:13:46PM -0500, Bjorn Helgaas wrote:
> On Wed, May 10, 2017 at 07:57:52PM +0200, Lucas Stach wrote:
> > The change in Linux 4.12 to make PCI configuartion requests non-posted
> > means that we are now getting a synchronous abort when the CFG space
> > read to probe for downstream devices times out.
> > 
> > Synchronous aborts need to be handled differently from the async aborts
> > we were getting before, in particular the PC needs to be advanced when
> > resolving the abort. This is mostly a copy of what other PCI drivers do
> > on ARM to handle those aborts.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> 
> Applied to for-linus for v4.12 with tested-by and acks from Fabio,
> Peter, and Richard.  I updated the subject line to:
> 
>   PCI: imx6: Fix config read timeout handling
> 
> because I don't think this change is specific to bus scanning and I
> wanted a hint that it's related to the config space mapping changes.
> 
> In fact, I'd really like to include the specific commit that caused
> this problem so that if the original commit is backported, there's a
> hint that we should backport this one, too.  I *think* it might be
> cc7b0d495589, so I updated the changelog to this:
> 
>   Commit cc7b0d495589 ("PCI: designware: Update PCI config space remap 
>   function") made PCI configuration requests non-posted, which means we now
>   get a synchronous abort when the CFG space read to probe for downstream
>   devices times out. 
> 
>   Synchronous aborts need to be handled differently from the async aborts we
>   were getting before, in particular the PC needs to be advanced when
>   resolving the abort.  This is mostly a copy of what other PCI drivers do on
>   ARM to handle those aborts.
> 
>   [bhelgaas: changelog, "Fixes"]
>   Fixes: cc7b0d495589 ("PCI: designware: Update PCI config space remap function")
> 
> Please let me know if this is the wrong commit.

I think it is the right commit but it is better if Lucas can confirm,
apologies for the breakage caused, I just could not test on iMX6.

Thanks,
Lorenzo

> > This is a fix that needs to go in for 4.12, but I would hope to get
> > some thorough testing before.
> > ---
> >  drivers/pci/dwc/pci-imx6.c | 33 ++++++++++++++++++++++++++++++---
> >  1 file changed, 30 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> > index a98cba55c7f0..19a289b8cc94 100644
> > --- a/drivers/pci/dwc/pci-imx6.c
> > +++ b/drivers/pci/dwc/pci-imx6.c
> > @@ -252,7 +252,34 @@ static void imx6_pcie_reset_phy(struct imx6_pcie *imx6_pcie)
> >  static int imx6q_pcie_abort_handler(unsigned long addr,
> >  		unsigned int fsr, struct pt_regs *regs)
> >  {
> > -	return 0;
> > +	unsigned long pc = instruction_pointer(regs);
> > +	unsigned long instr = *(unsigned long *)pc;
> > +	int reg = (instr >> 12) & 15;
> > +
> > +	/*
> > +	 * If the instruction being executed was a read,
> > +	 * make it look like it read all-ones.
> > +	 */
> > +	if ((instr & 0x0c100000) == 0x04100000) {
> > +		unsigned long val;
> > +
> > +		if (instr & 0x00400000)
> > +			val = 255;
> > +		else
> > +			val = -1;
> > +
> > +		regs->uregs[reg] = val;
> > +		regs->ARM_pc += 4;
> > +		return 0;
> > +	}
> > +
> > +	if ((instr & 0x0e100090) == 0x00100090) {
> > +		regs->uregs[reg] = -1;
> > +		regs->ARM_pc += 4;
> > +		return 0;
> > +	}
> > +
> > +	return 1;
> >  }
> >  
> >  static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > @@ -819,8 +846,8 @@ static int __init imx6_pcie_init(void)
> >  	 * we can install the handler here without risking it
> >  	 * accessing some uninitialized driver state.
> >  	 */
> > -	hook_fault_code(16 + 6, imx6q_pcie_abort_handler, SIGBUS, 0,
> > -			"imprecise external abort");
> > +	hook_fault_code(8, imx6q_pcie_abort_handler, SIGBUS, 0,
> > +			"external abort on non-linefetch");
> >  
> >  	return platform_driver_register(&imx6_pcie_driver);
> >  }
> > -- 
> > 2.11.0
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2017-05-23  9:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-10 17:57 [PATCH] PCI: imx6: fix downstream bus scanning Lucas Stach
2017-05-10 18:35 ` Fabio Estevam
2017-05-10 19:04   ` Fabio Estevam
2017-05-10 19:51 ` Peter Senna Tschudin
2017-05-11  2:00 ` Richard Zhu
2017-05-22 22:13 ` Bjorn Helgaas
2017-05-23  9:07   ` Lorenzo Pieralisi [this message]
2017-05-23 22:11   ` Fabio Estevam
2017-05-24 17:16     ` Bjorn Helgaas

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=20170523090729.GB18204@red-moon \
    --to=lorenzo.pieralisi@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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