mbox series

[v3,0/4] Prepare Armada 3700 PCIe suspend to RAM support

Message ID 20190627125245.26788-1-miquel.raynal@bootlin.com (mailing list archive)
Headers show
Series Prepare Armada 3700 PCIe suspend to RAM support | expand

Message

Miquel Raynal June 27, 2019, 12:52 p.m. UTC
Hello,

As part of an effort to bring suspend to RAM support to the Armada
3700 SoC (main target: ESPRESSObin board), there are small things to
do in the Armada 3700 peripherals clock driver:

* On this SoC, the PCIe controller gets fed by a gated clock in the
  south bridge. This clock is missing in the current driver, patch 1
  adds it.

* Because of a constraint in the PCI core, the resume function of a
  PCIe controller driver must be run at an early stage
  (->suspend/resume_noirq()), before the core tries to ->read/write()
  in the PCIe registers to do more configuration. Hence, the PCIe
  clock must be resumed before. This is enforced thanks to two
  changes:
  1/ Add device links to the clock framework. This enforce order in
     the PM core: the clocks are resumed before the consumers. Series
     has been posted, see [1].
  2/ Even with the above feature, the clock's resume() callback is
     called after the PCI controller's resume_noirq() callback. The
     only way to fix this is to change the "priority" of the clock
     suspend/resume callbacks. This is done in patch 2.

* The bindings are updated with the PCI clock in patch 4 while patch 3
  is just a typo correction in the same file.

If there is anything unclear please feel free to ask.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-November/614527.html

Thanks,
Miquèl

Changes in v3:
==============
* Fixed one typo: s/their register/their registers/.

Changes in v2:
==============
* Rebased on top of v5.2-rc1.
* Added Rob's R-by tags.
* No change on the "change suspend/resume time" patch as, despite my
  pings, I got no answer and IMHO the proposed approach is entirely
  valid.


Miquel Raynal (4):
  clk: mvebu: armada-37xx-periph: add PCIe gated clock
  clk: mvebu: armada-37xx-periph: change suspend/resume time
  dt-bindings: clk: armada3700: fix typo in SoC name
  dt-bindings: clk: armada3700: document the PCIe clock

 .../devicetree/bindings/clock/armada3700-periph-clock.txt   | 5 +++--
 drivers/clk/mvebu/armada-37xx-periph.c                      | 6 ++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

Comments

Miquel Raynal Sept. 20, 2019, 8:03 a.m. UTC | #1
Hi Stephen,

Stephen Boyd <sboyd@kernel.org> wrote on Tue, 17 Sep 2019 10:31:53
-0700:

> Quoting Miquel Raynal (2019-06-27 05:52:41)
> > Hello,
> > 
> > As part of an effort to bring suspend to RAM support to the Armada
> > 3700 SoC (main target: ESPRESSObin board), there are small things to
> > do in the Armada 3700 peripherals clock driver:
> > 
> > * On this SoC, the PCIe controller gets fed by a gated clock in the
> >   south bridge. This clock is missing in the current driver, patch 1
> >   adds it.
> > 
> > * Because of a constraint in the PCI core, the resume function of a
> >   PCIe controller driver must be run at an early stage
> >   (->suspend/resume_noirq()), before the core tries to ->read/write()
> >   in the PCIe registers to do more configuration. Hence, the PCIe
> >   clock must be resumed before. This is enforced thanks to two
> >   changes:
> >   1/ Add device links to the clock framework. This enforce order in
> >      the PM core: the clocks are resumed before the consumers. Series
> >      has been posted, see [1].
> >   2/ Even with the above feature, the clock's resume() callback is
> >      called after the PCI controller's resume_noirq() callback. The
> >      only way to fix this is to change the "priority" of the clock
> >      suspend/resume callbacks. This is done in patch 2.
> > 
> > * The bindings are updated with the PCI clock in patch 4 while patch 3
> >   is just a typo correction in the same file.
> > 
> > If there is anything unclear please feel free to ask.
> >   
> 
> Should I drop this patch series?
> 

No, if it is right for you I would really prefer to have it merged
(sorry for the delay in answering though) because it will be still
needed, no matter how clock dependencies are handled.


Thanks,
Miquèl
Stephen Boyd Sept. 20, 2019, 4:54 p.m. UTC | #2
Quoting Miquel Raynal (2019-09-20 01:03:01)
> Hi Stephen,
> 
> Stephen Boyd <sboyd@kernel.org> wrote on Tue, 17 Sep 2019 10:31:53
> -0700:
> 
> > Quoting Miquel Raynal (2019-06-27 05:52:41)
> > > Hello,
> > > 
> > > As part of an effort to bring suspend to RAM support to the Armada
> > > 3700 SoC (main target: ESPRESSObin board), there are small things to
> > > do in the Armada 3700 peripherals clock driver:
> > > 
> > > * On this SoC, the PCIe controller gets fed by a gated clock in the
> > >   south bridge. This clock is missing in the current driver, patch 1
> > >   adds it.
> > > 
> > > * Because of a constraint in the PCI core, the resume function of a
> > >   PCIe controller driver must be run at an early stage
> > >   (->suspend/resume_noirq()), before the core tries to ->read/write()
> > >   in the PCIe registers to do more configuration. Hence, the PCIe
> > >   clock must be resumed before. This is enforced thanks to two
> > >   changes:
> > >   1/ Add device links to the clock framework. This enforce order in
> > >      the PM core: the clocks are resumed before the consumers. Series
> > >      has been posted, see [1].
> > >   2/ Even with the above feature, the clock's resume() callback is
> > >      called after the PCI controller's resume_noirq() callback. The
> > >      only way to fix this is to change the "priority" of the clock
> > >      suspend/resume callbacks. This is done in patch 2.
> > > 
> > > * The bindings are updated with the PCI clock in patch 4 while patch 3
> > >   is just a typo correction in the same file.
> > > 
> > > If there is anything unclear please feel free to ask.
> > >   
> > 
> > Should I drop this patch series?
> > 
> 
> No, if it is right for you I would really prefer to have it merged
> (sorry for the delay in answering though) because it will be still
> needed, no matter how clock dependencies are handled.
> 
> 

Ok. I'll apply it after the merge window. Let me know if it's more
urgent than that.
Miquel Raynal Sept. 21, 2019, 9:23 a.m. UTC | #3
Hi Stephen,

Stephen Boyd <sboyd@kernel.org> wrote on Fri, 20 Sep 2019 09:54:01
-0700:

> Quoting Miquel Raynal (2019-09-20 01:03:01)
> > Hi Stephen,
> > 
> > Stephen Boyd <sboyd@kernel.org> wrote on Tue, 17 Sep 2019 10:31:53
> > -0700:
> >   
> > > Quoting Miquel Raynal (2019-06-27 05:52:41)  
> > > > Hello,
> > > > 
> > > > As part of an effort to bring suspend to RAM support to the Armada
> > > > 3700 SoC (main target: ESPRESSObin board), there are small things to
> > > > do in the Armada 3700 peripherals clock driver:
> > > > 
> > > > * On this SoC, the PCIe controller gets fed by a gated clock in the
> > > >   south bridge. This clock is missing in the current driver, patch 1
> > > >   adds it.
> > > > 
> > > > * Because of a constraint in the PCI core, the resume function of a
> > > >   PCIe controller driver must be run at an early stage
> > > >   (->suspend/resume_noirq()), before the core tries to ->read/write()
> > > >   in the PCIe registers to do more configuration. Hence, the PCIe
> > > >   clock must be resumed before. This is enforced thanks to two
> > > >   changes:
> > > >   1/ Add device links to the clock framework. This enforce order in
> > > >      the PM core: the clocks are resumed before the consumers. Series
> > > >      has been posted, see [1].
> > > >   2/ Even with the above feature, the clock's resume() callback is
> > > >      called after the PCI controller's resume_noirq() callback. The
> > > >      only way to fix this is to change the "priority" of the clock
> > > >      suspend/resume callbacks. This is done in patch 2.
> > > > 
> > > > * The bindings are updated with the PCI clock in patch 4 while patch 3
> > > >   is just a typo correction in the same file.
> > > > 
> > > > If there is anything unclear please feel free to ask.
> > > >     
> > > 
> > > Should I drop this patch series?
> > >   
> > 
> > No, if it is right for you I would really prefer to have it merged
> > (sorry for the delay in answering though) because it will be still
> > needed, no matter how clock dependencies are handled.
> > 
> >   
> 
> Ok. I'll apply it after the merge window. Let me know if it's more
> urgent than that.
> 

No it's not, 5.5 is fine.


Thanks,
Miquèl