sunxi-ng clocks: leaving certain clocks alone?

sunxi-ng clocks: leaving certain clocks alone?

[André Przywara]

Hi,

I was observing that the new sunxi-ng clock code apparently explicitly
turns off _all_ clocks that are not used or needed. I find this rather
unfortunate, as I wanted to use the THS temperature sensor from ARM
Trusted Firmware to implement emergency shutdown or DVFS throttling.
That works until the clock framework finds the clock (as enumerated in
ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS mod
clock register and bit 8 in the respective clock gate register.
Turning them manually back on via /dev/mem or removing the THS clock
from the sunxi-ng driver fixes this for me.

This was not happening with the old Allwinner clocks, since the kernel
wouldn't even know about it if there was no driver and the clock wasn't
mentioned in the DT.

Now with sunxi-ng even though the THS clock is not actually referenced
or used in the DT, the kernel turns it off. I would expect that upon
entering the kernel all unneeded clocks are turned off anyway, so there
is no need to mess with clocks that have no user, but are enumerated in
the ccu driver.

I wonder if this kills simplefb as well, for instance, since I believe
that U-Boot needs to turn on certain clocks and relies on them staying up.

So my questions:
1) Is this expected behaviour?
2) If yes, should it really be?
3) If yes, shouldn't there be way to explicitly tell Linux to leave that
clock alone, preferably via DT? Although the sunxi-ng clocks take
control over the whole CCU unit, I wonder if it should really mess with
clocks there are not referenced in the DT.

Maybe there is some way to reference those clocks via some dummy driver
or DT node to avoid this behaviour? Is there any prior art in this respect?

I think this issue will affect more future users (thinking of EFI RTC,
EFI graphics, etc.), so I wanted to start a discussion on this.

Any input welcome.

Cheers,
Andre.

P.S. For reference my use case: ARM Trusted Firmware (ATF) enables the
temperature sensor (THS) and programs a shutdown value. It programs the
respective interrupt as secure and registers an IRQ handler in ATF to
shutdown the system or take other appropriate matters to avoid
overheating. Additionally the sensor is exported via the generic SCPI
interface, so the existing scpi-hwmon driver picks it up and reports it
to whoever is interested in Linux land via the normal interfaces.

sunxi-ng clocks: leaving certain clocks alone?

[André Przywara]

Hi,

I was observing that the new sunxi-ng clock code apparently explicitly
turns off _all_ clocks that are not used or needed. I find this rather
unfortunate, as I wanted to use the THS temperature sensor from ARM
Trusted Firmware to implement emergency shutdown or DVFS throttling.
That works until the clock framework finds the clock (as enumerated in
ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS mod
clock register and bit 8 in the respective clock gate register.
Turning them manually back on via /dev/mem or removing the THS clock
from the sunxi-ng driver fixes this for me.

This was not happening with the old Allwinner clocks, since the kernel
wouldn't even know about it if there was no driver and the clock wasn't
mentioned in the DT.

Now with sunxi-ng even though the THS clock is not actually referenced
or used in the DT, the kernel turns it off. I would expect that upon
entering the kernel all unneeded clocks are turned off anyway, so there
is no need to mess with clocks that have no user, but are enumerated in
the ccu driver.

I wonder if this kills simplefb as well, for instance, since I believe
that U-Boot needs to turn on certain clocks and relies on them staying up.

So my questions:
1) Is this expected behaviour?
2) If yes, should it really be?
3) If yes, shouldn't there be way to explicitly tell Linux to leave that
clock alone, preferably via DT? Although the sunxi-ng clocks take
control over the whole CCU unit, I wonder if it should really mess with
clocks there are not referenced in the DT.

Maybe there is some way to reference those clocks via some dummy driver
or DT node to avoid this behaviour? Is there any prior art in this respect?

I think this issue will affect more future users (thinking of EFI RTC,
EFI graphics, etc.), so I wanted to start a discussion on this.

Any input welcome.

Cheers,
Andre.

P.S. For reference my use case: ARM Trusted Firmware (ATF) enables the
temperature sensor (THS) and programs a shutdown value. It programs the
respective interrupt as secure and registers an IRQ handler in ATF to
shutdown the system or take other appropriate matters to avoid
overheating. Additionally the sensor is exported via the generic SCPI
interface, so the existing scpi-hwmon driver picks it up and reports it
to whoever is interested in Linux land via the normal interfaces.

Re: [linux-sunxi] sunxi-ng clocks: leaving certain clocks alone?

[Chen-Yu Tsai]

On Mon, Dec 12, 2016 at 7:54 AM, Andr=C3=A9 Przywara <andre.przywara@arm.co=
m> wrote:
> Hi,
>
> I was observing that the new sunxi-ng clock code apparently explicitly
> turns off _all_ clocks that are not used or needed. I find this rather
> unfortunate, as I wanted to use the THS temperature sensor from ARM
> Trusted Firmware to implement emergency shutdown or DVFS throttling.
> That works until the clock framework finds the clock (as enumerated in
> ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS mod
> clock register and bit 8 in the respective clock gate register.
> Turning them manually back on via /dev/mem or removing the THS clock
> from the sunxi-ng driver fixes this for me.
>
> This was not happening with the old Allwinner clocks, since the kernel
> wouldn't even know about it if there was no driver and the clock wasn't
> mentioned in the DT.
>
> Now with sunxi-ng even though the THS clock is not actually referenced
> or used in the DT, the kernel turns it off. I would expect that upon
> entering the kernel all unneeded clocks are turned off anyway, so there
> is no need to mess with clocks that have no user, but are enumerated in
> the ccu driver.

I can't say that's absolutely true (wink).

>
> I wonder if this kills simplefb as well, for instance, since I believe
> that U-Boot needs to turn on certain clocks and relies on them staying up=
.

The simplefb bindings takes clocks and regulators expressly for the
purpose of keeping them enabled.

>
> So my questions:
> 1) Is this expected behaviour?

Yes.

> 2) If yes, should it really be?
> 3) If yes, shouldn't there be way to explicitly tell Linux to leave that
> clock alone, preferably via DT? Although the sunxi-ng clocks take
> control over the whole CCU unit, I wonder if it should really mess with
> clocks there are not referenced in the DT.

As it owns the whole CCU unit, why not? And how would it know if some
clock is referenced or not, unless going through the whole device tree?

Furthermore, nothing prevents another device driver from referencing
said clock and turning it off when it's not in use. Think THS driver
with runtime PM.

Are you also mapping the THS to secure only? Otherwise nothing would
prevent Linux from also claiming it.

>
> Maybe there is some way to reference those clocks via some dummy driver
> or DT node to avoid this behaviour? Is there any prior art in this respec=
t?

If you want a clock to not be disabled by anyone, adding CLK_IS_CRITICAL
to its flags is the proper option. This is done in the clk driver though.

If you just don't want the clk subsystem to disable unused clks at boot
time, you can add "clk_ignore_unused" to the kernel boot parameters.
I think this is more of a hack and debugging tool though.

About dummy drivers, simplefb comes to mind again. But simplefb disables
them when it gets kicked out by the drm driver. Maybe there are other
examples.

Regards
ChenYu

> I think this issue will affect more future users (thinking of EFI RTC,
> EFI graphics, etc.), so I wanted to start a discussion on this.
>
> Any input welcome.
>
> Cheers,
> Andre.
>
> P.S. For reference my use case: ARM Trusted Firmware (ATF) enables the
> temperature sensor (THS) and programs a shutdown value. It programs the
> respective interrupt as secure and registers an IRQ handler in ATF to
> shutdown the system or take other appropriate matters to avoid
> overheating. Additionally the sensor is exported via the generic SCPI
> interface, so the existing scpi-hwmon driver picks it up and reports it
> to whoever is interested in Linux land via the normal interfaces.
>
> --
> You received this message because you are subscribed to the Google Groups=
 "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an=
 email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] sunxi-ng clocks: leaving certain clocks alone?

[Chen-Yu Tsai]

On Mon, Dec 12, 2016 at 7:54 AM, Andr? Przywara <andre.przywara@arm.com> wrote:
> Hi,
>
> I was observing that the new sunxi-ng clock code apparently explicitly
> turns off _all_ clocks that are not used or needed. I find this rather
> unfortunate, as I wanted to use the THS temperature sensor from ARM
> Trusted Firmware to implement emergency shutdown or DVFS throttling.
> That works until the clock framework finds the clock (as enumerated in
> ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS mod
> clock register and bit 8 in the respective clock gate register.
> Turning them manually back on via /dev/mem or removing the THS clock
> from the sunxi-ng driver fixes this for me.
>
> This was not happening with the old Allwinner clocks, since the kernel
> wouldn't even know about it if there was no driver and the clock wasn't
> mentioned in the DT.
>
> Now with sunxi-ng even though the THS clock is not actually referenced
> or used in the DT, the kernel turns it off. I would expect that upon
> entering the kernel all unneeded clocks are turned off anyway, so there
> is no need to mess with clocks that have no user, but are enumerated in
> the ccu driver.

I can't say that's absolutely true (wink).

>
> I wonder if this kills simplefb as well, for instance, since I believe
> that U-Boot needs to turn on certain clocks and relies on them staying up.

The simplefb bindings takes clocks and regulators expressly for the
purpose of keeping them enabled.

>
> So my questions:
> 1) Is this expected behaviour?

Yes.

> 2) If yes, should it really be?
> 3) If yes, shouldn't there be way to explicitly tell Linux to leave that
> clock alone, preferably via DT? Although the sunxi-ng clocks take
> control over the whole CCU unit, I wonder if it should really mess with
> clocks there are not referenced in the DT.

As it owns the whole CCU unit, why not? And how would it know if some
clock is referenced or not, unless going through the whole device tree?

Furthermore, nothing prevents another device driver from referencing
said clock and turning it off when it's not in use. Think THS driver
with runtime PM.

Are you also mapping the THS to secure only? Otherwise nothing would
prevent Linux from also claiming it.

>
> Maybe there is some way to reference those clocks via some dummy driver
> or DT node to avoid this behaviour? Is there any prior art in this respect?

If you want a clock to not be disabled by anyone, adding CLK_IS_CRITICAL
to its flags is the proper option. This is done in the clk driver though.

If you just don't want the clk subsystem to disable unused clks at boot
time, you can add "clk_ignore_unused" to the kernel boot parameters.
I think this is more of a hack and debugging tool though.

About dummy drivers, simplefb comes to mind again. But simplefb disables
them when it gets kicked out by the drm driver. Maybe there are other
examples.

Regards
ChenYu

> I think this issue will affect more future users (thinking of EFI RTC,
> EFI graphics, etc.), so I wanted to start a discussion on this.
>
> Any input welcome.
>
> Cheers,
> Andre.
>
> P.S. For reference my use case: ARM Trusted Firmware (ATF) enables the
> temperature sensor (THS) and programs a shutdown value. It programs the
> respective interrupt as secure and registers an IRQ handler in ATF to
> shutdown the system or take other appropriate matters to avoid
> overheating. Additionally the sensor is exported via the generic SCPI
> interface, so the existing scpi-hwmon driver picks it up and reports it
> to whoever is interested in Linux land via the normal interfaces.
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] sunxi-ng clocks: leaving certain clocks alone?

[Andre Przywara]

Hi Chen-Yu,

thanks for the answer!

On 12/12/16 04:41, Chen-Yu Tsai wrote:
> On Mon, Dec 12, 2016 at 7:54 AM, André Przywara <andre.przywara@arm.com> wrote:
>> Hi,
>>
>> I was observing that the new sunxi-ng clock code apparently explicitly
>> turns off _all_ clocks that are not used or needed. I find this rather
>> unfortunate, as I wanted to use the THS temperature sensor from ARM
>> Trusted Firmware to implement emergency shutdown or DVFS throttling.
>> That works until the clock framework finds the clock (as enumerated in
>> ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS mod
>> clock register and bit 8 in the respective clock gate register.
>> Turning them manually back on via /dev/mem or removing the THS clock
>> from the sunxi-ng driver fixes this for me.
>>
>> This was not happening with the old Allwinner clocks, since the kernel
>> wouldn't even know about it if there was no driver and the clock wasn't
>> mentioned in the DT.
>>
>> Now with sunxi-ng even though the THS clock is not actually referenced
>> or used in the DT, the kernel turns it off. I would expect that upon
>> entering the kernel all unneeded clocks are turned off anyway, so there
>> is no need to mess with clocks that have no user, but are enumerated in
>> the ccu driver.
> 
> I can't say that's absolutely true (wink).
> 
>>
>> I wonder if this kills simplefb as well, for instance, since I believe
>> that U-Boot needs to turn on certain clocks and relies on them staying up.
> 
> The simplefb bindings takes clocks and regulators expressly for the
> purpose of keeping them enabled.

Right, I should have checked this before ...

>>
>> So my questions:
>> 1) Is this expected behaviour?
> 
> Yes.
> 
>> 2) If yes, should it really be?
>> 3) If yes, shouldn't there be way to explicitly tell Linux to leave that
>> clock alone, preferably via DT? Although the sunxi-ng clocks take
>> control over the whole CCU unit, I wonder if it should really mess with
>> clocks there are not referenced in the DT.
> 
> As it owns the whole CCU unit, why not? And how would it know if some
> clock is referenced or not, unless going through the whole device tree?

I was hoping that it just provides clocks to any users (drivers) and
wouldn't bother with tinkering with every clock unless explicitly being
asked for (by a driver being pointed to a specific clock through DT).
So it would need to iterate through anything - neither the whole DT nor
it's own list of clocks.

> Furthermore, nothing prevents another device driver from referencing
> said clock and turning it off when it's not in use. Think THS driver
> with runtime PM.

I am totally OK with that: Any potential Linux THS driver can take over,
if the DT references this device and the clock.
My point is that atm there is no such driver and so the clocks framework
shouldn't turn that clock off.

> Are you also mapping the THS to secure only? Otherwise nothing would
> prevent Linux from also claiming it.

Unfortunately the THS is always unsecure. And even if it could be
switched, after a recent IRC discussion I came to believe that those
secure peripherals features only works when the secure boot feature is
used, which requires to blow an efuse and thus is not easily doable on
most boards and also irreversible.
Also I am not sure whether this security feature actually extends to the
mod clocks and the bus reset and clock gates bits.

But I was relying on that Linux doesn't touch hardware that's not
referenced in the DT, so if firmware uses the THS, any Linux THS node
would need to go - or the other way around: if there is a Linux THS
node, firmware backs off.

>> Maybe there is some way to reference those clocks via some dummy driver
>> or DT node to avoid this behaviour? Is there any prior art in this respect?
> 
> If you want a clock to not be disabled by anyone, adding CLK_IS_CRITICAL
> to its flags is the proper option. This is done in the clk driver though.

Yes, I was thinking about that, but it indeed sounds like a hack to
follow this.

> If you just don't want the clk subsystem to disable unused clks at boot
> time, you can add "clk_ignore_unused" to the kernel boot parameters.
> I think this is more of a hack and debugging tool though.

Good point, but indeed looks like a debug feature.

> About dummy drivers, simplefb comes to mind again. But simplefb disables
> them when it gets kicked out by the drm driver.

Which I am fine with. If people are desperate about a THS driver, this
could take over, although I would expect a firmware driving the THS
would discourage this - for instance by removing a THS node.

> Maybe there are other examples.

OK, thanks for the pointer, I will look into this direction.

Cheers,
Andre.

>> I think this issue will affect more future users (thinking of EFI RTC,
>> EFI graphics, etc.), so I wanted to start a discussion on this.
>>
>> Any input welcome.
>>
>> Cheers,
>> Andre.
>>
>> P.S. For reference my use case: ARM Trusted Firmware (ATF) enables the
>> temperature sensor (THS) and programs a shutdown value. It programs the
>> respective interrupt as secure and registers an IRQ handler in ATF to
>> shutdown the system or take other appropriate matters to avoid
>> overheating. Additionally the sensor is exported via the generic SCPI
>> interface, so the existing scpi-hwmon driver picks it up and reports it
>> to whoever is interested in Linux land via the normal interfaces.
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] sunxi-ng clocks: leaving certain clocks alone?

[Andre Przywara]

Hi Chen-Yu,

thanks for the answer!

On 12/12/16 04:41, Chen-Yu Tsai wrote:
> On Mon, Dec 12, 2016 at 7:54 AM, Andr? Przywara <andre.przywara@arm.com> wrote:
>> Hi,
>>
>> I was observing that the new sunxi-ng clock code apparently explicitly
>> turns off _all_ clocks that are not used or needed. I find this rather
>> unfortunate, as I wanted to use the THS temperature sensor from ARM
>> Trusted Firmware to implement emergency shutdown or DVFS throttling.
>> That works until the clock framework finds the clock (as enumerated in
>> ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS mod
>> clock register and bit 8 in the respective clock gate register.
>> Turning them manually back on via /dev/mem or removing the THS clock
>> from the sunxi-ng driver fixes this for me.
>>
>> This was not happening with the old Allwinner clocks, since the kernel
>> wouldn't even know about it if there was no driver and the clock wasn't
>> mentioned in the DT.
>>
>> Now with sunxi-ng even though the THS clock is not actually referenced
>> or used in the DT, the kernel turns it off. I would expect that upon
>> entering the kernel all unneeded clocks are turned off anyway, so there
>> is no need to mess with clocks that have no user, but are enumerated in
>> the ccu driver.
> 
> I can't say that's absolutely true (wink).
> 
>>
>> I wonder if this kills simplefb as well, for instance, since I believe
>> that U-Boot needs to turn on certain clocks and relies on them staying up.
> 
> The simplefb bindings takes clocks and regulators expressly for the
> purpose of keeping them enabled.

Right, I should have checked this before ...

>>
>> So my questions:
>> 1) Is this expected behaviour?
> 
> Yes.
> 
>> 2) If yes, should it really be?
>> 3) If yes, shouldn't there be way to explicitly tell Linux to leave that
>> clock alone, preferably via DT? Although the sunxi-ng clocks take
>> control over the whole CCU unit, I wonder if it should really mess with
>> clocks there are not referenced in the DT.
> 
> As it owns the whole CCU unit, why not? And how would it know if some
> clock is referenced or not, unless going through the whole device tree?

I was hoping that it just provides clocks to any users (drivers) and
wouldn't bother with tinkering with every clock unless explicitly being
asked for (by a driver being pointed to a specific clock through DT).
So it would need to iterate through anything - neither the whole DT nor
it's own list of clocks.

> Furthermore, nothing prevents another device driver from referencing
> said clock and turning it off when it's not in use. Think THS driver
> with runtime PM.

I am totally OK with that: Any potential Linux THS driver can take over,
if the DT references this device and the clock.
My point is that atm there is no such driver and so the clocks framework
shouldn't turn that clock off.

> Are you also mapping the THS to secure only? Otherwise nothing would
> prevent Linux from also claiming it.

Unfortunately the THS is always unsecure. And even if it could be
switched, after a recent IRC discussion I came to believe that those
secure peripherals features only works when the secure boot feature is
used, which requires to blow an efuse and thus is not easily doable on
most boards and also irreversible.
Also I am not sure whether this security feature actually extends to the
mod clocks and the bus reset and clock gates bits.

But I was relying on that Linux doesn't touch hardware that's not
referenced in the DT, so if firmware uses the THS, any Linux THS node
would need to go - or the other way around: if there is a Linux THS
node, firmware backs off.

>> Maybe there is some way to reference those clocks via some dummy driver
>> or DT node to avoid this behaviour? Is there any prior art in this respect?
> 
> If you want a clock to not be disabled by anyone, adding CLK_IS_CRITICAL
> to its flags is the proper option. This is done in the clk driver though.

Yes, I was thinking about that, but it indeed sounds like a hack to
follow this.

> If you just don't want the clk subsystem to disable unused clks at boot
> time, you can add "clk_ignore_unused" to the kernel boot parameters.
> I think this is more of a hack and debugging tool though.

Good point, but indeed looks like a debug feature.

> About dummy drivers, simplefb comes to mind again. But simplefb disables
> them when it gets kicked out by the drm driver.

Which I am fine with. If people are desperate about a THS driver, this
could take over, although I would expect a firmware driving the THS
would discourage this - for instance by removing a THS node.

> Maybe there are other examples.

OK, thanks for the pointer, I will look into this direction.

Cheers,
Andre.

>> I think this issue will affect more future users (thinking of EFI RTC,
>> EFI graphics, etc.), so I wanted to start a discussion on this.
>>
>> Any input welcome.
>>
>> Cheers,
>> Andre.
>>
>> P.S. For reference my use case: ARM Trusted Firmware (ATF) enables the
>> temperature sensor (THS) and programs a shutdown value. It programs the
>> respective interrupt as secure and registers an IRQ handler in ATF to
>> shutdown the system or take other appropriate matters to avoid
>> overheating. Additionally the sensor is exported via the generic SCPI
>> interface, so the existing scpi-hwmon driver picks it up and reports it
>> to whoever is interested in Linux land via the normal interfaces.
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] sunxi-ng clocks: leaving certain clocks alone?

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 5224 bytes --]

Hi,

On Mon, Dec 12, 2016 at 12:16:07PM +0000, Andre Przywara wrote:
> Hi Chen-Yu,
> 
> thanks for the answer!
> 
> On 12/12/16 04:41, Chen-Yu Tsai wrote:
> > On Mon, Dec 12, 2016 at 7:54 AM, André Przywara <andre.przywara@arm.com> wrote:
> >> Hi,
> >>
> >> I was observing that the new sunxi-ng clock code apparently explicitly
> >> turns off _all_ clocks that are not used or needed. I find this rather
> >> unfortunate, as I wanted to use the THS temperature sensor from ARM
> >> Trusted Firmware to implement emergency shutdown or DVFS throttling.
> >> That works until the clock framework finds the clock (as enumerated in
> >> ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS mod
> >> clock register and bit 8 in the respective clock gate register.
> >> Turning them manually back on via /dev/mem or removing the THS clock
> >> from the sunxi-ng driver fixes this for me.
> >>
> >> This was not happening with the old Allwinner clocks, since the kernel
> >> wouldn't even know about it if there was no driver and the clock wasn't
> >> mentioned in the DT.
> >>
> >> Now with sunxi-ng even though the THS clock is not actually referenced
> >> or used in the DT, the kernel turns it off. I would expect that upon
> >> entering the kernel all unneeded clocks are turned off anyway, so there
> >> is no need to mess with clocks that have no user, but are enumerated in
> >> the ccu driver.
> > 
> > I can't say that's absolutely true (wink).
> > 
> >>
> >> I wonder if this kills simplefb as well, for instance, since I believe
> >> that U-Boot needs to turn on certain clocks and relies on them staying up.
> > 
> > The simplefb bindings takes clocks and regulators expressly for the
> > purpose of keeping them enabled.
> 
> Right, I should have checked this before ...
> 
> >>
> >> So my questions:
> >> 1) Is this expected behaviour?
> > 
> > Yes.
> > 
> >> 2) If yes, should it really be?
> >> 3) If yes, shouldn't there be way to explicitly tell Linux to leave that
> >> clock alone, preferably via DT? Although the sunxi-ng clocks take
> >> control over the whole CCU unit, I wonder if it should really mess with
> >> clocks there are not referenced in the DT.
> > 
> > As it owns the whole CCU unit, why not? And how would it know if some
> > clock is referenced or not, unless going through the whole device tree?
> 
> I was hoping that it just provides clocks to any users (drivers) and
> wouldn't bother with tinkering with every clock unless explicitly being
> asked for (by a driver being pointed to a specific clock through DT).
> So it would need to iterate through anything - neither the whole DT nor
> it's own list of clocks.
> 
> > Furthermore, nothing prevents another device driver from referencing
> > said clock and turning it off when it's not in use. Think THS driver
> > with runtime PM.
> 
> I am totally OK with that: Any potential Linux THS driver can take over,
> if the DT references this device and the clock.
> My point is that atm there is no such driver and so the clocks framework
> shouldn't turn that clock off.

You could turn that exact argument the other way though. If there's no
user in the system, why should we waste power and leave it enabled?

> > Are you also mapping the THS to secure only? Otherwise nothing would
> > prevent Linux from also claiming it.
> 
> Unfortunately the THS is always unsecure. And even if it could be
> switched, after a recent IRC discussion I came to believe that those
> secure peripherals features only works when the secure boot feature is
> used, which requires to blow an efuse and thus is not easily doable on
> most boards and also irreversible.
> Also I am not sure whether this security feature actually extends to the
> mod clocks and the bus reset and clock gates bits.
> 
> But I was relying on that Linux doesn't touch hardware that's not
> referenced in the DT, so if firmware uses the THS, any Linux THS node
> would need to go - or the other way around: if there is a Linux THS
> node, firmware backs off.

It's not just about node though, but also based on the kernel
configuration. If the kernel didn't have a THS driver compiled (or not
loaded), then if you want to implement such a behaviour, you should
also keep the THS driver in the firmware.

> >> Maybe there is some way to reference those clocks via some dummy driver
> >> or DT node to avoid this behaviour? Is there any prior art in this respect?
> > 
> > If you want a clock to not be disabled by anyone, adding CLK_IS_CRITICAL
> > to its flags is the proper option. This is done in the clk driver though.
> 
> Yes, I was thinking about that, but it indeed sounds like a hack to
> follow this.

You also have the option to add a clock-critical property.

Keep in mind that just preventing it from shutting down at boot gives
no warranty that the clock will remain enabled. Other clocks in the
same sub-tree might do a reparenting or a disable that would lead to
that clock being modified or disabled too as a side effect.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[linux-sunxi] sunxi-ng clocks: leaving certain clocks alone?

[Maxime Ripard]

Hi,

On Mon, Dec 12, 2016 at 12:16:07PM +0000, Andre Przywara wrote:
> Hi Chen-Yu,
> 
> thanks for the answer!
> 
> On 12/12/16 04:41, Chen-Yu Tsai wrote:
> > On Mon, Dec 12, 2016 at 7:54 AM, Andr? Przywara <andre.przywara@arm.com> wrote:
> >> Hi,
> >>
> >> I was observing that the new sunxi-ng clock code apparently explicitly
> >> turns off _all_ clocks that are not used or needed. I find this rather
> >> unfortunate, as I wanted to use the THS temperature sensor from ARM
> >> Trusted Firmware to implement emergency shutdown or DVFS throttling.
> >> That works until the clock framework finds the clock (as enumerated in
> >> ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS mod
> >> clock register and bit 8 in the respective clock gate register.
> >> Turning them manually back on via /dev/mem or removing the THS clock
> >> from the sunxi-ng driver fixes this for me.
> >>
> >> This was not happening with the old Allwinner clocks, since the kernel
> >> wouldn't even know about it if there was no driver and the clock wasn't
> >> mentioned in the DT.
> >>
> >> Now with sunxi-ng even though the THS clock is not actually referenced
> >> or used in the DT, the kernel turns it off. I would expect that upon
> >> entering the kernel all unneeded clocks are turned off anyway, so there
> >> is no need to mess with clocks that have no user, but are enumerated in
> >> the ccu driver.
> > 
> > I can't say that's absolutely true (wink).
> > 
> >>
> >> I wonder if this kills simplefb as well, for instance, since I believe
> >> that U-Boot needs to turn on certain clocks and relies on them staying up.
> > 
> > The simplefb bindings takes clocks and regulators expressly for the
> > purpose of keeping them enabled.
> 
> Right, I should have checked this before ...
> 
> >>
> >> So my questions:
> >> 1) Is this expected behaviour?
> > 
> > Yes.
> > 
> >> 2) If yes, should it really be?
> >> 3) If yes, shouldn't there be way to explicitly tell Linux to leave that
> >> clock alone, preferably via DT? Although the sunxi-ng clocks take
> >> control over the whole CCU unit, I wonder if it should really mess with
> >> clocks there are not referenced in the DT.
> > 
> > As it owns the whole CCU unit, why not? And how would it know if some
> > clock is referenced or not, unless going through the whole device tree?
> 
> I was hoping that it just provides clocks to any users (drivers) and
> wouldn't bother with tinkering with every clock unless explicitly being
> asked for (by a driver being pointed to a specific clock through DT).
> So it would need to iterate through anything - neither the whole DT nor
> it's own list of clocks.
> 
> > Furthermore, nothing prevents another device driver from referencing
> > said clock and turning it off when it's not in use. Think THS driver
> > with runtime PM.
> 
> I am totally OK with that: Any potential Linux THS driver can take over,
> if the DT references this device and the clock.
> My point is that atm there is no such driver and so the clocks framework
> shouldn't turn that clock off.

You could turn that exact argument the other way though. If there's no
user in the system, why should we waste power and leave it enabled?

> > Are you also mapping the THS to secure only? Otherwise nothing would
> > prevent Linux from also claiming it.
> 
> Unfortunately the THS is always unsecure. And even if it could be
> switched, after a recent IRC discussion I came to believe that those
> secure peripherals features only works when the secure boot feature is
> used, which requires to blow an efuse and thus is not easily doable on
> most boards and also irreversible.
> Also I am not sure whether this security feature actually extends to the
> mod clocks and the bus reset and clock gates bits.
> 
> But I was relying on that Linux doesn't touch hardware that's not
> referenced in the DT, so if firmware uses the THS, any Linux THS node
> would need to go - or the other way around: if there is a Linux THS
> node, firmware backs off.

It's not just about node though, but also based on the kernel
configuration. If the kernel didn't have a THS driver compiled (or not
loaded), then if you want to implement such a behaviour, you should
also keep the THS driver in the firmware.

> >> Maybe there is some way to reference those clocks via some dummy driver
> >> or DT node to avoid this behaviour? Is there any prior art in this respect?
> > 
> > If you want a clock to not be disabled by anyone, adding CLK_IS_CRITICAL
> > to its flags is the proper option. This is done in the clk driver though.
> 
> Yes, I was thinking about that, but it indeed sounds like a hack to
> follow this.

You also have the option to add a clock-critical property.

Keep in mind that just preventing it from shutting down at boot gives
no warranty that the clock will remain enabled. Other clocks in the
same sub-tree might do a reparenting or a disable that would lead to
that clock being modified or disabled too as a side effect.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161213/b593fc31/attachment.sig>

Re: [linux-sunxi] sunxi-ng clocks: leaving certain clocks alone?

[Chen-Yu Tsai]

On Tue, Dec 13, 2016 at 11:08 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Mon, Dec 12, 2016 at 12:16:07PM +0000, Andre Przywara wrote:
>> Hi Chen-Yu,
>>
>> thanks for the answer!
>>
>> On 12/12/16 04:41, Chen-Yu Tsai wrote:
>> > On Mon, Dec 12, 2016 at 7:54 AM, Andr=C3=A9 Przywara <andre.przywara@a=
rm.com> wrote:
>> >> Hi,
>> >>
>> >> I was observing that the new sunxi-ng clock code apparently explicitl=
y
>> >> turns off _all_ clocks that are not used or needed. I find this rathe=
r
>> >> unfortunate, as I wanted to use the THS temperature sensor from ARM
>> >> Trusted Firmware to implement emergency shutdown or DVFS throttling.
>> >> That works until the clock framework finds the clock (as enumerated i=
n
>> >> ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS m=
od
>> >> clock register and bit 8 in the respective clock gate register.
>> >> Turning them manually back on via /dev/mem or removing the THS clock
>> >> from the sunxi-ng driver fixes this for me.
>> >>
>> >> This was not happening with the old Allwinner clocks, since the kerne=
l
>> >> wouldn't even know about it if there was no driver and the clock wasn=
't
>> >> mentioned in the DT.
>> >>
>> >> Now with sunxi-ng even though the THS clock is not actually reference=
d
>> >> or used in the DT, the kernel turns it off. I would expect that upon
>> >> entering the kernel all unneeded clocks are turned off anyway, so the=
re
>> >> is no need to mess with clocks that have no user, but are enumerated =
in
>> >> the ccu driver.
>> >
>> > I can't say that's absolutely true (wink).
>> >
>> >>
>> >> I wonder if this kills simplefb as well, for instance, since I believ=
e
>> >> that U-Boot needs to turn on certain clocks and relies on them stayin=
g up.
>> >
>> > The simplefb bindings takes clocks and regulators expressly for the
>> > purpose of keeping them enabled.
>>
>> Right, I should have checked this before ...
>>
>> >>
>> >> So my questions:
>> >> 1) Is this expected behaviour?
>> >
>> > Yes.
>> >
>> >> 2) If yes, should it really be?
>> >> 3) If yes, shouldn't there be way to explicitly tell Linux to leave t=
hat
>> >> clock alone, preferably via DT? Although the sunxi-ng clocks take
>> >> control over the whole CCU unit, I wonder if it should really mess wi=
th
>> >> clocks there are not referenced in the DT.
>> >
>> > As it owns the whole CCU unit, why not? And how would it know if some
>> > clock is referenced or not, unless going through the whole device tree=
?
>>
>> I was hoping that it just provides clocks to any users (drivers) and
>> wouldn't bother with tinkering with every clock unless explicitly being
>> asked for (by a driver being pointed to a specific clock through DT).
>> So it would need to iterate through anything - neither the whole DT nor
>> it's own list of clocks.
>>
>> > Furthermore, nothing prevents another device driver from referencing
>> > said clock and turning it off when it's not in use. Think THS driver
>> > with runtime PM.
>>
>> I am totally OK with that: Any potential Linux THS driver can take over,
>> if the DT references this device and the clock.
>> My point is that atm there is no such driver and so the clocks framework
>> shouldn't turn that clock off.
>
> You could turn that exact argument the other way though. If there's no
> user in the system, why should we waste power and leave it enabled?
>
>> > Are you also mapping the THS to secure only? Otherwise nothing would
>> > prevent Linux from also claiming it.
>>
>> Unfortunately the THS is always unsecure. And even if it could be
>> switched, after a recent IRC discussion I came to believe that those
>> secure peripherals features only works when the secure boot feature is
>> used, which requires to blow an efuse and thus is not easily doable on
>> most boards and also irreversible.
>> Also I am not sure whether this security feature actually extends to the
>> mod clocks and the bus reset and clock gates bits.
>>
>> But I was relying on that Linux doesn't touch hardware that's not
>> referenced in the DT, so if firmware uses the THS, any Linux THS node
>> would need to go - or the other way around: if there is a Linux THS
>> node, firmware backs off.
>
> It's not just about node though, but also based on the kernel
> configuration. If the kernel didn't have a THS driver compiled (or not
> loaded), then if you want to implement such a behaviour, you should
> also keep the THS driver in the firmware.
>
>> >> Maybe there is some way to reference those clocks via some dummy driv=
er
>> >> or DT node to avoid this behaviour? Is there any prior art in this re=
spect?
>> >
>> > If you want a clock to not be disabled by anyone, adding CLK_IS_CRITIC=
AL
>> > to its flags is the proper option. This is done in the clk driver thou=
gh.
>>
>> Yes, I was thinking about that, but it indeed sounds like a hack to
>> follow this.
>
> You also have the option to add a clock-critical property.

This is not supported by the clk core though. Rather, the clk core just
provides the helper function of_clk_detect_critical() to set the flag.
We don't support it either. Furthermore, the function's comment says:

    Do not use this function. It exists only for legacy Device Tree
    bindings, such as the one-clock-per-node style that are outdated.
    Those bindings typically put all clock data into .dts and the Linux
    driver has no clock data, thus making it impossible to set this flag
    correctly from the driver. Only those drivers may call
    of_clk_detect_critical from their setup functions.

ChenYu

>
> Keep in mind that just preventing it from shutting down at boot gives
> no warranty that the clock will remain enabled. Other clocks in the
> same sub-tree might do a reparenting or a disable that would lead to
> that clock being modified or disabled too as a side effect.
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

[linux-sunxi] sunxi-ng clocks: leaving certain clocks alone?

[Chen-Yu Tsai]

On Tue, Dec 13, 2016 at 11:08 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Mon, Dec 12, 2016 at 12:16:07PM +0000, Andre Przywara wrote:
>> Hi Chen-Yu,
>>
>> thanks for the answer!
>>
>> On 12/12/16 04:41, Chen-Yu Tsai wrote:
>> > On Mon, Dec 12, 2016 at 7:54 AM, Andr? Przywara <andre.przywara@arm.com> wrote:
>> >> Hi,
>> >>
>> >> I was observing that the new sunxi-ng clock code apparently explicitly
>> >> turns off _all_ clocks that are not used or needed. I find this rather
>> >> unfortunate, as I wanted to use the THS temperature sensor from ARM
>> >> Trusted Firmware to implement emergency shutdown or DVFS throttling.
>> >> That works until the clock framework finds the clock (as enumerated in
>> >> ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS mod
>> >> clock register and bit 8 in the respective clock gate register.
>> >> Turning them manually back on via /dev/mem or removing the THS clock
>> >> from the sunxi-ng driver fixes this for me.
>> >>
>> >> This was not happening with the old Allwinner clocks, since the kernel
>> >> wouldn't even know about it if there was no driver and the clock wasn't
>> >> mentioned in the DT.
>> >>
>> >> Now with sunxi-ng even though the THS clock is not actually referenced
>> >> or used in the DT, the kernel turns it off. I would expect that upon
>> >> entering the kernel all unneeded clocks are turned off anyway, so there
>> >> is no need to mess with clocks that have no user, but are enumerated in
>> >> the ccu driver.
>> >
>> > I can't say that's absolutely true (wink).
>> >
>> >>
>> >> I wonder if this kills simplefb as well, for instance, since I believe
>> >> that U-Boot needs to turn on certain clocks and relies on them staying up.
>> >
>> > The simplefb bindings takes clocks and regulators expressly for the
>> > purpose of keeping them enabled.
>>
>> Right, I should have checked this before ...
>>
>> >>
>> >> So my questions:
>> >> 1) Is this expected behaviour?
>> >
>> > Yes.
>> >
>> >> 2) If yes, should it really be?
>> >> 3) If yes, shouldn't there be way to explicitly tell Linux to leave that
>> >> clock alone, preferably via DT? Although the sunxi-ng clocks take
>> >> control over the whole CCU unit, I wonder if it should really mess with
>> >> clocks there are not referenced in the DT.
>> >
>> > As it owns the whole CCU unit, why not? And how would it know if some
>> > clock is referenced or not, unless going through the whole device tree?
>>
>> I was hoping that it just provides clocks to any users (drivers) and
>> wouldn't bother with tinkering with every clock unless explicitly being
>> asked for (by a driver being pointed to a specific clock through DT).
>> So it would need to iterate through anything - neither the whole DT nor
>> it's own list of clocks.
>>
>> > Furthermore, nothing prevents another device driver from referencing
>> > said clock and turning it off when it's not in use. Think THS driver
>> > with runtime PM.
>>
>> I am totally OK with that: Any potential Linux THS driver can take over,
>> if the DT references this device and the clock.
>> My point is that atm there is no such driver and so the clocks framework
>> shouldn't turn that clock off.
>
> You could turn that exact argument the other way though. If there's no
> user in the system, why should we waste power and leave it enabled?
>
>> > Are you also mapping the THS to secure only? Otherwise nothing would
>> > prevent Linux from also claiming it.
>>
>> Unfortunately the THS is always unsecure. And even if it could be
>> switched, after a recent IRC discussion I came to believe that those
>> secure peripherals features only works when the secure boot feature is
>> used, which requires to blow an efuse and thus is not easily doable on
>> most boards and also irreversible.
>> Also I am not sure whether this security feature actually extends to the
>> mod clocks and the bus reset and clock gates bits.
>>
>> But I was relying on that Linux doesn't touch hardware that's not
>> referenced in the DT, so if firmware uses the THS, any Linux THS node
>> would need to go - or the other way around: if there is a Linux THS
>> node, firmware backs off.
>
> It's not just about node though, but also based on the kernel
> configuration. If the kernel didn't have a THS driver compiled (or not
> loaded), then if you want to implement such a behaviour, you should
> also keep the THS driver in the firmware.
>
>> >> Maybe there is some way to reference those clocks via some dummy driver
>> >> or DT node to avoid this behaviour? Is there any prior art in this respect?
>> >
>> > If you want a clock to not be disabled by anyone, adding CLK_IS_CRITICAL
>> > to its flags is the proper option. This is done in the clk driver though.
>>
>> Yes, I was thinking about that, but it indeed sounds like a hack to
>> follow this.
>
> You also have the option to add a clock-critical property.

This is not supported by the clk core though. Rather, the clk core just
provides the helper function of_clk_detect_critical() to set the flag.
We don't support it either. Furthermore, the function's comment says:

    Do not use this function. It exists only for legacy Device Tree
    bindings, such as the one-clock-per-node style that are outdated.
    Those bindings typically put all clock data into .dts and the Linux
    driver has no clock data, thus making it impossible to set this flag
    correctly from the driver. Only those drivers may call
    of_clk_detect_critical from their setup functions.

ChenYu

>
> Keep in mind that just preventing it from shutting down at boot gives
> no warranty that the clock will remain enabled. Other clocks in the
> same sub-tree might do a reparenting or a disable that would lead to
> that clock being modified or disabled too as a side effect.
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com