mbox series

[0/3] Add missing A2NoC QoS clocks for SDM660 interconnect driver

Message ID 20210823095603.5538-1-shawn.guo@linaro.org
Headers show
Series Add missing A2NoC QoS clocks for SDM660 interconnect driver | expand

Message

Shawn Guo Aug. 23, 2021, 9:56 a.m. UTC 
The series updates SDM660 interconnect bindings, driver and device tree
to add A2NoC clocks required for QoS registers programming per downstream
kernel[1].  Otherwise, qcom_icc_noc_set_qos_priority() call on mas_ufs or
mas_usb_hs node will simply result in a hardware hang on SDM660 SoC.

[1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/sdm660-bus.dtsi?h=LA.UM.8.2.r1-04800-sdm660.0#n43

Shawn Guo (3):
  dt-bindings: interconnect: sdm660: Add missing a2noc qos clocks
  interconnect: qcom: sdm660: Add missing a2noc qos clocks
  arm64: dts: qcom: sdm630: Add missing a2noc qos clocks

 .../bindings/interconnect/qcom,sdm660.yaml    | 41 ++++++++++++++++++-
 arch/arm64/boot/dts/qcom/sdm630.dtsi          | 17 ++++++--
 drivers/interconnect/qcom/sdm660.c            | 14 +++++++
 3 files changed, 67 insertions(+), 5 deletions(-)

-- 
2.17.1

Comments

Rob Herring (Arm) Aug. 23, 2021, 1:30 p.m. UTC | #1 
On Mon, 23 Aug 2021 17:56:01 +0800, Shawn Guo wrote:
> It adds the missing a2noc clocks required for QoS registers programming
> per downstream kernel[1].
> 
> [1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/sdm660-bus.dtsi?h=LA.UM.8.2.r1-04800-sdm660.0#n43
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  .../bindings/interconnect/qcom,sdm660.yaml    | 41 ++++++++++++++++++-
>  1 file changed, 39 insertions(+), 2 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/interconnect/qcom,sdm660.example.dts:63.32-33 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/interconnect/qcom,sdm660.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1419: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1519617

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Shawn Guo Aug. 24, 2021, 1:17 a.m. UTC | #2 
On Mon, Aug 23, 2021 at 07:51:32PM +0300, Dmitry Baryshkov wrote:
> On Mon, 23 Aug 2021 at 12:56, Shawn Guo <shawn.guo@linaro.org> wrote:
> >
> > It adds the missing a2noc clocks required for QoS registers programming
> > per downstream kernel[1].
> >
> > [1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/sdm660-bus.dtsi?h=LA.UM.8.2.r1-04800-sdm660.0#n43
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sdm630.dtsi | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> > index 9153e6616ba4..b3a7f3bf1560 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> > @@ -652,11 +652,22 @@
> >
> >                 a2noc: interconnect@1704000 {
> >                         compatible = "qcom,sdm660-a2noc";
> > -                       reg = <0x01704000 0xc100>;
> > +                       reg = <0x01704000 0x1c000>;
> 
> Shawn, as you are at it, do we want to keep these nocs shifted
> compared to the downstream dtsi (so that the offset of QoS registers
> is 0) or we'd better introduce QoS register offset and move noc start
> address to the same address as in downstream?

Dmitry, thanks for spotting this!  This is really an unintended leftover
from debugging.  I will drop it in v2.  For address alignment with
downstream, I do not really have a strong opinion and I can live with
either of them :)

Shawn

> >                         #interconnect-cells = <1>;
> > -                       clock-names = "bus", "bus_a";
> > +                       clock-names = "bus",
> > +                                     "bus_a",
> > +                                     "ipa",
> > +                                     "ufs_axi",
> > +                                     "aggre2_ufs_axi",
> > +                                     "aggre2_usb3_axi",
> > +                                     "cfg_noc_usb2_axi";
> >                         clocks = <&rpmcc RPM_SMD_AGGR2_NOC_CLK>,
> > -                                <&rpmcc RPM_SMD_AGGR2_NOC_A_CLK>;
> > +                                <&rpmcc RPM_SMD_AGGR2_NOC_A_CLK>,
> > +                                <&rpmcc RPM_SMD_IPA_CLK>,
> > +                                <&gcc GCC_UFS_AXI_CLK>,
> > +                                <&gcc GCC_AGGRE2_UFS_AXI_CLK>,
> > +                                <&gcc GCC_AGGRE2_USB3_AXI_CLK>,
> > +                                <&gcc GCC_CFG_NOC_USB2_AXI_CLK>;
> >                 };
> >
> >                 mnoc: interconnect@1745000 {
> > --
> > 2.17.1
> >
> 
> 
> -- 
> With best wishes
> Dmitry