* [PATCH 0/2] Apple M1 PCIe DT bindings @ 2021-05-16 21:18 Mark Kettenis 2021-05-16 21:18 ` [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie Mark Kettenis 2021-05-16 21:18 ` [PATCH 2/2] arm64: apple: Add PCIe node Mark Kettenis 0 siblings, 2 replies; 10+ messages in thread From: Mark Kettenis @ 2021-05-16 21:18 UTC (permalink / raw) To: devicetree Cc: maz, arnd, Mark Kettenis, Hector Martin, Bjorn Helgaas, Rob Herring, linux-arm-kernel, linux-pci, linux-kernel From: Mark Kettenis <kettenis@openbsd.org> This small series adds bindings for the PCIe controller found on the Apple M1 SoC. At this point, the primary consumer for these bindings is U-Boot. With these bindings U-Boot can bring up the links for the root ports of the PCIe root complex. A simple OS driver can then provide standard ECAM access and manage MSI interrupts to provide access to the built-in Ethernet and XHCI controllers of the Mac mini. The Apple controller incorporates Synopsys Designware PCIe logic to implement its root port. But unlike other hardware currently supported by U-Boot and the Linux kernel the Apple hardware integrates multiple root ports. As such the existing bindings for the DWC PCIe interface can't be used. There is a single ECAM space for all root space, but separate GPIOs to take the PCI devices on those ports out of reset. Therefore the standard "reset-gpio" and "max-link-speed" properties appear on the child nodes representing the PCI devices that correspond to the individual root ports. MSIs are handled by the PCIe controller and translated into "regular interrupts". A range of 32 MSIs is provided. These 32 MSIs can be distributed over the root ports as the OS sees fit by programming the PCIe controller port registers. These series depends on the pinctrl series I sent earlier (with a v2 respin today). Mark Kettenis (2): dt-bindings: pci: Add DT bindings for apple,pcie arm64: apple: Add PCIe node .../devicetree/bindings/pci/apple,pcie.yaml | 150 ++++++++++++++++++ MAINTAINERS | 1 + arch/arm64/boot/dts/apple/t8103.dtsi | 64 ++++++++ 3 files changed, 215 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml -- 2.31.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie 2021-05-16 21:18 [PATCH 0/2] Apple M1 PCIe DT bindings Mark Kettenis @ 2021-05-16 21:18 ` Mark Kettenis 2021-05-17 2:27 ` Rob Herring ` (2 more replies) 2021-05-16 21:18 ` [PATCH 2/2] arm64: apple: Add PCIe node Mark Kettenis 1 sibling, 3 replies; 10+ messages in thread From: Mark Kettenis @ 2021-05-16 21:18 UTC (permalink / raw) To: devicetree Cc: maz, arnd, Mark Kettenis, Hector Martin, Bjorn Helgaas, Rob Herring, linux-arm-kernel, linux-pci, linux-kernel From: Mark Kettenis <kettenis@openbsd.org> The Apple PCIe host controller is a PCIe host controller with multiple root ports present in Apple ARM SoC platforms, including various iPhone and iPad devices and the "Apple Silicon" Macs. Signed-off-by: Mark Kettenis <kettenis@openbsd.org> --- .../devicetree/bindings/pci/apple,pcie.yaml | 150 ++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 151 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml diff --git a/Documentation/devicetree/bindings/pci/apple,pcie.yaml b/Documentation/devicetree/bindings/pci/apple,pcie.yaml new file mode 100644 index 000000000000..af3c9f64e380 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml @@ -0,0 +1,150 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/apple,pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Apple PCIe host controller + +maintainers: + - Mark Kettenis <kettenis@openbsd.org> + +description: | + The Apple PCIe host controller is a PCIe host controller with + multiple root ports present in Apple ARM SoC platforms, including + various iPhone and iPad devices and the "Apple Silicon" Macs. + +allOf: + - $ref: /schemas/pci/pci-bus.yaml# + +properties: + compatible: + items: + - const: apple,t8103-pcie + - const: apple,pcie + + reg: + minItems: 4 + maxItems: 6 + + reg-names: + minItems: 4 + maxItems: 7 + items: + - const: ecam + - const: rc + - const: phy + - const: port0 + - const: port1 + - const: port2 + + ranges: + minItems: 2 + maxItems: 2 + + interrupts: + minItems: 3 + maxItems: 3 + + msi-ranges: + description: + A list of pairs <intid span>, where "intid" is the first + interrupt number that can be used as an MSI, and "span" the size + of that range. + $ref: /schemas/types.yaml#/definitions/uint32-matrix + items: + minItems: 2 + maxItems: 2 + +required: + - compatible + - reg + - reg-names + - bus-range + - interrupts + - msi-controller + - msi-parent + - msi-ranges + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/apple-aic.h> + #include <dt-bindings/pinctrl/apple.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + pcie0: pcie@690000000 { + compatible = "apple,t8103-pcie", "apple,pcie"; + device_type = "pci"; + + reg = <0x6 0x90000000 0x0 0x1000000>, + <0x6 0x80000000 0x0 0x4000>, + <0x6 0x8c000000 0x0 0x4000>, + <0x6 0x81000000 0x0 0x8000>, + <0x6 0x82000000 0x0 0x8000>, + <0x6 0x83000000 0x0 0x8000>; + reg-names = "ecam", "rc", "phy", "port0", "port1", "port2"; + + interrupt-parent = <&aic>; + interrupts = <AIC_IRQ 695 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 698 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 701 IRQ_TYPE_LEVEL_HIGH>; + + msi-controller; + msi-parent = <&pcie0>; + msi-ranges = <704 32>; + + iommu-map = <0x0 &dart0 0x8000 0x100>, + <0x100 &dart0 0x100 0x100>, + <0x200 &dart1 0x200 0x100>, + <0x300 &dart2 0x300 0x100>; + iommu-map-mask = <0xff00>; + + bus-range = <0 7>; + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x43000000 0x6 0xa0000000 0x6 0xa0000000 0x0 0x20000000>, + <0x02000000 0x0 0xc0000000 0x6 0xc0000000 0x0 0x40000000>; + + clocks = <&pcie_core_clk>, <&pcie_aux_clk>, <&pcie_ref_clk>; + pinctrl-0 = <&pcie_pins>; + pinctrl-names = "default"; + + pci@0,0 { + device_type = "pci"; + reg = <0x0 0x0 0x0 0x0 0x0>; + reset-gpios = <&pinctrl_ap 152 0>; + max-link-speed = <2>; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + }; + + pci@1,0 { + device_type = "pci"; + reg = <0x800 0x0 0x0 0x0 0x0>; + reset-gpios = <&pinctrl_ap 153 0>; + max-link-speed = <2>; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + }; + + pci@2,0 { + device_type = "pci"; + reg = <0x1000 0x0 0x0 0x0 0x0>; + reset-gpios = <&pinctrl_ap 33 0>; + max-link-speed = <1>; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + }; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 7327c9b778f1..789d79315485 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1654,6 +1654,7 @@ C: irc://chat.freenode.net/asahi-dev T: git https://github.com/AsahiLinux/linux.git F: Documentation/devicetree/bindings/arm/apple.yaml F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml +F: Documentation/devicetree/bindings/pci/apple,pcie.yaml F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml F: arch/arm64/boot/dts/apple/ F: drivers/irqchip/irq-apple-aic.c -- 2.31.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie 2021-05-16 21:18 ` [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie Mark Kettenis @ 2021-05-17 2:27 ` Rob Herring 2021-05-17 8:17 ` Mark Kettenis 2021-05-17 14:16 ` Rob Herring 2021-05-18 14:10 ` Robin Murphy 2 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2021-05-17 2:27 UTC (permalink / raw) To: Mark Kettenis Cc: arnd, devicetree, Rob Herring, maz, linux-arm-kernel, Hector Martin, linux-pci, Bjorn Helgaas, linux-kernel, Mark Kettenis On Sun, 16 May 2021 23:18:46 +0200, Mark Kettenis wrote: > From: Mark Kettenis <kettenis@openbsd.org> > > The Apple PCIe host controller is a PCIe host controller with > multiple root ports present in Apple ARM SoC platforms, including > various iPhone and iPad devices and the "Apple Silicon" Macs. > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > --- > .../devicetree/bindings/pci/apple,pcie.yaml | 150 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 151 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/pci/apple,pcie.example.dts:20:18: fatal error: dt-bindings/pinctrl/apple.h: No such file or directory 20 | #include <dt-bindings/pinctrl/apple.h> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/pci/apple,pcie.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1416: dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1479170 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie 2021-05-17 2:27 ` Rob Herring @ 2021-05-17 8:17 ` Mark Kettenis 0 siblings, 0 replies; 10+ messages in thread From: Mark Kettenis @ 2021-05-17 8:17 UTC (permalink / raw) To: Rob Herring Cc: arnd, devicetree, robh+dt, maz, linux-arm-kernel, marcan, linux-pci, bhelgaas, linux-kernel, kettenis > From: Rob Herring <robh@kernel.org> > Date: Sun, 16 May 2021 21:27:03 -0500 > > On Sun, 16 May 2021 23:18:46 +0200, Mark Kettenis wrote: > > From: Mark Kettenis <kettenis@openbsd.org> > > > > The Apple PCIe host controller is a PCIe host controller with > > multiple root ports present in Apple ARM SoC platforms, including > > various iPhone and iPad devices and the "Apple Silicon" Macs. > > > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > > --- > > .../devicetree/bindings/pci/apple,pcie.yaml | 150 ++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 151 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > Documentation/devicetree/bindings/pci/apple,pcie.example.dts:20:18: fatal error: dt-bindings/pinctrl/apple.h: No such file or directory > 20 | #include <dt-bindings/pinctrl/apple.h> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > compilation terminated. > make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/pci/apple,pcie.example.dt.yaml] Error 1 > make[1]: *** Waiting for unfinished jobs.... > make: *** [Makefile:1416: dt_binding_check] Error 2 While that header file is part of the dependency mentioned in the cover letter, it isn't even needed. Will fix this when I spin v2. Sorry for the noise. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie 2021-05-16 21:18 ` [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie Mark Kettenis 2021-05-17 2:27 ` Rob Herring @ 2021-05-17 14:16 ` Rob Herring 2021-05-17 19:41 ` Mark Kettenis 2021-05-18 14:10 ` Robin Murphy 2 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2021-05-17 14:16 UTC (permalink / raw) To: Mark Kettenis Cc: devicetree, Marc Zyngier, Arnd Bergmann, Mark Kettenis, Hector Martin, Bjorn Helgaas, linux-arm-kernel, PCI, linux-kernel On Sun, May 16, 2021 at 4:19 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > From: Mark Kettenis <kettenis@openbsd.org> > > The Apple PCIe host controller is a PCIe host controller with > multiple root ports present in Apple ARM SoC platforms, including > various iPhone and iPad devices and the "Apple Silicon" Macs. All the cover letter will be lost in the git history. Please mention some details like this is a DWC controller here. I disagree that you can't use the DWC binding. You can use it and extend it with what's needed here. And that way, we could move from generic ECAM to an actual driver in the OS if needed (hopefully not). More below. > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > --- > .../devicetree/bindings/pci/apple,pcie.yaml | 150 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 151 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml > > diff --git a/Documentation/devicetree/bindings/pci/apple,pcie.yaml b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > new file mode 100644 > index 000000000000..af3c9f64e380 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > @@ -0,0 +1,150 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pci/apple,pcie.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Apple PCIe host controller > + > +maintainers: > + - Mark Kettenis <kettenis@openbsd.org> > + > +description: | > + The Apple PCIe host controller is a PCIe host controller with > + multiple root ports present in Apple ARM SoC platforms, including > + various iPhone and iPad devices and the "Apple Silicon" Macs. > + > +allOf: > + - $ref: /schemas/pci/pci-bus.yaml# > + > +properties: > + compatible: > + items: > + - const: apple,t8103-pcie > + - const: apple,pcie > + > + reg: > + minItems: 4 > + maxItems: 6 6 or... > + reg-names: > + minItems: 4 > + maxItems: 7 7? > + items: > + - const: ecam 'config' The difference between ECAM or not in existing devices is really just the size. If you look at the addresses on other DWC bindings, the config region is just an iATU window within the host's PCI address range. > + - const: rc This would be 'dbi'? Also check if we need 'atu' (only if it's not at the default offset)? > + - const: phy Should there be a separate phy node? > + - const: port0 > + - const: port1 > + - const: port2 What's in these registers? > + > + ranges: > + minItems: 2 > + maxItems: 2 > + > + interrupts: > + minItems: 3 > + maxItems: 3 Need to define what each one is. > + > + msi-ranges: > + description: > + A list of pairs <intid span>, where "intid" is the first > + interrupt number that can be used as an MSI, and "span" the size > + of that range. Hopefully, Marc Z will comment on the MSI bits. msi-map doesn't work here? If we need something else, then it should be added to pci-msi.txt. One problem with this is it assumes 'intid' is one cell. It's really 2 for the AIC if we ignore flags (which would be another assumption that we can ignore the last cell). Or maybe this belongs in the AIC binding? > + $ref: /schemas/types.yaml#/definitions/uint32-matrix > + items: > + minItems: 2 > + maxItems: 2 > + > +required: > + - compatible > + - reg > + - reg-names > + - bus-range > + - interrupts > + - msi-controller > + - msi-parent > + - msi-ranges > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/apple-aic.h> > + #include <dt-bindings/pinctrl/apple.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + pcie0: pcie@690000000 { > + compatible = "apple,t8103-pcie", "apple,pcie"; > + device_type = "pci"; > + > + reg = <0x6 0x90000000 0x0 0x1000000>, > + <0x6 0x80000000 0x0 0x4000>, > + <0x6 0x8c000000 0x0 0x4000>, > + <0x6 0x81000000 0x0 0x8000>, > + <0x6 0x82000000 0x0 0x8000>, > + <0x6 0x83000000 0x0 0x8000>; > + reg-names = "ecam", "rc", "phy", "port0", "port1", "port2"; > + > + interrupt-parent = <&aic>; > + interrupts = <AIC_IRQ 695 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 698 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 701 IRQ_TYPE_LEVEL_HIGH>; > + > + msi-controller; > + msi-parent = <&pcie0>; > + msi-ranges = <704 32>; > + > + iommu-map = <0x0 &dart0 0x8000 0x100>, > + <0x100 &dart0 0x100 0x100>, > + <0x200 &dart1 0x200 0x100>, > + <0x300 &dart2 0x300 0x100>; > + iommu-map-mask = <0xff00>; These need to be documented. You can assume they have a type already, so just 'true' or any constraints. > + > + bus-range = <0 7>; > + #address-cells = <3>; > + #size-cells = <2>; > + ranges = <0x43000000 0x6 0xa0000000 0x6 0xa0000000 0x0 0x20000000>, > + <0x02000000 0x0 0xc0000000 0x6 0xc0000000 0x0 0x40000000>; > + > + clocks = <&pcie_core_clk>, <&pcie_aux_clk>, <&pcie_ref_clk>; > + pinctrl-0 = <&pcie_pins>; > + pinctrl-names = "default"; > + > + pci@0,0 { > + device_type = "pci"; > + reg = <0x0 0x0 0x0 0x0 0x0>; > + reset-gpios = <&pinctrl_ap 152 0>; > + max-link-speed = <2>; > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + }; > + > + pci@1,0 { > + device_type = "pci"; > + reg = <0x800 0x0 0x0 0x0 0x0>; > + reset-gpios = <&pinctrl_ap 153 0>; > + max-link-speed = <2>; > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + }; > + > + pci@2,0 { > + device_type = "pci"; > + reg = <0x1000 0x0 0x0 0x0 0x0>; > + reset-gpios = <&pinctrl_ap 33 0>; > + max-link-speed = <1>; > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + }; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index 7327c9b778f1..789d79315485 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1654,6 +1654,7 @@ C: irc://chat.freenode.net/asahi-dev > T: git https://github.com/AsahiLinux/linux.git > F: Documentation/devicetree/bindings/arm/apple.yaml > F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml > +F: Documentation/devicetree/bindings/pci/apple,pcie.yaml > F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml > F: arch/arm64/boot/dts/apple/ > F: drivers/irqchip/irq-apple-aic.c > -- > 2.31.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie 2021-05-17 14:16 ` Rob Herring @ 2021-05-17 19:41 ` Mark Kettenis 2021-05-17 20:21 ` Rob Herring 0 siblings, 1 reply; 10+ messages in thread From: Mark Kettenis @ 2021-05-17 19:41 UTC (permalink / raw) To: Rob Herring Cc: devicetree, maz, arnd, kettenis, marcan, bhelgaas, linux-arm-kernel, linux-pci, linux-kernel > From: Rob Herring <robh+dt@kernel.org> > Date: Mon, 17 May 2021 09:16:49 -0500 Hi Rob, Thanks for taking a look. Below are some further explanations and questions. I'll probably wait to collect a bit more feedback before re-rolling this series. > On Sun, May 16, 2021 at 4:19 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > > From: Mark Kettenis <kettenis@openbsd.org> > > > > The Apple PCIe host controller is a PCIe host controller with > > multiple root ports present in Apple ARM SoC platforms, including > > various iPhone and iPad devices and the "Apple Silicon" Macs. > > All the cover letter will be lost in the git history. Please mention > some details like this is a DWC controller here. I disagree that you > can't use the DWC binding. You can use it and extend it with what's > needed here. And that way, we could move from generic ECAM to an > actual driver in the OS if needed (hopefully not). More below. Should I mention the DWC heritage in the description of the device in the DT schema or in the commit message of the schema? Or both? Note that it looks as if Apple has replaced most of the DWC logic in the controller with their own logic. The DWC ATU isn't there, not in its original form nor in its newer "unrolled" form. The MSI logic is Apple's own as well. As such, none of the properties specified in Documentation/devicetree/bindings/pci/designware-pcie.txt that aren't specified by /schemas/pci/pci-bus.yaml apply. That said, the binding doesn't deliberately deviate from the DWC binding either. > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > > --- > > .../devicetree/bindings/pci/apple,pcie.yaml | 150 ++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 151 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > > diff --git a/Documentation/devicetree/bindings/pci/apple,pcie.yaml b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > > new file mode 100644 > > index 000000000000..af3c9f64e380 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > > @@ -0,0 +1,150 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pci/apple,pcie.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Apple PCIe host controller > > + > > +maintainers: > > + - Mark Kettenis <kettenis@openbsd.org> > > + > > +description: | > > + The Apple PCIe host controller is a PCIe host controller with > > + multiple root ports present in Apple ARM SoC platforms, including > > + various iPhone and iPad devices and the "Apple Silicon" Macs. > > + > > +allOf: > > + - $ref: /schemas/pci/pci-bus.yaml# > > + > > +properties: > > + compatible: > > + items: > > + - const: apple,t8103-pcie > > + - const: apple,pcie > > + > > + reg: > > + minItems: 4 > > + maxItems: 6 > > 6 or... > > > + reg-names: > > + minItems: 4 > > + maxItems: 7 > > 7? Oops. That is a leftover from when I was checking whether the schema would catch certain errors in the dts files. > > + items: > > + - const: ecam > > 'config' > > The difference between ECAM or not in existing devices is really just > the size. If you look at the addresses on other DWC bindings, the > config region is just an iATU window within the host's PCI address > range. If 'config' is a better name, sure, I can change that. I don't think the config region is just a window into the host's PCI address range though as config space requires a nGnRnE mapping whereas the PCI address range requires a nGnRE mapping. That's what the "nonposted-mmio" discussion was all about. > > + - const: rc > > This would be 'dbi'? I don't think so. As far as I can determine this registers block contains only Apple-specific registers and none of the registers that live in "Designware DBI space" on other hardware. > Also check if we need 'atu' (only if it's not at the default offset)? As far as I can tell there is no Designware ATU on the Apple hardware. > > + - const: phy > > Should there be a separate phy node? At this point neither the U-Boot driver nor the OpenBSD driver access registers in this block. Should I leave it out for now? > > + - const: port0 > > + - const: port1 > > + - const: port2 > > What's in these registers? This block contains registers that are per-port. This includes the Apple-specific link control registers, the Apple-specific MSI registers, PCI requestor ID to IOMMU stream ID mapping registers. > > + > > + ranges: > > + minItems: 2 > > + maxItems: 2 > > + > > + interrupts: > > + minItems: 3 > > + maxItems: 3 > > Need to define what each one is. Hmm, haven't figured that out yet. U-Boot doesn't need them and neither does my OpenBSD driver. Should I leave these out for now? > > + > > + msi-ranges: > > + description: > > + A list of pairs <intid span>, where "intid" is the first > > + interrupt number that can be used as an MSI, and "span" the size > > + of that range. > > Hopefully, Marc Z will comment on the MSI bits. msi-map doesn't work > here? If we need something else, then it should be added to > pci-msi.txt. This was actually based on discussions I had with Marc on #asahilinux some time ago. But certainly something that we need his blessing on ;). > One problem with this is it assumes 'intid' is one cell. It's really 2 > for the AIC if we ignore flags (which would be another assumption that > we can ignore the last cell). Or maybe this belongs in the AIC > binding? The MSI translation defenitely happens in the PCIe host bridge. The MSI target address is programmed in a per-port register and not translated by the IOMMU. > > > + $ref: /schemas/types.yaml#/definitions/uint32-matrix > > + items: > > + minItems: 2 > > + maxItems: 2 > > + > > +required: > > + - compatible > > + - reg > > + - reg-names > > + - bus-range > > + - interrupts > > + - msi-controller > > + - msi-parent > > + - msi-ranges > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/apple-aic.h> > > + #include <dt-bindings/pinctrl/apple.h> > > + > > + soc { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + pcie0: pcie@690000000 { > > + compatible = "apple,t8103-pcie", "apple,pcie"; > > + device_type = "pci"; > > + > > + reg = <0x6 0x90000000 0x0 0x1000000>, > > + <0x6 0x80000000 0x0 0x4000>, > > + <0x6 0x8c000000 0x0 0x4000>, > > + <0x6 0x81000000 0x0 0x8000>, > > + <0x6 0x82000000 0x0 0x8000>, > > + <0x6 0x83000000 0x0 0x8000>; > > + reg-names = "ecam", "rc", "phy", "port0", "port1", "port2"; > > + > > + interrupt-parent = <&aic>; > > + interrupts = <AIC_IRQ 695 IRQ_TYPE_LEVEL_HIGH>, > > + <AIC_IRQ 698 IRQ_TYPE_LEVEL_HIGH>, > > + <AIC_IRQ 701 IRQ_TYPE_LEVEL_HIGH>; > > + > > + msi-controller; > > + msi-parent = <&pcie0>; > > + msi-ranges = <704 32>; > > + > > + iommu-map = <0x0 &dart0 0x8000 0x100>, > > + <0x100 &dart0 0x100 0x100>, > > + <0x200 &dart1 0x200 0x100>, > > + <0x300 &dart2 0x300 0x100>; > > + iommu-map-mask = <0xff00>; > > These need to be documented. You can assume they have a type already, > so just 'true' or any constraints. No examples of that yet, but I can try to get that right... > > + > > + bus-range = <0 7>; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges = <0x43000000 0x6 0xa0000000 0x6 0xa0000000 0x0 0x20000000>, > > + <0x02000000 0x0 0xc0000000 0x6 0xc0000000 0x0 0x40000000>; > > + > > + clocks = <&pcie_core_clk>, <&pcie_aux_clk>, <&pcie_ref_clk>; > > + pinctrl-0 = <&pcie_pins>; > > + pinctrl-names = "default"; > > + > > + pci@0,0 { > > + device_type = "pci"; > > + reg = <0x0 0x0 0x0 0x0 0x0>; > > + reset-gpios = <&pinctrl_ap 152 0>; > > + max-link-speed = <2>; > > + > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges; > > + }; > > + > > + pci@1,0 { > > + device_type = "pci"; > > + reg = <0x800 0x0 0x0 0x0 0x0>; > > + reset-gpios = <&pinctrl_ap 153 0>; > > + max-link-speed = <2>; > > + > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges; > > + }; > > + > > + pci@2,0 { > > + device_type = "pci"; > > + reg = <0x1000 0x0 0x0 0x0 0x0>; > > + reset-gpios = <&pinctrl_ap 33 0>; > > + max-link-speed = <1>; > > + > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges; > > + }; > > + }; > > + }; > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 7327c9b778f1..789d79315485 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -1654,6 +1654,7 @@ C: irc://chat.freenode.net/asahi-dev > > T: git https://github.com/AsahiLinux/linux.git > > F: Documentation/devicetree/bindings/arm/apple.yaml > > F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml > > +F: Documentation/devicetree/bindings/pci/apple,pcie.yaml > > F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml > > F: arch/arm64/boot/dts/apple/ > > F: drivers/irqchip/irq-apple-aic.c > > -- > > 2.31.1 > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie 2021-05-17 19:41 ` Mark Kettenis @ 2021-05-17 20:21 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2021-05-17 20:21 UTC (permalink / raw) To: Mark Kettenis Cc: devicetree, Marc Zyngier, Arnd Bergmann, Mark Kettenis, Hector Martin, Bjorn Helgaas, linux-arm-kernel, PCI, linux-kernel On Mon, May 17, 2021 at 2:41 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > From: Rob Herring <robh+dt@kernel.org> > > Date: Mon, 17 May 2021 09:16:49 -0500 > > Hi Rob, > > Thanks for taking a look. Below are some further explanations and > questions. I'll probably wait to collect a bit more feedback before > re-rolling this series. > > > On Sun, May 16, 2021 at 4:19 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > > > > From: Mark Kettenis <kettenis@openbsd.org> > > > > > > The Apple PCIe host controller is a PCIe host controller with > > > multiple root ports present in Apple ARM SoC platforms, including > > > various iPhone and iPad devices and the "Apple Silicon" Macs. > > > > All the cover letter will be lost in the git history. Please mention > > some details like this is a DWC controller here. I disagree that you > > can't use the DWC binding. You can use it and extend it with what's > > needed here. And that way, we could move from generic ECAM to an > > actual driver in the OS if needed (hopefully not). More below. > > Should I mention the DWC heritage in the description of the device in > the DT schema or in the commit message of the schema? Or both? Probably in the schema is best. > Note that it looks as if Apple has replaced most of the DWC logic in > the controller with their own logic. The DWC ATU isn't there, not in > its original form nor in its newer "unrolled" form. The MSI logic is > Apple's own as well. As such, none of the properties specified in > Documentation/devicetree/bindings/pci/designware-pcie.txt that aren't > specified by /schemas/pci/pci-bus.yaml apply. That said, the binding > doesn't deliberately deviate from the DWC binding either. Okay, there might not be much to share. > > > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > > > --- > > > .../devicetree/bindings/pci/apple,pcie.yaml | 150 ++++++++++++++++++ > > > MAINTAINERS | 1 + > > > 2 files changed, 151 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/pci/apple,pcie.yaml b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > new file mode 100644 > > > index 000000000000..af3c9f64e380 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > @@ -0,0 +1,150 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/pci/apple,pcie.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Apple PCIe host controller > > > + > > > +maintainers: > > > + - Mark Kettenis <kettenis@openbsd.org> > > > + > > > +description: | > > > + The Apple PCIe host controller is a PCIe host controller with > > > + multiple root ports present in Apple ARM SoC platforms, including > > > + various iPhone and iPad devices and the "Apple Silicon" Macs. > > > + > > > +allOf: > > > + - $ref: /schemas/pci/pci-bus.yaml# > > > + > > > +properties: > > > + compatible: > > > + items: > > > + - const: apple,t8103-pcie > > > + - const: apple,pcie > > > + > > > + reg: > > > + minItems: 4 > > > + maxItems: 6 > > > > 6 or... > > > > > + reg-names: > > > + minItems: 4 > > > + maxItems: 7 > > > > 7? > > Oops. That is a leftover from when I was checking whether the schema > would catch certain errors in the dts files. > > > > + items: > > > + - const: ecam > > > > 'config' > > > > The difference between ECAM or not in existing devices is really just > > the size. If you look at the addresses on other DWC bindings, the > > config region is just an iATU window within the host's PCI address > > range. > > If 'config' is a better name, sure, I can change that. I don't think > the config region is just a window into the host's PCI address range > though as config space requires a nGnRnE mapping whereas the PCI > address range requires a nGnRE mapping. That's what the > "nonposted-mmio" discussion was all about. > > > > + - const: rc > > > > This would be 'dbi'? > > I don't think so. As far as I can determine this registers block > contains only Apple-specific registers and none of the registers that > live in "Designware DBI space" on other hardware. Humm, that's what I looked at to determine it's Designware based. I'll have to dig thru the irc logs to remember what I looked at. It's the 'port logic' registers that I compare. This sounds like it might be different enough that using the common DWC driver won't ever work since the bulk of the common parts for it are the iATU and MSI controller. > > Also check if we need 'atu' (only if it's not at the default offset)? > > As far as I can tell there is no Designware ATU on the Apple hardware. > > > > + - const: phy > > > > Should there be a separate phy node? > > At this point neither the U-Boot driver nor the OpenBSD driver access > registers in this block. Should I leave it out for now? Probably so. > > > + - const: port0 > > > + - const: port1 > > > + - const: port2 > > > > What's in these registers? > > This block contains registers that are per-port. This includes the > Apple-specific link control registers, the Apple-specific MSI > registers, PCI requestor ID to IOMMU stream ID mapping registers. > > > > + > > > + ranges: > > > + minItems: 2 > > > + maxItems: 2 > > > + > > > + interrupts: > > > + minItems: 3 > > > + maxItems: 3 > > > > Need to define what each one is. > > Hmm, haven't figured that out yet. U-Boot doesn't need them and > neither does my OpenBSD driver. Should I leave these out for now? Just add a note that they are unknown. It's at least useful to know there are 3. > > > + msi-ranges: > > > + description: > > > + A list of pairs <intid span>, where "intid" is the first > > > + interrupt number that can be used as an MSI, and "span" the size > > > + of that range. > > > > Hopefully, Marc Z will comment on the MSI bits. msi-map doesn't work > > here? If we need something else, then it should be added to > > pci-msi.txt. > > This was actually based on discussions I had with Marc on #asahilinux > some time ago. But certainly something that we need his blessing on > ;). > > > One problem with this is it assumes 'intid' is one cell. It's really 2 > > for the AIC if we ignore flags (which would be another assumption that > > we can ignore the last cell). Or maybe this belongs in the AIC > > binding? > > The MSI translation defenitely happens in the PCIe host bridge. The > MSI target address is programmed in a per-port register and not > translated by the IOMMU. Then perhaps you have more than 3 interrupts and need to list out 32 more? Or am I misunderstanding that '704' is an AIC interrupt number? > > > + $ref: /schemas/types.yaml#/definitions/uint32-matrix > > > + items: > > > + minItems: 2 > > > + maxItems: 2 > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - reg-names > > > + - bus-range > > > + - interrupts > > > + - msi-controller > > > + - msi-parent > > > + - msi-ranges > > > + > > > +unevaluatedProperties: false > > > + > > > +examples: > > > + - | > > > + #include <dt-bindings/interrupt-controller/apple-aic.h> > > > + #include <dt-bindings/pinctrl/apple.h> > > > + > > > + soc { > > > + #address-cells = <2>; > > > + #size-cells = <2>; > > > + > > > + pcie0: pcie@690000000 { > > > + compatible = "apple,t8103-pcie", "apple,pcie"; > > > + device_type = "pci"; > > > + > > > + reg = <0x6 0x90000000 0x0 0x1000000>, > > > + <0x6 0x80000000 0x0 0x4000>, > > > + <0x6 0x8c000000 0x0 0x4000>, > > > + <0x6 0x81000000 0x0 0x8000>, > > > + <0x6 0x82000000 0x0 0x8000>, > > > + <0x6 0x83000000 0x0 0x8000>; > > > + reg-names = "ecam", "rc", "phy", "port0", "port1", "port2"; > > > + > > > + interrupt-parent = <&aic>; > > > + interrupts = <AIC_IRQ 695 IRQ_TYPE_LEVEL_HIGH>, > > > + <AIC_IRQ 698 IRQ_TYPE_LEVEL_HIGH>, > > > + <AIC_IRQ 701 IRQ_TYPE_LEVEL_HIGH>; > > > + > > > + msi-controller; > > > + msi-parent = <&pcie0>; > > > + msi-ranges = <704 32>; > > > + > > > + iommu-map = <0x0 &dart0 0x8000 0x100>, > > > + <0x100 &dart0 0x100 0x100>, > > > + <0x200 &dart1 0x200 0x100>, > > > + <0x300 &dart2 0x300 0x100>; > > > + iommu-map-mask = <0xff00>; > > > > These need to be documented. You can assume they have a type already, > > so just 'true' or any constraints. > > No examples of that yet, but I can try to get that right... > > > > + > > > + bus-range = <0 7>; > > > + #address-cells = <3>; > > > + #size-cells = <2>; > > > + ranges = <0x43000000 0x6 0xa0000000 0x6 0xa0000000 0x0 0x20000000>, > > > + <0x02000000 0x0 0xc0000000 0x6 0xc0000000 0x0 0x40000000>; > > > + > > > + clocks = <&pcie_core_clk>, <&pcie_aux_clk>, <&pcie_ref_clk>; > > > + pinctrl-0 = <&pcie_pins>; > > > + pinctrl-names = "default"; > > > + > > > + pci@0,0 { > > > + device_type = "pci"; > > > + reg = <0x0 0x0 0x0 0x0 0x0>; > > > + reset-gpios = <&pinctrl_ap 152 0>; > > > + max-link-speed = <2>; > > > + > > > + #address-cells = <3>; > > > + #size-cells = <2>; > > > + ranges; > > > + }; > > > + > > > + pci@1,0 { > > > + device_type = "pci"; > > > + reg = <0x800 0x0 0x0 0x0 0x0>; > > > + reset-gpios = <&pinctrl_ap 153 0>; > > > + max-link-speed = <2>; > > > + > > > + #address-cells = <3>; > > > + #size-cells = <2>; > > > + ranges; > > > + }; > > > + > > > + pci@2,0 { > > > + device_type = "pci"; > > > + reg = <0x1000 0x0 0x0 0x0 0x0>; > > > + reset-gpios = <&pinctrl_ap 33 0>; > > > + max-link-speed = <1>; > > > + > > > + #address-cells = <3>; > > > + #size-cells = <2>; > > > + ranges; > > > + }; > > > + }; > > > + }; > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 7327c9b778f1..789d79315485 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -1654,6 +1654,7 @@ C: irc://chat.freenode.net/asahi-dev > > > T: git https://github.com/AsahiLinux/linux.git > > > F: Documentation/devicetree/bindings/arm/apple.yaml > > > F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml > > > +F: Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml > > > F: arch/arm64/boot/dts/apple/ > > > F: drivers/irqchip/irq-apple-aic.c > > > -- > > > 2.31.1 > > > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie 2021-05-16 21:18 ` [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie Mark Kettenis 2021-05-17 2:27 ` Rob Herring 2021-05-17 14:16 ` Rob Herring @ 2021-05-18 14:10 ` Robin Murphy 2021-05-22 13:57 ` Mark Kettenis 2 siblings, 1 reply; 10+ messages in thread From: Robin Murphy @ 2021-05-18 14:10 UTC (permalink / raw) To: Mark Kettenis, devicetree Cc: maz, arnd, Mark Kettenis, Hector Martin, Bjorn Helgaas, Rob Herring, linux-arm-kernel, linux-pci, linux-kernel On 2021-05-16 22:18, Mark Kettenis wrote: > From: Mark Kettenis <kettenis@openbsd.org> > > The Apple PCIe host controller is a PCIe host controller with > multiple root ports present in Apple ARM SoC platforms, including > various iPhone and iPad devices and the "Apple Silicon" Macs. > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > --- > .../devicetree/bindings/pci/apple,pcie.yaml | 150 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 151 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml > > diff --git a/Documentation/devicetree/bindings/pci/apple,pcie.yaml b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > new file mode 100644 > index 000000000000..af3c9f64e380 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > @@ -0,0 +1,150 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pci/apple,pcie.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Apple PCIe host controller > + > +maintainers: > + - Mark Kettenis <kettenis@openbsd.org> > + > +description: | > + The Apple PCIe host controller is a PCIe host controller with > + multiple root ports present in Apple ARM SoC platforms, including > + various iPhone and iPad devices and the "Apple Silicon" Macs. > + > +allOf: > + - $ref: /schemas/pci/pci-bus.yaml# > + > +properties: > + compatible: > + items: > + - const: apple,t8103-pcie > + - const: apple,pcie > + > + reg: > + minItems: 4 > + maxItems: 6 > + > + reg-names: > + minItems: 4 > + maxItems: 7 > + items: > + - const: ecam > + - const: rc > + - const: phy > + - const: port0 > + - const: port1 > + - const: port2 > + > + ranges: > + minItems: 2 > + maxItems: 2 > + > + interrupts: > + minItems: 3 > + maxItems: 3 > + > + msi-ranges: > + description: > + A list of pairs <intid span>, where "intid" is the first > + interrupt number that can be used as an MSI, and "span" the size > + of that range. > + $ref: /schemas/types.yaml#/definitions/uint32-matrix > + items: > + minItems: 2 > + maxItems: 2 > + > +required: > + - compatible > + - reg > + - reg-names > + - bus-range > + - interrupts > + - msi-controller > + - msi-parent > + - msi-ranges > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/apple-aic.h> > + #include <dt-bindings/pinctrl/apple.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + pcie0: pcie@690000000 { > + compatible = "apple,t8103-pcie", "apple,pcie"; > + device_type = "pci"; > + > + reg = <0x6 0x90000000 0x0 0x1000000>, > + <0x6 0x80000000 0x0 0x4000>, > + <0x6 0x8c000000 0x0 0x4000>, > + <0x6 0x81000000 0x0 0x8000>, > + <0x6 0x82000000 0x0 0x8000>, > + <0x6 0x83000000 0x0 0x8000>; > + reg-names = "ecam", "rc", "phy", "port0", "port1", "port2"; > + > + interrupt-parent = <&aic>; > + interrupts = <AIC_IRQ 695 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 698 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 701 IRQ_TYPE_LEVEL_HIGH>; > + > + msi-controller; > + msi-parent = <&pcie0>; > + msi-ranges = <704 32>; > + > + iommu-map = <0x0 &dart0 0x8000 0x100>, > + <0x100 &dart0 0x100 0x100>, > + <0x200 &dart1 0x200 0x100>, > + <0x300 &dart2 0x300 0x100>; > + iommu-map-mask = <0xff00>; This doesn't quite add up - if the mask is ignoring the bottom 8 bits, then each of those map entries is describing one single ID mapping, not 256. > + bus-range = <0 7>; Given that the iommu-map only covers buses 0-3, what happens to traffic from buses 4-7? Robin. > + #address-cells = <3>; > + #size-cells = <2>; > + ranges = <0x43000000 0x6 0xa0000000 0x6 0xa0000000 0x0 0x20000000>, > + <0x02000000 0x0 0xc0000000 0x6 0xc0000000 0x0 0x40000000>; > + > + clocks = <&pcie_core_clk>, <&pcie_aux_clk>, <&pcie_ref_clk>; > + pinctrl-0 = <&pcie_pins>; > + pinctrl-names = "default"; > + > + pci@0,0 { > + device_type = "pci"; > + reg = <0x0 0x0 0x0 0x0 0x0>; > + reset-gpios = <&pinctrl_ap 152 0>; > + max-link-speed = <2>; > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + }; > + > + pci@1,0 { > + device_type = "pci"; > + reg = <0x800 0x0 0x0 0x0 0x0>; > + reset-gpios = <&pinctrl_ap 153 0>; > + max-link-speed = <2>; > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + }; > + > + pci@2,0 { > + device_type = "pci"; > + reg = <0x1000 0x0 0x0 0x0 0x0>; > + reset-gpios = <&pinctrl_ap 33 0>; > + max-link-speed = <1>; > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + }; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index 7327c9b778f1..789d79315485 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1654,6 +1654,7 @@ C: irc://chat.freenode.net/asahi-dev > T: git https://github.com/AsahiLinux/linux.git > F: Documentation/devicetree/bindings/arm/apple.yaml > F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml > +F: Documentation/devicetree/bindings/pci/apple,pcie.yaml > F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml > F: arch/arm64/boot/dts/apple/ > F: drivers/irqchip/irq-apple-aic.c > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie 2021-05-18 14:10 ` Robin Murphy @ 2021-05-22 13:57 ` Mark Kettenis 0 siblings, 0 replies; 10+ messages in thread From: Mark Kettenis @ 2021-05-22 13:57 UTC (permalink / raw) To: Robin Murphy, sven Cc: devicetree, maz, arnd, kettenis, marcan, bhelgaas, robh+dt, linux-arm-kernel, linux-pci, linux-kernel > From: Robin Murphy <robin.murphy@arm.com> > Date: Tue, 18 May 2021 15:10:01 +0100 Hi Robin, > On 2021-05-16 22:18, Mark Kettenis wrote: > > From: Mark Kettenis <kettenis@openbsd.org> > > > > The Apple PCIe host controller is a PCIe host controller with > > multiple root ports present in Apple ARM SoC platforms, including > > various iPhone and iPad devices and the "Apple Silicon" Macs. > > > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > > --- > > .../devicetree/bindings/pci/apple,pcie.yaml | 150 ++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 151 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > > diff --git a/Documentation/devicetree/bindings/pci/apple,pcie.yaml b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > > new file mode 100644 > > index 000000000000..af3c9f64e380 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > > @@ -0,0 +1,150 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pci/apple,pcie.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Apple PCIe host controller > > + > > +maintainers: > > + - Mark Kettenis <kettenis@openbsd.org> > > + > > +description: | > > + The Apple PCIe host controller is a PCIe host controller with > > + multiple root ports present in Apple ARM SoC platforms, including > > + various iPhone and iPad devices and the "Apple Silicon" Macs. > > + > > +allOf: > > + - $ref: /schemas/pci/pci-bus.yaml# > > + > > +properties: > > + compatible: > > + items: > > + - const: apple,t8103-pcie > > + - const: apple,pcie > > + > > + reg: > > + minItems: 4 > > + maxItems: 6 > > + > > + reg-names: > > + minItems: 4 > > + maxItems: 7 > > + items: > > + - const: ecam > > + - const: rc > > + - const: phy > > + - const: port0 > > + - const: port1 > > + - const: port2 > > + > > + ranges: > > + minItems: 2 > > + maxItems: 2 > > + > > + interrupts: > > + minItems: 3 > > + maxItems: 3 > > + > > + msi-ranges: > > + description: > > + A list of pairs <intid span>, where "intid" is the first > > + interrupt number that can be used as an MSI, and "span" the size > > + of that range. > > + $ref: /schemas/types.yaml#/definitions/uint32-matrix > > + items: > > + minItems: 2 > > + maxItems: 2 > > + > > +required: > > + - compatible > > + - reg > > + - reg-names > > + - bus-range > > + - interrupts > > + - msi-controller > > + - msi-parent > > + - msi-ranges > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/apple-aic.h> > > + #include <dt-bindings/pinctrl/apple.h> > > + > > + soc { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + pcie0: pcie@690000000 { > > + compatible = "apple,t8103-pcie", "apple,pcie"; > > + device_type = "pci"; > > + > > + reg = <0x6 0x90000000 0x0 0x1000000>, > > + <0x6 0x80000000 0x0 0x4000>, > > + <0x6 0x8c000000 0x0 0x4000>, > > + <0x6 0x81000000 0x0 0x8000>, > > + <0x6 0x82000000 0x0 0x8000>, > > + <0x6 0x83000000 0x0 0x8000>; > > + reg-names = "ecam", "rc", "phy", "port0", "port1", "port2"; > > + > > + interrupt-parent = <&aic>; > > + interrupts = <AIC_IRQ 695 IRQ_TYPE_LEVEL_HIGH>, > > + <AIC_IRQ 698 IRQ_TYPE_LEVEL_HIGH>, > > + <AIC_IRQ 701 IRQ_TYPE_LEVEL_HIGH>; > > + > > + msi-controller; > > + msi-parent = <&pcie0>; > > + msi-ranges = <704 32>; > > + > > + iommu-map = <0x0 &dart0 0x8000 0x100>, > > + <0x100 &dart0 0x100 0x100>, > > + <0x200 &dart1 0x200 0x100>, > > + <0x300 &dart2 0x300 0x100>; > > + iommu-map-mask = <0xff00>; > > This doesn't quite add up - if the mask is ignoring the bottom 8 bits, > then each of those map entries is describing one single ID mapping, not 256. > > > + bus-range = <0 7>; > > Given that the iommu-map only covers buses 0-3, what happens to traffic > from buses 4-7? Yes, that probably needs a little bit of thought. The hardware is somewhat "interesting". The PCIe host bridge has (up to) three ports. Each port is associated with its own IOMMU/DART. Each port provides mapping logic that maps the RID to an SID. There are 16 mapping registers for the PCIe host bridge that connects the onboard devices and 64 mapping registers for the PCIe host bridges that are asociated with the Thunderbolt ports. If no mappings are enabled, it seems that all RIDs get mapped to SID 0. The Apple firmware doesn't enable any mappings and my U-Boot code doesn't change it either. The Corellium folks in their port chose a 1:1 mapping from bus number to SID and that is what the example above came from. Both my U-Boot driver and my OpenBSD actually ignore the SID and install the same IOMMU translation table for all the SIDs. That's probably good enough for U-Boot as long as we don't enable the Thunderbolt ports. But for the OS itself a bit more control is certainly desirable. Would it be reasonable to allow the device tree some flexibility in specifying the desired iommu mapping and let the OS PCIe host bride driver program the RID to SID mappings to match what's specified in the "iommu-map" and "iommu-map-mask" properties? Or is it better to just pick a mapping scheme like Corellium did and make that part of the DT binding? Thanks, Mark > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges = <0x43000000 0x6 0xa0000000 0x6 0xa0000000 0x0 0x20000000>, > > + <0x02000000 0x0 0xc0000000 0x6 0xc0000000 0x0 0x40000000>; > > + > > + clocks = <&pcie_core_clk>, <&pcie_aux_clk>, <&pcie_ref_clk>; > > + pinctrl-0 = <&pcie_pins>; > > + pinctrl-names = "default"; > > + > > + pci@0,0 { > > + device_type = "pci"; > > + reg = <0x0 0x0 0x0 0x0 0x0>; > > + reset-gpios = <&pinctrl_ap 152 0>; > > + max-link-speed = <2>; > > + > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges; > > + }; > > + > > + pci@1,0 { > > + device_type = "pci"; > > + reg = <0x800 0x0 0x0 0x0 0x0>; > > + reset-gpios = <&pinctrl_ap 153 0>; > > + max-link-speed = <2>; > > + > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges; > > + }; > > + > > + pci@2,0 { > > + device_type = "pci"; > > + reg = <0x1000 0x0 0x0 0x0 0x0>; > > + reset-gpios = <&pinctrl_ap 33 0>; > > + max-link-speed = <1>; > > + > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges; > > + }; > > + }; > > + }; > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 7327c9b778f1..789d79315485 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -1654,6 +1654,7 @@ C: irc://chat.freenode.net/asahi-dev > > T: git https://github.com/AsahiLinux/linux.git > > F: Documentation/devicetree/bindings/arm/apple.yaml > > F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml > > +F: Documentation/devicetree/bindings/pci/apple,pcie.yaml > > F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml > > F: arch/arm64/boot/dts/apple/ > > F: drivers/irqchip/irq-apple-aic.c > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] arm64: apple: Add PCIe node 2021-05-16 21:18 [PATCH 0/2] Apple M1 PCIe DT bindings Mark Kettenis 2021-05-16 21:18 ` [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie Mark Kettenis @ 2021-05-16 21:18 ` Mark Kettenis 1 sibling, 0 replies; 10+ messages in thread From: Mark Kettenis @ 2021-05-16 21:18 UTC (permalink / raw) To: devicetree Cc: maz, arnd, Mark Kettenis, Hector Martin, Bjorn Helgaas, Rob Herring, linux-arm-kernel, linux-pci, linux-kernel From: Mark Kettenis <kettenis@openbsd.org> Add node corresponding to the apcie,t8103 node in the Apple device tree for the Mac mini (M1, 2020). Clock references and DART (IOMMU) references are left out at the moment and will be added once the appropriate bindings have been settled upon. Signed-off-by: Mark Kettenis <kettenis@openbsd.org> --- arch/arm64/boot/dts/apple/t8103.dtsi | 64 ++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi index 503a76fc30e6..102947935d63 100644 --- a/arch/arm64/boot/dts/apple/t8103.dtsi +++ b/arch/arm64/boot/dts/apple/t8103.dtsi @@ -214,5 +214,69 @@ pinctrl_smc: pinctrl@23e820000 { <AIC_IRQ 396 IRQ_TYPE_LEVEL_HIGH>, <AIC_IRQ 397 IRQ_TYPE_LEVEL_HIGH>; }; + + pcie0: pcie@690000000 { + compatible = "apple,t8103-pcie", "apple,pcie"; + device_type = "pci"; + + reg = <0x6 0x90000000 0x0 0x1000000>, + <0x6 0x80000000 0x0 0x4000>, + <0x6 0x8c000000 0x0 0x4000>, + <0x6 0x81000000 0x0 0x8000>, + <0x6 0x82000000 0x0 0x8000>, + <0x6 0x83000000 0x0 0x8000>; + reg-names = "ecam", "rc", "phy", "port0", "port1", "port2"; + + interrupt-parent = <&aic>; + interrupts = <AIC_IRQ 695 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 698 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 701 IRQ_TYPE_LEVEL_HIGH>; + + msi-controller; + msi-parent = <&pcie0>; + msi-ranges = <704 32>; + + bus-range = <0 7>; + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x43000000 0x6 0xa0000000 0x6 0xa0000000 0x0 0x20000000>, + <0x02000000 0x0 0xc0000000 0x6 0xc0000000 0x0 0x40000000>; + + pinctrl-0 = <&pcie_pins>; + pinctrl-names = "default"; + + pci@0,0 { + device_type = "pci"; + reg = <0x0 0x0 0x0 0x0 0x0>; + reset-gpios = <&pinctrl_ap 152 0>; + max-link-speed = <2>; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + }; + + pci@1,0 { + device_type = "pci"; + reg = <0x800 0x0 0x0 0x0 0x0>; + reset-gpios = <&pinctrl_ap 153 0>; + max-link-speed = <2>; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + }; + + pci@2,0 { + device_type = "pci"; + reg = <0x1000 0x0 0x0 0x0 0x0>; + reset-gpios = <&pinctrl_ap 33 0>; + max-link-speed = <1>; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + }; + }; }; }; -- 2.31.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-05-22 13:59 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-16 21:18 [PATCH 0/2] Apple M1 PCIe DT bindings Mark Kettenis 2021-05-16 21:18 ` [PATCH 1/2] dt-bindings: pci: Add DT bindings for apple,pcie Mark Kettenis 2021-05-17 2:27 ` Rob Herring 2021-05-17 8:17 ` Mark Kettenis 2021-05-17 14:16 ` Rob Herring 2021-05-17 19:41 ` Mark Kettenis 2021-05-17 20:21 ` Rob Herring 2021-05-18 14:10 ` Robin Murphy 2021-05-22 13:57 ` Mark Kettenis 2021-05-16 21:18 ` [PATCH 2/2] arm64: apple: Add PCIe node Mark Kettenis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).