| Message ID | cover.1571484439.git.saiprakash.ranjan@codeaurora.org |
|---|---|
| Headers | show |
| Series | Add LLCC support for SC7180 SoC | expand |
Quoting Sai Prakash Ranjan (2019-10-19 04:37:11) > From: Vivek Gautam <vivek.gautam@codeaurora.org> > > Add LLCC configuration data for SC7180 SoC which controls > LLCC behaviour. > > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
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 Regards, Bjorn > v2: > * Convert bindings to YAML and add compatible for SC7180 > * Address Stephen's comments on const > > Sai Prakash Ranjan (2): > dt-bindings: msm: Convert LLCC bindings to YAML > dt-bindings: msm: Add LLCC for SC7180 > > Vivek Gautam (1): > soc: qcom: llcc: Add configuration data for SC7180 > > .../devicetree/bindings/arm/msm/qcom,llcc.txt | 41 -------------- > .../bindings/arm/msm/qcom,llcc.yaml | 55 +++++++++++++++++++ > drivers/soc/qcom/llcc-qcom.c | 15 ++++- > 3 files changed, 69 insertions(+), 42 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt > create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml > > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation >
On Sun, Oct 20, 2019 at 10:32 PM Bjorn Andersson <bjorn.andersson@linaro.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. Rob
Hi Rob, On 2019-10-24 01:19, Rob Herring wrote: > On Sun, Oct 20, 2019 at 10:32 PM Bjorn Andersson > <bjorn.andersson@linaro.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. [1] - http://patchwork.ozlabs.org/patch/1179800/ -Sai
On Thu, Oct 24, 2019 at 6:00 AM Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.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@linaro.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. 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'. Rob [1] https://github.com/devicetree-org/devicetree-specification/blob/master/source/devicenodes.rst#multi-level-and-shared-cache-nodes-cpuscpul-cache
On 2019-10-25 04:03, Rob Herring wrote: > On Thu, Oct 24, 2019 at 6:00 AM Sai Prakash Ranjan > <saiprakash.ranjan@codeaurora.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@linaro.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. -Sai
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@codeaurora.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@linaro.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? Thanks, Sai
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@codeaurora.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@linaro.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?
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@codeaurora.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@linaro.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