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

[Jia Hongtao-B38951]

Hi Kumar,

This PCI controller PM thing is pending for nearly a month without
further discussion. Maybe it's time now to reach an agreement.

- Hongtao.



> -----Original Message-----
> From: Jia Hongtao-B38951
> Sent: Friday, October 19, 2012 12:15 PM
> To: 'Kumar Gala'
> 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
> 
> > -----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.