diff mbox

[RFC,1/4] ARM: tegra: Move SoC drivers to drivers/soc/tegra

Message ID 1403888329-24755-1-git-send-email-thierry.reding@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Reding June 27, 2014, 4:58 p.m. UTC
From: Thierry Reding <treding@nvidia.com>

These drivers are closely coupled and need to be moved as a whole. One
reason for moving them out of arch/arm/mach-tegra is to allow them to be
shared with 64-bit ARM.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 arch/arm/mach-tegra/Makefile                       | 32 --------
 arch/arm/mach-tegra/common.h                       |  4 -
 arch/arm/mach-tegra/io.c                           | 27 +++++-
 arch/arm/mach-tegra/pmc.h                          | 62 --------------
 arch/arm/mach-tegra/tegra.c                        |  9 --
 drivers/soc/Makefile                               |  1 +
 drivers/soc/tegra/Makefile                         | 34 ++++++++
 .../soc/tegra}/cpuidle-tegra114.c                  |  0
 .../soc/tegra}/cpuidle-tegra20.c                   |  6 +-
 .../soc/tegra}/cpuidle-tegra30.c                   |  0
 .../arm/mach-tegra => drivers/soc/tegra}/cpuidle.c |  0
 .../arm/mach-tegra => drivers/soc/tegra}/cpuidle.h |  3 -
 .../mach-tegra => drivers/soc/tegra}/flowctrl.c    |  0
 .../mach-tegra => drivers/soc/tegra}/flowctrl.h    |  2 -
 .../arm/mach-tegra => drivers/soc/tegra}/headsmp.S |  0
 .../arm/mach-tegra => drivers/soc/tegra}/hotplug.c |  0
 {arch/arm/mach-tegra => drivers/soc/tegra}/iomap.h |  0
 .../arm/mach-tegra => drivers/soc/tegra}/irammap.h |  0
 {arch/arm/mach-tegra => drivers/soc/tegra}/irq.c   |  1 -
 {arch/arm/mach-tegra => drivers/soc/tegra}/irq.h   |  6 --
 .../arm/mach-tegra => drivers/soc/tegra}/platsmp.c |  5 --
 .../mach-tegra => drivers/soc/tegra}/pm-tegra20.c  |  0
 .../mach-tegra => drivers/soc/tegra}/pm-tegra30.c  |  0
 {arch/arm/mach-tegra => drivers/soc/tegra}/pm.c    |  0
 {arch/arm/mach-tegra => drivers/soc/tegra}/pm.h    |  4 +-
 {arch/arm/mach-tegra => drivers/soc/tegra}/pmc.c   |  0
 drivers/soc/tegra/pmc.h                            | 35 ++++++++
 .../mach-tegra => drivers/soc/tegra}/powergate.c   |  0
 .../soc/tegra}/reset-handler.S                     |  0
 {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c |  0
 {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h |  2 -
 .../soc/tegra}/sleep-tegra20.S                     |  0
 .../soc/tegra}/sleep-tegra30.S                     |  0
 {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.S |  0
 {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.h |  2 -
 include/linux/tegra-soc.h                          | 95 ++++++++++++++++++++++
 36 files changed, 195 insertions(+), 135 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/common.h
 delete mode 100644 arch/arm/mach-tegra/pmc.h
 create mode 100644 drivers/soc/tegra/Makefile
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/flowctrl.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/flowctrl.h (98%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/headsmp.S (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/hotplug.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/iomap.h (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/irammap.h (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/irq.c (99%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/irq.h (83%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/platsmp.c (98%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm-tegra20.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm-tegra30.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm.h (94%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/pmc.c (100%)
 create mode 100644 drivers/soc/tegra/pmc.h
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/powergate.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep-tegra20.S (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep-tegra30.S (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.S (100%)
 rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.h (98%)

Comments

Santosh Shilimkar June 27, 2014, 5:30 p.m. UTC | #1
+Arnd, Greg, Catalin and Kumar,

On Friday 27 June 2014 12:58 PM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> These drivers are closely coupled and need to be moved as a whole. One
> reason for moving them out of arch/arm/mach-tegra is to allow them to be
> shared with 64-bit ARM.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  arch/arm/mach-tegra/Makefile                       | 32 --------
>  arch/arm/mach-tegra/common.h                       |  4 -
>  arch/arm/mach-tegra/io.c                           | 27 +++++-
>  arch/arm/mach-tegra/pmc.h                          | 62 --------------
>  arch/arm/mach-tegra/tegra.c                        |  9 --
>  drivers/soc/Makefile                               |  1 +
>  drivers/soc/tegra/Makefile                         | 34 ++++++++
>  .../soc/tegra}/cpuidle-tegra114.c                  |  0
>  .../soc/tegra}/cpuidle-tegra20.c                   |  6 +-
>  .../soc/tegra}/cpuidle-tegra30.c                   |  0
>  .../arm/mach-tegra => drivers/soc/tegra}/cpuidle.c |  0
>  .../arm/mach-tegra => drivers/soc/tegra}/cpuidle.h |  3 -
>  .../mach-tegra => drivers/soc/tegra}/flowctrl.c    |  0
>  .../mach-tegra => drivers/soc/tegra}/flowctrl.h    |  2 -
>  .../arm/mach-tegra => drivers/soc/tegra}/headsmp.S |  0
>  .../arm/mach-tegra => drivers/soc/tegra}/hotplug.c |  0
>  {arch/arm/mach-tegra => drivers/soc/tegra}/iomap.h |  0
>  .../arm/mach-tegra => drivers/soc/tegra}/irammap.h |  0
>  {arch/arm/mach-tegra => drivers/soc/tegra}/irq.c   |  1 -
>  {arch/arm/mach-tegra => drivers/soc/tegra}/irq.h   |  6 --
>  .../arm/mach-tegra => drivers/soc/tegra}/platsmp.c |  5 --
>  .../mach-tegra => drivers/soc/tegra}/pm-tegra20.c  |  0
>  .../mach-tegra => drivers/soc/tegra}/pm-tegra30.c  |  0
>  {arch/arm/mach-tegra => drivers/soc/tegra}/pm.c    |  0
>  {arch/arm/mach-tegra => drivers/soc/tegra}/pm.h    |  4 +-
>  {arch/arm/mach-tegra => drivers/soc/tegra}/pmc.c   |  0
>  drivers/soc/tegra/pmc.h                            | 35 ++++++++
>  .../mach-tegra => drivers/soc/tegra}/powergate.c   |  0
>  .../soc/tegra}/reset-handler.S                     |  0
>  {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c |  0
>  {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h |  2 -
>  .../soc/tegra}/sleep-tegra20.S                     |  0
>  .../soc/tegra}/sleep-tegra30.S                     |  0
>  {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.S |  0
>  {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.h |  2 -
>  include/linux/tegra-soc.h                          | 95 ++++++++++++++++++++++
>  36 files changed, 195 insertions(+), 135 deletions(-)
>  delete mode 100644 arch/arm/mach-tegra/common.h
>  delete mode 100644 arch/arm/mach-tegra/pmc.h
>  create mode 100644 drivers/soc/tegra/Makefile
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/flowctrl.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/flowctrl.h (98%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/headsmp.S (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/hotplug.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/iomap.h (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/irammap.h (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/irq.c (99%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/irq.h (83%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/platsmp.c (98%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm-tegra20.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm-tegra30.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm.h (94%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pmc.c (100%)
>  create mode 100644 drivers/soc/tegra/pmc.h
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/powergate.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep-tegra20.S (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep-tegra30.S (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.S (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.h (98%)
> 
NAK for this patch.

You are using drivers/soc/* as a dump yard for your SOC code which
is not the intention we created drivers/soc/.

Its really for subsystem drivers which doesn't have appropriate
home in Linux kernel today. From your above patch ...

>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
This should go into drivers/idle/*. if you have dependencies, please sort
them out.

>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
subsystem: drivers/power/reset/

For tegra/*pm*/, you can use drivers/power or drivers/base/power/

For SMP boot, ARMv8 expecting to have either PSCI based implementation or
device tree based boot scheme. you can move towards that model if possible.

Please find appropriate subsystem to move your SOC code and if you don't
find an appropriate subsystem, we can discuss adding that under drivers/soc.
The drivers/soc/* isn't a dumping ground so please don't abuse it.

regards,
Santosh
Stephen Warren June 27, 2014, 9:10 p.m. UTC | #2
On 06/27/2014 10:58 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> These drivers are closely coupled and need to be moved as a whole. One
> reason for moving them out of arch/arm/mach-tegra is to allow them to be
> shared with 64-bit ARM.

Aside from any discussions re: splitting out cpuidle, power domains, ...
into separate subsystem directories, this patch looks conceptually fine
to me.

That said, given that I think arch/arm64 isn't going to have mach-*
directories or machine descriptors, I wonder how all this code is going
to be invoked at boot. arch/arm/mach-tegra/tegra.c calls a bunch of
these functions at boot right now. So, it seems like moving the code so
that it can compile in both places is only part of the solution? Are we
going to have an arm64-specific SoC driver that binds to the SoC
compatible in order to initialize everything? If so, I wonder if the
same can work for arch/arm so everything works the same way?
Thierry Reding June 27, 2014, 11:27 p.m. UTC | #3
On Fri, Jun 27, 2014 at 01:30:04PM -0400, Santosh Shilimkar wrote:
> +Arnd, Greg, Catalin and Kumar,
> 
> On Friday 27 June 2014 12:58 PM, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > These drivers are closely coupled and need to be moved as a whole. One
> > reason for moving them out of arch/arm/mach-tegra is to allow them to be
> > shared with 64-bit ARM.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  arch/arm/mach-tegra/Makefile                       | 32 --------
> >  arch/arm/mach-tegra/common.h                       |  4 -
> >  arch/arm/mach-tegra/io.c                           | 27 +++++-
> >  arch/arm/mach-tegra/pmc.h                          | 62 --------------
> >  arch/arm/mach-tegra/tegra.c                        |  9 --
> >  drivers/soc/Makefile                               |  1 +
> >  drivers/soc/tegra/Makefile                         | 34 ++++++++
> >  .../soc/tegra}/cpuidle-tegra114.c                  |  0
> >  .../soc/tegra}/cpuidle-tegra20.c                   |  6 +-
> >  .../soc/tegra}/cpuidle-tegra30.c                   |  0
> >  .../arm/mach-tegra => drivers/soc/tegra}/cpuidle.c |  0
> >  .../arm/mach-tegra => drivers/soc/tegra}/cpuidle.h |  3 -
> >  .../mach-tegra => drivers/soc/tegra}/flowctrl.c    |  0
> >  .../mach-tegra => drivers/soc/tegra}/flowctrl.h    |  2 -
> >  .../arm/mach-tegra => drivers/soc/tegra}/headsmp.S |  0
> >  .../arm/mach-tegra => drivers/soc/tegra}/hotplug.c |  0
> >  {arch/arm/mach-tegra => drivers/soc/tegra}/iomap.h |  0
> >  .../arm/mach-tegra => drivers/soc/tegra}/irammap.h |  0
> >  {arch/arm/mach-tegra => drivers/soc/tegra}/irq.c   |  1 -
> >  {arch/arm/mach-tegra => drivers/soc/tegra}/irq.h   |  6 --
> >  .../arm/mach-tegra => drivers/soc/tegra}/platsmp.c |  5 --
> >  .../mach-tegra => drivers/soc/tegra}/pm-tegra20.c  |  0
> >  .../mach-tegra => drivers/soc/tegra}/pm-tegra30.c  |  0
> >  {arch/arm/mach-tegra => drivers/soc/tegra}/pm.c    |  0
> >  {arch/arm/mach-tegra => drivers/soc/tegra}/pm.h    |  4 +-
> >  {arch/arm/mach-tegra => drivers/soc/tegra}/pmc.c   |  0
> >  drivers/soc/tegra/pmc.h                            | 35 ++++++++
> >  .../mach-tegra => drivers/soc/tegra}/powergate.c   |  0
> >  .../soc/tegra}/reset-handler.S                     |  0
> >  {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c |  0
> >  {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h |  2 -
> >  .../soc/tegra}/sleep-tegra20.S                     |  0
> >  .../soc/tegra}/sleep-tegra30.S                     |  0
> >  {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.S |  0
> >  {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.h |  2 -
> >  include/linux/tegra-soc.h                          | 95 ++++++++++++++++++++++
> >  36 files changed, 195 insertions(+), 135 deletions(-)
> >  delete mode 100644 arch/arm/mach-tegra/common.h
> >  delete mode 100644 arch/arm/mach-tegra/pmc.h
> >  create mode 100644 drivers/soc/tegra/Makefile
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/flowctrl.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/flowctrl.h (98%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/headsmp.S (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/hotplug.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/iomap.h (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/irammap.h (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/irq.c (99%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/irq.h (83%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/platsmp.c (98%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm-tegra20.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm-tegra30.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm.h (94%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pmc.c (100%)
> >  create mode 100644 drivers/soc/tegra/pmc.h
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/powergate.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep-tegra20.S (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep-tegra30.S (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.S (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.h (98%)
> > 
> NAK for this patch.
> 
> You are using drivers/soc/* as a dump yard for your SOC code which
> is not the intention we created drivers/soc/.

That was not my intention. What remains in arch/arm/mach-tegra at this
point doesn't actually have a corresponding subsystem (well, except
maybe the cpuidle code).

> Its really for subsystem drivers which doesn't have appropriate
> home in Linux kernel today. From your above patch ...
> 
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
> This should go into drivers/idle/*. if you have dependencies, please sort
> them out.

What exactly is the difference between drivers/idle and drivers/cpuidle?
There's an intel_idle driver in drivers/idle that includes cpuidle.h and
registers with that subsystem. But there's also an i7300_idle driver
that doesn't.

drivers/cpuidle seems like a better fit. I'll look into moving the code
there.

> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
> >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
> subsystem: drivers/power/reset/

drivers/power/reset seems to be for drivers that register functions to
reset a board. The above code for Tegra doesn't do that. Rather it sets
up the reset handlers for secondary CPUs and for suspend/resume.

> For tegra/*pm*/, you can use drivers/power or drivers/base/power/

drivers/power seems to be exclusively battery charger drivers. The pm.c,
pm-*.c and sleep-*.S set up suspend/resume. That doesn't seem to belong
in drivers/base/power either.

pmc.c implements various routines to access the power management
controller, some of which is needed by suspend/resume, some of it is
needed by SMP. powergate.c implements a subset of the PMC that needs to
be exported to drivers to enable power partitions on the SoC. I'm not
aware of subsystems that deal with this kind of driver.

> For SMP boot, ARMv8 expecting to have either PSCI based implementation or
> device tree based boot scheme. you can move towards that model if possible.

But we also have the code for SMP on 32-bit ARM. Should that remain in
arch/arm/mach-tegra or can it move to drivers/soc/tegra?

So the only thing in the above that I think could be moved somewhere
else is the cpuidle drivers. Or do you have any other suggestions for
the remaining code?

Thierry
Thierry Reding June 28, 2014, 1:24 a.m. UTC | #4
On Fri, Jun 27, 2014 at 03:10:25PM -0600, Stephen Warren wrote:
> On 06/27/2014 10:58 AM, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > These drivers are closely coupled and need to be moved as a whole. One
> > reason for moving them out of arch/arm/mach-tegra is to allow them to be
> > shared with 64-bit ARM.
> 
> Aside from any discussions re: splitting out cpuidle, power domains, ...
> into separate subsystem directories, this patch looks conceptually fine
> to me.
> 
> That said, given that I think arch/arm64 isn't going to have mach-*
> directories or machine descriptors, I wonder how all this code is going
> to be invoked at boot. arch/arm/mach-tegra/tegra.c calls a bunch of
> these functions at boot right now. So, it seems like moving the code so
> that it can compile in both places is only part of the solution?

Yes, that's correct. My primary motivation for moving this outside of
arch/arm/mach-tegra was because we need the powergate APIs for 64-bit
builds. I can look at moving out only that driver in a first step and
then look at moving cpuidle where it belongs. As for the other pieces
maybe a better solution would be to keep them in arch/arm/mach-tegra
for now until we've determined which of them are really needed for
64-bit and move them out on an as-needed basis. Untangling these on
by one will take some effort, though.

> Are we going to have an arm64-specific SoC driver that binds to the SoC
> compatible in order to initialize everything? If so, I wonder if the
> same can work for arch/arm so everything works the same way?

I think we'll have to do something along those lines. But I'm not sure
how well it will work out because some of these drivers need to be set
up very early, before any of the driver core is up.

Thierry
Santosh Shilimkar June 28, 2014, 5:15 p.m. UTC | #5
On Friday 27 June 2014 07:27 PM, Thierry Reding wrote:
> On Fri, Jun 27, 2014 at 01:30:04PM -0400, Santosh Shilimkar wrote:
>> +Arnd, Greg, Catalin and Kumar,
>>
>> On Friday 27 June 2014 12:58 PM, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> These drivers are closely coupled and need to be moved as a whole. One
>>> reason for moving them out of arch/arm/mach-tegra is to allow them to be
>>> shared with 64-bit ARM.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  arch/arm/mach-tegra/Makefile                       | 32 --------
>>>  arch/arm/mach-tegra/common.h                       |  4 -
>>>  arch/arm/mach-tegra/io.c                           | 27 +++++-
>>>  arch/arm/mach-tegra/pmc.h                          | 62 --------------
>>>  arch/arm/mach-tegra/tegra.c                        |  9 --
>>>  drivers/soc/Makefile                               |  1 +
>>>  drivers/soc/tegra/Makefile                         | 34 ++++++++
>>>  .../soc/tegra}/cpuidle-tegra114.c                  |  0
>>>  .../soc/tegra}/cpuidle-tegra20.c                   |  6 +-
>>>  .../soc/tegra}/cpuidle-tegra30.c                   |  0
>>>  .../arm/mach-tegra => drivers/soc/tegra}/cpuidle.c |  0
>>>  .../arm/mach-tegra => drivers/soc/tegra}/cpuidle.h |  3 -
>>>  .../mach-tegra => drivers/soc/tegra}/flowctrl.c    |  0
>>>  .../mach-tegra => drivers/soc/tegra}/flowctrl.h    |  2 -
>>>  .../arm/mach-tegra => drivers/soc/tegra}/headsmp.S |  0
>>>  .../arm/mach-tegra => drivers/soc/tegra}/hotplug.c |  0
>>>  {arch/arm/mach-tegra => drivers/soc/tegra}/iomap.h |  0
>>>  .../arm/mach-tegra => drivers/soc/tegra}/irammap.h |  0
>>>  {arch/arm/mach-tegra => drivers/soc/tegra}/irq.c   |  1 -
>>>  {arch/arm/mach-tegra => drivers/soc/tegra}/irq.h   |  6 --
>>>  .../arm/mach-tegra => drivers/soc/tegra}/platsmp.c |  5 --
>>>  .../mach-tegra => drivers/soc/tegra}/pm-tegra20.c  |  0
>>>  .../mach-tegra => drivers/soc/tegra}/pm-tegra30.c  |  0
>>>  {arch/arm/mach-tegra => drivers/soc/tegra}/pm.c    |  0
>>>  {arch/arm/mach-tegra => drivers/soc/tegra}/pm.h    |  4 +-
>>>  {arch/arm/mach-tegra => drivers/soc/tegra}/pmc.c   |  0
>>>  drivers/soc/tegra/pmc.h                            | 35 ++++++++
>>>  .../mach-tegra => drivers/soc/tegra}/powergate.c   |  0
>>>  .../soc/tegra}/reset-handler.S                     |  0
>>>  {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c |  0
>>>  {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h |  2 -
>>>  .../soc/tegra}/sleep-tegra20.S                     |  0
>>>  .../soc/tegra}/sleep-tegra30.S                     |  0
>>>  {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.S |  0
>>>  {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.h |  2 -
>>>  include/linux/tegra-soc.h                          | 95 ++++++++++++++++++++++
>>>  36 files changed, 195 insertions(+), 135 deletions(-)
>>>  delete mode 100644 arch/arm/mach-tegra/common.h
>>>  delete mode 100644 arch/arm/mach-tegra/pmc.h
>>>  create mode 100644 drivers/soc/tegra/Makefile
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/flowctrl.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/flowctrl.h (98%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/headsmp.S (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/hotplug.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/iomap.h (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/irammap.h (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/irq.c (99%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/irq.h (83%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/platsmp.c (98%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm-tegra20.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm-tegra30.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pm.h (94%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/pmc.c (100%)
>>>  create mode 100644 drivers/soc/tegra/pmc.h
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/powergate.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep-tegra20.S (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep-tegra30.S (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.S (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/sleep.h (98%)
>>>
>> NAK for this patch.
>>
>> You are using drivers/soc/* as a dump yard for your SOC code which
>> is not the intention we created drivers/soc/.
> 
> That was not my intention. What remains in arch/arm/mach-tegra at this
> point doesn't actually have a corresponding subsystem (well, except
> maybe the cpuidle code).
> 
>> Its really for subsystem drivers which doesn't have appropriate
>> home in Linux kernel today. From your above patch ...
>>
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
>> This should go into drivers/idle/*. if you have dependencies, please sort
>> them out.
> 
> What exactly is the difference between drivers/idle and drivers/cpuidle?
> There's an intel_idle driver in drivers/idle that includes cpuidle.h and
> registers with that subsystem. But there's also an i7300_idle driver
> that doesn't.
> 
> drivers/cpuidle seems like a better fit. I'll look into moving the code
> there.
> 
I meant drivers/cpuidle.

>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
>>>  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
>> subsystem: drivers/power/reset/
> 
> drivers/power/reset seems to be for drivers that register functions to
> reset a board. The above code for Tegra doesn't do that. Rather it sets
> up the reset handlers for secondary CPUs and for suspend/resume.
>
So this is the suspend code which should go along with rest of your
PM code.
 
>> For tegra/*pm*/, you can use drivers/power or drivers/base/power/
> 
> drivers/power seems to be exclusively battery charger drivers. The pm.c,
> pm-*.c and sleep-*.S set up suspend/resume. That doesn't seem to belong
> in drivers/base/power either.
>
It does have AVS PM code as well and also home for power controller
code. Similar efforts are happening on OMAP [1] to move the PM code
under drivers/power/* 
 
> pmc.c implements various routines to access the power management
> controller, some of which is needed by suspend/resume, some of it is
> needed by SMP. powergate.c implements a subset of the PMC that needs to
> be exported to drivers to enable power partitions on the SoC. I'm not
> aware of subsystems that deal with this kind of driver.
>
Please see above.
 
>> For SMP boot, ARMv8 expecting to have either PSCI based implementation or
>> device tree based boot scheme. you can move towards that model if possible.
> 
> But we also have the code for SMP on 32-bit ARM. Should that remain in
> arch/arm/mach-tegra or can it move to drivers/soc/tegra?
> 
You answered yourself. I don't see any point moving such a code to drivers
which is really SMP bring up code for ARM 32 bit. For ARM 64 bit, as
outline by maintainers (Catalin, Arnd), you should use PSCI or DT based
boot scheme.

> So the only thing in the above that I think could be moved somewhere
> else is the cpuidle drivers. Or do you have any other suggestions for
> the remaining code?
> 
Infact all of your code already has a potential home and I
suggest you discuss it with those subsystem maintainers.

[1] "[PATCH 00/55]: ARM: OMAP2+: PRCM move to drivers"
http://www.spinics.net/lists/arm-kernel/msg319360.html
Thierry Reding June 28, 2014, 8:40 p.m. UTC | #6
On Sat, Jun 28, 2014 at 01:15:09PM -0400, Santosh Shilimkar wrote:
> On Friday 27 June 2014 07:27 PM, Thierry Reding wrote:
[...]
> > drivers/power seems to be exclusively battery charger drivers. The pm.c,
> > pm-*.c and sleep-*.S set up suspend/resume. That doesn't seem to belong
> > in drivers/base/power either.
> >
> It does have AVS PM code as well and also home for power controller
> code. Similar efforts are happening on OMAP [1] to move the PM code
> under drivers/power/*

Quite frankly that doesn't sound any better than putting that code into
drivers/soc. If there is no common framework for a set of drivers, then
I don't see what improvement we get by putting all of them together. If
anything it makes things more chaotic because we sprinkle random driver
bits across the whole tree rather than keeping them together.

That sounds a lot like a dumping ground to me.

According to the MAINTAINERS file, drivers/power is:

	POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS

and that's just not what the Tegra PM code is. That said, it may have
been a little premature to move this code out of arch/arm/mach-tegra,
and I think I'll revisit at a later point.

> > pmc.c implements various routines to access the power management
> > controller, some of which is needed by suspend/resume, some of it is
> > needed by SMP. powergate.c implements a subset of the PMC that needs to
> > be exported to drivers to enable power partitions on the SoC. I'm not
> > aware of subsystems that deal with this kind of driver.
> >
> Please see above.

Like I said, I don't see what business suspend/resume related code has
in drivers/power. What we're talking about here really is functionality
very specific to Tegra. Also some of that code needs to be run at very
early points in the boot process, so we can't reasonably turn it into a
proper driver anyway.

Also, the real win we get from moving code out into drivers/ is if we
can provide a common framework for them. I don't see how we can possibly
do that for this code since there simply isn't enough commonality
between SoCs. At the same time we now have a situation where both 32-bit
and 64-bit variants of some SoCs share some of the same hardware at the
very low level and since we don't have mach-* directories for 64-bit ARM
and shouldn't be duplicating code either, we need to find a new home for
this type of code. drivers/soc seemed to fit perfectly well.

Thierry
Arnd Bergmann June 30, 2014, 7:20 a.m. UTC | #7
On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
> > > pmc.c implements various routines to access the power management
> > > controller, some of which is needed by suspend/resume, some of it is
> > > needed by SMP. powergate.c implements a subset of the PMC that needs to
> > > be exported to drivers to enable power partitions on the SoC. I'm not
> > > aware of subsystems that deal with this kind of driver.
> > >
> > Please see above.
> 
> Like I said, I don't see what business suspend/resume related code has
> in drivers/power. What we're talking about here really is functionality
> very specific to Tegra. Also some of that code needs to be run at very
> early points in the boot process, so we can't reasonably turn it into a
> proper driver anyway.

I believe the powergate.c stuff can be changed into pm_domain code, but
we don't have a good subsystem with generic DT bindings yet, so that
may need some more groundwork first. drivers/power or a subdirectory
of that may end up being the right place though.

> Also, the real win we get from moving code out into drivers/ is if we
> can provide a common framework for them. I don't see how we can possibly
> do that for this code since there simply isn't enough commonality
> between SoCs. At the same time we now have a situation where both 32-bit
> and 64-bit variants of some SoCs share some of the same hardware at the
> very low level and since we don't have mach-* directories for 64-bit ARM
> and shouldn't be duplicating code either, we need to find a new home for
> this type of code. drivers/soc seemed to fit perfectly well.

For the low-level stuff yes, but I agree with Santosh there needs to be
some more work trying to split out individual high-level drivers.

There are two common patterns for the interface between the low-level
register access and the more high-level stuff:  you can either use
a "syscon" driver that just exports a struct regmap and that gets referenced
from other drivers using a phandle in their device nodes, or you have
a driver in drivers/soc that exports a somewhat higher-level interface
and comes with its own header file that gets included by other drivers.
At the moment, the syscon/regmap variant can only be used once device
drivers are loaded, but there is some broad agreement that it should
be changed to allow calling syscon_regmap_lookup_by_phandle() at
early boot using just DT accessors.

	Arnd
Arnd Bergmann June 30, 2014, 7:23 a.m. UTC | #8
On Saturday 28 June 2014 13:15:09 Santosh Shilimkar wrote:
> >> For SMP boot, ARMv8 expecting to have either PSCI based implementation or
> >> device tree based boot scheme. you can move towards that model if possible.
> > 
> > But we also have the code for SMP on 32-bit ARM. Should that remain in
> > arch/arm/mach-tegra or can it move to drivers/soc/tegra?
> > 
> You answered yourself. I don't see any point moving such a code to drivers
> which is really SMP bring up code for ARM 32 bit. For ARM 64 bit, as
> outline by maintainers (Catalin, Arnd), you should use PSCI or DT based
> boot scheme.

I was hoping that one day we could unify the SMP bootup code with the
cpuidle infrastructure, as a lot of the underlying functions are shared
on the majority of SoCs. I have no plans to work on that myself though,
and it would have to be agreed on by the cpuidle maintainers.

For ARM64, the hope was indeed to use PSCI for all SoCs. I just have no
idea how that would be enforced for mobile SoCs that get shipped to
system integrators with an bootloader and kernel tree that doesn't even
get reviewed anywhere first.

	Arnd
Thierry Reding June 30, 2014, 9:01 a.m. UTC | #9
On Mon, Jun 30, 2014 at 09:20:30AM +0200, Arnd Bergmann wrote:
> On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
> > > > pmc.c implements various routines to access the power management
> > > > controller, some of which is needed by suspend/resume, some of it is
> > > > needed by SMP. powergate.c implements a subset of the PMC that needs to
> > > > be exported to drivers to enable power partitions on the SoC. I'm not
> > > > aware of subsystems that deal with this kind of driver.
> > > >
> > > Please see above.
> > 
> > Like I said, I don't see what business suspend/resume related code has
> > in drivers/power. What we're talking about here really is functionality
> > very specific to Tegra. Also some of that code needs to be run at very
> > early points in the boot process, so we can't reasonably turn it into a
> > proper driver anyway.
> 
> I believe the powergate.c stuff can be changed into pm_domain code, but
> we don't have a good subsystem with generic DT bindings yet, so that
> may need some more groundwork first. drivers/power or a subdirectory
> of that may end up being the right place though.

Last time I checked the PM domain code didn't seem like a good fit. I'll
go recheck and see if I can make it work.

One minor issue with putting the driver in drivers/power is that that
directory is only built when POWER_SUPPLY is selected. And since this
isn't really a power supply driver at all, either we need to change the
drivers/Makefile to descend unconditionally or we need to find another
home.

I'll see if I can come up with yet another generic binding for this in
an attempt to move this out of arch/arm/mach-tegra...

> > Also, the real win we get from moving code out into drivers/ is if we
> > can provide a common framework for them. I don't see how we can possibly
> > do that for this code since there simply isn't enough commonality
> > between SoCs. At the same time we now have a situation where both 32-bit
> > and 64-bit variants of some SoCs share some of the same hardware at the
> > very low level and since we don't have mach-* directories for 64-bit ARM
> > and shouldn't be duplicating code either, we need to find a new home for
> > this type of code. drivers/soc seemed to fit perfectly well.
> 
> For the low-level stuff yes, but I agree with Santosh there needs to be
> some more work trying to split out individual high-level drivers.
> 
> There are two common patterns for the interface between the low-level
> register access and the more high-level stuff:  you can either use
> a "syscon" driver that just exports a struct regmap and that gets referenced
> from other drivers using a phandle in their device nodes, or you have
> a driver in drivers/soc that exports a somewhat higher-level interface
> and comes with its own header file that gets included by other drivers.
> At the moment, the syscon/regmap variant can only be used once device
> drivers are loaded, but there is some broad agreement that it should
> be changed to allow calling syscon_regmap_lookup_by_phandle() at
> early boot using just DT accessors.

Note that we already have all these drivers and they do work for earlier
Tegra generations. My attempt to move these out of arch/arm/mach-tegra
is merely about being able to share them with upcoming 64-bit variants.
So it's not about adding new functionality but rather about keeping the
existing level of functionality on 64-bit.

I'm not a fan of the syscon/regmap approach at all and the existing
drivers use a higher-level API already, so I think we're going to stick
with that.

Thierry
Catalin Marinas June 30, 2014, 9:44 a.m. UTC | #10
On Mon, Jun 30, 2014 at 08:23:45AM +0100, Arnd Bergmann wrote:
> On Saturday 28 June 2014 13:15:09 Santosh Shilimkar wrote:
> > >> For SMP boot, ARMv8 expecting to have either PSCI based implementation or
> > >> device tree based boot scheme. you can move towards that model if possible.
> > > 
> > > But we also have the code for SMP on 32-bit ARM. Should that remain in
> > > arch/arm/mach-tegra or can it move to drivers/soc/tegra?
> > > 
> > You answered yourself. I don't see any point moving such a code to drivers
> > which is really SMP bring up code for ARM 32 bit. For ARM 64 bit, as
> > outline by maintainers (Catalin, Arnd), you should use PSCI or DT based
> > boot scheme.
> 
> I was hoping that one day we could unify the SMP bootup code with the
> cpuidle infrastructure, as a lot of the underlying functions are shared
> on the majority of SoCs. I have no plans to work on that myself though,
> and it would have to be agreed on by the cpuidle maintainers.

If you look at the arm64 implementation and Lorenzo's generic cpuidle
code, that's not far off. Basically for arm64 we have a cpu_operations
structure which implements boot, suspend, disable etc., placing SMP
booting, hotplug and cpuidle together.

While PSCI is the (strongly) recommended way, the current structure
allows for other implementations and spin-table is an example (though
this lacks any form of parking CPUs back to firmware). What I don't want
to see is every platform implementing its own booting method for no good
reasons and especially when functionality is duplicated with other
platforms.

In the tegra case, I suppose the arm32 implementation assumes Linux
running in secure mode. On arm64 we don't support this (though it may
work for certain configurations) and once you realise you need SMC calls
for CPU up/down/suspend, I would rather standardise on PSCI (unless a
better standard appears ;)).

> For ARM64, the hope was indeed to use PSCI for all SoCs. I just have no
> idea how that would be enforced for mobile SoCs that get shipped to
> system integrators with an bootloader and kernel tree that doesn't even
> get reviewed anywhere first.

We can't enforce this but at least with enough "marketing" support, we
can sell this as a good idea. For example, if one implements PSCI you
get (generic) cpuidle for free (just DT bindings). That's definitely not
enough and other things like a comprehensive validation suite for PSCI
also helps (currently ARM only works on bare metal but it would be good
to have a kernel-based test suite as well).
Catalin Marinas June 30, 2014, 10:25 a.m. UTC | #11
On Sat, Jun 28, 2014 at 12:27:58AM +0100, Thierry Reding wrote:
> On Fri, Jun 27, 2014 at 01:30:04PM -0400, Santosh Shilimkar wrote:
> > On Friday 27 June 2014 12:58 PM, Thierry Reding wrote:
> > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
> > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
> > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
> > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
> > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
> > This should go into drivers/idle/*. if you have dependencies, please sort
> > them out.
> 
> What exactly is the difference between drivers/idle and drivers/cpuidle?
> There's an intel_idle driver in drivers/idle that includes cpuidle.h and
> registers with that subsystem. But there's also an i7300_idle driver
> that doesn't.
> 
> drivers/cpuidle seems like a better fit. I'll look into moving the code
> there.

Actually, please look at Lorenzo's generic cpuidle series. Basically
most cpuidle drivers simply define some C states and call the
corresponding platform back-end. On arm64, we aim to separate the
back-end in the cpu_operations structure and just have a generic cpuidle
driver with states defined in the DT (and passed to the back-end).

As for the back-end, what about using PSCI?

> > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
> > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
> > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
> > subsystem: drivers/power/reset/
> 
> drivers/power/reset seems to be for drivers that register functions to
> reset a board. The above code for Tegra doesn't do that. Rather it sets
> up the reset handlers for secondary CPUs and for suspend/resume.

PSCI again?
Catalin Marinas June 30, 2014, 10:36 a.m. UTC | #12
On Mon, Jun 30, 2014 at 10:01:19AM +0100, Thierry Reding wrote:
> On Mon, Jun 30, 2014 at 09:20:30AM +0200, Arnd Bergmann wrote:
> > On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
> > > Also, the real win we get from moving code out into drivers/ is if we
> > > can provide a common framework for them. I don't see how we can possibly
> > > do that for this code since there simply isn't enough commonality
> > > between SoCs. At the same time we now have a situation where both 32-bit
> > > and 64-bit variants of some SoCs share some of the same hardware at the
> > > very low level and since we don't have mach-* directories for 64-bit ARM
> > > and shouldn't be duplicating code either, we need to find a new home for
> > > this type of code. drivers/soc seemed to fit perfectly well.
> > 
> > For the low-level stuff yes, but I agree with Santosh there needs to be
> > some more work trying to split out individual high-level drivers.
> > 
> > There are two common patterns for the interface between the low-level
> > register access and the more high-level stuff:  you can either use
> > a "syscon" driver that just exports a struct regmap and that gets referenced
> > from other drivers using a phandle in their device nodes, or you have
> > a driver in drivers/soc that exports a somewhat higher-level interface
> > and comes with its own header file that gets included by other drivers.
> > At the moment, the syscon/regmap variant can only be used once device
> > drivers are loaded, but there is some broad agreement that it should
> > be changed to allow calling syscon_regmap_lookup_by_phandle() at
> > early boot using just DT accessors.
> 
> Note that we already have all these drivers and they do work for earlier
> Tegra generations. My attempt to move these out of arch/arm/mach-tegra
> is merely about being able to share them with upcoming 64-bit variants.
> So it's not about adding new functionality but rather about keeping the
> existing level of functionality on 64-bit.

Only that for arm64 we try to do things slightly differently and not
because we just like to but because we want better standardisation
across SoCs. One example is the booting protocol. Another example, you
don't get some early initcalls for your SoC (no machine descriptor). The
hardware should be configured by firmware in such a way that it boots at
least up to arch_initcall() level (ideally, we should move anything to
device_initcall() but it's not always easy).

> I'm not a fan of the syscon/regmap approach at all and the existing
> drivers use a higher-level API already, so I think we're going to stick
> with that.

It's not about whether you are a fan of it or not but whether we can get
more code sharing across several SoCs and not just between tegra arm32
and arm64 versions.
Thierry Reding June 30, 2014, 10:48 a.m. UTC | #13
On Mon, Jun 30, 2014 at 11:36:49AM +0100, Catalin Marinas wrote:
> On Mon, Jun 30, 2014 at 10:01:19AM +0100, Thierry Reding wrote:
> > On Mon, Jun 30, 2014 at 09:20:30AM +0200, Arnd Bergmann wrote:
> > > On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
> > > > Also, the real win we get from moving code out into drivers/ is if we
> > > > can provide a common framework for them. I don't see how we can possibly
> > > > do that for this code since there simply isn't enough commonality
> > > > between SoCs. At the same time we now have a situation where both 32-bit
> > > > and 64-bit variants of some SoCs share some of the same hardware at the
> > > > very low level and since we don't have mach-* directories for 64-bit ARM
> > > > and shouldn't be duplicating code either, we need to find a new home for
> > > > this type of code. drivers/soc seemed to fit perfectly well.
> > > 
> > > For the low-level stuff yes, but I agree with Santosh there needs to be
> > > some more work trying to split out individual high-level drivers.
> > > 
> > > There are two common patterns for the interface between the low-level
> > > register access and the more high-level stuff:  you can either use
> > > a "syscon" driver that just exports a struct regmap and that gets referenced
> > > from other drivers using a phandle in their device nodes, or you have
> > > a driver in drivers/soc that exports a somewhat higher-level interface
> > > and comes with its own header file that gets included by other drivers.
> > > At the moment, the syscon/regmap variant can only be used once device
> > > drivers are loaded, but there is some broad agreement that it should
> > > be changed to allow calling syscon_regmap_lookup_by_phandle() at
> > > early boot using just DT accessors.
> > 
> > Note that we already have all these drivers and they do work for earlier
> > Tegra generations. My attempt to move these out of arch/arm/mach-tegra
> > is merely about being able to share them with upcoming 64-bit variants.
> > So it's not about adding new functionality but rather about keeping the
> > existing level of functionality on 64-bit.
> 
> Only that for arm64 we try to do things slightly differently and not
> because we just like to but because we want better standardisation
> across SoCs. One example is the booting protocol. Another example, you
> don't get some early initcalls for your SoC (no machine descriptor). The
> hardware should be configured by firmware in such a way that it boots at
> least up to arch_initcall() level (ideally, we should move anything to
> device_initcall() but it's not always easy).

Well, one of the problems on Tegra is that we need part of the PMC to
power up CPUs. There's no firmware that could do this for us. We need
also access to another block called flow controller. Both of those
drivers need to be available very early for things to work. At the same
time the driver exposes control over power domains. So while we possibly
can push CPU bringup/teardown to firmware for ARM64, we can't do the
same for the other parts of the PMC, so we end up with a weird kind of
driver.

Parts of it can't register with the driver model because it isn't
available that early, and at the same time we need to register parts
only later because they require the driver model.

> > I'm not a fan of the syscon/regmap approach at all and the existing
> > drivers use a higher-level API already, so I think we're going to stick
> > with that.
> 
> It's not about whether you are a fan of it or not but whether we can get
> more code sharing across several SoCs and not just between tegra arm32
> and arm64 versions.

syscon by definition cannot be shared between different SoCs.

Thierry
Thierry Reding June 30, 2014, 10:49 a.m. UTC | #14
On Mon, Jun 30, 2014 at 11:25:12AM +0100, Catalin Marinas wrote:
> On Sat, Jun 28, 2014 at 12:27:58AM +0100, Thierry Reding wrote:
> > On Fri, Jun 27, 2014 at 01:30:04PM -0400, Santosh Shilimkar wrote:
> > > On Friday 27 June 2014 12:58 PM, Thierry Reding wrote:
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
> > > This should go into drivers/idle/*. if you have dependencies, please sort
> > > them out.
> > 
> > What exactly is the difference between drivers/idle and drivers/cpuidle?
> > There's an intel_idle driver in drivers/idle that includes cpuidle.h and
> > registers with that subsystem. But there's also an i7300_idle driver
> > that doesn't.
> > 
> > drivers/cpuidle seems like a better fit. I'll look into moving the code
> > there.
> 
> Actually, please look at Lorenzo's generic cpuidle series. Basically
> most cpuidle drivers simply define some C states and call the
> corresponding platform back-end. On arm64, we aim to separate the
> back-end in the cpu_operations structure and just have a generic cpuidle
> driver with states defined in the DT (and passed to the back-end).
> 
> As for the back-end, what about using PSCI?
> 
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
> > > subsystem: drivers/power/reset/
> > 
> > drivers/power/reset seems to be for drivers that register functions to
> > reset a board. The above code for Tegra doesn't do that. Rather it sets
> > up the reset handlers for secondary CPUs and for suspend/resume.
> 
> PSCI again?

I have yet to look at what exactly needs to be done on 64-bit Tegra to
get CPUs up. So I'll revisit this part of the patches when that's
happened. It may yet turn out that most of this doesn't need to be
shared after all.

Thierry
Peter De Schrijver June 30, 2014, 11:46 a.m. UTC | #15
On Mon, Jun 30, 2014 at 12:25:12PM +0200, Catalin Marinas wrote:
> On Sat, Jun 28, 2014 at 12:27:58AM +0100, Thierry Reding wrote:
> > On Fri, Jun 27, 2014 at 01:30:04PM -0400, Santosh Shilimkar wrote:
> > > On Friday 27 June 2014 12:58 PM, Thierry Reding wrote:
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
> > > This should go into drivers/idle/*. if you have dependencies, please sort
> > > them out.
> > 
> > What exactly is the difference between drivers/idle and drivers/cpuidle?
> > There's an intel_idle driver in drivers/idle that includes cpuidle.h and
> > registers with that subsystem. But there's also an i7300_idle driver
> > that doesn't.
> > 
> > drivers/cpuidle seems like a better fit. I'll look into moving the code
> > there.
> 
> Actually, please look at Lorenzo's generic cpuidle series. Basically
> most cpuidle drivers simply define some C states and call the
> corresponding platform back-end. On arm64, we aim to separate the
> back-end in the cpu_operations structure and just have a generic cpuidle
> driver with states defined in the DT (and passed to the back-end).
> 
> As for the back-end, what about using PSCI?
> 
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
> > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
> > > subsystem: drivers/power/reset/
> > 
> > drivers/power/reset seems to be for drivers that register functions to
> > reset a board. The above code for Tegra doesn't do that. Rather it sets
> > up the reset handlers for secondary CPUs and for suspend/resume.
> 
> PSCI again?

Not possible for boards where linux runs in NS (roth and tn7). The secure
monitor on those is outside our control and does not implement PSCI. There
might be some value for Tegra114 and Tegra124 if we would like to use HYP
mode, but that's not on the radar today. It's a waste of time for Tegra20
and Tegra30.

Cheers,

Peter.
Lorenzo Pieralisi June 30, 2014, 1:16 p.m. UTC | #16
On Mon, Jun 30, 2014 at 11:48:15AM +0100, Thierry Reding wrote:
> On Mon, Jun 30, 2014 at 11:36:49AM +0100, Catalin Marinas wrote:
> > On Mon, Jun 30, 2014 at 10:01:19AM +0100, Thierry Reding wrote:
> > > On Mon, Jun 30, 2014 at 09:20:30AM +0200, Arnd Bergmann wrote:
> > > > On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
> > > > > Also, the real win we get from moving code out into drivers/ is if we
> > > > > can provide a common framework for them. I don't see how we can possibly
> > > > > do that for this code since there simply isn't enough commonality
> > > > > between SoCs. At the same time we now have a situation where both 32-bit
> > > > > and 64-bit variants of some SoCs share some of the same hardware at the
> > > > > very low level and since we don't have mach-* directories for 64-bit ARM
> > > > > and shouldn't be duplicating code either, we need to find a new home for
> > > > > this type of code. drivers/soc seemed to fit perfectly well.
> > > > 
> > > > For the low-level stuff yes, but I agree with Santosh there needs to be
> > > > some more work trying to split out individual high-level drivers.
> > > > 
> > > > There are two common patterns for the interface between the low-level
> > > > register access and the more high-level stuff:  you can either use
> > > > a "syscon" driver that just exports a struct regmap and that gets referenced
> > > > from other drivers using a phandle in their device nodes, or you have
> > > > a driver in drivers/soc that exports a somewhat higher-level interface
> > > > and comes with its own header file that gets included by other drivers.
> > > > At the moment, the syscon/regmap variant can only be used once device
> > > > drivers are loaded, but there is some broad agreement that it should
> > > > be changed to allow calling syscon_regmap_lookup_by_phandle() at
> > > > early boot using just DT accessors.
> > > 
> > > Note that we already have all these drivers and they do work for earlier
> > > Tegra generations. My attempt to move these out of arch/arm/mach-tegra
> > > is merely about being able to share them with upcoming 64-bit variants.
> > > So it's not about adding new functionality but rather about keeping the
> > > existing level of functionality on 64-bit.
> > 
> > Only that for arm64 we try to do things slightly differently and not
> > because we just like to but because we want better standardisation
> > across SoCs. One example is the booting protocol. Another example, you
> > don't get some early initcalls for your SoC (no machine descriptor). The
> > hardware should be configured by firmware in such a way that it boots at
> > least up to arch_initcall() level (ideally, we should move anything to
> > device_initcall() but it's not always easy).
> 
> Well, one of the problems on Tegra is that we need part of the PMC to
> power up CPUs. There's no firmware that could do this for us. We need
> also access to another block called flow controller. Both of those
> drivers need to be available very early for things to work. At the same
> time the driver exposes control over power domains. So while we possibly
> can push CPU bringup/teardown to firmware for ARM64, we can't do the
> same for the other parts of the PMC, so we end up with a weird kind of
> driver.
> 
> Parts of it can't register with the driver model because it isn't
> available that early, and at the same time we need to register parts
> only later because they require the driver model.

That sounds familiar and that's exactly the problem, and that's a problem
that won't disappear by just moving code around directories (hey, it is
not that we have not tried =)), as you (and I) know very well:

https://lkml.org/lkml/2013/8/16/399
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/190067.html

Either a common way of dealing with this early CPU bring-up code is
implemented (but it can't be platform specific otherwise we are back to
square one) or we decide that this code does not belong in the kernel and
should be abstracted away.

And I think that most of the early bring up issues are just related to
powering-up/resetting a cpu for it to be booted, probably that's the
abstraction we should be focusing on, because it should just require poking
a bunch of registers to kickstart a CPU, no more than that, FW should be
able to deal with that and that's yet another reason behind PSCI design.

I am not convinced that the early device model would be useful for any
other reason, that's why shoehorning kernel changes in the current driver
model just to bring up secondaries is not something I consider useful
and proper, in the first place, but I am happy to hear other possible
concerns.

Lorenzo

> 
> > > I'm not a fan of the syscon/regmap approach at all and the existing
> > > drivers use a higher-level API already, so I think we're going to stick
> > > with that.
> > 
> > It's not about whether you are a fan of it or not but whether we can get
> > more code sharing across several SoCs and not just between tegra arm32
> > and arm64 versions.
> 
> syscon by definition cannot be shared between different SoCs.
> 
> Thierry


> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Catalin Marinas June 30, 2014, 2:13 p.m. UTC | #17
On Mon, Jun 30, 2014 at 12:46:21PM +0100, Peter De Schrijver wrote:
> On Mon, Jun 30, 2014 at 12:25:12PM +0200, Catalin Marinas wrote:
> > On Sat, Jun 28, 2014 at 12:27:58AM +0100, Thierry Reding wrote:
> > > On Fri, Jun 27, 2014 at 01:30:04PM -0400, Santosh Shilimkar wrote:
> > > > On Friday 27 June 2014 12:58 PM, Thierry Reding wrote:
> > > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra114.c (100%)
> > > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra20.c (100%)
> > > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle-tegra30.c (100%)
> > > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.c (100%)
> > > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/cpuidle.h (91%)
> > > > This should go into drivers/idle/*. if you have dependencies, please sort
> > > > them out.
> > > 
> > > What exactly is the difference between drivers/idle and drivers/cpuidle?
> > > There's an intel_idle driver in drivers/idle that includes cpuidle.h and
> > > registers with that subsystem. But there's also an i7300_idle driver
> > > that doesn't.
> > > 
> > > drivers/cpuidle seems like a better fit. I'll look into moving the code
> > > there.
> > 
> > Actually, please look at Lorenzo's generic cpuidle series. Basically
> > most cpuidle drivers simply define some C states and call the
> > corresponding platform back-end. On arm64, we aim to separate the
> > back-end in the cpu_operations structure and just have a generic cpuidle
> > driver with states defined in the DT (and passed to the back-end).
> > 
> > As for the back-end, what about using PSCI?
> > 
> > > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset-handler.S (100%)
> > > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.c (100%)
> > > > >  rename {arch/arm/mach-tegra => drivers/soc/tegra}/reset.h (97%)
> > > > subsystem: drivers/power/reset/
> > > 
> > > drivers/power/reset seems to be for drivers that register functions to
> > > reset a board. The above code for Tegra doesn't do that. Rather it sets
> > > up the reset handlers for secondary CPUs and for suspend/resume.
> > 
> > PSCI again?
> 
> Not possible for boards where linux runs in NS (roth and tn7).

That's one of the main use-cases for PSCI - Linux running in NS mode.
AFAIK, the boards that you mentioned are not ARMv8? We don't intend to
enforce PSCI on ARMv7 platforms, mainly because of the legacy firmware
(though where starting from scratch it would be nice). That means that
the code won't be shared with arm64 and you can leave it under arch/arm
(it's a lot of AArch32 assembly anyway).

> The secure monitor on those is outside our control and does not
> implement PSCI.

As I said, I don't think we should change firmware on existing ARMv7
SoCs but for a new ARMv8 SoC I strongly recommend PSCI. The firmware
needs to be written anyway (different AArch64 exception model), so you
can raise such requirement with your firmware provider (ARM also
provides Trusted Firmware freely as a quick start which implements PSCI,
though you can only use it as an example).

> There might be some value for Tegra114 and Tegra124 if we would like
> to use HYP mode, but that's not on the radar today. It's a waste of
> time for Tegra20 and Tegra30.

I think you should keep the Hyp support in mind anyway and test Linux
booting in NS mode on your platforms. It may not be on your radar today
for a SoC but if you later decide that it is, we end up with needing
different code paths in Linux for the same SoC depending on how it boots
(and you can't easily detect this at boot time, nor you can pass it via
DT).
Peter De Schrijver June 30, 2014, 2:42 p.m. UTC | #18
> > > PSCI again?
> > 
> > Not possible for boards where linux runs in NS (roth and tn7).
> 
> That's one of the main use-cases for PSCI - Linux running in NS mode.
> AFAIK, the boards that you mentioned are not ARMv8? We don't intend to

They are v7 indeed...

> enforce PSCI on ARMv7 platforms, mainly because of the legacy firmware
> (though where starting from scratch it would be nice). That means that
> the code won't be shared with arm64 and you can leave it under arch/arm
> (it's a lot of AArch32 assembly anyway).
> 
> > The secure monitor on those is outside our control and does not
> > implement PSCI.
> 
> As I said, I don't think we should change firmware on existing ARMv7
> SoCs but for a new ARMv8 SoC I strongly recommend PSCI. The firmware
> needs to be written anyway (different AArch64 exception model), so you
> can raise such requirement with your firmware provider (ARM also
> provides Trusted Firmware freely as a quick start which implements PSCI,
> though you can only use it as an example).

For v8 this should be fine yes...

> 
> > There might be some value for Tegra114 and Tegra124 if we would like
> > to use HYP mode, but that's not on the radar today. It's a waste of
> > time for Tegra20 and Tegra30.
> 
> I think you should keep the Hyp support in mind anyway and test Linux
> booting in NS mode on your platforms. It may not be on your radar today
> for a SoC but if you later decide that it is, we end up with needing
> different code paths in Linux for the same SoC depending on how it boots
> (and you can't easily detect this at boot time, nor you can pass it via
> DT).

You can just look for a psci DT node? If this node is missing, fallback to
the existing tegra specific mechanism. At least, that's how I envision doing
PSCI for HYP mode on v7 based boards where linux today runs in secure mode.
u-boot loads a minimal secure monitor which only does PSCI (we don't care about
any security as there are no keys available for signing on those boards
anyway), adds the PSCI node to the DT and then boots linux in HYP mode. 
tegra_cpu_reset_handler_enable() checks for a PSCI node and uses it if
available. Else it will fallback to the existing methods. Similar for cpuidle.
No guarantees that this will ever be implemented though :) Maybe.

Cheers,

Peter.
Peter De Schrijver June 30, 2014, 2:50 p.m. UTC | #19
> > 
> > I think you should keep the Hyp support in mind anyway and test Linux
> > booting in NS mode on your platforms. It may not be on your radar today
> > for a SoC but if you later decide that it is, we end up with needing
> > different code paths in Linux for the same SoC depending on how it boots
> > (and you can't easily detect this at boot time, nor you can pass it via
> > DT).
> 
> You can just look for a psci DT node? If this node is missing, fallback to
> the existing tegra specific mechanism. At least, that's how I envision doing
> PSCI for HYP mode on v7 based boards where linux today runs in secure mode.
> u-boot loads a minimal secure monitor which only does PSCI (we don't care about
> any security as there are no keys available for signing on those boards
> anyway), adds the PSCI node to the DT and then boots linux in HYP mode. 
> tegra_cpu_reset_handler_enable() checks for a PSCI node and uses it if
> available. Else it will fallback to the existing methods. Similar for cpuidle.
> No guarantees that this will ever be implemented though :) Maybe.
> 

Come to think of it, we follow almost the same approach for Tegra114 where we
support boards with Trusted Foundations secure monitor and linux in secure
mode with the same kernel image.

Cheers,

Peter.
Catalin Marinas June 30, 2014, 4:08 p.m. UTC | #20
On Mon, Jun 30, 2014 at 03:42:17PM +0100, Peter De Schrijver wrote:
> > > There might be some value for Tegra114 and Tegra124 if we would like
> > > to use HYP mode, but that's not on the radar today. It's a waste of
> > > time for Tegra20 and Tegra30.
> > 
> > I think you should keep the Hyp support in mind anyway and test Linux
> > booting in NS mode on your platforms. It may not be on your radar today
> > for a SoC but if you later decide that it is, we end up with needing
> > different code paths in Linux for the same SoC depending on how it boots
> > (and you can't easily detect this at boot time, nor you can pass it via
> > DT).
> 
> You can just look for a psci DT node? If this node is missing, fallback to
> the existing tegra specific mechanism. At least, that's how I envision doing
> PSCI for HYP mode on v7 based boards where linux today runs in secure mode.

This should work, assuming that setup required before the DT is
available is already handled by firmware (for example, SMP coherency bit
in secure-only registers).

> u-boot loads a minimal secure monitor which only does PSCI (we don't care about
> any security as there are no keys available for signing on those boards
> anyway), adds the PSCI node to the DT and then boots linux in HYP mode. 

BTW, there are some patches under review for adding PSCI support to
U-Boot.
Stephen Warren June 30, 2014, 6:45 p.m. UTC | #21
On 06/30/2014 01:23 AM, Arnd Bergmann wrote:
> On Saturday 28 June 2014 13:15:09 Santosh Shilimkar wrote:
>>>> For SMP boot, ARMv8 expecting to have either PSCI based implementation or
>>>> device tree based boot scheme. you can move towards that model if possible.
>>>
>>> But we also have the code for SMP on 32-bit ARM. Should that remain in
>>> arch/arm/mach-tegra or can it move to drivers/soc/tegra?
>>>
>> You answered yourself. I don't see any point moving such a code to drivers
>> which is really SMP bring up code for ARM 32 bit. For ARM 64 bit, as
>> outline by maintainers (Catalin, Arnd), you should use PSCI or DT based
>> boot scheme.
> 
> I was hoping that one day we could unify the SMP bootup code with the
> cpuidle infrastructure, as a lot of the underlying functions are shared
> on the majority of SoCs. I have no plans to work on that myself though,
> and it would have to be agreed on by the cpuidle maintainers.
> 
> For ARM64, the hope was indeed to use PSCI for all SoCs. I just have no
> idea how that would be enforced for mobile SoCs that get shipped to
> system integrators with an bootloader and kernel tree that doesn't even
> get reviewed anywhere first.

Indeed, I'm not sure that our downstream kernels use PSCI. There has
been some discussion of that, but I'm not sure that it actually happened
at least on Tegra132. Even if we do use PSCI, it's likely to be
implemented in a binary blob firmware at the moment, which still seems
less palatable to me than putting the code directly into the kernel. If
the upstream mandate is "use PSCI or don't upstream", then given my and
Thierry's limited bandwidth (we are after all only human), we likely
can't port the kernel and write a whole new upstream-specific firmware
just to provide PSCI, so this kinda makes the choice for us...
Thierry Reding June 30, 2014, 7:21 p.m. UTC | #22
On Mon, Jun 30, 2014 at 09:20:30AM +0200, Arnd Bergmann wrote:
> On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
> > > > pmc.c implements various routines to access the power management
> > > > controller, some of which is needed by suspend/resume, some of it is
> > > > needed by SMP. powergate.c implements a subset of the PMC that needs to
> > > > be exported to drivers to enable power partitions on the SoC. I'm not
> > > > aware of subsystems that deal with this kind of driver.
> > > >
> > > Please see above.
> > 
> > Like I said, I don't see what business suspend/resume related code has
> > in drivers/power. What we're talking about here really is functionality
> > very specific to Tegra. Also some of that code needs to be run at very
> > early points in the boot process, so we can't reasonably turn it into a
> > proper driver anyway.
> 
> I believe the powergate.c stuff can be changed into pm_domain code, but
> we don't have a good subsystem with generic DT bindings yet, so that
> may need some more groundwork first. drivers/power or a subdirectory
> of that may end up being the right place though.

So I ended up implementing the powergates as generic_pm_domain, but that
breaks existing drivers currently. The reason is that generic_pm_domains
are automatically turned off on suspend and turned back on on resume. On
Tegra if a partition is powered off then the whole module looses state
and would need to be completely reinitialized. None of the drivers
currently support that and I'm also not sure if it's really what we want
since it means for example redetecting links and reenumerating devices
on PCI, or probing display outputs in DRM. That's going to take a lot of
time and may not be appropriate for suspend-to-RAM.

Unfortunately I haven't found a way to force a domain to remain powered
during suspend/resume.

Thierry
Thierry Reding June 30, 2014, 7:36 p.m. UTC | #23
On Mon, Jun 30, 2014 at 02:16:34PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Jun 30, 2014 at 11:48:15AM +0100, Thierry Reding wrote:
> > On Mon, Jun 30, 2014 at 11:36:49AM +0100, Catalin Marinas wrote:
> > > On Mon, Jun 30, 2014 at 10:01:19AM +0100, Thierry Reding wrote:
> > > > On Mon, Jun 30, 2014 at 09:20:30AM +0200, Arnd Bergmann wrote:
> > > > > On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
> > > > > > Also, the real win we get from moving code out into drivers/ is if we
> > > > > > can provide a common framework for them. I don't see how we can possibly
> > > > > > do that for this code since there simply isn't enough commonality
> > > > > > between SoCs. At the same time we now have a situation where both 32-bit
> > > > > > and 64-bit variants of some SoCs share some of the same hardware at the
> > > > > > very low level and since we don't have mach-* directories for 64-bit ARM
> > > > > > and shouldn't be duplicating code either, we need to find a new home for
> > > > > > this type of code. drivers/soc seemed to fit perfectly well.
> > > > > 
> > > > > For the low-level stuff yes, but I agree with Santosh there needs to be
> > > > > some more work trying to split out individual high-level drivers.
> > > > > 
> > > > > There are two common patterns for the interface between the low-level
> > > > > register access and the more high-level stuff:  you can either use
> > > > > a "syscon" driver that just exports a struct regmap and that gets referenced
> > > > > from other drivers using a phandle in their device nodes, or you have
> > > > > a driver in drivers/soc that exports a somewhat higher-level interface
> > > > > and comes with its own header file that gets included by other drivers.
> > > > > At the moment, the syscon/regmap variant can only be used once device
> > > > > drivers are loaded, but there is some broad agreement that it should
> > > > > be changed to allow calling syscon_regmap_lookup_by_phandle() at
> > > > > early boot using just DT accessors.
> > > > 
> > > > Note that we already have all these drivers and they do work for earlier
> > > > Tegra generations. My attempt to move these out of arch/arm/mach-tegra
> > > > is merely about being able to share them with upcoming 64-bit variants.
> > > > So it's not about adding new functionality but rather about keeping the
> > > > existing level of functionality on 64-bit.
> > > 
> > > Only that for arm64 we try to do things slightly differently and not
> > > because we just like to but because we want better standardisation
> > > across SoCs. One example is the booting protocol. Another example, you
> > > don't get some early initcalls for your SoC (no machine descriptor). The
> > > hardware should be configured by firmware in such a way that it boots at
> > > least up to arch_initcall() level (ideally, we should move anything to
> > > device_initcall() but it's not always easy).
> > 
> > Well, one of the problems on Tegra is that we need part of the PMC to
> > power up CPUs. There's no firmware that could do this for us. We need
> > also access to another block called flow controller. Both of those
> > drivers need to be available very early for things to work. At the same
> > time the driver exposes control over power domains. So while we possibly
> > can push CPU bringup/teardown to firmware for ARM64, we can't do the
> > same for the other parts of the PMC, so we end up with a weird kind of
> > driver.
> > 
> > Parts of it can't register with the driver model because it isn't
> > available that early, and at the same time we need to register parts
> > only later because they require the driver model.
> 
> That sounds familiar and that's exactly the problem, and that's a problem
> that won't disappear by just moving code around directories (hey, it is
> not that we have not tried =)), as you (and I) know very well:
> 
> https://lkml.org/lkml/2013/8/16/399
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/190067.html
> 
> Either a common way of dealing with this early CPU bring-up code is
> implemented (but it can't be platform specific otherwise we are back to
> square one)

I prototyped this based on an OF init table (much like IRQCHIP_DECLARE
or CLOCKSOURCE_OF_DECLARE) that's run early in the boot process (at the
very end of setup_arch()).

> or we decide that this code does not belong in the kernel and
> should be abstracted away.
> 
> And I think that most of the early bring up issues are just related to
> powering-up/resetting a cpu for it to be booted, probably that's the
> abstraction we should be focusing on, because it should just require poking
> a bunch of registers to kickstart a CPU, no more than that, FW should be
> able to deal with that and that's yet another reason behind PSCI design.

Right, I agree that if we can push that into PSCI that would be great
(provided there's an open-source implementation of PSCI). But for 32-bit
ARM that's no longer a viable option, so we're pretty much stuck with
what we have.

> I am not convinced that the early device model would be useful for any
> other reason, that's why shoehorning kernel changes in the current driver
> model just to bring up secondaries is not something I consider useful
> and proper, in the first place, but I am happy to hear other possible
> concerns.

I agree, but we still want to support early and driver model code in the
same source file to avoid duplication. What I prototyped is a driver
that's initialized to a bare minimum very early on and then takes over
when the driver model is up (using regular driver .probe()). I hope to
get the patches cleaned up and send them out tomorrow. If you want I can
add you on Cc.

Thierry
Peter De Schrijver July 1, 2014, 7:51 a.m. UTC | #24
On Mon, Jun 30, 2014 at 09:21:22PM +0200, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Mon, Jun 30, 2014 at 09:20:30AM +0200, Arnd Bergmann wrote:
> > On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
> > > > > pmc.c implements various routines to access the power management
> > > > > controller, some of which is needed by suspend/resume, some of it is
> > > > > needed by SMP. powergate.c implements a subset of the PMC that needs to
> > > > > be exported to drivers to enable power partitions on the SoC. I'm not
> > > > > aware of subsystems that deal with this kind of driver.
> > > > >
> > > > Please see above.
> > > 
> > > Like I said, I don't see what business suspend/resume related code has
> > > in drivers/power. What we're talking about here really is functionality
> > > very specific to Tegra. Also some of that code needs to be run at very
> > > early points in the boot process, so we can't reasonably turn it into a
> > > proper driver anyway.
> > 
> > I believe the powergate.c stuff can be changed into pm_domain code, but
> > we don't have a good subsystem with generic DT bindings yet, so that
> > may need some more groundwork first. drivers/power or a subdirectory
> > of that may end up being the right place though.
> 
> So I ended up implementing the powergates as generic_pm_domain, but that
> breaks existing drivers currently. The reason is that generic_pm_domains
> are automatically turned off on suspend and turned back on on resume. On
> Tegra if a partition is powered off then the whole module looses state
> and would need to be completely reinitialized. None of the drivers
> currently support that and I'm also not sure if it's really what we want
> since it means for example redetecting links and reenumerating devices
> on PCI, or probing display outputs in DRM. That's going to take a lot of
> time and may not be appropriate for suspend-to-RAM.
> 
> Unfortunately I haven't found a way to force a domain to remain powered
> during suspend/resume.

If we make power off a NOP until LP0 gets implemented? Then device state
will have to be saved/restored.

Cheers,

Peter.
Catalin Marinas July 1, 2014, 10:50 a.m. UTC | #25
On Mon, Jun 30, 2014 at 08:36:36PM +0100, Thierry Reding wrote:
> On Mon, Jun 30, 2014 at 02:16:34PM +0100, Lorenzo Pieralisi wrote:
> > And I think that most of the early bring up issues are just related to
> > powering-up/resetting a cpu for it to be booted, probably that's the
> > abstraction we should be focusing on, because it should just require poking
> > a bunch of registers to kickstart a CPU, no more than that, FW should be
> > able to deal with that and that's yet another reason behind PSCI design.
> 
> Right, I agree that if we can push that into PSCI that would be great
> (provided there's an open-source implementation of PSCI).

At least one (BSD-style license):

https://github.com/ARM-software/arm-trusted-firmware

There are also some patches for U-Boot (only covering secondary CPU
booting for now and not merged yet).
Stephen Warren July 1, 2014, 3:05 p.m. UTC | #26
On 07/01/2014 04:50 AM, Catalin Marinas wrote:
> On Mon, Jun 30, 2014 at 08:36:36PM +0100, Thierry Reding wrote:
>> On Mon, Jun 30, 2014 at 02:16:34PM +0100, Lorenzo Pieralisi wrote:
>>> And I think that most of the early bring up issues are just related to
>>> powering-up/resetting a cpu for it to be booted, probably that's the
>>> abstraction we should be focusing on, because it should just require poking
>>> a bunch of registers to kickstart a CPU, no more than that, FW should be
>>> able to deal with that and that's yet another reason behind PSCI design.
>>
>> Right, I agree that if we can push that into PSCI that would be great
>> (provided there's an open-source implementation of PSCI).
> 
> At least one (BSD-style license):
> 
> https://github.com/ARM-software/arm-trusted-firmware
> 
> There are also some patches for U-Boot (only covering secondary CPU
> booting for now and not merged yet).

Are the U-Boot patches:

a) Patches to make U-Boot provide a PSCI implementation.

b) Patches to make U-Boot use PSCI as a client.

If you have a link or patch subject to search for, that'd be
interesting. Thanks.
Catalin Marinas July 1, 2014, 5 p.m. UTC | #27
On Tue, Jul 01, 2014 at 04:05:47PM +0100, Stephen Warren wrote:
> On 07/01/2014 04:50 AM, Catalin Marinas wrote:
> > On Mon, Jun 30, 2014 at 08:36:36PM +0100, Thierry Reding wrote:
> >> On Mon, Jun 30, 2014 at 02:16:34PM +0100, Lorenzo Pieralisi wrote:
> >>> And I think that most of the early bring up issues are just related to
> >>> powering-up/resetting a cpu for it to be booted, probably that's the
> >>> abstraction we should be focusing on, because it should just require poking
> >>> a bunch of registers to kickstart a CPU, no more than that, FW should be
> >>> able to deal with that and that's yet another reason behind PSCI design.
> >>
> >> Right, I agree that if we can push that into PSCI that would be great
> >> (provided there's an open-source implementation of PSCI).
> > 
> > At least one (BSD-style license):
> > 
> > https://github.com/ARM-software/arm-trusted-firmware
> > 
> > There are also some patches for U-Boot (only covering secondary CPU
> > booting for now and not merged yet).
> 
> Are the U-Boot patches:
> 
> a) Patches to make U-Boot provide a PSCI implementation.
> 
> b) Patches to make U-Boot use PSCI as a client.
> 
> If you have a link or patch subject to search for, that'd be
> interesting.

It's (a). See the base stuff:

https://git.kernel.org/cgit/linux/kernel/git/maz/u-boot.git/log/?h=wip/psci-v4

and Allwinner specifics:

https://git.kernel.org/cgit/linux/kernel/git/maz/u-boot.git/log/?h=wip/psci-v4-a20
Olof Johansson July 16, 2014, 7:31 p.m. UTC | #28
[ Gee, I had completely missed this thread because nobody bothered to
cc me. Seems to be standard procedure on 64-bit these days. :( ]


On Mon, Jun 30, 2014 at 12:20 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
>> > > pmc.c implements various routines to access the power management
>> > > controller, some of which is needed by suspend/resume, some of it is
>> > > needed by SMP. powergate.c implements a subset of the PMC that needs to
>> > > be exported to drivers to enable power partitions on the SoC. I'm not
>> > > aware of subsystems that deal with this kind of driver.
>> > >
>> > Please see above.
>>
>> Like I said, I don't see what business suspend/resume related code has
>> in drivers/power. What we're talking about here really is functionality
>> very specific to Tegra. Also some of that code needs to be run at very
>> early points in the boot process, so we can't reasonably turn it into a
>> proper driver anyway.
>
> I believe the powergate.c stuff can be changed into pm_domain code, but
> we don't have a good subsystem with generic DT bindings yet, so that
> may need some more groundwork first. drivers/power or a subdirectory
> of that may end up being the right place though.
>
>> Also, the real win we get from moving code out into drivers/ is if we
>> can provide a common framework for them. I don't see how we can possibly
>> do that for this code since there simply isn't enough commonality
>> between SoCs. At the same time we now have a situation where both 32-bit
>> and 64-bit variants of some SoCs share some of the same hardware at the
>> very low level and since we don't have mach-* directories for 64-bit ARM
>> and shouldn't be duplicating code either, we need to find a new home for
>> this type of code. drivers/soc seemed to fit perfectly well.
>
> For the low-level stuff yes, but I agree with Santosh there needs to be
> some more work trying to split out individual high-level drivers.
>
> There are two common patterns for the interface between the low-level
> register access and the more high-level stuff:  you can either use
> a "syscon" driver that just exports a struct regmap and that gets referenced
> from other drivers using a phandle in their device nodes, or you have
> a driver in drivers/soc that exports a somewhat higher-level interface
> and comes with its own header file that gets included by other drivers.
> At the moment, the syscon/regmap variant can only be used once device
> drivers are loaded, but there is some broad agreement that it should
> be changed to allow calling syscon_regmap_lookup_by_phandle() at
> early boot using just DT accessors.

I'd strongly prefer to NOT tie this into DT and keep it as in-kernel
implementation until we know more what a common subsystem might look
like, if any.

If we keep it only in the kernel, then we're free to change it as
needed. If we tie it into DT/syscon, then we get stick with
stable-only ABIs from day one. That doesn't seem like a good idea.


-Olof
Thierry Reding July 16, 2014, 7:47 p.m. UTC | #29
On Wed, Jul 16, 2014 at 12:31:00PM -0700, Olof Johansson wrote:
> [ Gee, I had completely missed this thread because nobody bothered to
> cc me. Seems to be standard procedure on 64-bit these days. :( ]
> 
> 
> On Mon, Jun 30, 2014 at 12:20 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
> >> > > pmc.c implements various routines to access the power management
> >> > > controller, some of which is needed by suspend/resume, some of it is
> >> > > needed by SMP. powergate.c implements a subset of the PMC that needs to
> >> > > be exported to drivers to enable power partitions on the SoC. I'm not
> >> > > aware of subsystems that deal with this kind of driver.
> >> > >
> >> > Please see above.
> >>
> >> Like I said, I don't see what business suspend/resume related code has
> >> in drivers/power. What we're talking about here really is functionality
> >> very specific to Tegra. Also some of that code needs to be run at very
> >> early points in the boot process, so we can't reasonably turn it into a
> >> proper driver anyway.
> >
> > I believe the powergate.c stuff can be changed into pm_domain code, but
> > we don't have a good subsystem with generic DT bindings yet, so that
> > may need some more groundwork first. drivers/power or a subdirectory
> > of that may end up being the right place though.
> >
> >> Also, the real win we get from moving code out into drivers/ is if we
> >> can provide a common framework for them. I don't see how we can possibly
> >> do that for this code since there simply isn't enough commonality
> >> between SoCs. At the same time we now have a situation where both 32-bit
> >> and 64-bit variants of some SoCs share some of the same hardware at the
> >> very low level and since we don't have mach-* directories for 64-bit ARM
> >> and shouldn't be duplicating code either, we need to find a new home for
> >> this type of code. drivers/soc seemed to fit perfectly well.
> >
> > For the low-level stuff yes, but I agree with Santosh there needs to be
> > some more work trying to split out individual high-level drivers.
> >
> > There are two common patterns for the interface between the low-level
> > register access and the more high-level stuff:  you can either use
> > a "syscon" driver that just exports a struct regmap and that gets referenced
> > from other drivers using a phandle in their device nodes, or you have
> > a driver in drivers/soc that exports a somewhat higher-level interface
> > and comes with its own header file that gets included by other drivers.
> > At the moment, the syscon/regmap variant can only be used once device
> > drivers are loaded, but there is some broad agreement that it should
> > be changed to allow calling syscon_regmap_lookup_by_phandle() at
> > early boot using just DT accessors.
> 
> I'd strongly prefer to NOT tie this into DT and keep it as in-kernel
> implementation until we know more what a common subsystem might look
> like, if any.
> 
> If we keep it only in the kernel, then we're free to change it as
> needed. If we tie it into DT/syscon, then we get stick with
> stable-only ABIs from day one. That doesn't seem like a good idea.

There are two sides to keeping to an in-kernel API too: it may seem like
we're free to change this at will and aren't subject to a stable ABI.
However that's not entirely accurate.

Assume we don't put any information about a feature into DT but use an
in-kernel API only. Once we've come up with a good way to represent that
in DT and have a generic framework that we can use, we'll most likely
still need to keep around the old API in the kernel to make sure the
kernel still works with a DT that doesn't have nodes and/or properties
for the new binding.

So the question arises: at that point is it really worth using a DT
binding? Since we already have an API that works, why bother coming up
with a new one? Especially if we can't get rid of the one we already
have anyway?

Thierry
Catalin Marinas July 17, 2014, 9:31 a.m. UTC | #30
On Wed, Jul 16, 2014 at 08:31:00PM +0100, Olof Johansson wrote:
> [ Gee, I had completely missed this thread because nobody bothered to
> cc me. Seems to be standard procedure on 64-bit these days. :( ]

Just to clarify this point, as per prior agreement I'm expecting the
arm-soc guys to help reviewing/merging arm64-soc as well, even though
there are no arch/arm64/{mach,plat}-* directories. That means that
arm-soc folks should also be cc'ed on such patches.

However, the "ARM SUB-ARCHITECTURES" MAINTAINERS entry does not mention
arm@kernel.org (neither Olof nor Arnd). Was this not meant as an arm-soc
email alias?
Olof Johansson July 17, 2014, 4:21 p.m. UTC | #31
On Thu, Jul 17, 2014 at 2:31 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Wed, Jul 16, 2014 at 08:31:00PM +0100, Olof Johansson wrote:
>> [ Gee, I had completely missed this thread because nobody bothered to
>> cc me. Seems to be standard procedure on 64-bit these days. :( ]
>
> Just to clarify this point, as per prior agreement I'm expecting the
> arm-soc guys to help reviewing/merging arm64-soc as well, even though
> there are no arch/arm64/{mach,plat}-* directories. That means that
> arm-soc folks should also be cc'ed on such patches.
>
> However, the "ARM SUB-ARCHITECTURES" MAINTAINERS entry does not mention
> arm@kernel.org (neither Olof nor Arnd). Was this not meant as an arm-soc
> email alias?

Yes, however we've been reluctant to add it because we don't want
people to post patches directly to us, we mostly expect maintainers to
send pull request and/or direct patches there.

We can do it and add annotation to not send patches directly there.
I'll post a patch for that update.


-Olof
diff mbox

Patch

diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index c303b55de22e..bed5b9b0c17a 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -1,38 +1,6 @@ 
 asflags-y				+= -march=armv7-a
 
 obj-y                                   += io.o
-obj-y                                   += irq.o
-obj-y					+= pmc.o
-obj-y					+= flowctrl.o
-obj-y					+= powergate.o
-obj-y					+= pm.o
-obj-y					+= reset.o
-obj-y					+= reset-handler.o
-obj-y					+= sleep.o
 obj-y					+= tegra.o
-obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
-obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= sleep-tegra20.o
-obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= pm-tegra20.o
-ifeq ($(CONFIG_CPU_IDLE),y)
-obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= cpuidle-tegra20.o
-endif
-obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= sleep-tegra30.o
-obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= pm-tegra30.o
-ifeq ($(CONFIG_CPU_IDLE),y)
-obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= cpuidle-tegra30.o
-endif
-obj-$(CONFIG_SMP)			+= platsmp.o headsmp.o
-obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o
-
-obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= sleep-tegra30.o
-obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= pm-tegra30.o
-ifeq ($(CONFIG_CPU_IDLE),y)
-obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= cpuidle-tegra114.o
-endif
-obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= sleep-tegra30.o
-obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= pm-tegra30.o
-ifeq ($(CONFIG_CPU_IDLE),y)
-obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= cpuidle-tegra114.o
-endif
 
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= board-paz00.o
diff --git a/arch/arm/mach-tegra/common.h b/arch/arm/mach-tegra/common.h
deleted file mode 100644
index 5900cc44f780..000000000000
--- a/arch/arm/mach-tegra/common.h
+++ /dev/null
@@ -1,4 +0,0 @@ 
-extern struct smp_operations tegra_smp_ops;
-
-extern int tegra_cpu_kill(unsigned int cpu);
-extern void tegra_cpu_die(unsigned int cpu);
diff --git a/arch/arm/mach-tegra/io.c b/arch/arm/mach-tegra/io.c
index bb9c9c29d181..2d68275aeadb 100644
--- a/arch/arm/mach-tegra/io.c
+++ b/arch/arm/mach-tegra/io.c
@@ -28,7 +28,32 @@ 
 #include <asm/mach/map.h>
 
 #include "board.h"
-#include "iomap.h"
+
+/*
+ * On TEGRA, many peripherals are very closely packed in
+ * two 256MB io windows (that actually only use about 64KB
+ * at the start of each).
+ *
+ * We will just map the first MMU section of each window (to minimize
+ * pt entries needed) and provide a macro to transform physical
+ * io addresses to an appropriate void __iomem *.
+ */
+
+#define IO_IRAM_PHYS	0x40000000
+#define IO_IRAM_VIRT	IOMEM(0xFE400000)
+#define IO_IRAM_SIZE	SZ_256K
+
+#define IO_CPU_PHYS	0x50040000
+#define IO_CPU_VIRT	IOMEM(0xFE440000)
+#define IO_CPU_SIZE	SZ_16K
+
+#define IO_PPSB_PHYS	0x60000000
+#define IO_PPSB_VIRT	IOMEM(0xFE200000)
+#define IO_PPSB_SIZE	SECTION_SIZE
+
+#define IO_APB_PHYS	0x70000000
+#define IO_APB_VIRT	IOMEM(0xFE000000)
+#define IO_APB_SIZE	SECTION_SIZE
 
 static struct map_desc tegra_io_desc[] __initdata = {
 	{
diff --git a/arch/arm/mach-tegra/pmc.h b/arch/arm/mach-tegra/pmc.h
deleted file mode 100644
index 139a30867cb2..000000000000
--- a/arch/arm/mach-tegra/pmc.h
+++ /dev/null
@@ -1,62 +0,0 @@ 
-/*
- * Copyright (C) 2012 NVIDIA CORPORATION. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- */
-
-#ifndef __MACH_TEGRA_PMC_H
-#define __MACH_TEGRA_PMC_H
-
-#include <linux/io.h>
-#include <linux/reboot.h>
-
-enum tegra_suspend_mode {
-	TEGRA_SUSPEND_NONE = 0,
-	TEGRA_SUSPEND_LP2,	/* CPU voltage off */
-	TEGRA_SUSPEND_LP1,	/* CPU voltage off, DRAM self-refresh */
-	TEGRA_SUSPEND_LP0,      /* CPU + core voltage off, DRAM self-refresh */
-	TEGRA_MAX_SUSPEND_MODE,
-};
-
-#ifdef CONFIG_PM_SLEEP
-enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void);
-void tegra_pmc_set_suspend_mode(enum tegra_suspend_mode mode);
-void tegra_pmc_suspend(void);
-void tegra_pmc_resume(void);
-void tegra_pmc_pm_set(enum tegra_suspend_mode mode);
-void tegra_pmc_suspend_init(void);
-#endif
-
-extern void __iomem *tegra_pmc_base;
-
-static inline u32 tegra_pmc_readl(u32 reg)
-{
-	return readl(tegra_pmc_base + reg);
-}
-
-static inline void tegra_pmc_writel(u32 val, u32 reg)
-{
-	writel(val, tegra_pmc_base + reg);
-}
-
-bool tegra_pmc_cpu_is_powered(int cpuid);
-int tegra_pmc_cpu_power_on(int cpuid);
-int tegra_pmc_cpu_remove_clamping(int cpuid);
-
-void tegra_pmc_restart(enum reboot_mode mode, const char *cmd);
-
-void tegra_pmc_init_irq(void);
-void tegra_pmc_init(void);
-
-#endif
diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index ab6544576eac..cea2db665b1f 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -44,15 +44,6 @@ 
 #include <asm/trusted_foundations.h>
 
 #include "board.h"
-#include "common.h"
-#include "cpuidle.h"
-#include "flowctrl.h"
-#include "iomap.h"
-#include "irq.h"
-#include "pmc.h"
-#include "pm.h"
-#include "reset.h"
-#include "sleep.h"
 
 /*
  * Storage for debug-macro.S's state.
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 0f7c44793b29..3b1b95d932d1 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -3,3 +3,4 @@ 
 #
 
 obj-$(CONFIG_ARCH_QCOM)		+= qcom/
+obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
diff --git a/drivers/soc/tegra/Makefile b/drivers/soc/tegra/Makefile
new file mode 100644
index 000000000000..6fd8790c314d
--- /dev/null
+++ b/drivers/soc/tegra/Makefile
@@ -0,0 +1,34 @@ 
+asflags-y += -march=armv7-a
+
+obj-y += flowctrl.o
+obj-y += irq.o
+obj-y += pm.o
+obj-y += pmc.o
+obj-y += powergate.o
+obj-y += reset.o
+obj-y += reset-handler.o
+obj-y += sleep.o
+
+
+obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
+obj-$(CONFIG_SMP) += platsmp.o headsmp.o
+
+obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += pm-tegra20.o
+obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += sleep-tegra20.o
+
+obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += pm-tegra30.o
+obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += sleep-tegra30.o
+
+obj-$(CONFIG_ARCH_TEGRA_114_SOC) += pm-tegra30.o
+obj-$(CONFIG_ARCH_TEGRA_114_SOC) += sleep-tegra30.o
+
+obj-$(CONFIG_ARCH_TEGRA_124_SOC) += pm-tegra30.o
+obj-$(CONFIG_ARCH_TEGRA_124_SOC) += sleep-tegra30.o
+
+ifeq ($(CONFIG_CPU_IDLE),y)
+obj-y += cpuidle.o
+obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += cpuidle-tegra20.o
+obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += cpuidle-tegra30.o
+obj-$(CONFIG_ARCH_TEGRA_114_SOC) += cpuidle-tegra114.o
+obj-$(CONFIG_ARCH_TEGRA_124_SOC) += cpuidle-tegra114.o
+endif
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/drivers/soc/tegra/cpuidle-tegra114.c
similarity index 100%
rename from arch/arm/mach-tegra/cpuidle-tegra114.c
rename to drivers/soc/tegra/cpuidle-tegra114.c
diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/drivers/soc/tegra/cpuidle-tegra20.c
similarity index 100%
rename from arch/arm/mach-tegra/cpuidle-tegra20.c
rename to drivers/soc/tegra/cpuidle-tegra20.c
index b82dcaee2ef4..0710e48f79e0 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/drivers/soc/tegra/cpuidle-tegra20.c
@@ -31,11 +31,11 @@ 
 #include <asm/suspend.h>
 #include <asm/smp_plat.h>
 
-#include "pm.h"
-#include "sleep.h"
+#include "flowctrl.h"
 #include "iomap.h"
 #include "irq.h"
-#include "flowctrl.h"
+#include "pm.h"
+#include "sleep.h"
 
 #ifdef CONFIG_PM_SLEEP
 static bool abort_flag;
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/drivers/soc/tegra/cpuidle-tegra30.c
similarity index 100%
rename from arch/arm/mach-tegra/cpuidle-tegra30.c
rename to drivers/soc/tegra/cpuidle-tegra30.c
diff --git a/arch/arm/mach-tegra/cpuidle.c b/drivers/soc/tegra/cpuidle.c
similarity index 100%
rename from arch/arm/mach-tegra/cpuidle.c
rename to drivers/soc/tegra/cpuidle.c
diff --git a/arch/arm/mach-tegra/cpuidle.h b/drivers/soc/tegra/cpuidle.h
similarity index 91%
rename from arch/arm/mach-tegra/cpuidle.h
rename to drivers/soc/tegra/cpuidle.h
index c017dab60ffa..6f5b61d317fd 100644
--- a/arch/arm/mach-tegra/cpuidle.h
+++ b/drivers/soc/tegra/cpuidle.h
@@ -22,9 +22,6 @@  int tegra20_cpuidle_init(void);
 void tegra20_cpuidle_pcie_irqs_in_use(void);
 int tegra30_cpuidle_init(void);
 int tegra114_cpuidle_init(void);
-void tegra_cpuidle_init(void);
-#else
-static inline void tegra_cpuidle_init(void) {}
 #endif
 
 #endif
diff --git a/arch/arm/mach-tegra/flowctrl.c b/drivers/soc/tegra/flowctrl.c
similarity index 100%
rename from arch/arm/mach-tegra/flowctrl.c
rename to drivers/soc/tegra/flowctrl.c
diff --git a/arch/arm/mach-tegra/flowctrl.h b/drivers/soc/tegra/flowctrl.h
similarity index 98%
rename from arch/arm/mach-tegra/flowctrl.h
rename to drivers/soc/tegra/flowctrl.h
index 73a9c5016c1a..c89aac60a143 100644
--- a/arch/arm/mach-tegra/flowctrl.h
+++ b/drivers/soc/tegra/flowctrl.h
@@ -59,8 +59,6 @@  void flowctrl_write_cpu_halt(unsigned int cpuid, u32 value);
 
 void flowctrl_cpu_suspend_enter(unsigned int cpuid);
 void flowctrl_cpu_suspend_exit(unsigned int cpuid);
-
-void tegra_flowctrl_init(void);
 #endif
 
 #endif
diff --git a/arch/arm/mach-tegra/headsmp.S b/drivers/soc/tegra/headsmp.S
similarity index 100%
rename from arch/arm/mach-tegra/headsmp.S
rename to drivers/soc/tegra/headsmp.S
diff --git a/arch/arm/mach-tegra/hotplug.c b/drivers/soc/tegra/hotplug.c
similarity index 100%
rename from arch/arm/mach-tegra/hotplug.c
rename to drivers/soc/tegra/hotplug.c
diff --git a/arch/arm/mach-tegra/iomap.h b/drivers/soc/tegra/iomap.h
similarity index 100%
rename from arch/arm/mach-tegra/iomap.h
rename to drivers/soc/tegra/iomap.h
diff --git a/arch/arm/mach-tegra/irammap.h b/drivers/soc/tegra/irammap.h
similarity index 100%
rename from arch/arm/mach-tegra/irammap.h
rename to drivers/soc/tegra/irammap.h
diff --git a/arch/arm/mach-tegra/irq.c b/drivers/soc/tegra/irq.c
similarity index 99%
rename from arch/arm/mach-tegra/irq.c
rename to drivers/soc/tegra/irq.c
index 1a74d562dca1..57807a79f5fd 100644
--- a/arch/arm/mach-tegra/irq.c
+++ b/drivers/soc/tegra/irq.c
@@ -27,7 +27,6 @@ 
 #include <linux/irqchip/arm-gic.h>
 #include <linux/syscore_ops.h>
 
-#include "board.h"
 #include "iomap.h"
 
 #define ICTLR_CPU_IEP_VFIQ	0x08
diff --git a/arch/arm/mach-tegra/irq.h b/drivers/soc/tegra/irq.h
similarity index 83%
rename from arch/arm/mach-tegra/irq.h
rename to drivers/soc/tegra/irq.h
index bc05ce5613fb..5142649bba05 100644
--- a/arch/arm/mach-tegra/irq.h
+++ b/drivers/soc/tegra/irq.h
@@ -19,10 +19,4 @@ 
 
 bool tegra_pending_sgi(void);
 
-#ifdef CONFIG_PM_SLEEP
-int tegra_legacy_irq_syscore_init(void);
-#else
-static inline int tegra_legacy_irq_syscore_init(void) { return 0; }
-#endif
-
 #endif
diff --git a/arch/arm/mach-tegra/platsmp.c b/drivers/soc/tegra/platsmp.c
similarity index 98%
rename from arch/arm/mach-tegra/platsmp.c
rename to drivers/soc/tegra/platsmp.c
index d9878acae3d2..f3622181d39e 100644
--- a/arch/arm/mach-tegra/platsmp.c
+++ b/drivers/soc/tegra/platsmp.c
@@ -27,10 +27,6 @@ 
 #include <asm/smp_plat.h>
 
 #include "flowctrl.h"
-#include "reset.h"
-#include "pmc.h"
-
-#include "common.h"
 #include "iomap.h"
 
 static cpumask_t tegra_cpu_init_mask;
@@ -40,7 +36,6 @@  static void tegra_secondary_init(unsigned int cpu)
 	cpumask_set_cpu(cpu, &tegra_cpu_init_mask);
 }
 
-
 static int tegra20_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
 	cpu = cpu_logical_map(cpu);
diff --git a/arch/arm/mach-tegra/pm-tegra20.c b/drivers/soc/tegra/pm-tegra20.c
similarity index 100%
rename from arch/arm/mach-tegra/pm-tegra20.c
rename to drivers/soc/tegra/pm-tegra20.c
diff --git a/arch/arm/mach-tegra/pm-tegra30.c b/drivers/soc/tegra/pm-tegra30.c
similarity index 100%
rename from arch/arm/mach-tegra/pm-tegra30.c
rename to drivers/soc/tegra/pm-tegra30.c
diff --git a/arch/arm/mach-tegra/pm.c b/drivers/soc/tegra/pm.c
similarity index 100%
rename from arch/arm/mach-tegra/pm.c
rename to drivers/soc/tegra/pm.c
diff --git a/arch/arm/mach-tegra/pm.h b/drivers/soc/tegra/pm.h
similarity index 94%
rename from arch/arm/mach-tegra/pm.h
rename to drivers/soc/tegra/pm.h
index f4a89698e5b0..0d899557ffeb 100644
--- a/arch/arm/mach-tegra/pm.h
+++ b/drivers/soc/tegra/pm.h
@@ -21,7 +21,7 @@ 
 #ifndef _MACH_TEGRA_PM_H_
 #define _MACH_TEGRA_PM_H_
 
-#include "pmc.h"
+#include <linux/tegra-soc.h>
 
 struct tegra_lp1_iram {
 	void	*start_addr;
@@ -44,14 +44,12 @@  extern void (*tegra_tear_down_cpu)(void);
 #ifdef CONFIG_PM_SLEEP
 enum tegra_suspend_mode tegra_pm_validate_suspend_mode(
 				enum tegra_suspend_mode mode);
-void tegra_init_suspend(void);
 #else
 static inline enum tegra_suspend_mode tegra_pm_validate_suspend_mode(
 				enum tegra_suspend_mode mode)
 {
 	return TEGRA_SUSPEND_NONE;
 }
-static inline void tegra_init_suspend(void) {}
 #endif
 
 #endif /* _MACH_TEGRA_PM_H_ */
diff --git a/arch/arm/mach-tegra/pmc.c b/drivers/soc/tegra/pmc.c
similarity index 100%
rename from arch/arm/mach-tegra/pmc.c
rename to drivers/soc/tegra/pmc.c
diff --git a/drivers/soc/tegra/pmc.h b/drivers/soc/tegra/pmc.h
new file mode 100644
index 000000000000..45643454e5cd
--- /dev/null
+++ b/drivers/soc/tegra/pmc.h
@@ -0,0 +1,35 @@ 
+/*
+ * Copyright (C) 2012 NVIDIA CORPORATION. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef __MACH_TEGRA_PMC_H
+#define __MACH_TEGRA_PMC_H
+
+#include <linux/io.h>
+
+extern void __iomem *tegra_pmc_base;
+
+static inline u32 tegra_pmc_readl(u32 reg)
+{
+	return readl(tegra_pmc_base + reg);
+}
+
+static inline void tegra_pmc_writel(u32 val, u32 reg)
+{
+	writel(val, tegra_pmc_base + reg);
+}
+
+#endif
diff --git a/arch/arm/mach-tegra/powergate.c b/drivers/soc/tegra/powergate.c
similarity index 100%
rename from arch/arm/mach-tegra/powergate.c
rename to drivers/soc/tegra/powergate.c
diff --git a/arch/arm/mach-tegra/reset-handler.S b/drivers/soc/tegra/reset-handler.S
similarity index 100%
rename from arch/arm/mach-tegra/reset-handler.S
rename to drivers/soc/tegra/reset-handler.S
diff --git a/arch/arm/mach-tegra/reset.c b/drivers/soc/tegra/reset.c
similarity index 100%
rename from arch/arm/mach-tegra/reset.c
rename to drivers/soc/tegra/reset.c
diff --git a/arch/arm/mach-tegra/reset.h b/drivers/soc/tegra/reset.h
similarity index 97%
rename from arch/arm/mach-tegra/reset.h
rename to drivers/soc/tegra/reset.h
index 76a93434c6ee..e054cca2abea 100644
--- a/arch/arm/mach-tegra/reset.h
+++ b/drivers/soc/tegra/reset.h
@@ -57,7 +57,5 @@  void tegra_secondary_startup(void);
 		(__tegra_cpu_reset_handler_end - \
 		 __tegra_cpu_reset_handler_start)
 
-void __init tegra_cpu_reset_handler_init(void);
-
 #endif
 #endif
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/drivers/soc/tegra/sleep-tegra20.S
similarity index 100%
rename from arch/arm/mach-tegra/sleep-tegra20.S
rename to drivers/soc/tegra/sleep-tegra20.S
diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/drivers/soc/tegra/sleep-tegra30.S
similarity index 100%
rename from arch/arm/mach-tegra/sleep-tegra30.S
rename to drivers/soc/tegra/sleep-tegra30.S
diff --git a/arch/arm/mach-tegra/sleep.S b/drivers/soc/tegra/sleep.S
similarity index 100%
rename from arch/arm/mach-tegra/sleep.S
rename to drivers/soc/tegra/sleep.S
diff --git a/arch/arm/mach-tegra/sleep.h b/drivers/soc/tegra/sleep.h
similarity index 98%
rename from arch/arm/mach-tegra/sleep.h
rename to drivers/soc/tegra/sleep.h
index 339fe42cd6fb..41b257f22b80 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/drivers/soc/tegra/sleep.h
@@ -121,7 +121,6 @@ 
 .endm
 
 #else
-void tegra_pen_lock(void);
 void tegra_pen_unlock(void);
 void tegra_resume(void);
 int tegra_sleep_cpu_finish(unsigned long);
@@ -130,7 +129,6 @@  void tegra_disable_clean_inv_dcache(u32 flag);
 #ifdef CONFIG_HOTPLUG_CPU
 void tegra20_hotplug_shutdown(void);
 void tegra30_hotplug_shutdown(void);
-void tegra_hotplug_init(void);
 #else
 static inline void tegra_hotplug_init(void) {}
 #endif
diff --git a/include/linux/tegra-soc.h b/include/linux/tegra-soc.h
index fcf65ecbecca..9fb8c7261403 100644
--- a/include/linux/tegra-soc.h
+++ b/include/linux/tegra-soc.h
@@ -17,6 +17,10 @@ 
 #ifndef __LINUX_TEGRA_SOC_H_
 #define __LINUX_TEGRA_SOC_H_
 
+#ifndef __ASSEMBLY__
+#include <linux/reboot.h>
+#endif
+
 #define TEGRA20		0x20
 #define TEGRA30		0x30
 #define TEGRA114	0x35
@@ -59,6 +63,97 @@  int tegra_fuse_readl(u32 offset, u32 *val);
 extern int tegra_chip_id;
 extern struct tegra_sku_info tegra_sku_info;
 
+/*
+ * flow controller
+ */
+
+void tegra_flowctrl_init(void);
+
+/*
+ * CPU hotplug
+ */
+#ifdef CONFIG_HOTPLUG_CPU
+void tegra_cpu_die(unsigned int cpu);
+int tegra_cpu_kill(unsigned int cpu);
+void tegra_hotplug_init(void);
+#else
+static inline void tegra_hotplug_init(void)
+{
+}
+#endif
+
+/*
+ * CPU idle
+ */
+#ifdef CONFIG_CPU_IDLE
+void tegra_cpuidle_init(void);
+#else
+static inline void tegra_cpuidle_init(void)
+{
+}
+#endif
+
+/*
+ * IRQ
+ */
+#ifdef CONFIG_PM_SLEEP
+int tegra_legacy_irq_syscore_init(void);
+#else
+static inline int tegra_legacy_irq_syscore_init(void)
+{
+	return 0;
+}
+#endif
+
+/*
+ * power management controller (PMC)
+ */
+enum tegra_suspend_mode {
+	TEGRA_SUSPEND_NONE = 0,
+	TEGRA_SUSPEND_LP2,	/* CPU voltage off */
+	TEGRA_SUSPEND_LP1,	/* CPU voltage off, DRAM self-refresh */
+	TEGRA_SUSPEND_LP0,      /* CPU + core voltage off, DRAM self-refresh */
+	TEGRA_MAX_SUSPEND_MODE,
+};
+
+#ifdef CONFIG_PM_SLEEP
+enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void);
+void tegra_pmc_set_suspend_mode(enum tegra_suspend_mode mode);
+void tegra_pmc_suspend(void);
+void tegra_pmc_resume(void);
+void tegra_pmc_pm_set(enum tegra_suspend_mode mode);
+void tegra_pmc_suspend_init(void);
+#endif
+
+void tegra_pmc_restart(enum reboot_mode mode, const char *cmd);
+
+void tegra_pmc_init_irq(void);
+void tegra_pmc_init(void);
+
+bool tegra_pmc_cpu_is_powered(int cpuid);
+int tegra_pmc_cpu_power_on(int cpuid);
+int tegra_pmc_cpu_remove_clamping(int cpuid);
+
+/*
+ * power management
+ */
+#ifdef CONFIG_PM_SLEEP
+void tegra_init_suspend(void);
+#else
+static inline void tegra_init_suspend(void)
+{
+}
+#endif
+
+/* reset */
+void __init tegra_cpu_reset_handler_init(void);
+
+/* sleep */
+void tegra_pen_lock(void);
+
+/* SMP */
+extern struct smp_operations tegra_smp_ops;
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __LINUX_TEGRA_SOC_H_ */