The PCIe controller on SDX55 has five entries in its iommu-map, MSM8998
has one and SDM845 has sixteen, so allow wider number of items to fix
dtbs_check warnings like:
qcom-sdx55-mtp.dtb: pcie@1c00000: iommu-map: [[0, 21, 512, 1], [256, 21, 513, 1],
[512, 21, 514, 1], [768, 21, 515, 1], [1024, 21, 516, 1]] is too long
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Acked-by: Conor Dooley <[email protected]>
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
Changes in v2:
1. Add Acs/Rb.
---
Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 8bfae8eb79a3..14d25e8a18e4 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -62,7 +62,8 @@ properties:
maxItems: 8
iommu-map:
- maxItems: 2
+ minItems: 1
+ maxItems: 16
# Common definitions for clocks, clock-names and reset.
# Platform constraints are described later.
--
2.34.1
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 <[email protected]>
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
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
+ - 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 Mon, 20 Nov 2023 at 09:09, Krzysztof Kozlowski
<[email protected]> 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 <[email protected]>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>
> ---
>
> 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
>
>
--
With best wishes
Dmitry
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
>
--
மணிவண்ணன் சதாசிவம்
> + minItems: 1
Hello Krzysztof,
the driver will accept 0 just fine, so I think this definition may be wrong.
I sent just generic "dt-bindings: PCI: qcom: delimit number of iommu-map entries" which doesn't care about the numbers (in similar fashion as other bindings having iommu-map).
Tell me what you think.
David
On Fri, Dec 29, 2023 at 04:36:31PM +0100, David wrote:
> > + minItems: 1
> Hello Krzysztof,
>
> the driver will accept 0 just fine, so I think this definition may be wrong.
>
It's not entirely wrong but the actual SID mapping differs between SoCs.
> I sent just generic "dt-bindings: PCI: qcom: delimit number of iommu-map entries" which doesn't care about the numbers (in similar fashion as other bindings having iommu-map).
>
No, we should not just ignore the MAX limit. If you add <N> number of entries
exceeding the max SID assigned to PCIe bus, it will fail.
- Mani
> Tell me what you think.
>
> David
>
--
மணிவண்ணன் சதாசிவம்
On 29/12/2023 18:17, Manivannan Sadhasivam wrote:
> On Fri, Dec 29, 2023 at 04:36:31PM +0100, David wrote:
>>> + minItems: 1
>> Hello Krzysztof,
>>
>> the driver will accept 0 just fine, so I think this definition may be wrong.
>>
> It's not entirely wrong but the actual SID mapping differs between SoCs.
Sure, I think I can live with this.
>
>> I sent just generic "dt-bindings: PCI: qcom: delimit number of iommu-map entries" which doesn't care about the numbers (in similar fashion as other bindings having iommu-map).
>>
> No, we should not just ignore the MAX limit. If you add <N> number of entries
> exceeding the max SID assigned to PCIe bus, it will fail.
>
> - Mani
Make sense, thanks for explanation.
Reviewed-by: David Heidelberg <[email protected]>
>> Tell me what you think.
>>
>> David
>>
--
David Heidelberg