diff mbox

[10/11] dt-bindings: misc: add bindings for throttler

Message ID 20180525203043.249193-11-mka@chromium.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Matthias Kaehlcke May 25, 2018, 8:30 p.m. UTC
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 .../devicetree/bindings/misc/throttler.txt    | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/throttler.txt

Comments

Rob Herring (Arm) May 31, 2018, 4:31 p.m. UTC | #1
On Fri, May 25, 2018 at 01:30:42PM -0700, Matthias Kaehlcke wrote:

Commit msg?

> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  .../devicetree/bindings/misc/throttler.txt    | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/throttler.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/throttler.txt b/Documentation/devicetree/bindings/misc/throttler.txt
> new file mode 100644
> index 000000000000..92f13e94451a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/throttler.txt
> @@ -0,0 +1,41 @@
> +Throttler driver
> +
> +The Throttler is used for non-thermal throttling of system components like
> +CPUs or devfreq devices.

This all looks very Linux specific and not a h/w device. Perhaps you can 
add hint properties to OPP tables as to what entries can be used for 
throttling, but otherwise this doesn't belong in DT.
Matthias Kaehlcke May 31, 2018, 6:34 p.m. UTC | #2
Hi Rob,

On Thu, May 31, 2018 at 11:31:59AM -0500, Rob Herring wrote:
> On Fri, May 25, 2018 at 01:30:42PM -0700, Matthias Kaehlcke wrote:
> 
> Commit msg?

Will add some more info in the next revision.

> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  .../devicetree/bindings/misc/throttler.txt    | 41 +++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/throttler.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/misc/throttler.txt b/Documentation/devicetree/bindings/misc/throttler.txt
> > new file mode 100644
> > index 000000000000..92f13e94451a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/throttler.txt
> > @@ -0,0 +1,41 @@
> > +Throttler driver
> > +
> > +The Throttler is used for non-thermal throttling of system components like
> > +CPUs or devfreq devices.
> 
> This all looks very Linux specific and not a h/w device. Perhaps you can 
> add hint properties to OPP tables as to what entries can be used for 
> throttling, but otherwise this doesn't belong in DT.

My idea is to allow multiple throttlers, which might operate on
different throttling devices or use different OPPs for the same
device. To support this a simple boolean hint that the OPP can be used
for throttling would not be sufficient.

What should work is a property with an array of phandles of the
throttlers that use a given OPP.

AFAIK it is currently not possible to enumerate the devfreq devices in
the system, so besides the info in the OPPs the throttler itself would
still need a phandle to the devfreq device(s) it uses.

I envision something like this:

gpu_opp_table: opp-table2 {
  compatible = "operating-points-v2";

  opp00 {
    opp-hz = /bits/ 64 <200000000>;
    opp-microvolt = <800000>;
  };
  opp01 {
    opp-hz = /bits/ 64 <297000000>;
    opp-microvolt = <800000>;
    opp-throttlers = <&cros_ec_throttler>;
  };
  ...
};

cros_ec_throttler: cros-ec-throttler {
  compatible = "google,cros-ec-throttler";

  devfreq-devices = <&gpu>;
};

Would this be acceptable?

Thanks

Matthias
Rob Herring (Arm) May 31, 2018, 8:04 p.m. UTC | #3
On Thu, May 31, 2018 at 1:34 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Hi Rob,
>
> On Thu, May 31, 2018 at 11:31:59AM -0500, Rob Herring wrote:
>> On Fri, May 25, 2018 at 01:30:42PM -0700, Matthias Kaehlcke wrote:
>>
>> Commit msg?
>
> Will add some more info in the next revision.
>
>> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>> > ---
>> >  .../devicetree/bindings/misc/throttler.txt    | 41 +++++++++++++++++++
>> >  1 file changed, 41 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/misc/throttler.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/misc/throttler.txt b/Documentation/devicetree/bindings/misc/throttler.txt
>> > new file mode 100644
>> > index 000000000000..92f13e94451a
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/misc/throttler.txt
>> > @@ -0,0 +1,41 @@
>> > +Throttler driver
>> > +
>> > +The Throttler is used for non-thermal throttling of system components like
>> > +CPUs or devfreq devices.
>>
>> This all looks very Linux specific and not a h/w device. Perhaps you can
>> add hint properties to OPP tables as to what entries can be used for
>> throttling, but otherwise this doesn't belong in DT.
>
> My idea is to allow multiple throttlers, which might operate on
> different throttling devices or use different OPPs for the same
> device. To support this a simple boolean hint that the OPP can be used
> for throttling would not be sufficient.
>
> What should work is a property with an array of phandles of the
> throttlers that use a given OPP.
>
> AFAIK it is currently not possible to enumerate the devfreq devices in
> the system, so besides the info in the OPPs the throttler itself would
> still need a phandle to the devfreq device(s) it uses.

Why don't you fix that OS problem instead of working around it in DT?

>
> I envision something like this:
>
> gpu_opp_table: opp-table2 {
>   compatible = "operating-points-v2";
>
>   opp00 {
>     opp-hz = /bits/ 64 <200000000>;
>     opp-microvolt = <800000>;
>   };
>   opp01 {
>     opp-hz = /bits/ 64 <297000000>;
>     opp-microvolt = <800000>;
>     opp-throttlers = <&cros_ec_throttler>;
>   };
>   ...
> };
>
> cros_ec_throttler: cros-ec-throttler {
>   compatible = "google,cros-ec-throttler";

Is this an actual h/w device?

>
>   devfreq-devices = <&gpu>;
> };
>
> Would this be acceptable?
>
> Thanks
>
> Matthias
Matthias Kaehlcke May 31, 2018, 9:10 p.m. UTC | #4
On Thu, May 31, 2018 at 03:04:18PM -0500, Rob Herring wrote:
> On Thu, May 31, 2018 at 1:34 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> > Hi Rob,
> >
> > On Thu, May 31, 2018 at 11:31:59AM -0500, Rob Herring wrote:
> >> On Fri, May 25, 2018 at 01:30:42PM -0700, Matthias Kaehlcke wrote:
> >>
> >> Commit msg?
> >
> > Will add some more info in the next revision.
> >
> >> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >> > ---
> >> >  .../devicetree/bindings/misc/throttler.txt    | 41 +++++++++++++++++++
> >> >  1 file changed, 41 insertions(+)
> >> >  create mode 100644 Documentation/devicetree/bindings/misc/throttler.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/misc/throttler.txt b/Documentation/devicetree/bindings/misc/throttler.txt
> >> > new file mode 100644
> >> > index 000000000000..92f13e94451a
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/misc/throttler.txt
> >> > @@ -0,0 +1,41 @@
> >> > +Throttler driver
> >> > +
> >> > +The Throttler is used for non-thermal throttling of system components like
> >> > +CPUs or devfreq devices.
> >>
> >> This all looks very Linux specific and not a h/w device. Perhaps you can
> >> add hint properties to OPP tables as to what entries can be used for
> >> throttling, but otherwise this doesn't belong in DT.
> >
> > My idea is to allow multiple throttlers, which might operate on
> > different throttling devices or use different OPPs for the same
> > device. To support this a simple boolean hint that the OPP can be used
> > for throttling would not be sufficient.
> >
> > What should work is a property with an array of phandles of the
> > throttlers that use a given OPP.
> >
> > AFAIK it is currently not possible to enumerate the devfreq devices in
> > the system, so besides the info in the OPPs the throttler itself would
> > still need a phandle to the devfreq device(s) it uses.
> 
> Why don't you fix that OS problem instead of working around it in DT?

I can try, though it's not exclusively in my hands, depends on what
the devfreq maintainers think about it.

> > I envision something like this:
> >
> > gpu_opp_table: opp-table2 {
> >   compatible = "operating-points-v2";
> >
> >   opp00 {
> >     opp-hz = /bits/ 64 <200000000>;
> >     opp-microvolt = <800000>;
> >   };
> >   opp01 {
> >     opp-hz = /bits/ 64 <297000000>;
> >     opp-microvolt = <800000>;
> >     opp-throttlers = <&cros_ec_throttler>;
> >   };
> >   ...
> > };
> >
> > cros_ec_throttler: cros-ec-throttler {
> >   compatible = "google,cros-ec-throttler";
> 
> Is this an actual h/w device?

The Chrome OS Embedded Controller is a MCU that communicates with
Linux over SPI or I2C. The low-level communication with the EC is
handled by drivers/mfd/cros_ec* and then there are multiple drivers
representing 'remote devices' on top of this (rtc-cros-ec.c,
extcon-usbc-cros-ec.c, pwm-cros-ec.c, ...). cros_ec_throttler is a
similar 'device' that responds to signals from the EC with frequency
adjustments.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/misc/throttler.txt b/Documentation/devicetree/bindings/misc/throttler.txt
new file mode 100644
index 000000000000..92f13e94451a
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/throttler.txt
@@ -0,0 +1,41 @@ 
+Throttler driver
+
+The Throttler is used for non-thermal throttling of system components like
+CPUs or devfreq devices.
+
+
+Optional properties:
+--------------------
+- cpufreq:			A sub-node which is container of only cpufreq
+  Type: sub-mode		nodes, used to describe the throttling settings
+				for a CPU (or CPU group sharing the same policy).
+
+- devfreq:			A sub-node which is container of only devfreq
+  Type: sub-mode		nodes, used to describe the throttling settings
+				for a devfreq device.
+
+cpufreq node:
+=============
+
+Required properties:
+--------------------
+- cpu:				The id of the CPU to be throttled.
+  Type: unsigned
+  Size: one cell
+
+- throttling-frequencies:	Frequencies used for throttling, corresponding
+  Type: unsigned		to throttling level 1, 2, ...
+  Size: array
+
+
+devfreq node:
+=============
+
+Required properties:
+--------------------
+- devfreq:			A phandle of the devfreq device to be throttled.
+  Type: phandle
+
+- throttling-frequencies:	Frequencies used for throttling, corresponding
+  Type: unsigned		to throttling level 1, 2, ...
+  Size: array