[Freedreno] [PATCH v6 2/2] arm64: dts: sdm845: Add gpu and gmu device nodes

[Doug Anderson]

Hi,

On Tue, Dec 18, 2018 at 10:40 AM Stephen Boyd <sboyd at kernel.org> wrote:
>
> Quoting Doug Anderson (2018-12-17 16:34:31)
> > Hi,
> >
> > On Sun, Dec 16, 2018 at 11:06 PM Viresh Kumar <viresh.kumar at linaro.org> wrote:
> > >
> > > +Rob +Stephen,
> > >
> > > On 14-12-18, 09:04, Doug Anderson wrote:
> > > > Hi,
> > > >
> > > > On Thu, Dec 13, 2018 at 8:41 PM Viresh Kumar <viresh.kumar at linaro.org> wrote:
> > > > >
> > > > > On 12-12-18, 14:18, Jordan Crouse wrote:
> > > > > > +                     gpu_opp_table: opp-table {
> > > > > > +                             compatible = "operating-points-v2-qcom-level";
> > > > >
> > > > > I think you need to mention "operating-points-v2" as well here.
> > > >
> > > > Are you saying the above should be:
> > > >   compatible = "operating-points-v2-qcom-level", "operating-points-v2";
> > > >
> > > > If so I'm not sure I agree.
> > >
> > > Well I have my doubts as well on this. This is where the ordering was discussed
> > > earlier:
> > >
> > > https://lore.kernel.org/lkml/152328979897.180276.15963925877442657158@swboyd.mtv.corp.google.com/
> > >
> > > @Rob/Stephen: Should the opp-table node above also have "operating-points-v2"
> > > string in the compatible property ?
> > >
> > > > It's _not_ really compatible with the
> > > > "operating-points-v2" binding.  If you had a driver that had never
> > > > heard of "operating-points-v2-qcom-level" and had only heard of
> > > > "operating-points-v2" and it took a look at this node it would have no
> > > > idea what to do with it.
> > >
> > > Well it will parse everything apart from the qcom,level thing, so it can
> > > actually parse stuff here.
> > >
> > > > I'll also note that other instances posted to the list don't list both:
> > > >
> > > > https://patchwork.kernel.org/patch/10725801/ - RPM / RPMH PD bindings
> > > > https://patchwork.kernel.org/patch/10725793/ - RPMH PD device tree for sdm845
> > > >
> > > > The bindings patch also makes no mention of needing
> > > > "operating-points-v2".  I think the common case when a fallback is
> > > > required it is explicitly called out in the bindings:
> > > >
> > > > https://patchwork.kernel.org/patch/10725803/ - qcom-opp bindings
> > >
> > > Sure, maybe I am wrong but its better to get some clarity on it from Rob/Stephen.
> >
> > In case it helps:
> >
> > https://lkml.kernel.org/r/20181217210849.GA15490@bogus
> >
> > ...shows Rob giving a Reviewed-by with just
> >
> >   compatible = "operating-points-v2-qcom-level";
> >
> > ...and not:
> >
> >   compatible = "operating-points-v2-qcom-level", "operating-points-v2";
> >
>
> I don't see a problem if both compatible strings are there. Does that
> change anything? I suppose the platform bus populate code won't create a
> device for the subnode if it has 'operating-points-v2', so maybe the
> fallback compatible string should be there? And then the generic OPP
> code can parse out the frequency at least without having to know that
> it's a qcom specific OPP table.

OK, it's fine with me to have the fallback, but if we do we should be
consistent about it and make sure it's in all the bindings and device
tree files...

-Doug