Re: [PATCH v2 1/2] dt-bindings: rockchip: Add DesignWare based PCIe controller【请注意,邮件由robherring2@xxxxxxxxx代发】

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rob,

在 2021/1/22 23:55, Rob Herring 写道:
On Wed, Jan 20, 2021 at 06:07:29PM +0100, Johan Jonker wrote:
Hi Simon,

Thanks you for version 2.
A few comments, have a look if it is useful or that you disagree.

This patch has no commit message. Add one in version 3.

Submit all patches in one batch with the same sort message ID to all
maintainers including Heiko.

Heiko Stuebner <heiko@xxxxxxxxx>

Example message ID:
20210120101554.241029-1-xxm@xxxxxxxxxxxxxx

/////

Included is a copy of the Rockchip pcie nodes in a sort of test.dts below.
Could you confirm that the properties in that dts are the one that we
can expect for Linux mainline and can base our YAML document on?

With rk3568-cru.h and rk3568-power.h manualy added we do some tests with
the following commands:

make ARCH=arm64 dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml

make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml

make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/schemas/pci/pci-bus.yaml

/////

Example notifications:

/arch/arm64/boot/dts/rockchip/test.dt.yaml: pcie@fe270000: reg: [[3,
3225419776, 0, 4194304], [0, 4263968768, 0, 65536]] is too long

/arch/arm64/boot/dts/rockchip/test.dt.yaml: pcie@fe270000: ranges:
'oneOf' conditional failed, one must be fixed:

Before you submit version 3 make sure that all warnings gone as much as
possible.

On 1/20/21 11:15 AM, Simon Xue wrote:
Signed-off-by: Simon Xue <xxm@xxxxxxxxxxxxxx>
---
  .../bindings/pci/rockchip-dw-pcie.yaml        | 140 ++++++++++++++++++
  1 file changed, 140 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
new file mode 100644
index 000000000000..9d3a57f5305e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -0,0 +1,140 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/rockchip-dw-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: DesignWare based PCIe RC controller on Rockchip SoCs
+
+maintainers:
+  - Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
+  - Simon Xue <xxm@xxxxxxxxxxxxxx>
      - Heiko Stuebner <heiko@xxxxxxxxx> ;)
+
+description: |+
+  RK3568 SoC PCIe host controller is based on the Synopsys DesignWare
+  PCIe IP and thus inherits all the common properties defined in
+  designware-pcie.txt.
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+
+# We need a select here so we don't match all nodes with 'snps,dw-pcie'
+select:
+  properties:
+    compatible:
+      contains:
+        const: rockchip,rk3568-pcie
+  required:
+    - compatible
+
+properties:
+  compatible:
+    item:
     items:

+      - const: rockchip,rk3568-pcie
+      - const: snps,dw-pcie
Add empty line

+  reg:    items:
       - description:
       - description:

Add some description for regs.

+    maxItems: 1
remove

This reg maxItems gives errors.

+
+  interrupt:
interrupts:
    items:

+      - description: system information
+      - description: power management control
+      - description: PCIe message
+      - description: legacy interrupt
+      - description: error report
+
+  interrupt-names:
+    items:
+      - const: sys
+      - const: pmc
+      - const: msg
MSI? If so, use 'msi'. The DWC core will handle setting it up now.

No, it is SoC designed interrupt for some purpose. Currently, we don't use interrupt, will remove

these.

+      - const: legacy
+      - const: err
+
+  clocks:
+    items:
+      - description: AHB clock for PCIe master
+      - description: AHB clock for PCIe slave
+      - description: AHB clock for PCIe dbi
+      - description: APB clock for PCIe
+      - description: Auxiliary clock for PCIe
+
+  clock-names:
+    items:
+      - const: aclk_mst
+      - const: aclk_slv
+      - const: aclk_dbi
+      - const: pclk
+      - const: aux
+
+  msi-map: true
+
+  power-domains:
+    maxItems: 1
/////
These properties come from designware-pcie.txt
Maybe add them here for now till there's a common yaml?

   num-ib-windows: number of inbound address translation windows
   num-ob-windows: number of outbound address translation windows
These can be and are now detected at runtime.

Will remove.

Simon Xue


Rob








[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux