From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: MIME-Version: 1.0 In-Reply-To: <20170308223938.GA28050@bhelgaas-glaptop.roam.corp.google.com> References: <1483425255-101923-1-git-send-email-rajatja@google.com> <1483425255-101923-6-git-send-email-rajatja@google.com> <58C05114.5030202@arm.com> <20170308223938.GA28050@bhelgaas-glaptop.roam.corp.google.com> From: Rajat Jain Date: Wed, 8 Mar 2017 15:08:59 -0800 Message-ID: Subject: Re: [PATCH 5/6] PCI/ASPM: Actually configure the L1 substate settings to the device To: Bjorn Helgaas Cc: James Morse , Rajat Jain , Bjorn Helgaas , Keith Busch , Andreas Ziegler , Jonathan Yong , Shawn Lin , David Daney , Julia Lawall , Ram Amrani , Doug Ledford , Wang Sheng-Hui , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Brian Norris Content-Type: multipart/alternative; boundary=001a114fb0501f7c4a054a403968 List-ID: --001a114fb0501f7c4a054a403968 Content-Type: text/plain; charset=UTF-8 On Wed, Mar 8, 2017 at 2:39 PM, Bjorn Helgaas wrote: > Hi James, > > On Wed, Mar 08, 2017 at 06:44:36PM +0000, James Morse wrote: > > Hi! > > > > On 03/01/17 06:34, Rajat Jain wrote: > > > Add code to actually configure the L1 substate settigns on the > > > upstream and downstream device, while taking care of the rules > > > dictated by the PCIe spec. > > > > While testing hibernate on an arm64 juno with v4.11-rc1, I get a NULL > pointer > > dereference from pcie_config_aspm_link(): > > > > > > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > > > index a70afdf..6735f38 100644 > > > --- a/drivers/pci/pcie/aspm.c > > > +++ b/drivers/pci/pcie/aspm.c > > > @@ -597,11 +683,23 @@ static void pcie_config_aspm_dev(struct pci_dev > *pdev, u32 val) > > > static void pcie_config_aspm_link(struct pcie_link_state *link, u32 > state) > > > { > > > u32 upstream = 0, dwstream = 0; > > > - struct pci_dev *child, *parent = link->pdev; > > > + struct pci_dev *child = link->downstream, *parent = link->pdev; > > > > > > Here link->downstream is NULL, > > Sorry about the breakage. Can you try also cherry-picking this > commit: > > https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci. > git/commit/?h=for-linus&id=3bd7db63a841e8c5297bb18ad801df67d5e38ad2 > > Yinghai tripped over a similar problem in a different way, but I > suspect his fix might also fix the problem you're seeing. > Yes, I think that should fix it. Sorry for the breakage, can you please let me know if it doesn't fix it for you. Rajat > > If that doesn't fix it, we'll have to look farther. > > I'm planning to ask Linus to pull this fix for v4.11-rc2. > > Bjorn > --001a114fb0501f7c4a054a403968 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

On Wed, Mar 8, 2017 at 2:39 PM, Bjorn Helgaas <helgaas@kernel.org= > wrote:

Hi James,

On Wed, Mar 08, 2017 at 06:44:36PM +0000, James Morse wrote:
> Hi!
>
> On 03/01/17 06:34, Rajat Jain wrote:
> > Add code to actually configure the L1 substate settigns on the > > upstream and downstream device, while taking care of the rules > > dictated by the PCIe spec.
>
> While testing hibernate on an arm64 juno with v4.11-rc1, I get = a NULL pointer
> dereference from pcie_config_aspm_link():
>
>
> > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > > index a70afdf..6735f38 100644
> > --- a/drivers/pci/pcie/aspm.c
> > +++ b/drivers/pci/pcie/aspm.c
> > @@ -597,11 +683,23 @@ static void pcie_co= nfig_aspm_dev(struct pci_dev *pdev, u32 val)
> >=C2=A0 static void pcie_config_aspm_link(struct pcie_link_state *l= ink, u32 state)
> >=C2=A0 {
> >=C2=A0 =C2=A0 =C2=A0u32 upstream =3D 0, dwstream =3D 0;
> > -=C2=A0 =C2=A0struct pci_dev *child, *parent =3D link->pdev; > > +=C2=A0 =C2=A0struct pci_dev *child =3D link->downstream, *par= ent =3D link->pdev;
>
>
> Here link->downstream is NULL,

Sorry about the breakage.=C2=A0 Can you try also cherry-picking this
commit:

https://git.kernel.org/cgit/linux/ke= rnel/git/helgaas/pci.git/commit/?h=3Dfor-linus&id=3D3bd7db63a= 841e8c5297bb18ad801df67d5e38ad2

Yinghai tripped over a similar problem in a different way, but I
suspect his fix might also fix the problem you're seeing.

Yes, I think that should fix it.

Sorry for the breakage, can you please let me know if it doesn't = fix it for you.

Rajat
=C2=A0

If that doesn't fix it, we'll have to look farther.

I'm planning to ask Linus to pull this fix for v4.11-rc2.

Bjorn

--001a114fb0501f7c4a054a403968--