diff mbox

[1/2] PM / OPP: compatible is an optional property

Message ID be2c4fb0bb58f5b29facbc6f5aaf3dca13cf209e.1474450324.git.viresh.kumar@linaro.org
State Rejected, archived
Headers show

Commit Message

Viresh Kumar Sept. 21, 2016, 9:32 a.m. UTC
It was never compulsory to have a compatible string in the OPP table.
Fix the documentation to mark it optional.

Also update its description a bit.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/devicetree/bindings/opp/opp.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Stephen Boyd Sept. 22, 2016, 7:24 p.m. UTC | #1
On 09/21/2016 02:32 AM, Viresh Kumar wrote:
> It was never compulsory to have a compatible string in the OPP table.
> Fix the documentation to mark it optional.
>
> Also update its description a bit.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---

Why? I'd prefer the compatible string to be required so we know what
sort of node it is.
Viresh Kumar Sept. 23, 2016, 5:15 a.m. UTC | #2
On 22-09-16, 12:24, Stephen Boyd wrote:
> On 09/21/2016 02:32 AM, Viresh Kumar wrote:
> > It was never compulsory to have a compatible string in the OPP table.
> > Fix the documentation to mark it optional.
> >
> > Also update its description a bit.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> 
> Why? I'd prefer the compatible string to be required so we know what
> sort of node it is.

Okay, the code doesn't have any checks for it then and that needs to be fixed.

Just for my clarity, for platforms with special OPP bindings and so a different
compatible string like: "operating-points-v2-XYZ", should the compatible string
contain both "operating-points-v2" and the above one? It would be easier to
check for "operating-points-v2" in that case from core code.
Rob Herring (Arm) Sept. 23, 2016, 7:55 p.m. UTC | #3
On Fri, Sep 23, 2016 at 10:45:26AM +0530, Viresh Kumar wrote:
> On 22-09-16, 12:24, Stephen Boyd wrote:
> > On 09/21/2016 02:32 AM, Viresh Kumar wrote:
> > > It was never compulsory to have a compatible string in the OPP table.
> > > Fix the documentation to mark it optional.
> > >

NAK.

> > > Also update its description a bit.
> > >
> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > > ---
> > 
> > Why? I'd prefer the compatible string to be required so we know what
> > sort of node it is.

Agreed.

> Okay, the code doesn't have any checks for it then and that needs to be fixed.

Why? The kernel is not a DT validator.
 
> Just for my clarity, for platforms with special OPP bindings and so a different
> compatible string like: "operating-points-v2-XYZ", should the compatible string
> contain both "operating-points-v2" and the above one? It would be easier to
> check for "operating-points-v2" in that case from core code.

That would imply operating-points-v2-XYZ has extra properties or is 
different in some way. If an OS only understanding operating-points-v2 
will work, then yes it should have both. If not, then no.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar Sept. 26, 2016, 4:55 a.m. UTC | #4
On 23-09-16, 14:55, Rob Herring wrote:
> On Fri, Sep 23, 2016 at 10:45:26AM +0530, Viresh Kumar wrote:
> > On 22-09-16, 12:24, Stephen Boyd wrote:
> > > On 09/21/2016 02:32 AM, Viresh Kumar wrote:
> > > > It was never compulsory to have a compatible string in the OPP table.
> > > > Fix the documentation to mark it optional.
> > > >
> 
> NAK.
> 
> > > > Also update its description a bit.
> > > >
> > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > > > ---
> > > 
> > > Why? I'd prefer the compatible string to be required so we know what
> > > sort of node it is.
> 
> Agreed.
> 
> > Okay, the code doesn't have any checks for it then and that needs to be fixed.
> 
> Why? The kernel is not a DT validator.

Hmm.. I thought it should be checking if it can parse those bindings or not.
What if someone adds compatible property as "foo" for OPP node? Should the OPP
code even try to parse it?

> > Just for my clarity, for platforms with special OPP bindings and so a different
> > compatible string like: "operating-points-v2-XYZ", should the compatible string
> > contain both "operating-points-v2" and the above one? It would be easier to
> > check for "operating-points-v2" in that case from core code.
> 
> That would imply operating-points-v2-XYZ has extra properties or is 
> different in some way. If an OS only understanding operating-points-v2 
> will work, then yes it should have both. If not, then no.

Well, in this case that can't be done fully, so we should have only the -xyz
one. Got it, thanks.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index ee91cbdd95ee..5eab6f0215d1 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -55,14 +55,14 @@  This describes the OPPs belonging to a device. This node can have following
 properties:
 
 Required properties:
-- compatible: Allow OPPs to express their compatibility. It should be:
-  "operating-points-v2".
-
 - OPP nodes: One or more OPP nodes describing voltage-current-frequency
   combinations. Their name isn't significant but their phandle can be used to
   reference an OPP.
 
 Optional properties:
+- compatible: Allow OPPs to express their compatibility. It should be
+  "operating-points-v2" or a vendor specific string.
+
 - opp-shared: Indicates that device nodes using this OPP Table Node's phandle
   switch their DVFS state together, i.e. they share clock/voltage/current lines.
   Missing property means devices have independent clock/voltage/current lines,