diff mbox

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

Message ID 1347934234-18223-1-git-send-email-B38951@freescale.com (mailing list archive)
State Superseded
Headers show

Commit Message

Hongtao Jia Sept. 18, 2012, 2:10 a.m. UTC
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(-)

Comments

Kumar Gala Sept. 18, 2012, 5:03 a.m. UTC | #1
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
Hongtao Jia Sept. 19, 2012, 7:10 a.m. UTC | #2
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, September 18, 2012 1:04 PM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> 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@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.

Anyway, if setup_pci_atmu() at resume is functionally right then re-parsing
is a good way for PM. I also test the latency of re-parsing and save/restore,
both way are acceptable.


- Hongtao.
Kumar Gala Sept. 19, 2012, 2:27 p.m. UTC | #3
On Sep 19, 2012, at 2:10 AM, Jia Hongtao-B38951 wrote:

> 
> 
>> -----Original Message-----
>> From: Kumar Gala [mailto:galak@kernel.crashing.org]
>> Sent: Tuesday, September 18, 2012 1:04 PM
>> To: Jia Hongtao-B38951
>> Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
>> 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@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.

> 
> Anyway, if setup_pci_atmu() at resume is functionally right then re-parsing
> is a good way for PM. I also test the latency of re-parsing and save/restore,
> both way are acceptable.
> 
> 
> - Hongtao.
Hongtao Jia Sept. 19, 2012, 3:41 p.m. UTC | #4

Kumar Gala Sept. 19, 2012, 3:49 p.m. UTC | #5
>>> 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" ?

Trying to understand why you'd hit the reassignment of io_resource.

- k

> 
> 
> 
>> 
>> Anyway, if setup_pci_atmu() at resume is functionally right then re-parsing
>> is a good way for PM. I also test the latency of re-parsing and save/restore,
>> both way are acceptable.
>> 
>> 
>> - Hongtao.
> 
>
Hongtao Jia Sept. 21, 2012, 3:13 a.m. UTC | #6
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Wednesday, September 19, 2012 11:49 PM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> 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@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?

- Hongtao.
Li Yang-R58472 Sept. 21, 2012, 3:50 a.m. UTC | #7
> -----Original Message-----
> From: Jia Hongtao-B38951
> Sent: Friday, September 21, 2012 11:14 AM
> To: Kumar Gala
> Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> Subject: RE: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM
> support
> 
> 
> 
> > -----Original Message-----
> > From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > Sent: Wednesday, September 19, 2012 11:49 PM
> > To: Jia Hongtao-B38951
> > Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> > 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@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?

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
Hongtao Jia Sept. 21, 2012, 5:15 a.m. UTC | #8
> -----Original Message-----
> From: Li Yang-R58472
> Sent: Friday, September 21, 2012 11:51 AM
> To: Jia Hongtao-B38951; Kumar Gala
> Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421
> Subject: RE: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM
> support
> 
> 
> 
> > -----Original Message-----
> > From: Jia Hongtao-B38951
> > Sent: Friday, September 21, 2012 11:14 AM
> > To: Kumar Gala
> > Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> > Subject: RE: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM
> > support
> >
> >
> >
> > > -----Original Message-----
> > > From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > > Sent: Wednesday, September 19, 2012 11:49 PM
> > > To: Jia Hongtao-B38951
> > > Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> > > 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@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?
> 
> 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.
Kumar Gala Sept. 21, 2012, 1:15 p.m. UTC | #9
>>>>>>> 
>>>>>>> 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.

>> 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)

However, I think the change of the io_resource.start is normal and correct behavior.

- k
Hongtao Jia Sept. 24, 2012, 2:47 a.m. UTC | #10
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Friday, September 21, 2012 9:16 PM
> To: Jia Hongtao-B38951
> Cc: Li Yang-R58472; linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421
> 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@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
>
Hongtao Jia Sept. 27, 2012, 2:58 a.m. UTC | #11
> -----Original Message-----
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+b38951=freescale.com@lists.ozlabs.org] On Behalf Of Jia Hongtao-
> B38951
> Sent: Monday, September 24, 2012 10:47 AM
> To: Kumar Gala
> Cc: Wood Scott-B07421; linuxppc-dev@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@kernel.crashing.org]
> > Sent: Friday, September 21, 2012 9:16 PM
> > To: Jia Hongtao-B38951
> > Cc: Li Yang-R58472; linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421
> > 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@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.
Kumar Gala Sept. 27, 2012, 12:05 p.m. UTC | #12
>>>>>>>>>> 
>>>>>>>>>> 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?

- k
Yang Li Sept. 27, 2012, 1:24 p.m. UTC | #13
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.
Kumar Gala Sept. 27, 2012, 4:04 p.m. UTC | #14
>>> 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.

I was under that the impression that was the normal boot case, not the values from after wakeup.

- k

>>> 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.
Li Yang-R58472 Sept. 27, 2012, 4:27 p.m. UTC | #15
在 Sep 28, 2012,0:07,"Kumar Gala" <galak@kernel.crashing.org> 写道:

>>>> 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.

> 

> I was under that the impression that was the normal boot case, not the values from after wakeup.


It is for the normal boot.  But re-parse will use the io resource after pic scan to initialize atmu.  Instead, the original atmu is initialized use the io resource before the scan.

Leo
> 

> - k

> 

>>>> 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.

> 

>
Kumar Gala Sept. 27, 2012, 9:38 p.m. UTC | #16
On Sep 27, 2012, at 11:27 AM, Li Yang-R58472 wrote:

> 
> 在 Sep 28, 2012,0:07,"Kumar Gala" <galak@kernel.crashing.org> 写道:
> 
>>>>> 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.
>> 
>> I was under that the impression that was the normal boot case, not the values from after wakeup.
> 
> It is for the normal boot.  But re-parse will use the io resource after pic scan to initialize atmu.  Instead, the original atmu is initialized use the io resource before the scan.
> 
> Leo

I think I see, so isn't the mem resources also wrong?

Can we dump the following:

1. enable pr_debug() in pcibios_setup_phb_resources so we get "PHB: " prints
2. Can we dump hose->io_resource & hose->mem_resources[] right after wakeup?

I think I see what direction but would be useful to get a few more answers.

- k
Hongtao Jia Sept. 28, 2012, 2:57 a.m. UTC | #17
> -----Original Message-----

> From: Kumar Gala [mailto:galak@kernel.crashing.org]

> Sent: Friday, September 28, 2012 5:38 AM

> To: Li Yang-R58472

> Cc: Jia Hongtao-B38951; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org

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

> support

> 

> 

> On Sep 27, 2012, at 11:27 AM, Li Yang-R58472 wrote:

> 

> >

> > 在 Sep 28, 2012,0:07,"Kumar Gala" <galak@kernel.crashing.org> 写道:

> >

> >>>>> 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.

> >>

> >> I was under that the impression that was the normal boot case, not the

> values from after wakeup.

> >

> > It is for the normal boot.  But re-parse will use the io resource after

> pic scan to initialize atmu.  Instead, the original atmu is initialized

> use the io resource before the scan.

> >

> > Leo

> 

> I think I see, so isn't the mem resources also wrong?

> 

> Can we dump the following:

> 

> 1. enable pr_debug() in pcibios_setup_phb_resources so we get "PHB: "

> prints 2. Can we dump hose->io_resource & hose->mem_resources[] right

> after wakeup?

> 

> I think I see what direction but would be useful to get a few more

> answers.

> 

> - k


I enable pr_debug() on these three files:
arch/powerpc/sysdev/fsl_pci.c
arch/powerpc/kernel/pci-common.c
arch/powerpc/kernel/pci_32.c

Also I print the io_resource and mem_resources after wakeup.
(actually pr_debug already done this)

You can see that io resource is changed after pci scan in normal boot.

To see the log please refer to the attachment.

- Hongtao.
Hongtao Jia Oct. 19, 2012, 4:15 a.m. UTC | #18
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Thursday, September 27, 2012 8:06 PM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linuxppc-dev@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@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?
> 
> - k


Hi Kumar,

About this topic do you have any further comments?

Thanks.
- Hongtao.
Hongtao Jia Oct. 24, 2012, 1:58 a.m. UTC | #19
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@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@kernel.crashing.org]
> > Sent: Thursday, September 27, 2012 8:06 PM
> > To: Jia Hongtao-B38951
> > Cc: Wood Scott-B07421; linuxppc-dev@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@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?
> >
> > - k
> 
> 
> Hi Kumar,
> 
> About this topic do you have any further comments?
> 
> Thanks.
> - Hongtao.
Hongtao Jia Oct. 30, 2012, 2:57 a.m. UTC | #20
Hi Kumar,

Since PCI controller PM support is inactive for a long while I'd
like to submit a new patch using re-setup atmu to restore PCI states.
Maybe the outbound IO issue during first bootup will be discussed
later when you have time.

- Hongtao.

> -----Original Message-----
> From: Jia Hongtao-B38951
> Sent: Wednesday, October 24, 2012 9:59 AM
> To: Jia Hongtao-B38951; Kumar Gala
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; Li Yang-R58472
> Subject: RE: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM
> support
> 
> 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@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@kernel.crashing.org]
> > > Sent: Thursday, September 27, 2012 8:06 PM
> > > To: Jia Hongtao-B38951
> > > Cc: Wood Scott-B07421; linuxppc-dev@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@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?
> > >
> > > - k
> >
> >
> > Hi Kumar,
> >
> > About this topic do you have any further comments?
> >
> > Thanks.
> > - Hongtao.
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index ac39e6a..823e000 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -89,9 +89,9 @@  struct pci_controller {
 
 #ifdef CONFIG_PPC64
 	unsigned long buid;
+#endif	/* CONFIG_PPC64 */
 
 	void *private_data;
-#endif	/* CONFIG_PPC64 */
 };
 
 /* These are used for config access before all the PCI probing
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index e577cb5..8c15177 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -887,12 +887,133 @@  static int __devinit fsl_pci_probe(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_SUSPEND
+
+#define PCI_POW_PIW_OFFSET	0xc00
+#define PCI_POW_PIW_SIZE	0x200
+#define PCI_POW_NUMBER		5
+
+static int fsl_pci_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct pci_controller *hose;
+	struct pci_outbound_window_regs *pci_saved_pow;
+	struct pci_inbound_window_regs *pci_saved_piw, *temp_piw;
+	struct resource pci_rsrc;
+	unsigned int i;
+	struct fsl_pci_private_data *sus_info;
+
+	hose = pci_find_hose_for_OF_device(pdev->dev.of_node);
+	of_address_to_resource(pdev->dev.of_node, 0, &pci_rsrc);
+
+	sus_info = kmalloc(
+			sizeof(struct fsl_pci_private_data), GFP_KERNEL);
+	if (!sus_info)
+		return -ENOMEM;
+
+	hose->private_data = sus_info;
+
+	sus_info->pci_pow = ioremap(pci_rsrc.start + PCI_POW_PIW_OFFSET,
+			PCI_POW_PIW_SIZE);
+	if (!sus_info->pci_pow) {
+		dev_err(&pdev->dev, "pci outbound/inbound windows ioremap error!\n");
+		goto err1;
+	}
+
+	sus_info->pci_piw = (struct pci_inbound_window_regs *)
+		((void *)sus_info->pci_pow + PCI_POW_PIW_SIZE) - 1;
+
+	if (of_device_is_compatible(pdev->dev.of_node, "fsl,qoriq-pcie-v2.2"))
+		sus_info->inbound_num = 4;
+	else
+		sus_info->inbound_num = 3;
+
+	sus_info->saved_regs = kmalloc(
+		sizeof(struct pci_outbound_window_regs) * PCI_POW_NUMBER +
+		sizeof(struct pci_inbound_window_regs) * sus_info->inbound_num,
+		GFP_KERNEL);
+	if (!sus_info->saved_regs)
+		goto err2;
+
+	pci_saved_pow = sus_info->saved_regs;
+	for (i = 0; i < PCI_POW_NUMBER; i++) {
+		pci_saved_pow[i].potar = in_be32(&sus_info->pci_pow[i].potar);
+		pci_saved_pow[i].potear = in_be32(&sus_info->pci_pow[i].potear);
+		pci_saved_pow[i].powbar = in_be32(&sus_info->pci_pow[i].powbar);
+		pci_saved_pow[i].powar = in_be32(&sus_info->pci_pow[i].powar);
+	}
+
+	pci_saved_piw = (struct pci_inbound_window_regs *)
+		(pci_saved_pow + PCI_POW_NUMBER);
+	temp_piw = sus_info->pci_piw;
+	for (i = 0; i < sus_info->inbound_num; i++, temp_piw--) {
+		pci_saved_piw[i].pitar = in_be32(&temp_piw->pitar);
+		pci_saved_piw[i].piwbar = in_be32(&temp_piw->piwbar);
+		pci_saved_piw[i].piwbear = in_be32(&temp_piw->piwbear);
+		pci_saved_piw[i].piwar = in_be32(&temp_piw->piwar);
+	}
+
+	return 0;
+
+err2:
+	iounmap(sus_info->pci_pow);
+
+err1:
+	kfree(sus_info);
+	return -ENOMEM;
+}
+
+static int fsl_pci_resume(struct platform_device *pdev)
+{
+	struct pci_controller *hose;
+	struct pci_outbound_window_regs *pci_saved_pow;
+	struct pci_inbound_window_regs *pci_saved_piw, *temp_piw;
+	unsigned int i;
+	struct fsl_pci_private_data *sus_info;
+
+	hose = pci_find_hose_for_OF_device(pdev->dev.of_node);
+	sus_info = (struct fsl_pci_private_data *)hose->private_data;
+
+	if (!sus_info->pci_pow || !sus_info->pci_piw || !sus_info->saved_regs)
+		return 0;
+
+	pci_saved_pow = sus_info->saved_regs;
+	for (i = 0; i < PCI_POW_NUMBER; i++) {
+		out_be32(&sus_info->pci_pow[i].potar, pci_saved_pow[i].potar);
+		out_be32(&sus_info->pci_pow[i].potear, pci_saved_pow[i].potear);
+		out_be32(&sus_info->pci_pow[i].powbar, pci_saved_pow[i].powbar);
+		out_be32(&sus_info->pci_pow[i].powar, pci_saved_pow[i].powar);
+	}
+
+	pci_saved_piw = (struct pci_inbound_window_regs *)
+		(pci_saved_pow + PCI_POW_NUMBER);
+	temp_piw = sus_info->pci_piw;
+	for (i = 0; i < sus_info->inbound_num; i++, temp_piw--) {
+		out_be32(&temp_piw->pitar, pci_saved_piw[i].pitar);
+		out_be32(&temp_piw->piwbar, pci_saved_piw[i].piwbar);
+		out_be32(&temp_piw->piwbear, pci_saved_piw[i].piwbear);
+		out_be32(&temp_piw->piwar, pci_saved_piw[i].piwar);
+	}
+	iounmap(sus_info->pci_pow);
+	kfree(sus_info->saved_regs);
+	sus_info->saved_regs = NULL;
+	kfree(sus_info);
+	sus_info = NULL;
+	hose->private_data = NULL;
+
+	return 0;
+}
+#endif
+
 static struct platform_driver fsl_pci_driver = {
 	.driver = {
 		.name = "fsl-pci",
 		.of_match_table = pci_ids,
 	},
 	.probe = fsl_pci_probe,
+#ifdef CONFIG_SUSPEND
+	.suspend	= fsl_pci_suspend,
+	.resume		= fsl_pci_resume,
+#endif
 };
 
 static int __init fsl_pci_init(void)