All of lore.kernel.org
 help / color / mirror / Atom feed
From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 12/17] ARM: mvebu: Armada XP GP specific suspend/resume code
Date: Fri, 24 Oct 2014 16:28:24 +0200	[thread overview]
Message-ID: <20141024162824.67f9ce3d@free-electrons.com> (raw)
In-Reply-To: <20141024142044.GB3142@lunn.ch>

Dear Andrew Lunn,

On Fri, 24 Oct 2014 16:20:44 +0200, Andrew Lunn wrote:

> Does Marvell mandate this PIC and gpio interface? Or is a board
> designer free to implement it some other way? It seems to me, this
> should be considered specific to the Marvell reference design.

They don't mandate this interface, it's really a board-specific
decision, which is why I've split my implementation between:

 * SoC-specific code, in mach-mvebu/pm.c.

 * Board-specific code, in mach-mvebu/pm-board.c.

> I'm wondering if this code should be a power driver, living in
> drivers/power/reset/.

I'm fine with that, but have you seen the *very* tight interaction
between the SoC-specific code and the board-specific code? The problem
is that the board-specific code needs to put the SDRAM into
self-refresh *right* before shutting down the SoC, and all that while
making sure the code doing both of these operations remains in the
I-Cache, and does not touch any other location in memory (which has
become inaccessible due to being in self-refresh mode).

Look at the mvebu_armada_xp_gp_pm_enter() function: it takes two
arguments, received from the SoC-level code. How to handle this thing
with a driver in drivers/power/reset/ ?

> A few kirkwood boards have a PIC controlling the power. QNAP devices
> use a serial port to talk to it. Others use a single gpio line. Is
> there something to stop a designer doing this for XP?

Nothing stops that. I think Marvell told me they wanted to standardize
this GPIO interface with a PIC, but that's only for their own
development boards. Since it remains something outside the SoC, it is
by nature board-specific, and designers can do pretty much what they
want.

> It seems like you would have a more generic solution by allowing DT to
> specify a power off driver to use.

Sure, but see above. If you have some suggestion, I'm surely interested.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
To: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
Cc: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>,
	Sebastian Hesselbarth
	<sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Gregory Clement
	<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Tawfik Bayouk <tawfik-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Nadav Haklai <nadavh-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Lior Amsalem <alior-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Ezequiel Garcia
	<ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 12/17] ARM: mvebu: Armada XP GP specific suspend/resume code
Date: Fri, 24 Oct 2014 16:28:24 +0200	[thread overview]
Message-ID: <20141024162824.67f9ce3d@free-electrons.com> (raw)
In-Reply-To: <20141024142044.GB3142-g2DYL2Zd6BY@public.gmane.org>

Dear Andrew Lunn,

On Fri, 24 Oct 2014 16:20:44 +0200, Andrew Lunn wrote:

> Does Marvell mandate this PIC and gpio interface? Or is a board
> designer free to implement it some other way? It seems to me, this
> should be considered specific to the Marvell reference design.

They don't mandate this interface, it's really a board-specific
decision, which is why I've split my implementation between:

 * SoC-specific code, in mach-mvebu/pm.c.

 * Board-specific code, in mach-mvebu/pm-board.c.

> I'm wondering if this code should be a power driver, living in
> drivers/power/reset/.

I'm fine with that, but have you seen the *very* tight interaction
between the SoC-specific code and the board-specific code? The problem
is that the board-specific code needs to put the SDRAM into
self-refresh *right* before shutting down the SoC, and all that while
making sure the code doing both of these operations remains in the
I-Cache, and does not touch any other location in memory (which has
become inaccessible due to being in self-refresh mode).

Look at the mvebu_armada_xp_gp_pm_enter() function: it takes two
arguments, received from the SoC-level code. How to handle this thing
with a driver in drivers/power/reset/ ?

> A few kirkwood boards have a PIC controlling the power. QNAP devices
> use a serial port to talk to it. Others use a single gpio line. Is
> there something to stop a designer doing this for XP?

Nothing stops that. I think Marvell told me they wanted to standardize
this GPIO interface with a PIC, but that's only for their own
development boards. Since it remains something outside the SoC, it is
by nature board-specific, and designers can do pretty much what they
want.

> It seems like you would have a more generic solution by allowing DT to
> specify a power off driver to use.

Sure, but see above. If you have some suggestion, I'm surely interested.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2014-10-24 14:28 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-24 11:59 [PATCH 00/17] Suspend to RAM support for Armada XP Thomas Petazzoni
2014-10-24 11:59 ` Thomas Petazzoni
2014-10-24 11:59 ` [PATCH 01/17] Documentation: dt-bindings: minimal documentation for MVEBU SDRAM controller Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-03 17:05   ` Gregory CLEMENT
2014-11-03 17:05     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 02/17] ARM: mvebu: enable strex backoff delay Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-03 17:08   ` Gregory CLEMENT
2014-11-03 17:08     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 03/17] irqchip: irq-armada-370-xp: use proper return value for ->set_affinity() Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-03 17:20   ` Gregory CLEMENT
2014-11-03 17:20     ` Gregory CLEMENT
2014-11-07  4:09   ` Jason Cooper
2014-11-07  4:09     ` Jason Cooper
2014-11-07  4:09     ` Jason Cooper
2014-10-24 11:59 ` [PATCH 04/17] irqchip: irq-armada-370-xp: suspend/resume support Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-03 17:38   ` Gregory CLEMENT
2014-11-03 17:38     ` Gregory CLEMENT
2014-11-13 16:32     ` Thomas Petazzoni
2014-11-13 16:32       ` Thomas Petazzoni
2014-10-24 11:59 ` [PATCH 05/17] clocksource: time-armada-370-xp: add " Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-03 17:45   ` Gregory CLEMENT
2014-11-03 17:45     ` Gregory CLEMENT
2014-11-03 17:45     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 06/17] gpio: mvebu: " Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-10-24 16:30   ` David Cohen
2014-10-24 16:30     ` David Cohen
2014-10-24 20:45     ` Andrew Lunn
2014-10-24 20:45       ` Andrew Lunn
2014-10-27  5:27       ` Alexandre Courbot
2014-10-27  5:27         ` Alexandre Courbot
2014-10-27 17:45         ` David Cohen
2014-10-27 17:45           ` David Cohen
2014-10-31  7:00   ` Linus Walleij
2014-10-31  7:00     ` Linus Walleij
2014-10-31  7:52     ` Gregory CLEMENT
2014-10-31  7:52       ` Gregory CLEMENT
2014-10-31  8:14       ` Thomas Petazzoni
2014-10-31  8:14         ` Thomas Petazzoni
2014-11-03 13:26       ` Linus Walleij
2014-11-03 13:26         ` Linus Walleij
2014-11-03 13:29   ` Linus Walleij
2014-11-03 13:29     ` Linus Walleij
2014-11-03 17:53   ` Gregory CLEMENT
2014-11-03 17:53     ` Gregory CLEMENT
2014-11-03 21:21     ` Thomas Petazzoni
2014-11-03 21:21       ` Thomas Petazzoni
2014-10-24 11:59 ` [PATCH 07/17] bus: mvebu-mbus: " Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-03 18:08   ` Gregory CLEMENT
2014-11-03 18:08     ` Gregory CLEMENT
2014-11-03 21:20     ` Thomas Petazzoni
2014-11-03 21:20       ` Thomas Petazzoni
2014-10-24 11:59 ` [PATCH 08/17] bus: mvebu-mbus: provide a mechanism to save SDRAM window configuration Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-04  9:17   ` Gregory CLEMENT
2014-11-04  9:17     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 09/17] clk: mvebu: add suspend/resume for gatable clocks Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-04  9:32   ` Gregory CLEMENT
2014-11-04  9:32     ` Gregory CLEMENT
2014-11-04  9:32     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 10/17] ARM: mvebu: implement suspend/resume support for Armada XP Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-04 10:00   ` Gregory CLEMENT
2014-11-04 10:00     ` Gregory CLEMENT
2014-11-13 17:00     ` Thomas Petazzoni
2014-11-13 17:00       ` Thomas Petazzoni
2014-10-24 11:59 ` [PATCH 11/17] ARM: mvebu: reserve the first 10 KB of each memory bank for suspend/resume Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-04 10:09   ` Gregory CLEMENT
2014-11-04 10:09     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 12/17] ARM: mvebu: Armada XP GP specific suspend/resume code Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-10-24 14:20   ` Andrew Lunn
2014-10-24 14:20     ` Andrew Lunn
2014-10-24 14:28     ` Thomas Petazzoni [this message]
2014-10-24 14:28       ` Thomas Petazzoni
2014-10-24 14:51       ` Andrew Lunn
2014-10-24 14:51         ` Andrew Lunn
2014-10-27 12:51         ` Thomas Petazzoni
2014-10-27 12:51           ` Thomas Petazzoni
2014-10-27 14:19           ` Andrew Lunn
2014-10-27 14:19             ` Andrew Lunn
2014-10-27 14:40             ` Thomas Petazzoni
2014-10-27 14:40               ` Thomas Petazzoni
2014-10-27 14:59               ` Andrew Lunn
2014-10-27 14:59                 ` Andrew Lunn
2014-10-27 15:12                 ` Thomas Petazzoni
2014-10-27 15:12                   ` Thomas Petazzoni
2014-10-27 15:15                   ` Andrew Lunn
2014-10-27 15:15                     ` Andrew Lunn
2014-11-10 13:53   ` Gregory CLEMENT
2014-11-10 13:53     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 13/17] ARM: mvebu: make sure MMU is disabled in armada_370_xp_cpu_resume Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-10 14:05   ` Gregory CLEMENT
2014-11-10 14:05     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 14/17] ARM: mvebu: synchronize secondary CPU clocks on resume Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-10 14:12   ` Gregory CLEMENT
2014-11-10 14:12     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 15/17] ARM: mvebu: add suspend/resume DT information for Armada XP GP Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-10 14:14   ` Gregory CLEMENT
2014-11-10 14:14     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 16/17] ARM: mvebu: adjust mbus controller description on Armada 370/XP Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-10 14:15   ` Gregory CLEMENT
2014-11-10 14:15     ` Gregory CLEMENT
2014-10-24 11:59 ` [PATCH 17/17] ARM: mvebu: add SDRAM controller description for Armada XP Thomas Petazzoni
2014-10-24 11:59   ` Thomas Petazzoni
2014-11-10 14:25   ` Gregory CLEMENT
2014-11-10 14:25     ` Gregory CLEMENT

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141024162824.67f9ce3d@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.