| Message ID | 20210815042525.36878-2-alyssa@rosenzweig.io (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Add PCI driver for the Apple M1 | expand |
Hi Alyssa, On Sun, 15 Aug 2021 05:25:24 +0100, Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote: > > Document the properties used by the Apple PCI controller. This is a > fairly standard PCI controller, although it is not derived from any > known non-Apple IP. > > Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io> I would rather you post something as an extension to Mark's work, for multiple reasons: - Mark's patch is still being discussed, and is the current reference (specially given that it is already in use in OpenBSD and u-boot). - we cannot have multiple bindings. There can only be one, shared across implementations. Otherwise, you need a different kernel depending on whether you are booting from m1n1 or u-boot. - what you have here is vastly inconsistent (you are describing the MSIs twice, using two different methods). Thanks, M. > --- > .../devicetree/bindings/pci/apple,pcie.yaml | 153 ++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 159 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..4378f5a05804 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > @@ -0,0 +1,153 @@ > +# 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 SoC PCIe Controller Device Tree Bindings > + > +maintainers: > + - Alyssa Rosenzweig <alyssa@rosenzweig.io> > + > +description: |+ > + Apple SoC PCIe host controller. > + > +allOf: > + - $ref: /schemas/pci/pci-bus.yaml# > + > +properties: > + compatible: > + const: apple,pcie > + > + reg: > + items: > + - description: PCIe configuration region. > + - description: Core registers. > + - description: AXI bridge registers. > + - description: Port 0 (radio) registers. > + - description: Port 1 (USB) registers. > + - description: Port 2 (Ethernet) registers. > + > + reg-names: > + items: > + - const: config > + - const: rc > + - const: phy > + - const: port0 > + - const: port1 > + - const: port2 > + > + interrupts: > + maxItems: 35 > + > + msi-controller: > + description: Identifies the node as an MSI controller. > + > + msi-parent: > + description: MSI controller the device is capable of using. > + > + reset-gpios: > + description: Reset lines for each of the ports of the controller. > + > + pinctrl-0: > + description: Pin controller for the reset lines. > + > + pinctrl-names: > + description: Names for the pin controller. > + > +required: > + - reg > + - reg-names > + - interrupt-parent > + - interrupts > + - msi-controller > + - msi-parent > + - msi-interrupts > + - iommu-map > + - iommu-map-mask > + - pinctrl-0 > + - pinctrl-names > + - reset-gpios > + - bus-range > + - "#address-cells" > + - "#size-cells" > + - ranges > + - device_type > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/apple-aic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + pcie0: pcie@690000000 { > + compatible = "apple,pcie"; > + reg = <0x6 0x90000000 0x0 0x1000000>, > + <0x6 0x80000000 0x0 0x100000>, > + <0x6 0x8c000000 0x0 0x100000>, > + <0x6 0x81000000 0x0 0x4000>, > + <0x6 0x82000000 0x0 0x4000>, > + <0x6 0x83000000 0x0 0x4000>; > + reg-names = "config", "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>, > + <AIC_IRQ 704 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 705 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 706 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 707 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 708 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 709 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 710 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 711 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 712 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 713 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 714 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 715 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 716 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 717 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 718 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 719 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 720 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 721 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 722 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 723 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 724 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 725 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 726 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 727 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 728 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 729 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 730 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 731 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 732 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 733 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 734 IRQ_TYPE_LEVEL_HIGH>, > + <AIC_IRQ 735 IRQ_TYPE_LEVEL_HIGH>; > + msi-controller; > + msi-parent = <&pcie0>; > + msi-interrupts = <704 32>; > + iommu-map = <0x100 &pcie0_dart_0 0 1>, > + <0x200 &pcie0_dart_1 0 1>, > + <0x300 &pcie0_dart_2 0 1>; > + iommu-map-mask = <0xff00>; > + pinctrl-0 = <&pcie_pins>; > + pinctrl-names = "default"; > + reset-gpios = <&gpio 152 0 &gpio 153 0 &gpio 33 0>; > + bus-range = <0 15>; > + #address-cells = <3>; > + #size-cells = <2>; > + ranges = <0x43000000 0x6 0xa0000000 0x6 0xa0000000 > + 0x0 0x20000000>, > + <0x02000000 0x0 0xc0000000 0x6 0xc0000000 > + 0x0 0x40000000>; > + device_type = "pci"; > + }; > + }; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index b63403793c81..d7d2e1d1e2f2 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1269,6 +1269,12 @@ S: Maintained > F: Documentation/devicetree/bindings/iommu/apple,dart.yaml > F: drivers/iommu/apple-dart.c > > +APPLE PCIE CONTROLLER DRIVER > +M: Alyssa Rosenzweig <alyssa@rosenzweig.io> > +L: linux-pci@vger.kernel.org > +S: Maintained > +F: Documentation/devicetree/bindings/pci/apple,pcie.yaml > + > APPLE SMC DRIVER > M: Henrik Rydberg <rydberg@bitmath.org> > L: linux-hwmon@vger.kernel.org > -- > 2.30.2 > >
Hi Mark, On Sun, 15 Aug 2021 09:10:53 +0100, Mark Kettenis <openbsd@xs4all.nl> wrote: > > Hi Marc, > > What can I do to make progress with my binding proposal? It seems we're stuck > on the MSI issue where you and robh disagree. I still think your idea of > describing the MSIs as a range makes much more sense than describing them > individually and bunching them together with the host bridge port interrupts. > It looks like I missed an email from Rob, which explains why we're in limbo (it was left unread and unmarked, which in my flow means "read once I have too much time on my hands"). Apologies for that, I'll try and reply tonight (travelling at the moment). > Op 15-08-2021 09:09 schreef Marc Zyngier <maz@kernel.org>: > > Hi Alyssa, > > On Sun, 15 Aug 2021 05:25:24 +0100, > Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote: > > Document the properties used by the Apple PCI controller. This is a > fairly standard PCI controller, although it is not derived from any > known non-Apple IP. > > Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io> > > I would rather you post something as an extension to Mark's work, for > multiple reasons: > > - Mark's patch is still being discussed, and is the current > reference (specially given that it is already in use in OpenBSD and > u-boot). > > - we cannot have multiple bindings. There can only be one, shared > across implementations. Otherwise, you need a different kernel > depending on whether you are booting from m1n1 or u-boot. > > - what you have here is vastly inconsistent (you are describing the > MSIs twice, using two different methods). > > That's probably my fault. The current u-boot device tree is a bit of a > Frankenstein thing to ease the transition from my initial binding to the > current proposal. I should clean that up at some point. That would certainly help. There are a lot of moving pieces at the moment, and it is getting hard to get a clear picture of what is using what. Thanks, M.
Hi Marc, > > Document the properties used by the Apple PCI controller. This is a > > fairly standard PCI controller, although it is not derived from any > > known non-Apple IP. > > > > Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io> > > I would rather you post something as an extension to Mark's work, for > multiple reasons: > > - Mark's patch is still being discussed, and is the current > reference (specially given that it is already in use in OpenBSD and > u-boot). > > - we cannot have multiple bindings. There can only be one, shared > across implementations. Otherwise, you need a different kernel > depending on whether you are booting from m1n1 or u-boot. > > - what you have here is vastly inconsistent (you are describing the > MSIs twice, using two different methods). Absolutely agree, the frankenstein bindings here were the main reason v1 was marked RFC. For v2, I've rebased on Mark's patch, which makes a bunch of driver magic disappear. Alyssa
> Date: Sun, 15 Aug 2021 21:34:36 -0400 > From: Alyssa Rosenzweig <alyssa@rosenzweig.io> > > Hi Marc, > > > > Document the properties used by the Apple PCI controller. This is a > > > fairly standard PCI controller, although it is not derived from any > > > known non-Apple IP. > > > > > > Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io> > > > > I would rather you post something as an extension to Mark's work, for > > multiple reasons: > > > > - Mark's patch is still being discussed, and is the current > > reference (specially given that it is already in use in OpenBSD and > > u-boot). > > > > - we cannot have multiple bindings. There can only be one, shared > > across implementations. Otherwise, you need a different kernel > > depending on whether you are booting from m1n1 or u-boot. > > > > - what you have here is vastly inconsistent (you are describing the > > MSIs twice, using two different methods). > > Absolutely agree, the frankenstein bindings here were the main reason v1 > was marked RFC. For v2, I've rebased on Mark's patch, which makes a > bunch of driver magic disappear. I updated the t8103.dtsi bindings on the apple-m1-m1n1-nvme branch in my u-boot repository to be more in line with the current DT binding proposal. Note that the format of the msi-ranges property is still under discussion. See: http://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210726083204.93196-2-mark.kettenis@xs4all.nl/ Cheers, Mark
diff --git a/Documentation/devicetree/bindings/pci/apple,pcie.yaml b/Documentation/devicetree/bindings/pci/apple,pcie.yaml new file mode 100644 index 000000000000..4378f5a05804 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml @@ -0,0 +1,153 @@ +# 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 SoC PCIe Controller Device Tree Bindings + +maintainers: + - Alyssa Rosenzweig <alyssa@rosenzweig.io> + +description: |+ + Apple SoC PCIe host controller. + +allOf: + - $ref: /schemas/pci/pci-bus.yaml# + +properties: + compatible: + const: apple,pcie + + reg: + items: + - description: PCIe configuration region. + - description: Core registers. + - description: AXI bridge registers. + - description: Port 0 (radio) registers. + - description: Port 1 (USB) registers. + - description: Port 2 (Ethernet) registers. + + reg-names: + items: + - const: config + - const: rc + - const: phy + - const: port0 + - const: port1 + - const: port2 + + interrupts: + maxItems: 35 + + msi-controller: + description: Identifies the node as an MSI controller. + + msi-parent: + description: MSI controller the device is capable of using. + + reset-gpios: + description: Reset lines for each of the ports of the controller. + + pinctrl-0: + description: Pin controller for the reset lines. + + pinctrl-names: + description: Names for the pin controller. + +required: + - reg + - reg-names + - interrupt-parent + - interrupts + - msi-controller + - msi-parent + - msi-interrupts + - iommu-map + - iommu-map-mask + - pinctrl-0 + - pinctrl-names + - reset-gpios + - bus-range + - "#address-cells" + - "#size-cells" + - ranges + - device_type + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/apple-aic.h> + #include <dt-bindings/interrupt-controller/irq.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + pcie0: pcie@690000000 { + compatible = "apple,pcie"; + reg = <0x6 0x90000000 0x0 0x1000000>, + <0x6 0x80000000 0x0 0x100000>, + <0x6 0x8c000000 0x0 0x100000>, + <0x6 0x81000000 0x0 0x4000>, + <0x6 0x82000000 0x0 0x4000>, + <0x6 0x83000000 0x0 0x4000>; + reg-names = "config", "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>, + <AIC_IRQ 704 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 705 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 706 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 707 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 708 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 709 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 710 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 711 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 712 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 713 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 714 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 715 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 716 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 717 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 718 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 719 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 720 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 721 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 722 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 723 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 724 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 725 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 726 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 727 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 728 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 729 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 730 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 731 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 732 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 733 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 734 IRQ_TYPE_LEVEL_HIGH>, + <AIC_IRQ 735 IRQ_TYPE_LEVEL_HIGH>; + msi-controller; + msi-parent = <&pcie0>; + msi-interrupts = <704 32>; + iommu-map = <0x100 &pcie0_dart_0 0 1>, + <0x200 &pcie0_dart_1 0 1>, + <0x300 &pcie0_dart_2 0 1>; + iommu-map-mask = <0xff00>; + pinctrl-0 = <&pcie_pins>; + pinctrl-names = "default"; + reset-gpios = <&gpio 152 0 &gpio 153 0 &gpio 33 0>; + bus-range = <0 15>; + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x43000000 0x6 0xa0000000 0x6 0xa0000000 + 0x0 0x20000000>, + <0x02000000 0x0 0xc0000000 0x6 0xc0000000 + 0x0 0x40000000>; + device_type = "pci"; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index b63403793c81..d7d2e1d1e2f2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1269,6 +1269,12 @@ S: Maintained F: Documentation/devicetree/bindings/iommu/apple,dart.yaml F: drivers/iommu/apple-dart.c +APPLE PCIE CONTROLLER DRIVER +M: Alyssa Rosenzweig <alyssa@rosenzweig.io> +L: linux-pci@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/pci/apple,pcie.yaml + APPLE SMC DRIVER M: Henrik Rydberg <rydberg@bitmath.org> L: linux-hwmon@vger.kernel.org
Document the properties used by the Apple PCI controller. This is a fairly standard PCI controller, although it is not derived from any known non-Apple IP. Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io> --- .../devicetree/bindings/pci/apple,pcie.yaml | 153 ++++++++++++++++++ MAINTAINERS | 6 + 2 files changed, 159 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml