[PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM support

[Jia Hongtao-B38951]

> -----Original Message-----
> From: Kumar Gala [mailto:galak at kernel.crashing.org]
> Sent: Thursday, September 27, 2012 8:06 PM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linuxppc-dev at lists.ozlabs.org; Li Yang-R58472
> Subject: Re: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM
> support
> 
> >>>>>>>>>>
> >>>>>>>>>> On Sep 17, 2012, at 9:10 PM, Jia Hongtao wrote:
> >>>>>>>>>>
> >>>>>>>>>>> Power supply for PCI inbound/outbound window registers is
> >>>>>>>>>>> off when system go to deep-sleep state. We save the values
> >>>>>>>>>>> of registers
> >>>>>>> before
> >>>>>>>>>>> suspend and restore to registers after resume.
> >>>>>>>>>>>
> >>>>>>>>>>> Signed-off-by: Jiang Yutang <b14898 at freescale.com>
> >>>>>>>>>>> Signed-off-by: Jia Hongtao <B38951 at freescale.com>
> >>>>>>>>>>> Signed-off-by: Li Yang <leoli at freescale.com>
> >>>>>>>>>>> ---
> >>>>>>>>>>> Changes for V4:
> >>>>>>>>>>> We just rebase the patch upon following patch:
> >>>>>>>>>>> powerpc/fsl-pci: Unify pci/pcie initialization code
> >>>>>>>>>>>
> >>>>>>>>>>> arch/powerpc/include/asm/pci-bridge.h |    2 +-
> >>>>>>>>>>> arch/powerpc/sysdev/fsl_pci.c         |  121
> >>>>>>>>>> +++++++++++++++++++++++++++++++++
> >>>>>>>>>>> 2 files changed, 122 insertions(+), 1 deletions(-)
> >>>>>>>>>>
> >>>>>>>>>> Did you ever compare this to just re-parsing device tree
> method?
> >>>>>>>>>>
> >>>>>>>>>> - k
> >>>>>>>>>
> >>>>>>>>> I tested the re-parsing way by using setup_pci_atmu() when
> >> resume.
> >>>>>>>>> And I found out that re-parsing will *change* outbound IO
> >>>>>>>>> translation address regitster.
> >>>>>>>>>
> >>>>>>>>> It seems that in the first bootup, after setup_atmu()
> >>>>>>>>> pcibios_setup_phb_resources() may update hose->io_resource,
> >>>>>>>>> but atmu is not updated according to the new hose->io_resource
> value.
> >>>>>>>>> In resume from sleep setup_atmu() will reset atmu according to
> >>>>>>>>> the new hose->io_resource value. So the setup_atmu() will
> >>>>>>>>> cause different result on outbound IO register between first
> >>>>>>>>> bootup and resume from sleep.
> >>>>>>>>>
> >>>>>>>>> So... There's a possibility that in the first bootup atmu is
> >>>>>>>>> not setup properly.
> >>>>>>>>
> >>>>>>>> [Are you seeing this happen in your testing?  If so its a bug
> >>>>>>>> we need
> >>>>>>> to look at fixing.]
> >>>>>>>>
> >>>>>>>> Yes, I see this in my testing.
> >>>>>>>> Also PCIe ethernet card works well after resuming from sleep in
> >>>>>>>> both
> >>>>>>> save/restore
> >>>>>>>> and re-parsing way. (Maybe PCIe ethernet card don't need
> >>>>>>>> outbound IO
> >>>>>>> resource)
> >>>>>>>> So, I guess the result of re-parsing (actually it's re-setup)
> >>>>>>>> is right
> >>>>>>> and ATMU is not setup
> >>>>>>>> properly at the first bootup.
> >>>>>>>
> >>>>>>> Are you seeing the following message - "PCI: I/O resource not
> >>>>>>> set for host bridge" ?
> >>>>>>
> >>>>>> No.
> >>>>>>
> >>>>>>>
> >>>>>>> Trying to understand why you'd hit the reassignment of
> io_resource.
> >>>>>>>
> >>>>>>> - k
> >>>>>>>
> >>>>>>
> >>>>>> I did some investigations and the conclusion is:
> >>>>>>
> >>>>>> io_resource.flags & IORESOURCE_IO are both positive but
> >>>>>> io_resource.start is 0 before pcibios_setup_phb_io_space() is done.
> >>>>>>
> >>>>>> The sequence of related process listed below:
> >>>>>> fsl_add_bridge() -> setup_pci_atmu()
> >>>>>> pcibios_init() -> pcibios_scan_phb() ->
> >>>>>> pcibios_setup_phb_io_space()
> >>>>>>
> >>>>>> Because fsl_add_bridge() must be finished before pcibios_init()
> >>>>>> so ATMU is set when io_resource.start is 0. That means outbound
> >>>>>> IO regs are not set.
> >>>>>>
> >>>>>> If system re-setup ATMU the io_resource.start has already updated
> >>>>>> so outbound IO regs are set.
> >>>>>>
> >>>>>> My question is:
> >>>>>> Is there any problem if outbound IO regs are not set in first
> >> bootup?
> >>>
> >>> Yes, it means that IO transactions would not work.
> >>
> >> I agree.
> >>
> >>>
> >>>>> Please also provide the IO resource address range before and after
> >>>>> the pci scan.  Then we can evaluate if the range is needed to be
> >>>>> mapped
> >>> via
> >>>>> ATMU.
> >>>>>
> >>>>> Leo
> >>>>
> >>>> Since potar is set by out_be32(&pci->pow[j].potar, (hose-
> >>>> io_resource.start >> 12);  I provide the result of
> >>>> hose->io_resource.start >> 12 as follows:
> >>>>
> >>>> pcie at ffe09000:
> >>>> before pci scan: io_resource.start >> 12: 0 after pci scan :
> >>>> io_resource.start >> 12: ff7ed
> >>>>
> >>>> pcie at ffe0a000:
> >>>> before pci scan: io_resource.start >> 12: 0 after pci scan :
> >>>> io_resource.start >> 12: ff7db
> >>>>
> >>>> pcie at ffe0b000:
> >>>> before pci scan: io_resource.start >> 12: 0 after pci scan :
> >>>> io_resource.start >> 12: ff7c9
> >>>>
> >>>> Note that I tested on P1022DS.
> >>>>
> >>>> - Hongtao.
> >>>
> >>> 1. What's the device tree nodes for PCIe look like?
> >>> 2. Can you get the pr_debug() in setup_pci_atmu() to print and
> >>> report results (as well as full boot log)
> >>
> >> Please refer to the attached file.
> >> In the log file I also print the device tree.
> >>
> >> - Hongtao.
> >>
> >>>
> >>> However, I think the change of the io_resource.start is normal and
> >>> correct behavior.
> >>>
> >>> - k
> >>>
> >>
> >
> > Hi Kumar,
> > I have already sent the log.
> > Do you have any comment on it?
> >
> > Thanks.
> > - Hongtao.
> >
> 
> Hongtao,
> 
> You mentioned:
> 
> > I tested the re-parsing way by using setup_pci_atmu() when resume.
> > And I found out that re-parsing will *change* outbound IO translation
> > address regitster.
> 
> What do the values look like in both ATMU registers and io_resource if
> you reparse?
> 
> - k


Hi Kumar,

About this topic do you have any further comments?

Thanks.
- Hongtao.