| Message ID | 20201026203148.47416-5-cristian.marussi@arm.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add support for SCMIv3.0 Voltage Domain Protocol and SCMI-Regulator | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Mon, Oct 26, 2020 at 08:31:48PM +0000, Cristian Marussi wrote: > Add devicetree bindings to support regulators based on SCMI Voltage > Domain Protocol. > > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> > --- > v2 --> v3 > - avoid awkard examples based on _cpu/_gpu regulators > v1 --> v2 > - removed any reference to negative voltages > --- > .../devicetree/bindings/arm/arm,scmi.txt | 42 +++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt > index 55deb68230eb..0cef83a60f03 100644 > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt > @@ -62,6 +62,28 @@ Required properties: > - #power-domain-cells : Should be 1. Contains the device or the power > domain ID value used by SCMI commands. > > +Regulator bindings for the SCMI Regulator based on SCMI Message Protocol > +------------------------------------------------------------ > + > +An SCMI Regulator is permanently bound to a well defined SCMI Voltage Domain, > +and should be always positioned as a root regulator. > +It does not support any current operation. > + > +This binding uses the common regulator binding[6]. > + > +SCMI Regulators are grouped under a 'regulators' node which in turn is a child > +of the SCMI Voltage protocol node inside the desired SCMI instance node. > + > +Required properties: > + - reg : shall identify an existent SCMI Voltage Domain. > + > +Optional properties: > + - all of the other standard regulator bindings as in [6]: note that, since > + the SCMI Protocol itself aims in fact to hide away many of the operational > + capabilities usually exposed by the properties of a standard regulator, > + most of the usual regulator bindings could have just no effect in the > + context of this SCMI regulator. You can't have it both ways... You should list out which ones apply. I'm a bit worried that now we're changing CPUs (at least?) from clocks to 'performance domains' while at the same time here we're adding low level, virtual regulators. Are we going to end up wanting something more abstract here too? > + > Sensor bindings for the sensors based on SCMI Message Protocol > -------------------------------------------------------------- > SCMI provides an API to access the various sensors on the SoC. > @@ -105,6 +127,7 @@ Required sub-node properties: > [3] Documentation/devicetree/bindings/thermal/thermal*.yaml > [4] Documentation/devicetree/bindings/sram/sram.yaml > [5] Documentation/devicetree/bindings/reset/reset.txt > +[6] Documentation/devicetree/bindings/regulator/regulator.yaml > > Example: > > @@ -169,6 +192,25 @@ firmware { > reg = <0x16>; > #reset-cells = <1>; > }; > + > + scmi_voltage: protocol@17 { > + reg = <0x17>; > + > + regulators { > + regulator_devX: regulator_scmi_devX@0 { Node names should be generic: regulator@0 > + reg = <0x0>; > + regulator-max-microvolt = <3300000>; > + }; > + > + regulator_devY: regulator_scmi_devY@9 { > + reg = <0x9>; > + regulator-min-microvolt = <500000>; > + regulator-max-microvolt = <4200000>; > + }; > + > + ... > + }; > + }; > }; > }; > > -- > 2.17.1 >
On Fri, Oct 30, 2020 at 01:55:14PM -0500, Rob Herring wrote: > I'm a bit worried that now we're changing CPUs (at least?) from clocks > to 'performance domains' while at the same time here we're adding > low level, virtual regulators. Are we going to end up wanting something > more abstract here too? My understanding is that this is aimed at systems which have done the more abstract thing where regulators just aren't visible at all to the kernel but then find that they actually need to control some of the regulators explicitly for things like MMC so need a mechanism for the firmware to expose select regulators.
On Fri, Oct 30, 2020 at 07:06:07PM +0000, Mark Brown wrote: > On Fri, Oct 30, 2020 at 01:55:14PM -0500, Rob Herring wrote: > > > I'm a bit worried that now we're changing CPUs (at least?) from clocks > > to 'performance domains' while at the same time here we're adding > > low level, virtual regulators. Are we going to end up wanting something > > more abstract here too? Valid concern and I too am with the same opinion. However as Mark Brown points out this was added to just satisfy some exiting consumers that rely on regulators. I had or still argue that we need a way to check if this is not getting misused with devices like CPUs. I was thinking of some check with DT where such properties are not allowed in certain device type. > > My understanding is that this is aimed at systems which have done the > more abstract thing where regulators just aren't visible at all to the > kernel but then find that they actually need to control some of the > regulators explicitly for things like MMC so need a mechanism for the > firmware to expose select regulators. Thanks Mark for the explaining this, saved me time 😄.
Hi Rob thanks for the review. On Fri, Oct 30, 2020 at 01:55:14PM -0500, Rob Herring wrote: > On Mon, Oct 26, 2020 at 08:31:48PM +0000, Cristian Marussi wrote: > > Add devicetree bindings to support regulators based on SCMI Voltage > > Domain Protocol. > > > > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> > > --- > > v2 --> v3 > > - avoid awkard examples based on _cpu/_gpu regulators > > v1 --> v2 > > - removed any reference to negative voltages > > --- > > .../devicetree/bindings/arm/arm,scmi.txt | 42 +++++++++++++++++++ > > 1 file changed, 42 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt > > index 55deb68230eb..0cef83a60f03 100644 > > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt > > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt > > @@ -62,6 +62,28 @@ Required properties: > > - #power-domain-cells : Should be 1. Contains the device or the power > > domain ID value used by SCMI commands. > > > > +Regulator bindings for the SCMI Regulator based on SCMI Message Protocol > > +------------------------------------------------------------ > > + > > +An SCMI Regulator is permanently bound to a well defined SCMI Voltage Domain, > > +and should be always positioned as a root regulator. > > +It does not support any current operation. > > + > > +This binding uses the common regulator binding[6]. > > + > > +SCMI Regulators are grouped under a 'regulators' node which in turn is a child > > +of the SCMI Voltage protocol node inside the desired SCMI instance node. > > + > > +Required properties: > > + - reg : shall identify an existent SCMI Voltage Domain. > > + > > +Optional properties: > > + - all of the other standard regulator bindings as in [6]: note that, since > > + the SCMI Protocol itself aims in fact to hide away many of the operational > > + capabilities usually exposed by the properties of a standard regulator, > > + most of the usual regulator bindings could have just no effect in the > > + context of this SCMI regulator. > > You can't have it both ways... You should list out which ones apply. > I'm double checking now every regulator binding that can apply, and I'll append here the full list of supported standard regulator bindings. > I'm a bit worried that now we're changing CPUs (at least?) from clocks > to 'performance domains' while at the same time here we're adding > low level, virtual regulators. Are we going to end up wanting something > more abstract here too? > As said already in a different thread by Mark and Sudeep, this was needed to accomodate some driver like MMC which expects to be able to fine tune some regulator voltages at some point. The SCMI abstraction exposed by the SCMI Power protocol indeed already provided a coarse grain tuning in term of abstract power levels and on/off ops but it was not able to provide fine grained voltage tuning in a clean way (without horribly abusing IMP DEFINED values), so Voltage Domain protocol with a regulator on top of it was added. (and I think anyway partners were already starting thinking about developing their own custom protocols and regulators to bypass these limitations...so providing a unified solution seemed sensible) Anyway consider that everything remains still in control of the SCMI fw which has the last word and can anyway drop your 'fine' tuned request if deemed unreasonable/invalid. > > + > > Sensor bindings for the sensors based on SCMI Message Protocol > > -------------------------------------------------------------- > > SCMI provides an API to access the various sensors on the SoC. > > @@ -105,6 +127,7 @@ Required sub-node properties: > > [3] Documentation/devicetree/bindings/thermal/thermal*.yaml > > [4] Documentation/devicetree/bindings/sram/sram.yaml > > [5] Documentation/devicetree/bindings/reset/reset.txt > > +[6] Documentation/devicetree/bindings/regulator/regulator.yaml > > > > Example: > > > > @@ -169,6 +192,25 @@ firmware { > > reg = <0x16>; > > #reset-cells = <1>; > > }; > > + > > + scmi_voltage: protocol@17 { > > + reg = <0x17>; > > + > > + regulators { > > + regulator_devX: regulator_scmi_devX@0 { > > Node names should be generic: > > regulator@0 > Yes, I was not sure if it this kind of naming for reg-based property was preferable of mandatory and so, since supporting using a <common-nam>@<unit> format in this driver needs a small patch in the regulator core framework (at least it seems so to me...), I wanted to be sure this was needed. I'll repost soon with naming fixed and with any additional patch added. Thanks Cristian > > + reg = <0x0>; > > + regulator-max-microvolt = <3300000>; > > + }; > > + > > + regulator_devY: regulator_scmi_devY@9 { > > + reg = <0x9>; > > + regulator-min-microvolt = <500000>; > > + regulator-max-microvolt = <4200000>; > > + }; > > + > > + ... > > + }; > > + }; > > }; > > }; > > > > -- > > 2.17.1 > >
diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt index 55deb68230eb..0cef83a60f03 100644 --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt @@ -62,6 +62,28 @@ Required properties: - #power-domain-cells : Should be 1. Contains the device or the power domain ID value used by SCMI commands. +Regulator bindings for the SCMI Regulator based on SCMI Message Protocol +------------------------------------------------------------ + +An SCMI Regulator is permanently bound to a well defined SCMI Voltage Domain, +and should be always positioned as a root regulator. +It does not support any current operation. + +This binding uses the common regulator binding[6]. + +SCMI Regulators are grouped under a 'regulators' node which in turn is a child +of the SCMI Voltage protocol node inside the desired SCMI instance node. + +Required properties: + - reg : shall identify an existent SCMI Voltage Domain. + +Optional properties: + - all of the other standard regulator bindings as in [6]: note that, since + the SCMI Protocol itself aims in fact to hide away many of the operational + capabilities usually exposed by the properties of a standard regulator, + most of the usual regulator bindings could have just no effect in the + context of this SCMI regulator. + Sensor bindings for the sensors based on SCMI Message Protocol -------------------------------------------------------------- SCMI provides an API to access the various sensors on the SoC. @@ -105,6 +127,7 @@ Required sub-node properties: [3] Documentation/devicetree/bindings/thermal/thermal*.yaml [4] Documentation/devicetree/bindings/sram/sram.yaml [5] Documentation/devicetree/bindings/reset/reset.txt +[6] Documentation/devicetree/bindings/regulator/regulator.yaml Example: @@ -169,6 +192,25 @@ firmware { reg = <0x16>; #reset-cells = <1>; }; + + scmi_voltage: protocol@17 { + reg = <0x17>; + + regulators { + regulator_devX: regulator_scmi_devX@0 { + reg = <0x0>; + regulator-max-microvolt = <3300000>; + }; + + regulator_devY: regulator_scmi_devY@9 { + reg = <0x9>; + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <4200000>; + }; + + ... + }; + }; }; };
Add devicetree bindings to support regulators based on SCMI Voltage Domain Protocol. Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> --- v2 --> v3 - avoid awkard examples based on _cpu/_gpu regulators v1 --> v2 - removed any reference to negative voltages --- .../devicetree/bindings/arm/arm,scmi.txt | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+)