Date:   Fri, 15 Nov 2019 16:54:08 +0530
From:   Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Mark Rutland <mark.rutland@....com>,
        devicetree@...r.kernel.org, Andy Gross <agross@...nel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        linux-kernel@...r.kernel.org,
        Rajendra Nayak <rnayak@...eaurora.org>,
        Rishabh Bhatnagar <rishabhb@...eaurora.org>,
        Doug Anderson <dianders@...omium.org>,
        linux-arm-msm-owner@...r.kernel.org
Subject: Re: [PATCHv2 0/3] Add LLCC support for SC7180 SoC

On 2019-11-14 22:19, Stephen Boyd wrote:
> Quoting Sai Prakash Ranjan (2019-11-13 07:00:40)
>> Hello Rob,
>> 
>> On 2019-10-25 13:24, Sai Prakash Ranjan wrote:
>> > On 2019-10-25 04:03, Rob Herring wrote:
>> >> On Thu, Oct 24, 2019 at 6:00 AM Sai Prakash Ranjan
>> >> <saiprakash.ranjan@...eaurora.org> wrote:
>> >>>
>> >>> Hi Rob,
>> >>>
>> >>> On 2019-10-24 01:19, Rob Herring wrote:
>> >>> > On Sun, Oct 20, 2019 at 10:32 PM Bjorn Andersson
>> >>> > <bjorn.andersson@...aro.org> wrote:
>> >>> >>
>> >>> >> On Sat 19 Oct 04:37 PDT 2019, Sai Prakash Ranjan wrote:
>> >>> >>
>> >>> >> > LLCC behaviour is controlled by the configuration data set
>> >>> >> > in the llcc-qcom driver, add the same for SC7180 SoC.
>> >>> >> > Also convert the existing bindings to json-schema and add
>> >>> >> > the compatible for SC7180 SoC.
>> >>> >> >
>> >>> >>
>> >>> >> Thanks for the patches and thanks for the review Stephen. Series
>> >>> >> applied
>> >>> >
>> >>> > And they break dt_binding_check. Please fix.
>> >>> >
>> >>>
>> >>> I did check this and think that the error log from dt_binding_check
>> >>> is
>> >>> not valid because it says cache-level is a required property [1], but
>> >>> there is no such property in LLCC bindings.
>> >>
>> >> Then you should point out the issue and not just submit stuff ignoring
>> >> it. It has to be resolved one way or another.
>> >>
>> >
>> > I did not ignore it. When I ran the dt-binding check locally, it did
>> > not
>> > error out and just passed on [1] and it was my bad that I did not check
>> > the entire build logs to see if llcc dt binding check had some warning
>> > or
>> > not. But this is the usual case where most of us don't look at the
>> > entire
>> > build logs to check if there is a warning or not. We notice if there is
>> > an
>> > immediate exit/fail in case of some warning/error. So it would be good
>> > if
>> > we fail the dt-binding check build if there is some warning/error or
>> > atleast
>> > provide some option to strict build to fail on warning, maybe there is
>> > already
>> > a flag to do this?
>> >
>> > After submitting the patch, I noticed this build failure on
>> > patchwork.ozlabs.org and was waiting for your reply.
>> >
>> > [1] https://paste.ubuntu.com/p/jNK8yfVkMG/
>> >
>> >> If you refer to the DT spec[1], cache-level is required. The schema is
>> >> just enforcing that now. It's keying off the node name of
>> >> 'cache-controller'.
>> >>
>> >
>> > This is not L2 or L3 cache, this is a system cache (last level cache)
>> > shared by
>> > clients other than just CPU. So I don't know how do we specify
>> > cache-level for
>> > this, let me know if you have some pointers.
>> >
>> 
>> Any ideas on specifying the cache-level for system cache? Does
>> dt-binding-check
>> needs to be updated for this case?
>> 
> 
> I don't see how 'cache-level' fits here. Maybe the node name should be
> changed to 'system-cache-controller' and then the schema checker can
> skip it?

Sounds good and correct. I made this change and ran the dt binding check
and no warning was observed.

Sent a patch - 
https://lore.kernel.org/lkml/cover.1573814758.git.saiprakash.ranjan@codeaurora.org/

-Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Powered by blists - more mailing lists