RE: [PATCH v2] PCI: imx6: Save and restore MSI control of RC in suspend and resume

From: Hongxing Zhu
Date: Sun Mar 12 2023 - 22:50:42 EST

Next message: Bagas Sanjaya: "[PATCH bpf-next] bpf, doc: use internal linking for link to netdev FAQ"
Previous message: Zheng Hacker: "Re: [PATCH v2] power: supply: da9150: Fix use after free bug in da9150_charger_remove due to race condition"
In reply to: Lorenzo Pieralisi: "Re: [PATCH v2] PCI: imx6: Save and restore MSI control of RC in suspend and resume"
Next in thread: Bjorn Helgaas: "Re: [PATCH v2] PCI: imx6: Save and restore MSI control of RC in suspend and resume"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

> -----Original Message-----
> From: Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx>
> Sent: 2023年3月11日 0:14
> To: Hongxing Zhu <hongxing.zhu@xxxxxxx>
> Cc: l.stach@xxxxxxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; dl-linux-imx
> <linux-imx@xxxxxxx>
> Subject: Re: [PATCH v2] PCI: imx6: Save and restore MSI control of RC in
> suspend and resume
>
> On Mon, Jan 09, 2023 at 02:08:06AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx>
> > > Sent: 2022年12月30日 23:06
> > > To: Hongxing Zhu <hongxing.zhu@xxxxxxx>; l.stach@xxxxxxxxxxxxxx;
> > > bhelgaas@xxxxxxxxxx
> > > Cc: linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> > > linux-kernel@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; dl-linux-imx
> > > <linux-imx@xxxxxxx>
> > > Subject: Re: [PATCH v2] PCI: imx6: Save and restore MSI control of
> > > RC in suspend and resume
> > >
> > > On Thu, Dec 08, 2022 at 02:05:34PM +0800, Richard Zhu wrote:
> > > > The MSI Enable bit controls delivery of MSI interrupts from
> > > > components below the Root Port. This bit might lost during the
> > > > suspend, should be re-stored during resume.
> > > >
> > > > Save the MSI control during suspend, and restore it in resume.
> > >
> > > I believe that what Lucas and Bjorn asked on v1 is still not answered.
> > >
> > > The root port is a PCI device, why do we need to save and restore
> > > the MSI cap on top of what PCI core already does ? The RP should be
> > > enumerated as a PCI device and therefore I expect the MSI cap to be
> > > saved/restored in the suspend/resume execution.
> > >
> > > I don't think there is anything iMX6 specific in this.
> > Hi Lorenzo:
> > Thanks for your comments.
> > Sorry to reply late, since I got a high fever in the past days.
> >
> > Based on i.MX6QP SABRESD board and XHCI PCIe2USB3.0 device, the MSI
> > cap save/restore of PCI core is not executed(dev->msi_enabled is
> > zero) during my suspend/resume tests.
>
> I still do not understand. The register you are saving/restoring in the RC is not
> the root port Message control field in the root port MSI capability, it is a
> separate register that controls the root complex MSI forwarding, is that
> correct ?
>
> The root port MSI capability does not control the root complex forwarding of
> MSIs TLPs.
>
> So the bits you are saving and restoring IIUC should be MMIO space in the
> root complex, dressed as an MSI capability, that has nothing to do with the
> root port MSI capability.
>
> Is that correct ?
Hi Lorenzo:
Thanks for your reply.
It's not a separate register.
The bit I manipulated is the MSI Enable bit of the Message Control Register
for MSI (Offset 02h) contained in the MSI-capability of Root Complex.
In addition, on i.MX6, the MSI Enable bit controls delivery of MSI
interrupts from components below the Root Port.
So, set MSI Enable in imx6q-pcie to let the MSI from downstream
components works.

Best Regards
Richard Zhu

>
> Thanks,
> Lorenzo
> >
> > It seems that some device might shutdown msi when do the suspend
> operations.
> > >
> > > Would you mind investigating it please ?
> > Sure, I did further investigation on i.MX6QP platform.
> > The MSI_EN bit of RC MSI capability would be cleared to zero, when
> > PCIE_RESET(BIT29 of IOMUXC_GPR1) is toggled (assertion 1b'1, then
> > de-assertion 1b'0).
> >
> > Verification steps:
> > MSI_EN of RC is set to 1b'1 when system is boot up.
> > ./memtool 1ffc050 1
> > 0x01FFC050: 01017005
> >
> > Toggle PCIe reset of i.MX6QP.
> > root@imx6qpdlsolox:~# ./memtool 20e0004=68691005 Writing 32-bit value
> > 0x68691005 to address 0x020E0004 root@imx6qpdlsolox:~# ./memtool
> > 20e0004=48691005 Writing 32-bit value 0x48691005 to address
> 0x020E0004
> >
> > The MSI_EN bit of RC had been cleared to 1b'0.
> > ./memtool 1ffc050 1
> > 0x01FFC050: 01807005
> >
> > This is why I used to reply to Bjorn the MSI_EN of RC is cleared when
> > RESETs are toggled during the imx6_pcie_host_init() in
> > imx6_pcie_resume_noirq() callback.
> >
> > Best Regards
> > Richard Zhu
> > >
> > > Lorenzo
> > >
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx>
> > > > ---
> > > > Changes v1-->v2:
> > > > New create one save/restore function, used save the setting in
> > > > suspend and restore the configuration in resume.
> > > > v1
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > >
> patc%2F&data=05%7C01%7Chongxing.zhu%40nxp.com%7C24971d8de9b54b
> 0b10
> > > >
> ad08db2182774d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
> 38140
> > > >
> 616456052078%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
> QIjoiV
> > > >
> 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=vE
> tRxL
> > > > BVi5lYmpwTNZfafMms3263LZXodneLChjEaOM%3D&reserved=0
> > > >
> > >
> hwork.kernel.org%2Fproject%2Flinux-pci%2Fpatch%2F1667289595-12440-1-
> > > g
> > > i
> > > >
> > >
> t-send-email-hongxing.zhu%40nxp.com%2F&data=05%7C01%7Chongxing.zhu
> > > %40n
> > > >
> > >
> xp.com%7C3aeb1d128f854dad1a5608daea77706d%7C686ea1d3bc2b4c6fa9
> 2
> > > cd99c5c
> > > >
> > >
> 301635%7C0%7C0%7C638080095954881374%7CUnknown%7CTWFpbGZsb3
> > > d8eyJWIjoiMC
> > > >
> > >
> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
> %
> > > 7C%7C%
> > > >
> > >
> 7C&sdata=V8yVvvpTKGoR1UyQP5HD2IdlSjJdznBeD12bdI67dEI%3D&reserved
> =
> > > 0
> > > >
> > > > ---
> > > > drivers/pci/controller/dwc/pci-imx6.c | 23
> > > > +++++++++++++++++++++++
> > > > 1 file changed, 23 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > > index 1dde5c579edc..aa3096890c3b 100644
> > > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > > @@ -76,6 +76,7 @@ struct imx6_pcie {
> > > > struct clk *pcie;
> > > > struct clk *pcie_aux;
> > > > struct regmap *iomuxc_gpr;
> > > > + u16 msi_ctrl;
> > > > u32 controller_id;
> > > > struct reset_control *pciephy_reset;
> > > > struct reset_control *apps_reset;
> > > > @@ -1042,6 +1043,26 @@ static void imx6_pcie_pm_turnoff(struct
> > > imx6_pcie *imx6_pcie)
> > > > usleep_range(1000, 10000);
> > > > }
> > > >
> > > > +static void imx6_pcie_msi_save_restore(struct imx6_pcie
> > > > +*imx6_pcie, bool save) {
> > > > + u8 offset;
> > > > + u16 val;
> > > > + struct dw_pcie *pci = imx6_pcie->pci;
> > > > +
> > > > + if (pci_msi_enabled()) {
> > > > + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> > > > + if (save) {
> > > > + val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> > > > + imx6_pcie->msi_ctrl = val;
> > > > + } else {
> > > > + dw_pcie_dbi_ro_wr_en(pci);
> > > > + val = imx6_pcie->msi_ctrl;
> > > > + dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> > > > + dw_pcie_dbi_ro_wr_dis(pci);
> > > > + }
> > > > + }
> > > > +}
> > > > +
> > > > static int imx6_pcie_suspend_noirq(struct device *dev) {
> > > > struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); @@ -1050,6
> > > > +1071,7 @@ static int imx6_pcie_suspend_noirq(struct device *dev)
> > > > if (!(imx6_pcie->drvdata->flags &
> > > IMX6_PCIE_FLAG_SUPPORTS_SUSPEND))
> > > > return 0;
> > > >
> > > > + imx6_pcie_msi_save_restore(imx6_pcie, true);
> > > > imx6_pcie_pm_turnoff(imx6_pcie);
> > > > imx6_pcie_stop_link(imx6_pcie->pci);
> > > > imx6_pcie_host_exit(pp);
> > > > @@ -1069,6 +1091,7 @@ static int imx6_pcie_resume_noirq(struct
> > > > device
> > > *dev)
> > > > ret = imx6_pcie_host_init(pp);
> > > > if (ret)
> > > > return ret;
> > > > + imx6_pcie_msi_save_restore(imx6_pcie, false);
> > > > dw_pcie_setup_rc(pp);
> > > >
> > > > if (imx6_pcie->link_is_up)
> > > > --
> > > > 2.25.1
> > > >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=05%7C01
> %7C
> >
> hongxing.zhu%40nxp.com%7C24971d8de9b54b0b10ad08db2182774d%7C68
> 6ea1d3bc
> >
> 2b4c6fa92cd99c5c301635%7C0%7C0%7C638140616456052078%7CUnknow
> n%7CTWFpbG
> >
> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn0%
> >
> 3D%7C3000%7C%7C%7C&sdata=JR6JKVNQGiZtxYbdD%2B4P9u7qgSMVGqQ
> PdN1CpN%2BrV
> > Ik%3D&reserved=0

Next message: Bagas Sanjaya: "[PATCH bpf-next] bpf, doc: use internal linking for link to netdev FAQ"
Previous message: Zheng Hacker: "Re: [PATCH v2] power: supply: da9150: Fix use after free bug in da9150_charger_remove due to race condition"
In reply to: Lorenzo Pieralisi: "Re: [PATCH v2] PCI: imx6: Save and restore MSI control of RC in suspend and resume"
Next in thread: Bjorn Helgaas: "Re: [PATCH v2] PCI: imx6: Save and restore MSI control of RC in suspend and resume"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]