| Message ID | 20240325181628.9407-4-quic_okukatla@quicinc.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for QoS configuration | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 25/03/2024 19:16, Odelu Kukatla wrote: > Added clock property to enable clocks required for accessing > qos registers. > > Signed-off-by: Odelu Kukatla <quic_okukatla@quicinc.com> > --- > .../bindings/interconnect/qcom,sc7280-rpmh.yaml | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml > index b135597d9489..950ecdd5252e 100644 > --- a/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml > +++ b/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml > @@ -35,6 +35,10 @@ properties: > reg: > maxItems: 1 > > + clocks: > + minItems: 1 > + maxItems: 2 Why is this flexible? Nothing in commit msg explains that. I gave the same talk twice, gave there examples, yet it is not enough... https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132 > + > required: > - compatible > > @@ -57,6 +61,7 @@ unevaluatedProperties: false > > examples: > - | > + #include <dt-bindings/clock/qcom,gcc-sc7280.h> > interconnect { > compatible = "qcom,sc7280-clk-virt"; > #interconnect-cells = <2>; > @@ -69,3 +74,12 @@ examples: > #interconnect-cells = <2>; > qcom,bcm-voters = <&apps_bcm_voter>; If all devices have clocks, then you could add them here. It seems not all of them have clocks... Best regards, Krzysztof
On 3/26/2024 1:00 PM, Krzysztof Kozlowski wrote: > On 25/03/2024 19:16, Odelu Kukatla wrote: >> Added clock property to enable clocks required for accessing >> qos registers. >> >> Signed-off-by: Odelu Kukatla <quic_okukatla@quicinc.com> >> --- >> .../bindings/interconnect/qcom,sc7280-rpmh.yaml | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml >> index b135597d9489..950ecdd5252e 100644 >> --- a/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml >> +++ b/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml >> @@ -35,6 +35,10 @@ properties: >> reg: >> maxItems: 1 >> >> + clocks: >> + minItems: 1 >> + maxItems: 2 > > Why is this flexible? Nothing in commit msg explains that. I gave the > same talk twice, gave there examples, yet it is not enough... > Clocks property is optional, and can be either 1 or 2 or none. I think "minItems: 1" should be removed. If no clock property is mentioned in node that means no clock is required to be enabled for QoS configuration. I will add back specific number of clocks based on interconnect compatible similar to what i did in v3. > https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132 > >> + >> required: >> - compatible >> >> @@ -57,6 +61,7 @@ unevaluatedProperties: false >> >> examples: >> - | >> + #include <dt-bindings/clock/qcom,gcc-sc7280.h> >> interconnect { >> compatible = "qcom,sc7280-clk-virt"; >> #interconnect-cells = <2>; >> @@ -69,3 +74,12 @@ examples: >> #interconnect-cells = <2>; >> qcom,bcm-voters = <&apps_bcm_voter>; > > If all devices have clocks, then you could add them here. It seems not > all of them have clocks... > > Best regards, > Krzysztof > Thanks, Odelu
On 27/03/2024 12:35, Odelu Kukatla wrote: >>> maxItems: 1 >>> >>> + clocks: >>> + minItems: 1 >>> + maxItems: 2 >> >> Why is this flexible? Nothing in commit msg explains that. I gave the >> same talk twice, gave there examples, yet it is not enough... >> > > Clocks property is optional, and can be either 1 or 2 or none. > I think "minItems: 1" should be removed. If no clock property is mentioned in node that means no clock is required to be enabled for QoS configuration. > > I will add back specific number of clocks based on interconnect compatible similar to what i did in v3. Just be sure you read the example I gave you. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml index b135597d9489..950ecdd5252e 100644 --- a/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml +++ b/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml @@ -35,6 +35,10 @@ properties: reg: maxItems: 1 + clocks: + minItems: 1 + maxItems: 2 + required: - compatible @@ -57,6 +61,7 @@ unevaluatedProperties: false examples: - | + #include <dt-bindings/clock/qcom,gcc-sc7280.h> interconnect { compatible = "qcom,sc7280-clk-virt"; #interconnect-cells = <2>; @@ -69,3 +74,12 @@ examples: #interconnect-cells = <2>; qcom,bcm-voters = <&apps_bcm_voter>; }; + + interconnect@16e0000 { + reg = <0x016e0000 0x1c080>; + compatible = "qcom,sc7280-aggre1-noc"; + #interconnect-cells = <2>; + qcom,bcm-voters = <&apps_bcm_voter>; + clocks = <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>, + <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>; + };
Added clock property to enable clocks required for accessing qos registers. Signed-off-by: Odelu Kukatla <quic_okukatla@quicinc.com> --- .../bindings/interconnect/qcom,sc7280-rpmh.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)