2015-06-15 16:15:12

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 00/14] ARM: shmobile: Add CPG Clock Domains

On Thu, May 28, 2015 at 8:53 PM, Geert Uytterhoeven
<[email protected]> wrote:
> Hi all,
>
> This patch series adds Clock Domain support to the Clock Pulse Generator
> (CPG) Module Stop (MSTP) Clocks driver using the generic PM Domain, to
> be used on shmobile SoCs without device power domains (R-Car Gen1 and
> Gen2, RZ). This allows to power-manage the module clocks of SoC devices
> that are part of the CPG Clock Domain using Runtime PM, or for system
> suspend/resume, similar to SoCs with device power domains (SH-Mobile and
> R-Mobile).
>
> SoC devices that are part of the CPG Clock Domain and can be
> power-managed through an MSTP clock are tagged in DT with a proper
> "power-domains" property. This applies to most on-SoC devices, which
> have a one-to-one mapping from SoC device to DT device node.
> Notable exceptions are "display" and "sound" device nodes, which
> represent multiple SoC devices, each having their own MSTP clocks. Hence
> drivers for such devices still have to manage their (multiple module)
> clocks themselves.
>
> The (MSTP) clock to use for power-management is found by scanning for
> clocks that are compatible with "renesas,cpg-mstp-clocks".
> Before, the "first" clock tied to each device (con_id NULL) was used,
> being a bit ad-hoc. It was suggested to use the "fck" clock instead,
> but this may conflict with DT bindings for devices we don't control
> (e.g. GIC-400 plans to mandate "clk" for the clk-name of its single
> clock). Looking for real MSTP clocks avoids this problem.
>
> Logically, the CPG Clock Domain operates on the SoC CPG/MSTP block.
> As there's no single device node in DT representing this block (there
> are separate device nodes for the CPG and for the individual MSTP
> clocks), I bound the logic to the CPG device node.
> Perhaps this is something we should change for future SoCs?

Inside Renesas, we've been discussing this face-to-face, but haven't
reached a conclusion yet.

In Linux terminology, "PM domain" is a higher-level abstraction than
just (hardware) "power domain" (sometimes called "power area").
A "PM domain" is any collection of devices that are power-managed
similarly. As such it covers not only hardware power domains, but also
clock domains, and even firmware controlled devices (e.g. as used by the
Linux ACPI subsystem).

I find it a bit unfortunate this was not reflected in the DT bindings
for Generic PM domains, which use "power-domains" properties, making
believe people this is about hardware power domains only.

One other point of confusion is that there are multiple kernel
subsystems that can (or seem to be able to) be used for the same
purpose. Both regulators and power domains are used to "control power".
The same is true for clocks vs. clock domains.
My point of view is that the regulator and clock subsystems are more
about the properties of regulators (voltage, current) resp. clocks
(frequencies), while power/clock domains are about being active or
inactive.

On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the MSTP (Module Standby and
Software Reset) block is very intimately tied to the CPG (Clock Pulse
Generator) block.

The MSTP block provides two functions:
1. Module Standby: "Clock supply to specified modules is stopped by
setting the module stop control register bits."
However, the clock supply to a module is not stopped until all CPUs
in the SoC agree. Indeed, there are separate MSTP registers for
application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
2. Reset control. to perform a software reset of a specific module.

Given the second function, perhaps the MSTP bits shouldn't have been
moduled as clocks, but it made sense at the time of introduction, and
IMHO it still does.

However, due to the module standby function, all connected devices are
grouped into a collection of devices that are power-managed similarly,
by controlling the clock supply to the individual modules. So this
warrants the use of a PM domain.

Alternative solutions that have been proposed are:

1. | Explicit opt-in in drivers (from Laurent)
| As the driver knows best which clock it wants to manage, the
| driver could tell runtime PM if/when which clock to use.

My rebuttal here is twofold:
- Does the driver know best? It may know it may need to enable a
clock. But the clock may be optional: on some SoCs, the same IP
core may be present without the Module Standby feature (e.g.
the GPIO blocks on R-Car Gen1 are not documented to have MSTP
bits, while they do on R-Car Gen2). Why would the driver have
to care?

The hardware documentation clearly states the purpose of the
MSTP clocks: when a module is not in use, its module clock can
be stopped to reduce power consumption. All of this should be
described in DT. We already have the clocks in DT. If a module
has an MSTP clock, it means the module can be put in standby
mode. This is the same for all modules, hence for all drivers.

- The idea is to reduce the amount of boilerplate code, not to
increase it. The more code we can move into platform code, the
less drivers have to care.

This is the real power behind the abstraction of runtime PM.
The driver only has to tell runtime PM when it wants to "use"
the hardware module, using pm_runtime_{get_sync,put}().
It doesn't have to know this involves enabling clocks and/or
power domains, or parent devices. All of this is taken care of
by a small piece of platform code, and the generic code.
As not all drivers are runtime PM-aware yet, calls to
pm_runtime_*() functions may have to be added, though. But
that's it.

Side note: Laurent has been mostly involved with multimedia devices.
And let display and sound be the two exceptions where there's no
one-to-one mapping from SoC devices to DT device nodes...
Hence the multimedia drivers would have to manage the (multiple)
module clocks anyway.

Perhaps the display and sound bindings can be reworked, to better
describe the hardware structure, and expose a one-to-one mapping
between MSTP clocks and hardware modules, too?

2. | Handling MSTP clocks automatically in a similar way that the current
| code handles the first clock, without requiring usage of a
| power-domain property in DT (from Magnus)
| As there are already "clocks = <...>" links from device nodes to MSTP
| clocks in DT, we can just scan for those, without requiring
| (superfluous) "power-domains = <...>" properties in DT.

Indeed, given the presence of a link to an MSTP clock in a device
node, we know the module can be put in standby mode.
But without the standard "power-domains" property, we would need
our own specialized code to scan all nodes for MSTP clocks (through
a platform_bus notifier again?), and add the corresponding devices
to the clock domain. Hence the "power-domains" properties allow to
use the generic code, and thus share more code with other SoCs.
In addition, a "power-domains" property gives a strong clue to
people not familiar with Renesas SoCs and their MSTP clocks.

Thanks for your comments/suggestions/supports/flames/...!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


2015-06-15 18:39:59

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH v2 00/14] ARM: shmobile: Add CPG Clock Domains

Geert Uytterhoeven <[email protected]> writes:

> On Thu, May 28, 2015 at 8:53 PM, Geert Uytterhoeven
> <[email protected]> wrote:
>> Hi all,
>>
>> This patch series adds Clock Domain support to the Clock Pulse Generator
>> (CPG) Module Stop (MSTP) Clocks driver using the generic PM Domain, to
>> be used on shmobile SoCs without device power domains (R-Car Gen1 and
>> Gen2, RZ). This allows to power-manage the module clocks of SoC devices
>> that are part of the CPG Clock Domain using Runtime PM, or for system
>> suspend/resume, similar to SoCs with device power domains (SH-Mobile and
>> R-Mobile).
>>
>> SoC devices that are part of the CPG Clock Domain and can be
>> power-managed through an MSTP clock are tagged in DT with a proper
>> "power-domains" property. This applies to most on-SoC devices, which
>> have a one-to-one mapping from SoC device to DT device node.
>> Notable exceptions are "display" and "sound" device nodes, which
>> represent multiple SoC devices, each having their own MSTP clocks. Hence
>> drivers for such devices still have to manage their (multiple module)
>> clocks themselves.
>>
>> The (MSTP) clock to use for power-management is found by scanning for
>> clocks that are compatible with "renesas,cpg-mstp-clocks".
>> Before, the "first" clock tied to each device (con_id NULL) was used,
>> being a bit ad-hoc. It was suggested to use the "fck" clock instead,
>> but this may conflict with DT bindings for devices we don't control
>> (e.g. GIC-400 plans to mandate "clk" for the clk-name of its single
>> clock). Looking for real MSTP clocks avoids this problem.
>>
>> Logically, the CPG Clock Domain operates on the SoC CPG/MSTP block.
>> As there's no single device node in DT representing this block (there
>> are separate device nodes for the CPG and for the individual MSTP
>> clocks), I bound the logic to the CPG device node.
>> Perhaps this is something we should change for future SoCs?
>
> Inside Renesas, we've been discussing this face-to-face, but haven't
> reached a conclusion yet.
>
> In Linux terminology, "PM domain" is a higher-level abstraction than
> just (hardware) "power domain" (sometimes called "power area").
> A "PM domain" is any collection of devices that are power-managed
> similarly. As such it covers not only hardware power domains, but also
> clock domains, and even firmware controlled devices (e.g. as used by the
> Linux ACPI subsystem).
>
> I find it a bit unfortunate this was not reflected in the DT bindings
> for Generic PM domains, which use "power-domains" properties, making
> believe people this is about hardware power domains only.
>
> One other point of confusion is that there are multiple kernel
> subsystems that can (or seem to be able to) be used for the same
> purpose. Both regulators and power domains are used to "control power".
> The same is true for clocks vs. clock domains.
> My point of view is that the regulator and clock subsystems are more
> about the properties of regulators (voltage, current) resp. clocks
> (frequencies), while power/clock domains are about being active or
> inactive.
>
> On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the MSTP (Module Standby and
> Software Reset) block is very intimately tied to the CPG (Clock Pulse
> Generator) block.
>
> The MSTP block provides two functions:
> 1. Module Standby: "Clock supply to specified modules is stopped by
> setting the module stop control register bits."
> However, the clock supply to a module is not stopped until all CPUs
> in the SoC agree. Indeed, there are separate MSTP registers for
> application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
> 2. Reset control. to perform a software reset of a specific module.
>
> Given the second function, perhaps the MSTP bits shouldn't have been
> moduled as clocks, but it made sense at the time of introduction, and
> IMHO it still does.
>
> However, due to the module standby function, all connected devices are
> grouped into a collection of devices that are power-managed similarly,
> by controlling the clock supply to the individual modules. So this
> warrants the use of a PM domain.

Agreed.

So, what is the main objection to using PM domains for this? Is it
only a terminology issue?

> Alternative solutions that have been proposed are:
>
> 1. | Explicit opt-in in drivers (from Laurent)
> | As the driver knows best which clock it wants to manage, the
> | driver could tell runtime PM if/when which clock to use.
>
> My rebuttal here is twofold:
> - Does the driver know best? It may know it may need to enable a
> clock. But the clock may be optional: on some SoCs, the same IP
> core may be present without the Module Standby feature (e.g.
> the GPIO blocks on R-Car Gen1 are not documented to have MSTP
> bits, while they do on R-Car Gen2). Why would the driver have
> to care?
>
> The hardware documentation clearly states the purpose of the
> MSTP clocks: when a module is not in use, its module clock can
> be stopped to reduce power consumption. All of this should be
> described in DT. We already have the clocks in DT. If a module
> has an MSTP clock, it means the module can be put in standby
> mode. This is the same for all modules, hence for all drivers.
>
> - The idea is to reduce the amount of boilerplate code, not to
> increase it. The more code we can move into platform code, the
> less drivers have to care.
>
> This is the real power behind the abstraction of runtime PM.
> The driver only has to tell runtime PM when it wants to "use"
> the hardware module, using pm_runtime_{get_sync,put}().
> It doesn't have to know this involves enabling clocks and/or
> power domains, or parent devices. All of this is taken care of
> by a small piece of platform code, and the generic code.

Exactly, this is real power of this abstraction when used fully.

> As not all drivers are runtime PM-aware yet, calls to
> pm_runtime_*() functions may have to be added, though. But
> that's it.
>
> Side note: Laurent has been mostly involved with multimedia devices.
> And let display and sound be the two exceptions where there's no
> one-to-one mapping from SoC devices to DT device nodes...
> Hence the multimedia drivers would have to manage the (multiple)
> module clocks anyway.
>
> Perhaps the display and sound bindings can be reworked, to better
> describe the hardware structure, and expose a one-to-one mapping
> between MSTP clocks and hardware modules, too?
>
> 2. | Handling MSTP clocks automatically in a similar way that the current
> | code handles the first clock, without requiring usage of a
> | power-domain property in DT (from Magnus)
> | As there are already "clocks = <...>" links from device nodes to MSTP
> | clocks in DT, we can just scan for those, without requiring
> | (superfluous) "power-domains = <...>" properties in DT.
>
> Indeed, given the presence of a link to an MSTP clock in a device
> node, we know the module can be put in standby mode.
> But without the standard "power-domains" property, we would need
> our own specialized code to scan all nodes for MSTP clocks (through
> a platform_bus notifier again?), and add the corresponding devices
> to the clock domain. Hence the "power-domains" properties allow to
> use the generic code, and thus share more code with other SoCs.

IMO, a clear win.

> In addition, a "power-domains" property gives a strong clue to
> people not familiar with Renesas SoCs and their MSTP clocks.

As a regular reviewer of PM code for SoCs that I'm not familiar with,
this is another big win from my perspective.

Kevin

2015-06-22 20:46:34

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH v2 00/14] ARM: shmobile: Add CPG Clock Domains

Quoting Geert Uytterhoeven (2015-06-15 09:15:04)
> On Thu, May 28, 2015 at 8:53 PM, Geert Uytterhoeven
> <[email protected]> wrote:
> > Hi all,
> >
> > This patch series adds Clock Domain support to the Clock Pulse Generator
> > (CPG) Module Stop (MSTP) Clocks driver using the generic PM Domain, to
> > be used on shmobile SoCs without device power domains (R-Car Gen1 and
> > Gen2, RZ). This allows to power-manage the module clocks of SoC devices
> > that are part of the CPG Clock Domain using Runtime PM, or for system
> > suspend/resume, similar to SoCs with device power domains (SH-Mobile and
> > R-Mobile).
> >
> > SoC devices that are part of the CPG Clock Domain and can be
> > power-managed through an MSTP clock are tagged in DT with a proper
> > "power-domains" property. This applies to most on-SoC devices, which
> > have a one-to-one mapping from SoC device to DT device node.
> > Notable exceptions are "display" and "sound" device nodes, which
> > represent multiple SoC devices, each having their own MSTP clocks. Hence
> > drivers for such devices still have to manage their (multiple module)
> > clocks themselves.
> >
> > The (MSTP) clock to use for power-management is found by scanning for
> > clocks that are compatible with "renesas,cpg-mstp-clocks".
> > Before, the "first" clock tied to each device (con_id NULL) was used,
> > being a bit ad-hoc. It was suggested to use the "fck" clock instead,
> > but this may conflict with DT bindings for devices we don't control
> > (e.g. GIC-400 plans to mandate "clk" for the clk-name of its single
> > clock). Looking for real MSTP clocks avoids this problem.
> >
> > Logically, the CPG Clock Domain operates on the SoC CPG/MSTP block.
> > As there's no single device node in DT representing this block (there
> > are separate device nodes for the CPG and for the individual MSTP
> > clocks), I bound the logic to the CPG device node.
> > Perhaps this is something we should change for future SoCs?
>

Hello Geert,

I'm still going through this series but I have a few comments/questions.

> Inside Renesas, we've been discussing this face-to-face, but haven't
> reached a conclusion yet.
>
> In Linux terminology, "PM domain" is a higher-level abstraction than
> just (hardware) "power domain" (sometimes called "power area").
> A "PM domain" is any collection of devices that are power-managed
> similarly. As such it covers not only hardware power domains, but also
> clock domains, and even firmware controlled devices (e.g. as used by the
> Linux ACPI subsystem).
>
> I find it a bit unfortunate this was not reflected in the DT bindings
> for Generic PM domains, which use "power-domains" properties, making
> believe people this is about hardware power domains only.

"Power island" is used commonly to describe the hardware power domain.
I like that one since it conveys the concept of power isolation/clamping
in a power distribution network. It might be more clear to refer to the
hardware as power islands and reserve "power domains" for the Linux
construct?

>
> One other point of confusion is that there are multiple kernel
> subsystems that can (or seem to be able to) be used for the same
> purpose. Both regulators and power domains are used to "control power".
> The same is true for clocks vs. clock domains.
> My point of view is that the regulator and clock subsystems are more
> about the properties of regulators (voltage, current) resp. clocks
> (frequencies), while power/clock domains are about being active or
> inactive.

Only speaking for the clock framework, but the prepare/unprepare and
enable/disable semantics are very important here, not just the
rate-change stuff.

>
> On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the MSTP (Module Standby and
> Software Reset) block is very intimately tied to the CPG (Clock Pulse
> Generator) block.
>
> The MSTP block provides two functions:
> 1. Module Standby: "Clock supply to specified modules is stopped by
> setting the module stop control register bits."
> However, the clock supply to a module is not stopped until all CPUs
> in the SoC agree. Indeed, there are separate MSTP registers for
> application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
> 2. Reset control. to perform a software reset of a specific module.
>
> Given the second function, perhaps the MSTP bits shouldn't have been
> moduled as clocks, but it made sense at the time of introduction, and
> IMHO it still does.

Does the clk.enable_count refcount for an MSTP "clock" mirror your
statement, "the clock supply to a module is not stopped until all CPUs
in the SoC agree"? Put another way, is it possble in a shmobile system
for an MSTP "clock" to have an enable_count of zero, but still by
physically enabled in hardware?

Regards,
Mike

2015-06-23 08:59:31

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 00/14] ARM: shmobile: Add CPG Clock Domains

Hi Mike,

On Mon, Jun 22, 2015 at 10:46 PM, Michael Turquette
<[email protected]> wrote:
>> The MSTP block provides two functions:
>> 1. Module Standby: "Clock supply to specified modules is stopped by
>> setting the module stop control register bits."
>> However, the clock supply to a module is not stopped until all CPUs
>> in the SoC agree. Indeed, there are separate MSTP registers for
>> application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
>> 2. Reset control. to perform a software reset of a specific module.
>>
>> Given the second function, perhaps the MSTP bits shouldn't have been
>> moduled as clocks, but it made sense at the time of introduction, and
>> IMHO it still does.
>
> Does the clk.enable_count refcount for an MSTP "clock" mirror your
> statement, "the clock supply to a module is not stopped until all CPUs
> in the SoC agree"? Put another way, is it possble in a shmobile system
> for an MSTP "clock" to have an enable_count of zero, but still by
> physically enabled in hardware?

Yes. The enable_count only matches the bit in the application core MSTP
control register. The real-time core MSTP control register may still have the
same module clock enabled (i.e. module not put in standby).
On some SoCs (e.g. R-Mobile APE6) there can be 3 sets of control registers
(for application, real-time, and baseband core).

Most (not all) module clocks have a status register to check if the module
clock is actually enabled or not. This shows that the module clock is stopped
iff all cores agree to stop it.

However, I found a few module clocks that never can be stopped, according to
the status register. Perhaps there's another set of control registers for an
undocumented core...

In addition, there are many undocumented bits in the MSTP control registers,
which may represent existing but undocumented clocks. Disabling such bits
may lock up the system.

As Linux doesn't touch the control registers for other CPU cores (they're
not described in DT), a module clock may be enabled (read: not stopped) in
a control registers meant for another CPU core, depending on reset state or
on the boot loader. Or by another OS running on another CPU core.
I've been running for quite some time with extra code that disables all module
clocks (for all cores) at early boot time, which let me find lots of drivers
that relied on implicit module clocks. Most got fixed, the only major
offender left is the module clock for the GIC ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds