[PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Each module clock has actually two disable bits: one for the System Core
(ARM) in an SMSTPCRx register, and one for the Realtime Core (SH) in an
RMSTPRx register. Currently we don't touch the bits meant for the
Realtime Core, so some clocks may inadvertently be enabled and still run,
wasting power, while they're disabled in the SMSTPCRx register.
The actual state of the clock is indicated in the corresponding status
register.

Set the disable bits for the Realtime Core for all module clocks we
currently care about to fix this. After this, e.g. QSPI fails to work if
we deliberately keep its clock gated.

Thanks to Laurent for the Realtime Core hint.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
This is a quick hack, NOT to be applied!

Notes/Questions/...:
  - The good news is that the system booted fine, hence so far I didn't
    notice any clocks that were not enabled correctly in our driver code,
    for both legacy and DT case (I used a slightly different version of
    this hack to test the DT case).
  - After bootup, the following clocks seem to be enabled for the Realtime
    Core:
      SCIFA0-2, SCIFB0-2, QSPI, MSIOF0-2, I2C0-5
  - The following clocks were disabled for the Realtime Core:
      LVDS0, DU0-1, SCIF0-5, SCIFA3-5, SDHI0-2, CMT0, Thermal, Ether,
      VIN0-1, SATA0-1
  - These are only the clocks we're currently using. There exist other
    clocks, I did not check their states.
  - Just setting all RMSTPCRx registers to 0xffffffff is not an option, as
    reserved bits should be written back using their read values.
  - Should we add a new enable_reg to struct clk for the legacy case? Or
    just disable them at initialization time, without enlarging struct clk?
  - Add more registers to the bindings for the DT case?
  - How to tell the system that it should (not) disable the RMSTP clocks?
    Someone may want to run something on the SH core, and we don't want to
    interfere with that.

Thanks for your feedback!

 arch/arm/mach-shmobile/clock-r8a7791.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-r8a7791.c b/arch/arm/mach-shmobile/clock-r8a7791.c
index 8e4c12a752b7..595111f79428 100644
--- a/arch/arm/mach-shmobile/clock-r8a7791.c
+++ b/arch/arm/mach-shmobile/clock-r8a7791.c
@@ -305,6 +305,7 @@ void __init r8a7791_clock_init(void)
 	void __iomem *modemr = ioremap_nocache(MODEMR, PAGE_SIZE);
 	u32 mode;
 	int k, ret = 0;
+	unsigned int i;
 
 	BUG_ON(!modemr);
 	mode = ioread32(modemr);
@@ -349,6 +350,19 @@ void __init r8a7791_clock_init(void)
 	else
 		goto epanic;
 
+	printk("Disabling all MSTP clocks for the Realtime Core\n");
+	for (i = 0; i < ARRAY_SIZE(mstp_clks); i++) {
+		const struct clk *clk = &mstp_clks[i];
+		void __iomem *reg = clk->mapped_reg - 0x20;
+		unsigned long physreg = (unsigned long)clk->enable_reg;
+		if ((unsigned long)clk->enable_reg >= SMSTPCR8) {
+			reg += 0x10;
+			physreg += 0x10;
+		}
+		iowrite32(ioread32(reg) | (1 << clk->enable_bit), reg);
+	}
+	printk("DONE\n");
+
 	return;
 
 epanic:
-- 
1.7.9.5

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Laurent Pinchart]

Hi Geert,

On Thursday 06 March 2014 20:54:08 Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Each module clock has actually two disable bits: one for the System Core
> (ARM) in an SMSTPCRx register, and one for the Realtime Core (SH) in an
> RMSTPRx register. Currently we don't touch the bits meant for the
> Realtime Core, so some clocks may inadvertently be enabled and still run,
> wasting power, while they're disabled in the SMSTPCRx register.
> The actual state of the clock is indicated in the corresponding status
> register.
> 
> Set the disable bits for the Realtime Core for all module clocks we
> currently care about to fix this. After this, e.g. QSPI fails to work if
> we deliberately keep its clock gated.
> 
> Thanks to Laurent for the Realtime Core hint.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> ---
> This is a quick hack, NOT to be applied!
> 
> Notes/Questions/...:
>   - The good news is that the system booted fine, hence so far I didn't
>     notice any clocks that were not enabled correctly in our driver code,
>     for both legacy and DT case (I used a slightly different version of
>     this hack to test the DT case).
>   - After bootup, the following clocks seem to be enabled for the Realtime
>     Core:
>       SCIFA0-2, SCIFB0-2, QSPI, MSIOF0-2, I2C0-5
>   - The following clocks were disabled for the Realtime Core:
>       LVDS0, DU0-1, SCIF0-5, SCIFA3-5, SDHI0-2, CMT0, Thermal, Ether,
>       VIN0-1, SATA0-1
>   - These are only the clocks we're currently using. There exist other
>     clocks, I did not check their states.
>   - Just setting all RMSTPCRx registers to 0xffffffff is not an option, as
>     reserved bits should be written back using their read values.
>   - Should we add a new enable_reg to struct clk for the legacy case? Or
>     just disable them at initialization time, without enlarging struct clk?
>   - Add more registers to the bindings for the DT case?
>   - How to tell the system that it should (not) disable the RMSTP clocks?
>     Someone may want to run something on the SH core, and we don't want to
>     interfere with that.

Those are tricky questions. RMSTPCRs are supposed to be managed by the SH-4A 
core. If the SH-4A core isn't booted the only option we have is to manage them 
elsewhere. In that case the registers should be configured at boot time to 
disable all modules from the SH-4A side. This could be done either when 
booting the kernel, or in the boot loader.

If the SH-4A core is booted by Linux we could require the SH-4A code to 
disable all unused clocks, but we could also do so before booting it without 
any drawback (I expect the SH-4A firmware to enable the clocks it needs).

The SH-4A core could be booted first, and then only boot the ARM cores running 
Linux. I don't know if that configuration is commonly used, or even supported, 
but in theory that should be possible. In that case the Linux kernel must not 
touch the RMSTPCRs.

We could disable all realtime clocks from the Linux kernel at boot time (with 
a way to bypass that depending on whether booting the SH-4A first is a use 
case we need to support), or decide to leave this to the boot loader.

> Thanks for your feedback!
> 
>  arch/arm/mach-shmobile/clock-r8a7791.c |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm/mach-shmobile/clock-r8a7791.c
> b/arch/arm/mach-shmobile/clock-r8a7791.c index 8e4c12a752b7..595111f79428
> 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7791.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7791.c
> @@ -305,6 +305,7 @@ void __init r8a7791_clock_init(void)
>  	void __iomem *modemr = ioremap_nocache(MODEMR, PAGE_SIZE);
>  	u32 mode;
>  	int k, ret = 0;
> +	unsigned int i;
> 
>  	BUG_ON(!modemr);
>  	mode = ioread32(modemr);
> @@ -349,6 +350,19 @@ void __init r8a7791_clock_init(void)
>  	else
>  		goto epanic;
> 
> +	printk("Disabling all MSTP clocks for the Realtime Core\n");
> +	for (i = 0; i < ARRAY_SIZE(mstp_clks); i++) {
> +		const struct clk *clk = &mstp_clks[i];
> +		void __iomem *reg = clk->mapped_reg - 0x20;
> +		unsigned long physreg = (unsigned long)clk->enable_reg;
> +		if ((unsigned long)clk->enable_reg >= SMSTPCR8) {
> +			reg += 0x10;
> +			physreg += 0x10;
> +		}
> +		iowrite32(ioread32(reg) | (1 << clk->enable_bit), reg);
> +	}
> +	printk("DONE\n");
> +
>  	return;
> 
>  epanic:

-- 
Regards,

Laurent Pinchart

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Ben Dooks]

On 07/03/14 15:23, Laurent Pinchart wrote:
> Hi Geert,
>
> On Thursday 06 March 2014 20:54:08 Geert Uytterhoeven wrote:
>> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>>
>> Each module clock has actually two disable bits: one for the System Core
>> (ARM) in an SMSTPCRx register, and one for the Realtime Core (SH) in an
>> RMSTPRx register. Currently we don't touch the bits meant for the
>> Realtime Core, so some clocks may inadvertently be enabled and still run,
>> wasting power, while they're disabled in the SMSTPCRx register.
>> The actual state of the clock is indicated in the corresponding status
>> register.
>>
>> Set the disable bits for the Realtime Core for all module clocks we
>> currently care about to fix this. After this, e.g. QSPI fails to work if
>> we deliberately keep its clock gated.
>>
>> Thanks to Laurent for the Realtime Core hint.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>> ---
>> This is a quick hack, NOT to be applied!
>>
>> Notes/Questions/...:
>>    - The good news is that the system booted fine, hence so far I didn't
>>      notice any clocks that were not enabled correctly in our driver code,
>>      for both legacy and DT case (I used a slightly different version of
>>      this hack to test the DT case).
>>    - After bootup, the following clocks seem to be enabled for the Realtime
>>      Core:
>>        SCIFA0-2, SCIFB0-2, QSPI, MSIOF0-2, I2C0-5
>>    - The following clocks were disabled for the Realtime Core:
>>        LVDS0, DU0-1, SCIF0-5, SCIFA3-5, SDHI0-2, CMT0, Thermal, Ether,
>>        VIN0-1, SATA0-1
>>    - These are only the clocks we're currently using. There exist other
>>      clocks, I did not check their states.
>>    - Just setting all RMSTPCRx registers to 0xffffffff is not an option, as
>>      reserved bits should be written back using their read values.
>>    - Should we add a new enable_reg to struct clk for the legacy case? Or
>>      just disable them at initialization time, without enlarging struct clk?
>>    - Add more registers to the bindings for the DT case?
>>    - How to tell the system that it should (not) disable the RMSTP clocks?
>>      Someone may want to run something on the SH core, and we don't want to
>>      interfere with that.
>
> Those are tricky questions. RMSTPCRs are supposed to be managed by the SH-4A
> core. If the SH-4A core isn't booted the only option we have is to manage them
> elsewhere. In that case the registers should be configured at boot time to
> disable all modules from the SH-4A side. This could be done either when
> booting the kernel, or in the boot loader.
>
> If the SH-4A core is booted by Linux we could require the SH-4A code to
> disable all unused clocks, but we could also do so before booting it without
> any drawback (I expect the SH-4A firmware to enable the clocks it needs).
>
> The SH-4A core could be booted first, and then only boot the ARM cores running
> Linux. I don't know if that configuration is commonly used, or even supported,
> but in theory that should be possible. In that case the Linux kernel must not
> touch the RMSTPCRs.
>
> We could disable all realtime clocks from the Linux kernel at boot time (with
> a way to bypass that depending on whether booting the SH-4A first is a use
> case we need to support), or decide to leave this to the boot loader.

I agree with Laurent, we don't really have a good way yet to know
if the SH-4A is going to be used.

My preference would be to either fix it in the bootloader (possibly
add a script in before loading the kernel).


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Laurent Pinchart]

Hi Ben,

On Friday 07 March 2014 15:44:20 Ben Dooks wrote:
> On 07/03/14 15:23, Laurent Pinchart wrote:
> > On Thursday 06 March 2014 20:54:08 Geert Uytterhoeven wrote:
> >> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> >> 
> >> Each module clock has actually two disable bits: one for the System Core
> >> (ARM) in an SMSTPCRx register, and one for the Realtime Core (SH) in an
> >> RMSTPRx register. Currently we don't touch the bits meant for the
> >> Realtime Core, so some clocks may inadvertently be enabled and still run,
> >> wasting power, while they're disabled in the SMSTPCRx register.
> >> The actual state of the clock is indicated in the corresponding status
> >> register.
> >> 
> >> Set the disable bits for the Realtime Core for all module clocks we
> >> currently care about to fix this. After this, e.g. QSPI fails to work if
> >> we deliberately keep its clock gated.
> >> 
> >> Thanks to Laurent for the Realtime Core hint.
> >> 
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> >> ---
> >> This is a quick hack, NOT to be applied!
> >> 
> >> Notes/Questions/...:
> >>    - The good news is that the system booted fine, hence so far I didn't
> >>      notice any clocks that were not enabled correctly in our driver
> >>      code, for both legacy and DT case (I used a slightly different
> >>      version of this hack to test the DT case).
> >>    
> >>    - After bootup, the following clocks seem to be enabled for the
> >>      Realtime Core:
> >>        SCIFA0-2, SCIFB0-2, QSPI, MSIOF0-2, I2C0-5
> >>    
> >>    - The following clocks were disabled for the Realtime Core:
> >>        LVDS0, DU0-1, SCIF0-5, SCIFA3-5, SDHI0-2, CMT0, Thermal, Ether,
> >>        VIN0-1, SATA0-1
> >>    
> >>    - These are only the clocks we're currently using. There exist other
> >>      clocks, I did not check their states.
> >>    
> >>    - Just setting all RMSTPCRx registers to 0xffffffff is not an option,
> >>      as reserved bits should be written back using their read values.
> >>    
> >>    - Should we add a new enable_reg to struct clk for the legacy case? Or
> >>      just disable them at initialization time, without enlarging struct
> >>      clk?
> >>    
> >>    - Add more registers to the bindings for the DT case?
> >>    - How to tell the system that it should (not) disable the RMSTP
> >>      clocks? Someone may want to run something on the SH core, and we
> >>      don't want to interfere with that.
> > 
> > Those are tricky questions. RMSTPCRs are supposed to be managed by the
> > SH-4A core. If the SH-4A core isn't booted the only option we have is to
> > manage them elsewhere. In that case the registers should be configured at
> > boot time to disable all modules from the SH-4A side. This could be done
> > either when booting the kernel, or in the boot loader.
> > 
> > If the SH-4A core is booted by Linux we could require the SH-4A code to
> > disable all unused clocks, but we could also do so before booting it
> > without any drawback (I expect the SH-4A firmware to enable the clocks it
> > needs).
> > 
> > The SH-4A core could be booted first, and then only boot the ARM cores
> > running Linux. I don't know if that configuration is commonly used, or
> > even supported, but in theory that should be possible. In that case the
> > Linux kernel must not touch the RMSTPCRs.
> > 
> > We could disable all realtime clocks from the Linux kernel at boot time
> > (with a way to bypass that depending on whether booting the SH-4A first
> > is a use case we need to support), or decide to leave this to the boot
> > loader.
>
> I agree with Laurent, we don't really have a good way yet to know
> if the SH-4A is going to be used.

Are you aware of any use case where SH-4A would be the main core and would 
then boot Linux on the ARM cores ?

> My preference would be to either fix it in the bootloader (possibly
> add a script in before loading the kernel).

Either that, or what ? :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Ben Dooks]

On 07/03/14 15:50, Laurent Pinchart wrote:
> Hi Ben,
>
> On Friday 07 March 2014 15:44:20 Ben Dooks wrote:
>> On 07/03/14 15:23, Laurent Pinchart wrote:
>>> On Thursday 06 March 2014 20:54:08 Geert Uytterhoeven wrote:
>>>> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>>>>
>>>> Each module clock has actually two disable bits: one for the System Core
>>>> (ARM) in an SMSTPCRx register, and one for the Realtime Core (SH) in an
>>>> RMSTPRx register. Currently we don't touch the bits meant for the
>>>> Realtime Core, so some clocks may inadvertently be enabled and still run,
>>>> wasting power, while they're disabled in the SMSTPCRx register.
>>>> The actual state of the clock is indicated in the corresponding status
>>>> register.
>>>>
>>>> Set the disable bits for the Realtime Core for all module clocks we
>>>> currently care about to fix this. After this, e.g. QSPI fails to work if
>>>> we deliberately keep its clock gated.
>>>>
>>>> Thanks to Laurent for the Realtime Core hint.
>>>>
>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>>>> ---
>>>> This is a quick hack, NOT to be applied!
>>>>
>>>> Notes/Questions/...:
>>>>     - The good news is that the system booted fine, hence so far I didn't
>>>>       notice any clocks that were not enabled correctly in our driver
>>>>       code, for both legacy and DT case (I used a slightly different
>>>>       version of this hack to test the DT case).
>>>>
>>>>     - After bootup, the following clocks seem to be enabled for the
>>>>       Realtime Core:
>>>>         SCIFA0-2, SCIFB0-2, QSPI, MSIOF0-2, I2C0-5
>>>>
>>>>     - The following clocks were disabled for the Realtime Core:
>>>>         LVDS0, DU0-1, SCIF0-5, SCIFA3-5, SDHI0-2, CMT0, Thermal, Ether,
>>>>         VIN0-1, SATA0-1
>>>>
>>>>     - These are only the clocks we're currently using. There exist other
>>>>       clocks, I did not check their states.
>>>>
>>>>     - Just setting all RMSTPCRx registers to 0xffffffff is not an option,
>>>>       as reserved bits should be written back using their read values.
>>>>
>>>>     - Should we add a new enable_reg to struct clk for the legacy case? Or
>>>>       just disable them at initialization time, without enlarging struct
>>>>       clk?
>>>>
>>>>     - Add more registers to the bindings for the DT case?
>>>>     - How to tell the system that it should (not) disable the RMSTP
>>>>       clocks? Someone may want to run something on the SH core, and we
>>>>       don't want to interfere with that.
>>>
>>> Those are tricky questions. RMSTPCRs are supposed to be managed by the
>>> SH-4A core. If the SH-4A core isn't booted the only option we have is to
>>> manage them elsewhere. In that case the registers should be configured at
>>> boot time to disable all modules from the SH-4A side. This could be done
>>> either when booting the kernel, or in the boot loader.
>>>
>>> If the SH-4A core is booted by Linux we could require the SH-4A code to
>>> disable all unused clocks, but we could also do so before booting it
>>> without any drawback (I expect the SH-4A firmware to enable the clocks it
>>> needs).
>>>
>>> The SH-4A core could be booted first, and then only boot the ARM cores
>>> running Linux. I don't know if that configuration is commonly used, or
>>> even supported, but in theory that should be possible. In that case the
>>> Linux kernel must not touch the RMSTPCRs.
>>>
>>> We could disable all realtime clocks from the Linux kernel at boot time
>>> (with a way to bypass that depending on whether booting the SH-4A first
>>> is a use case we need to support), or decide to leave this to the boot
>>> loader.
>>
>> I agree with Laurent, we don't really have a good way yet to know
>> if the SH-4A is going to be used.
>
> Are you aware of any use case where SH-4A would be the main core and would
> then boot Linux on the ARM cores ?

I've been in discussion on a case where the SH core may have been
booted by something else before Linux was run. We're currently not
sure how this is going to be played out (SH loaded from Linux, or
from somewhere else)

>> My preference would be to either fix it in the bootloader (possibly
>> add a script in before loading the kernel).
>
> Either that, or what ? :-)
>


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Geert Uytterhoeven]

On Fri, Mar 7, 2014 at 4:44 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>>>    - How to tell the system that it should (not) disable the RMSTP
>>> clocks?
>>>      Someone may want to run something on the SH core, and we don't want
>>> to
>>>      interfere with that.
>>
>>
>> Those are tricky questions. RMSTPCRs are supposed to be managed by the
>> SH-4A
>> core. If the SH-4A core isn't booted the only option we have is to manage
>> them
>> elsewhere. In that case the registers should be configured at boot time to
>> disable all modules from the SH-4A side. This could be done either when
>> booting the kernel, or in the boot loader.
>>
>> If the SH-4A core is booted by Linux we could require the SH-4A code to
>> disable all unused clocks, but we could also do so before booting it
>> without
>> any drawback (I expect the SH-4A firmware to enable the clocks it needs).
>>
>> The SH-4A core could be booted first, and then only boot the ARM cores
>> running
>> Linux. I don't know if that configuration is commonly used, or even
>> supported,
>> but in theory that should be possible. In that case the Linux kernel must
>> not
>> touch the RMSTPCRs.
>>
>> We could disable all realtime clocks from the Linux kernel at boot time
>> (with
>> a way to bypass that depending on whether booting the SH-4A first is a use
>> case we need to support), or decide to leave this to the boot loader.
>
> I agree with Laurent, we don't really have a good way yet to know
> if the SH-4A is going to be used.

If the SH-4A is used, there must be some synchronization between the
two cores, to avoid both cores touching the same hardware at the same time.

If there is no sharing (which CPU uses which hardware block is fixed),
Linux can just disable the RMSTP clocks for the hardware blocks that are
used by Linux only (i.e. those that are enabled in DT --- I'm ignoring
the legacy
case for now). This requires adding new regs to the bindings[*].

If there is sharing (both CPUs access the same hardware block, but not
at the same time), synchronization code needs to be written.

As the sharing case needs new code anyway, I think we can ignore it for now?

That leaves us with the hardware blocks that are not used by Linux. I.e.
(1) blocks we have in the .dtsi, but not enabled in the .dts, and
(2) blocks that are not in the .dtsi because we don't have a driver yet.
These may or may not be used by the SH-4A, but if not, we want to disable
the clocks for power management reasons.

[*] Currently the bindings in renesas,cpg-mstp-clocks.txt say:

    "reg: Base address and length of the I/O mapped registers used by the MSTP
    clocks. The first register is the clock control register and is mandatory.
    The second register is the clock status register and is optional when not
    implemented in hardware."

    The RMSTPCR registers can be the third (optional) register, unless there
    exist SoCs with RMSTPCR registers but no MSTPSR registers (one more
    reason for going with reg-names rather sooner than later ;-).

<going wild>
As DT describes the hardware, we could have both ARM and SH in the DTS,
too? Anyone who wants to run an AMP system? ;-)
</going wild>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Ben Dooks]

On 07/03/14 16:17, Geert Uytterhoeven wrote:
> On Fri, Mar 7, 2014 at 4:44 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>>>>     - How to tell the system that it should (not) disable the RMSTP
>>>> clocks?
>>>>       Someone may want to run something on the SH core, and we don't want
>>>> to
>>>>       interfere with that.
>>>
>>>
>>> Those are tricky questions. RMSTPCRs are supposed to be managed by the
>>> SH-4A
>>> core. If the SH-4A core isn't booted the only option we have is to manage
>>> them
>>> elsewhere. In that case the registers should be configured at boot time to
>>> disable all modules from the SH-4A side. This could be done either when
>>> booting the kernel, or in the boot loader.
>>>
>>> If the SH-4A core is booted by Linux we could require the SH-4A code to
>>> disable all unused clocks, but we could also do so before booting it
>>> without
>>> any drawback (I expect the SH-4A firmware to enable the clocks it needs).
>>>
>>> The SH-4A core could be booted first, and then only boot the ARM cores
>>> running
>>> Linux. I don't know if that configuration is commonly used, or even
>>> supported,
>>> but in theory that should be possible. In that case the Linux kernel must
>>> not
>>> touch the RMSTPCRs.
>>>
>>> We could disable all realtime clocks from the Linux kernel at boot time
>>> (with
>>> a way to bypass that depending on whether booting the SH-4A first is a use
>>> case we need to support), or decide to leave this to the boot loader.
>>
>> I agree with Laurent, we don't really have a good way yet to know
>> if the SH-4A is going to be used.
>
> If the SH-4A is used, there must be some synchronization between the
> two cores, to avoid both cores touching the same hardware at the same time.
>
> If there is no sharing (which CPU uses which hardware block is fixed),
> Linux can just disable the RMSTP clocks for the hardware blocks that are
> used by Linux only (i.e. those that are enabled in DT --- I'm ignoring
> the legacy
> case for now). This requires adding new regs to the bindings[*].
>
> If there is sharing (both CPUs access the same hardware block, but not
> at the same time), synchronization code needs to be written.
>
> As the sharing case needs new code anyway, I think we can ignore it for now?

No it doesn't... it could be hidden behind trustzone or similar hiding
technology

> That leaves us with the hardware blocks that are not used by Linux. I.e.
> (1) blocks we have in the .dtsi, but not enabled in the .dts, and
> (2) blocks that are not in the .dtsi because we don't have a driver yet.
> These may or may not be used by the SH-4A, but if not, we want to disable
> the clocks for power management reasons.
>
> [*] Currently the bindings in renesas,cpg-mstp-clocks.txt say:
>
>      "reg: Base address and length of the I/O mapped registers used by the MSTP
>      clocks. The first register is the clock control register and is mandatory.
>      The second register is the clock status register and is optional when not
>      implemented in hardware."
>
>      The RMSTPCR registers can be the third (optional) register, unless there
>      exist SoCs with RMSTPCR registers but no MSTPSR registers (one more
>      reason for going with reg-names rather sooner than later ;-).
>
> <going wild>
> As DT describes the hardware, we could have both ARM and SH in the DTS,
> too? Anyone who wants to run an AMP system? ;-)
> </going wild>

Already doing it.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Laurent Pinchart]

Hi Geert,

On Friday 07 March 2014 17:17:04 Geert Uytterhoeven wrote:
> On Fri, Mar 7, 2014 at 4:44 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> >>>    - How to tell the system that it should (not) disable the RMSTP
> >>>      clocks? Someone may want to run something on the SH core, and we
> >>>      don't want to interfere with that.
> >> 
> >> Those are tricky questions. RMSTPCRs are supposed to be managed by the
> >> SH-4A core. If the SH-4A core isn't booted the only option we have is to
> >> manage them elsewhere. In that case the registers should be configured at
> >> boot time to disable all modules from the SH-4A side. This could be done
> >> either when booting the kernel, or in the boot loader.
> >> 
> >> If the SH-4A core is booted by Linux we could require the SH-4A code to
> >> disable all unused clocks, but we could also do so before booting it
> >> without any drawback (I expect the SH-4A firmware to enable the clocks it
> >> needs).
> >> 
> >> The SH-4A core could be booted first, and then only boot the ARM cores
> >> running Linux. I don't know if that configuration is commonly used, or
> >> even supported, but in theory that should be possible. In that case the
> >> Linux kernel must not touch the RMSTPCRs.
> >> 
> >> We could disable all realtime clocks from the Linux kernel at boot time
> >> (with a way to bypass that depending on whether booting the SH-4A first
> >> is a use case we need to support), or decide to leave this to the boot
> >> loader.
> > 
> > I agree with Laurent, we don't really have a good way yet to know
> > if the SH-4A is going to be used.
> 
> If the SH-4A is used, there must be some synchronization between the
> two cores, to avoid both cores touching the same hardware at the same time.
> 
> If there is no sharing (which CPU uses which hardware block is fixed),
> Linux can just disable the RMSTP clocks for the hardware blocks that are
> used by Linux only (i.e. those that are enabled in DT --- I'm ignoring
> the legacy case for now). This requires adding new regs to the bindings[*].
> 
> If there is sharing (both CPUs access the same hardware block, but not
> at the same time), synchronization code needs to be written.
> 
> As the sharing case needs new code anyway, I think we can ignore it for now?
> 
> That leaves us with the hardware blocks that are not used by Linux. I.e.
> (1) blocks we have in the .dtsi, but not enabled in the .dts, and
> (2) blocks that are not in the .dtsi because we don't have a driver yet.
> These may or may not be used by the SH-4A, but if not, we want to disable
> the clocks for power management reasons.

As soon as SH-4A is involved I believe Linux shouldn't touch the RMSTPCRs. As 
we currently have no standard way to handle this, I would be inclined to leave 
the responsibility of disabling clocks for unused peripherals to the boot 
loader. If the boot loader doesn't behave we'll end up using more power than 
we should, but I don't think there would be any other adverse effect. On the 
other hand, it's tempting to let the kernel cover the bugs introduced by the 
boot loader developers, especially when the boot loader is not easily 
modifiable.

> [*] Currently the bindings in renesas,cpg-mstp-clocks.txt say:
> 
>     "reg: Base address and length of the I/O mapped registers used by the
> MSTP clocks. The first register is the clock control register and is
> mandatory. The second register is the clock status register and is optional
> when not implemented in hardware."
> 
>     The RMSTPCR registers can be the third (optional) register, unless there
> exist SoCs with RMSTPCR registers but no MSTPSR registers (one more reason
> for going with reg-names rather sooner than later ;-).

Yes, there are such SoCs I'm afraid.

> <going wild>
> As DT describes the hardware, we could have both ARM and SH in the DTS,
> too? Anyone who wants to run an AMP system? ;-)
> </going wild>

Sure, just submit patches to implement DT support for SH :-) On a more serious 
note, DT for AMP sounds interesting, but I would expect interesting technical 
challenges (especially with DT ABI compatibility issues).

-- 
Regards,

Laurent Pinchart

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Geert Uytterhoeven]

On Fri, Mar 7, 2014 at 5:34 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Friday 07 March 2014 17:17:04 Geert Uytterhoeven wrote:
>> On Fri, Mar 7, 2014 at 4:44 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> >>>    - How to tell the system that it should (not) disable the RMSTP
>> >>>      clocks? Someone may want to run something on the SH core, and we
>> >>>      don't want to interfere with that.
>> >>
>> >> Those are tricky questions. RMSTPCRs are supposed to be managed by the
>> >> SH-4A core. If the SH-4A core isn't booted the only option we have is to
>> >> manage them elsewhere. In that case the registers should be configured at
>> >> boot time to disable all modules from the SH-4A side. This could be done
>> >> either when booting the kernel, or in the boot loader.
>> >>
>> >> If the SH-4A core is booted by Linux we could require the SH-4A code to
>> >> disable all unused clocks, but we could also do so before booting it
>> >> without any drawback (I expect the SH-4A firmware to enable the clocks it
>> >> needs).
>> >>
>> >> The SH-4A core could be booted first, and then only boot the ARM cores
>> >> running Linux. I don't know if that configuration is commonly used, or
>> >> even supported, but in theory that should be possible. In that case the
>> >> Linux kernel must not touch the RMSTPCRs.
>> >>
>> >> We could disable all realtime clocks from the Linux kernel at boot time
>> >> (with a way to bypass that depending on whether booting the SH-4A first
>> >> is a use case we need to support), or decide to leave this to the boot
>> >> loader.
>> >
>> > I agree with Laurent, we don't really have a good way yet to know
>> > if the SH-4A is going to be used.
>>
>> If the SH-4A is used, there must be some synchronization between the
>> two cores, to avoid both cores touching the same hardware at the same time.
>>
>> If there is no sharing (which CPU uses which hardware block is fixed),
>> Linux can just disable the RMSTP clocks for the hardware blocks that are
>> used by Linux only (i.e. those that are enabled in DT --- I'm ignoring
>> the legacy case for now). This requires adding new regs to the bindings[*].
>>
>> If there is sharing (both CPUs access the same hardware block, but not
>> at the same time), synchronization code needs to be written.
>>
>> As the sharing case needs new code anyway, I think we can ignore it for now?
>>
>> That leaves us with the hardware blocks that are not used by Linux. I.e.
>> (1) blocks we have in the .dtsi, but not enabled in the .dts, and
>> (2) blocks that are not in the .dtsi because we don't have a driver yet.
>> These may or may not be used by the SH-4A, but if not, we want to disable
>> the clocks for power management reasons.
>
> As soon as SH-4A is involved I believe Linux shouldn't touch the RMSTPCRs. As
> we currently have no standard way to handle this, I would be inclined to leave
> the responsibility of disabling clocks for unused peripherals to the boot
> loader. If the boot loader doesn't behave we'll end up using more power than
> we should, but I don't think there would be any other adverse effect. On the
> other hand, it's tempting to let the kernel cover the bugs introduced by the
> boot loader developers, especially when the boot loader is not easily
> modifiable.

Sounds reasonable.

Currently I'm most worried by bugs slipping in in our drivers' clock
handling due
to the clock still being enabled on the SH-4A side.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Laurent Pinchart]

Hi Geert,

On Monday 10 March 2014 10:00:29 Geert Uytterhoeven wrote:
> On Fri, Mar 7, 2014 at 5:34 PM, Laurent Pinchart wrote:
> > On Friday 07 March 2014 17:17:04 Geert Uytterhoeven wrote:
> >> On Fri, Mar 7, 2014 at 4:44 PM, Ben Dooks wrote:
> >> >>>  - How to tell the system that it should (not) disable the RMSTP
> >> >>>    clocks? Someone may want to run something on the SH core, and we
> >> >>>    don't want to interfere with that.
> >> >> 
> >> >> Those are tricky questions. RMSTPCRs are supposed to be managed by the
> >> >> SH-4A core. If the SH-4A core isn't booted the only option we have is
> >> >> to manage them elsewhere. In that case the registers should be
> >> >> configured at boot time to disable all modules from the SH-4A side.
> >> >> This could be done either when booting the kernel, or in the boot
> >> >> loader.
> >> >> 
> >> >> If the SH-4A core is booted by Linux we could require the SH-4A code
> >> >> to disable all unused clocks, but we could also do so before booting
> >> >> it without any drawback (I expect the SH-4A firmware to enable the
> >> >> clocks it needs).
> >> >> 
> >> >> The SH-4A core could be booted first, and then only boot the ARM cores
> >> >> running Linux. I don't know if that configuration is commonly used, or
> >> >> even supported, but in theory that should be possible. In that case
> >> >> the Linux kernel must not touch the RMSTPCRs.
> >> >> 
> >> >> We could disable all realtime clocks from the Linux kernel at boot
> >> >> time (with a way to bypass that depending on whether booting the SH-4A
> >> >> first is a use case we need to support), or decide to leave this to
> >> >> the boot loader.
> >> > 
> >> > I agree with Laurent, we don't really have a good way yet to know
> >> > if the SH-4A is going to be used.
> >> 
> >> If the SH-4A is used, there must be some synchronization between the
> >> two cores, to avoid both cores touching the same hardware at the same
> >> time.
> >> 
> >> If there is no sharing (which CPU uses which hardware block is fixed),
> >> Linux can just disable the RMSTP clocks for the hardware blocks that are
> >> used by Linux only (i.e. those that are enabled in DT --- I'm ignoring
> >> the legacy case for now). This requires adding new regs to the
> >> bindings[*].
> >> 
> >> If there is sharing (both CPUs access the same hardware block, but not
> >> at the same time), synchronization code needs to be written.
> >> 
> >> As the sharing case needs new code anyway, I think we can ignore it for
> >> now?
> >> 
> >> That leaves us with the hardware blocks that are not used by Linux. I.e.
> >> (1) blocks we have in the .dtsi, but not enabled in the .dts, and
> >> (2) blocks that are not in the .dtsi because we don't have a driver yet.
> >> These may or may not be used by the SH-4A, but if not, we want to disable
> >> the clocks for power management reasons.
> > 
> > As soon as SH-4A is involved I believe Linux shouldn't touch the RMSTPCRs.
> > As we currently have no standard way to handle this, I would be inclined
> > to leave the responsibility of disabling clocks for unused peripherals to
> > the boot loader. If the boot loader doesn't behave we'll end up using
> > more power than we should, but I don't think there would be any other
> > adverse effect. On the other hand, it's tempting to let the kernel cover
> > the bugs introduced by the boot loader developers, especially when the
> > boot loader is not easily modifiable.
> 
> Sounds reasonable.
> 
> Currently I'm most worried by bugs slipping in in our drivers' clock
> handling due to the clock still being enabled on the SH-4A side.

I can share that concern. One possible solution would be a debug option to 
force disabling of all RMSTPCRs from the Linux side. It's a bit hackish 
though. Another one would be to fix the boot loader for our development 
boards, but I don't know where the sources are located, and whether we could 
push the modification "upstream" internally.

Magnus, I'd also like to hear your opinion on all this.

-- 
Regards,

Laurent Pinchart

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Ben Dooks]

On 10/03/14 11:45, Laurent Pinchart wrote:
> Hi Geert,
>
> On Monday 10 March 2014 10:00:29 Geert Uytterhoeven wrote:
>> On Fri, Mar 7, 2014 at 5:34 PM, Laurent Pinchart wrote:
>>> On Friday 07 March 2014 17:17:04 Geert Uytterhoeven wrote:
>>>> On Fri, Mar 7, 2014 at 4:44 PM, Ben Dooks wrote:
>>>>>>>   - How to tell the system that it should (not) disable the RMSTP
>>>>>>>     clocks? Someone may want to run something on the SH core, and we
>>>>>>>     don't want to interfere with that.
>>>>>>
>>>>>> Those are tricky questions. RMSTPCRs are supposed to be managed by the
>>>>>> SH-4A core. If the SH-4A core isn't booted the only option we have is
>>>>>> to manage them elsewhere. In that case the registers should be
>>>>>> configured at boot time to disable all modules from the SH-4A side.
>>>>>> This could be done either when booting the kernel, or in the boot
>>>>>> loader.
>>>>>>
>>>>>> If the SH-4A core is booted by Linux we could require the SH-4A code
>>>>>> to disable all unused clocks, but we could also do so before booting
>>>>>> it without any drawback (I expect the SH-4A firmware to enable the
>>>>>> clocks it needs).
>>>>>>
>>>>>> The SH-4A core could be booted first, and then only boot the ARM cores
>>>>>> running Linux. I don't know if that configuration is commonly used, or
>>>>>> even supported, but in theory that should be possible. In that case
>>>>>> the Linux kernel must not touch the RMSTPCRs.
>>>>>>
>>>>>> We could disable all realtime clocks from the Linux kernel at boot
>>>>>> time (with a way to bypass that depending on whether booting the SH-4A
>>>>>> first is a use case we need to support), or decide to leave this to
>>>>>> the boot loader.
>>>>>
>>>>> I agree with Laurent, we don't really have a good way yet to know
>>>>> if the SH-4A is going to be used.
>>>>
>>>> If the SH-4A is used, there must be some synchronization between the
>>>> two cores, to avoid both cores touching the same hardware at the same
>>>> time.
>>>>
>>>> If there is no sharing (which CPU uses which hardware block is fixed),
>>>> Linux can just disable the RMSTP clocks for the hardware blocks that are
>>>> used by Linux only (i.e. those that are enabled in DT --- I'm ignoring
>>>> the legacy case for now). This requires adding new regs to the
>>>> bindings[*].
>>>>
>>>> If there is sharing (both CPUs access the same hardware block, but not
>>>> at the same time), synchronization code needs to be written.
>>>>
>>>> As the sharing case needs new code anyway, I think we can ignore it for
>>>> now?
>>>>
>>>> That leaves us with the hardware blocks that are not used by Linux. I.e.
>>>> (1) blocks we have in the .dtsi, but not enabled in the .dts, and
>>>> (2) blocks that are not in the .dtsi because we don't have a driver yet.
>>>> These may or may not be used by the SH-4A, but if not, we want to disable
>>>> the clocks for power management reasons.
>>>
>>> As soon as SH-4A is involved I believe Linux shouldn't touch the RMSTPCRs.
>>> As we currently have no standard way to handle this, I would be inclined
>>> to leave the responsibility of disabling clocks for unused peripherals to
>>> the boot loader. If the boot loader doesn't behave we'll end up using
>>> more power than we should, but I don't think there would be any other
>>> adverse effect. On the other hand, it's tempting to let the kernel cover
>>> the bugs introduced by the boot loader developers, especially when the
>>> boot loader is not easily modifiable.
>>
>> Sounds reasonable.
>>
>> Currently I'm most worried by bugs slipping in in our drivers' clock
>> handling due to the clock still being enabled on the SH-4A side.
>
> I can share that concern. One possible solution would be a debug option to
> force disabling of all RMSTPCRs from the Linux side. It's a bit hackish
> though. Another one would be to fix the boot loader for our development
> boards, but I don't know where the sources are located, and whether we could
> push the modification "upstream" internally.
>
> Magnus, I'd also like to hear your opinion on all this.

It isn't as if most boot-loaders are script-able, it would not be
difficult to do the relevant pokes.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Geert Uytterhoeven]

On Mon, Mar 10, 2014 at 12:45 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> Currently I'm most worried by bugs slipping in in our drivers' clock
>> handling due to the clock still being enabled on the SH-4A side.
>
> I can share that concern. One possible solution would be a debug option to
> force disabling of all RMSTPCRs from the Linux side. It's a bit hackish
> though. Another one would be to fix the boot loader for our development
> boards, but I don't know where the sources are located, and whether we could
> push the modification "upstream" internally.

My koelsch has U-Boot b6af5fc, which presumably can be found at
http://git.denx.de/?p=u-boot/u-boot-sh.git;a=commit;h¶af5fc

Haven't tried building and FLASHing my own, though.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Magnus Damm]

Hi Laurent,

On Mon, Mar 10, 2014 at 8:45 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Geert,
>
> On Monday 10 March 2014 10:00:29 Geert Uytterhoeven wrote:
>> On Fri, Mar 7, 2014 at 5:34 PM, Laurent Pinchart wrote:
>> > On Friday 07 March 2014 17:17:04 Geert Uytterhoeven wrote:
>> >> On Fri, Mar 7, 2014 at 4:44 PM, Ben Dooks wrote:
>> >> >>>  - How to tell the system that it should (not) disable the RMSTP
>> >> >>>    clocks? Someone may want to run something on the SH core, and we
>> >> >>>    don't want to interfere with that.
>> >> >>
>> >> >> Those are tricky questions. RMSTPCRs are supposed to be managed by the
>> >> >> SH-4A core. If the SH-4A core isn't booted the only option we have is
>> >> >> to manage them elsewhere. In that case the registers should be
>> >> >> configured at boot time to disable all modules from the SH-4A side.
>> >> >> This could be done either when booting the kernel, or in the boot
>> >> >> loader.
>> >> >>
>> >> >> If the SH-4A core is booted by Linux we could require the SH-4A code
>> >> >> to disable all unused clocks, but we could also do so before booting
>> >> >> it without any drawback (I expect the SH-4A firmware to enable the
>> >> >> clocks it needs).
>> >> >>
>> >> >> The SH-4A core could be booted first, and then only boot the ARM cores
>> >> >> running Linux. I don't know if that configuration is commonly used, or
>> >> >> even supported, but in theory that should be possible. In that case
>> >> >> the Linux kernel must not touch the RMSTPCRs.
>> >> >>
>> >> >> We could disable all realtime clocks from the Linux kernel at boot
>> >> >> time (with a way to bypass that depending on whether booting the SH-4A
>> >> >> first is a use case we need to support), or decide to leave this to
>> >> >> the boot loader.
>> >> >
>> >> > I agree with Laurent, we don't really have a good way yet to know
>> >> > if the SH-4A is going to be used.
>> >>
>> >> If the SH-4A is used, there must be some synchronization between the
>> >> two cores, to avoid both cores touching the same hardware at the same
>> >> time.
>> >>
>> >> If there is no sharing (which CPU uses which hardware block is fixed),
>> >> Linux can just disable the RMSTP clocks for the hardware blocks that are
>> >> used by Linux only (i.e. those that are enabled in DT --- I'm ignoring
>> >> the legacy case for now). This requires adding new regs to the
>> >> bindings[*].
>> >>
>> >> If there is sharing (both CPUs access the same hardware block, but not
>> >> at the same time), synchronization code needs to be written.
>> >>
>> >> As the sharing case needs new code anyway, I think we can ignore it for
>> >> now?
>> >>
>> >> That leaves us with the hardware blocks that are not used by Linux. I.e.
>> >> (1) blocks we have in the .dtsi, but not enabled in the .dts, and
>> >> (2) blocks that are not in the .dtsi because we don't have a driver yet.
>> >> These may or may not be used by the SH-4A, but if not, we want to disable
>> >> the clocks for power management reasons.
>> >
>> > As soon as SH-4A is involved I believe Linux shouldn't touch the RMSTPCRs.
>> > As we currently have no standard way to handle this, I would be inclined
>> > to leave the responsibility of disabling clocks for unused peripherals to
>> > the boot loader. If the boot loader doesn't behave we'll end up using
>> > more power than we should, but I don't think there would be any other
>> > adverse effect. On the other hand, it's tempting to let the kernel cover
>> > the bugs introduced by the boot loader developers, especially when the
>> > boot loader is not easily modifiable.
>>
>> Sounds reasonable.
>>
>> Currently I'm most worried by bugs slipping in in our drivers' clock
>> handling due to the clock still being enabled on the SH-4A side.
>
> I can share that concern. One possible solution would be a debug option to
> force disabling of all RMSTPCRs from the Linux side. It's a bit hackish
> though. Another one would be to fix the boot loader for our development
> boards, but I don't know where the sources are located, and whether we could
> push the modification "upstream" internally.
>
> Magnus, I'd also like to hear your opinion on all this.

I think we should come up with a reasonable default. Not so keen on
Kconfig dependencies.

Perhaps it is possible to determine boot mode from the MD pins
settings and in such case simply overwrite the RMSTP register with
sane defaults in case we are booting from ARM. Anyone running multiple
OS:es will have to decide about how to share devices anyway, and in
such case handling the clocks and virtualizing them if needed is
probably not too much to ask for.

Cheers,

/ magnus

Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks

[Laurent Pinchart]

Hi Magnus,

On Wednesday 12 March 2014 18:36:38 Magnus Damm wrote:
> On Mon, Mar 10, 2014 at 8:45 PM, Laurent Pinchart wrote:
> > On Monday 10 March 2014 10:00:29 Geert Uytterhoeven wrote:
> >> On Fri, Mar 7, 2014 at 5:34 PM, Laurent Pinchart wrote:
> >> > On Friday 07 March 2014 17:17:04 Geert Uytterhoeven wrote:
> >> >> On Fri, Mar 7, 2014 at 4:44 PM, Ben Dooks wrote:
> >> >> >>>  - How to tell the system that it should (not) disable the RMSTP
> >> >> >>>    clocks? Someone may want to run something on the SH core, and
> >> >> >>>    we don't want to interfere with that.
> >> >> >> 
> >> >> >> Those are tricky questions. RMSTPCRs are supposed to be managed by
> >> >> >> the SH-4A core. If the SH-4A core isn't booted the only option we
> >> >> >> have is to manage them elsewhere. In that case the registers should
> >> >> >> be configured at boot time to disable all modules from the SH-4A
> >> >> >> side. This could be done either when booting the kernel, or in the
> >> >> >> boot loader.
> >> >> >> 
> >> >> >> If the SH-4A core is booted by Linux we could require the SH-4A
> >> >> >> code to disable all unused clocks, but we could also do so before
> >> >> >> booting it without any drawback (I expect the SH-4A firmware to
> >> >> >> enable the clocks it needs).
> >> >> >> 
> >> >> >> The SH-4A core could be booted first, and then only boot the ARM
> >> >> >> cores running Linux. I don't know if that configuration is commonly
> >> >> >> used, or even supported, but in theory that should be possible. In
> >> >> >> that case the Linux kernel must not touch the RMSTPCRs.
> >> >> >> 
> >> >> >> We could disable all realtime clocks from the Linux kernel at boot
> >> >> >> time (with a way to bypass that depending on whether booting the
> >> >> >> SH-4A first is a use case we need to support), or decide to leave
> >> >> >> this to the boot loader.
> >> >> > 
> >> >> > I agree with Laurent, we don't really have a good way yet to know
> >> >> > if the SH-4A is going to be used.
> >> >> 
> >> >> If the SH-4A is used, there must be some synchronization between the
> >> >> two cores, to avoid both cores touching the same hardware at the same
> >> >> time.
> >> >> 
> >> >> If there is no sharing (which CPU uses which hardware block is fixed),
> >> >> Linux can just disable the RMSTP clocks for the hardware blocks that
> >> >> are used by Linux only (i.e. those that are enabled in DT --- I'm
> >> >> ignoring the legacy case for now). This requires adding new regs to
> >> >> the bindings[*].
> >> >> 
> >> >> If there is sharing (both CPUs access the same hardware block, but not
> >> >> at the same time), synchronization code needs to be written.
> >> >> 
> >> >> As the sharing case needs new code anyway, I think we can ignore it
> >> >> for now?
> >> >> 
> >> >> That leaves us with the hardware blocks that are not used by Linux.
> >> >> I.e.
> >> >> (1) blocks we have in the .dtsi, but not enabled in the .dts, and
> >> >> (2) blocks that are not in the .dtsi because we don't have a driver
> >> >> yet.
> >> >> These may or may not be used by the SH-4A, but if not, we want to
> >> >> disable the clocks for power management reasons.
> >> > 
> >> > As soon as SH-4A is involved I believe Linux shouldn't touch the
> >> > RMSTPCRs. As we currently have no standard way to handle this, I would
> >> > be inclined to leave the responsibility of disabling clocks for unused
> >> > peripherals to the boot loader. If the boot loader doesn't behave we'll
> >> > end up using more power than we should, but I don't think there would
> >> > be any other adverse effect. On the other hand, it's tempting to let
> >> > the kernel cover the bugs introduced by the boot loader developers,
> >> > especially when the boot loader is not easily modifiable.
> >> 
> >> Sounds reasonable.
> >> 
> >> Currently I'm most worried by bugs slipping in in our drivers' clock
> >> handling due to the clock still being enabled on the SH-4A side.
> > 
> > I can share that concern. One possible solution would be a debug option to
> > force disabling of all RMSTPCRs from the Linux side. It's a bit hackish
> > though. Another one would be to fix the boot loader for our development
> > boards, but I don't know where the sources are located, and whether we
> > could push the modification "upstream" internally.
> > 
> > Magnus, I'd also like to hear your opinion on all this.
> 
> I think we should come up with a reasonable default. Not so keen on
> Kconfig dependencies.
> 
> Perhaps it is possible to determine boot mode from the MD pins
> settings and in such case simply overwrite the RMSTP register with
> sane defaults in case we are booting from ARM. Anyone running multiple
> OS:es will have to decide about how to share devices anyway, and in
> such case handling the clocks and virtualizing them if needed is
> probably not too much to ask for.

That's a good point. We can just read the boot mode and disable all RMSTPCRs 
when booting from ARM, and leave them untouched when booting from SH4. Booting 
SH4 from ARM isn't supported in mainline at the moment, so we don't have to 
care too much about that case yet.

-- 
Regards,

Laurent Pinchart