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

[Li Yang <leoli@freescale.com>]

On Thu, Sep 27, 2012 at 8:05 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
>>>>>>>>>>>
>>>>>>>>>>> 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@freescale.com>
>>>>>>>>>>>> Signed-off-by: Jia Hongtao <B38951@freescale.com>
>>>>>>>>>>>> Signed-off-by: Li Yang <leoli@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@ffe09000:
>>>>> before pci scan: io_resource.start >> 12: 0 after pci scan :
>>>>> io_resource.start >> 12: ff7ed
>>>>>
>>>>> pcie@ffe0a000:
>>>>> before pci scan: io_resource.start >> 12: 0 after pci scan :
>>>>> io_resource.start >> 12: ff7db
>>>>>
>>>>> pcie@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?

I think Hongtao mentioned in previous email as follows, the ATMU
registers are inline with the io_resource address.

> > 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@ffe09000:
> > before pci scan: io_resource.start >> 12: 0
> > after pci scan : io_resource.start >> 12: ff7ed
> >
> > pcie@ffe0a000:
> > before pci scan: io_resource.start >> 12: 0
> > after pci scan : io_resource.start >> 12: ff7db
> >
> > pcie@ffe0b000:
> > before pci scan: io_resource.start >> 12: 0
> > after pci scan : io_resource.start >> 12: ff7c9
> >
> > Note that I tested on P1022DS.