Hey Tomasz, On Wed, Jul 19, 2023 at 12:33:47PM -0700, Tomasz Jeznach wrote: > From: Anup Patel <apatel@xxxxxxxxxxxxxxxx> > > We add DT bindings document for RISC-V IOMMU platform and PCI devices > defined by the RISC-V IOMMU specification. > > Signed-off-by: Anup Patel <apatel@xxxxxxxxxxxxxxxx> Your signoff is missing from here. Secondly, as get_maintainer.pl would have told you, dt-bindings patches need to be sent to the dt-binding maintainers and list. +CC maintainers & list. Thirdly, dt-binding patches should come before their users. > --- > .../bindings/iommu/riscv,iommu.yaml | 146 ++++++++++++++++++ > 1 file changed, 146 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iommu/riscv,iommu.yaml > > diff --git a/Documentation/devicetree/bindings/iommu/riscv,iommu.yaml b/Documentation/devicetree/bindings/iommu/riscv,iommu.yaml > new file mode 100644 > index 000000000000..8a9aedb61768 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/riscv,iommu.yaml > @@ -0,0 +1,146 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iommu/riscv,iommu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: RISC-V IOMMU Implementation > + > +maintainers: > + - Tomasz Jeznach <tjeznach@xxxxxxxxxxxx> What about Anup, who seems to have written this? Or your co-authors of the drivers? > + > +description: > + The RISC-V IOMMU specificaiton defines an IOMMU for RISC-V platforms > + which can be a regular platform device or a PCI device connected to > + the host root port. > + > + The RISC-V IOMMU provides two stage translation, device directory table, > + command queue and fault reporting as wired interrupt or MSIx event for > + both PCI and platform devices. > + > + Visit https://github.com/riscv-non-isa/riscv-iommu for more details. > + > +properties: > + compatible: > + oneOf: > + - description: RISC-V IOMMU as a platform device > + items: > + - enum: > + - vendor,chip-iommu These dummy compatibles are not valid, as was pointed out to Anup on the AIA series. Please go look at what was done there instead: https://lore.kernel.org/all/20230719113542.2293295-7-apatel@xxxxxxxxxxxxxxxx/ > + - const: riscv,iommu > + > + - description: RISC-V IOMMU as a PCI device connected to root port > + items: > + - enum: > + - vendor,chip-pci-iommu > + - const: riscv,pci-iommu I'm not really au fait with the arm smmu stuff, but do any of its versions support being connected to a root port? > + reg: > + maxItems: 1 > + description: > + For RISC-V IOMMU as a platform device, this represents the MMIO base > + address of registers. > + > + For RISC-V IOMMU as a PCI device, this represents the PCI-PCI bridge > + details as described in Documentation/devicetree/bindings/pci/pci.txt > + > + '#iommu-cells': > + const: 2 > + description: | |s are only needed where formatting needs to be preserved. > + Each IOMMU specifier represents the base device ID and number of > + device IDs. > + > + interrupts: > + minItems: 1 > + maxItems: 16 What are any of these interrupts? > + description: > + The presence of this property implies that given RISC-V IOMMU uses > + wired interrupts to notify the RISC-V HARTS (or CPUs). > + > + msi-parent: > + description: > + The presence of this property implies that given RISC-V IOMMU uses > + MSIx to notify the RISC-V HARTs (or CPUs). This property should be > + considered only when the interrupts property is absent. > + > + dma-coherent: RISC-V is dma-coherent by default, should this not be dma-noncoherent instead? > + description: > + Present if page table walks and DMA accessed made by the RISC-V IOMMU > + are cache coherent with the CPU. > + > + power-domains: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - '#iommu-cells' > + > +additionalProperties: false > + > +examples: > + - | > + /* Example 1 (IOMMU platform device with wired interrupts) */ > + immu1: iommu@1bccd000 { Why is this "immu"? typo or intentional? > + compatible = "vendor,chip-iommu", "riscv,iommu"; > + reg = <0x1bccd000 0x1000>; > + interrupt-parent = <&aplic_smode>; > + interrupts = <32 4>, <33 4>, <34 4>, <35 4>; > + #iommu-cells = <2>; > + }; > + > + /* Device with two IOMMU device IDs, 0 and 7 */ > + master1 { > + iommus = <&immu1 0 1>, <&immu1 7 1>; > + }; > + > + - | > + /* Example 2 (IOMMU platform device with MSIs) */ > + immu2: iommu@1bcdd000 { > + compatible = "vendor,chip-iommu", "riscv,iommu"; > + reg = <0x1bccd000 0x1000>; > + msi-parent = <&imsics_smode>; > + #iommu-cells = <2>; > + }; > + > + bus { > + #address-cells = <2>; > + #size-cells = <2>; > + > + /* Device with IOMMU device IDs ranging from 32 to 64 */ > + master1 { > + iommus = <&immu2 32 32>; > + }; > + > + pcie@40000000 { > + compatible = "pci-host-cam-generic"; > + device_type = "pci"; > + #address-cells = <3>; > + #size-cells = <2>; > + bus-range = <0x0 0x1>; > + > + /* CPU_PHYSICAL(2) SIZE(2) */ These sort of comments seem to just repeat what address-cells & size-cells has already said, no? Thanks, Conor.
Attachment:
signature.asc
Description: PGP signature
