| Message ID | 20231120070910.16697-2-krzysztof.kozlowski@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/2] dt-bindings: PCI: qcom: adjust iommu-map for different SoC | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 71 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Mon, 20 Nov 2023 at 09:09, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > PCI node in Qualcomm SC8180x DTS has 8 clocks, while one on SM8150 has 7 > clocks: > > sc8180x-primus.dtb: pci@1c00000: 'oneOf' conditional failed, one must be fixed: > ['pipe', 'aux', 'cfg', 'bus_master', 'bus_slave', 'slave_q2a', 'ref', 'tbu'] is too short > > sm8150-hdk.dtb: pci@1c00000: 'oneOf' conditional failed, one must be fixed: > ['pipe', 'aux', 'cfg', 'bus_master', 'bus_slave', 'slave_q2a', 'tbu'] is too short > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > Changes in v2: > 1. Add Acs/Rb. > 2. Correct error message for sm8150. > --- > .../devicetree/bindings/pci/qcom,pcie.yaml | 58 ++++++++++++++++++- > 1 file changed, 57 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > index 14d25e8a18e4..4c993ea97d7c 100644 > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml > @@ -479,6 +479,35 @@ allOf: > items: > - const: pci # PCIe core reset > > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,pcie-sc8180x > + then: > + oneOf: > + - properties: > + clocks: > + minItems: 8 > + maxItems: 8 > + clock-names: > + items: > + - const: pipe # PIPE clock > + - const: aux # Auxiliary clock > + - const: cfg # Configuration clock > + - const: bus_master # Master AXI clock > + - const: bus_slave # Slave AXI clock > + - const: slave_q2a # Slave Q2A clock > + - const: ref # REFERENCE clock > + - const: tbu # PCIe TBU clock > + properties: > + resets: > + maxItems: 1 > + reset-names: > + items: > + - const: pci # PCIe core reset > + > - if: > properties: > compatible: > @@ -527,8 +556,35 @@ allOf: > compatible: > contains: > enum: > - - qcom,pcie-sc8180x > - qcom,pcie-sm8150 > + then: > + oneOf: > + - properties: > + clocks: > + minItems: 7 > + maxItems: 7 > + clock-names: > + items: > + - const: pipe # PIPE clock > + - const: aux # Auxiliary clock > + - const: cfg # Configuration clock > + - const: bus_master # Master AXI clock > + - const: bus_slave # Slave AXI clock > + - const: slave_q2a # Slave Q2A clock Mani promised to check whether we should use the 'ref' clock for the PCIe hosts or not. I'd ask to delay this patch until we finish that investigation. > + - const: tbu # PCIe TBU clock > + properties: > + resets: > + maxItems: 1 > + reset-names: > + items: > + - const: pci # PCIe core reset > + > + > + - if: > + properties: > + compatible: > + contains: > + enum: > - qcom,pcie-sm8250 > then: > oneOf: > -- > 2.34.1 > >
On 20/11/2023 11:11, Dmitry Baryshkov wrote: >> + then: >> + oneOf: >> + - properties: >> + clocks: >> + minItems: 7 >> + maxItems: 7 >> + clock-names: >> + items: >> + - const: pipe # PIPE clock >> + - const: aux # Auxiliary clock >> + - const: cfg # Configuration clock >> + - const: bus_master # Master AXI clock >> + - const: bus_slave # Slave AXI clock >> + - const: slave_q2a # Slave Q2A clock > > Mani promised to check whether we should use the 'ref' clock for the > PCIe hosts or not. > I'd ask to delay this patch until we finish that investigation. Right. I thought that his Rb-tag solves it, but if not - let's wait. Best regards, Krzysztof
On Mon, Nov 20, 2023 at 11:48:25AM +0100, Krzysztof Kozlowski wrote: > On 20/11/2023 11:11, Dmitry Baryshkov wrote: > >> + then: > >> + oneOf: > >> + - properties: > >> + clocks: > >> + minItems: 7 > >> + maxItems: 7 > >> + clock-names: > >> + items: > >> + - const: pipe # PIPE clock > >> + - const: aux # Auxiliary clock > >> + - const: cfg # Configuration clock > >> + - const: bus_master # Master AXI clock > >> + - const: bus_slave # Slave AXI clock > >> + - const: slave_q2a # Slave Q2A clock > > > > Mani promised to check whether we should use the 'ref' clock for the > > PCIe hosts or not. > > I'd ask to delay this patch until we finish that investigation. > > Right. I thought that his Rb-tag solves it, but if not - let's wait. > We discussed mostly offline, after I gave my R-b tag. I checked with Qcom on the use of "ref" clock in both PCIe and PHY nodes. It turned out that both nodes indeed need the "ref" clock, but not the GCC.*CLKREF that comes out of GCC. GCC.*CLKREF is only needed by the PHY node since PHY uses it for it's internal logic. For PCIe node, RPMH_CXO_CLK should be used as "ref" clock since it is used by the PCIe IP internally. This behavior applies to other peripherals like UFS, USB as well with same inconsistency in DT. So we need to fix this for those peripherals also. I can take up PCIe and UFS, and someone needs to fix USB. And for this patch, "ref" clock needs to be added to SM8150. Thanks Dmitry for pointing this out mess! - Mani > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml index 14d25e8a18e4..4c993ea97d7c 100644 --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml @@ -479,6 +479,35 @@ allOf: items: - const: pci # PCIe core reset + - if: + properties: + compatible: + contains: + enum: + - qcom,pcie-sc8180x + then: + oneOf: + - properties: + clocks: + minItems: 8 + maxItems: 8 + clock-names: + items: + - const: pipe # PIPE clock + - const: aux # Auxiliary clock + - const: cfg # Configuration clock + - const: bus_master # Master AXI clock + - const: bus_slave # Slave AXI clock + - const: slave_q2a # Slave Q2A clock + - const: ref # REFERENCE clock + - const: tbu # PCIe TBU clock + properties: + resets: + maxItems: 1 + reset-names: + items: + - const: pci # PCIe core reset + - if: properties: compatible: @@ -527,8 +556,35 @@ allOf: compatible: contains: enum: - - qcom,pcie-sc8180x - qcom,pcie-sm8150 + then: + oneOf: + - properties: + clocks: + minItems: 7 + maxItems: 7 + clock-names: + items: + - const: pipe # PIPE clock + - const: aux # Auxiliary clock + - const: cfg # Configuration clock + - const: bus_master # Master AXI clock + - const: bus_slave # Slave AXI clock + - const: slave_q2a # Slave Q2A clock + - const: tbu # PCIe TBU clock + properties: + resets: + maxItems: 1 + reset-names: + items: + - const: pci # PCIe core reset + + + - if: + properties: + compatible: + contains: + enum: - qcom,pcie-sm8250 then: oneOf:
PCI node in Qualcomm SC8180x DTS has 8 clocks, while one on SM8150 has 7 clocks: sc8180x-primus.dtb: pci@1c00000: 'oneOf' conditional failed, one must be fixed: ['pipe', 'aux', 'cfg', 'bus_master', 'bus_slave', 'slave_q2a', 'ref', 'tbu'] is too short sm8150-hdk.dtb: pci@1c00000: 'oneOf' conditional failed, one must be fixed: ['pipe', 'aux', 'cfg', 'bus_master', 'bus_slave', 'slave_q2a', 'tbu'] is too short Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Changes in v2: 1. Add Acs/Rb. 2. Correct error message for sm8150. --- .../devicetree/bindings/pci/qcom,pcie.yaml | 58 ++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-)