Message ID | 20240626104544.14233-1-svarbanov@suse.de (mailing list archive) |
---|---|
Headers | show |
Series | Add PCIe support for bcm2712 | expand |
On 26/06/2024 11:45, Stanimir Varbanov wrote: > Adds DT bindings for bcm2712 MSI-X interrupt peripheral controller. > > Signed-off-by: Stanimir Varbanov <svarbanov@suse.de> > --- > .../brcm,bcm2712-msix.yaml | 74 +++++++++++++++++++ > 1 file changed, 74 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml > new file mode 100644 > index 000000000000..ca610e4467d9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml > @@ -0,0 +1,74 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/brcm,bcm2712-msix.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Broadcom bcm2712 MSI-X Interrupt Peripheral support > + > +maintainers: > + - Stanimir Varbanov <svarbanov@suse.de> > + > +description: > > + This interrupt controller is used to provide intterupt vectors to the > + generic interrupt controller (GIC) on bcm2712. It will be used as > + external MSI-X controller for PCIe root complex. > + > +allOf: > + - $ref: /schemas/interrupt-controller/msi-controller.yaml# > + > +properties: > + compatible: > + items: > + - enum: > + - "brcm,bcm2712-mip-intc" > + reg: > + maxItems: 1 > + description: > > + Specifies the base physical address and size of the registers > + > + interrupt-controller: true > + > + "#interrupt-cells": > + const: 2 > + > + msi-controller: true > + > + brcm,msi-base-spi: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: The SGI number that MSIs start. > + > + brcm,msi-num-spis: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: The number of SGIs for MSIs. > + > + brcm,msi-offset: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Shift the allocated MSIs up by N. > + > + brcm,msi-pci-addr: > + $ref: /schemas/types.yaml#/definitions/uint64 > + description: MSI-X message address. > + > +additionalProperties: false > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - "#interrupt-cells" > + - msi-controller From the implementation of the driver, it looks like all properties are required, except for brcm,msi-offset which has a fallback to the value 0.
Hi, On 26/06/2024 11:45, Stanimir Varbanov wrote: > This patchset aims to add bare minimum support for bcm2712 > in brcmstb PCIe driver needed to support the peripherals from > RP1 south-bridge found in RPi5. In order to support RP1 > PCIe endpoint peripherals a new interrupt controller is added. > The interrupt controller supports 64 interrupt sources which > are enough to handle 61 RP1 peripherals. > > Patch 1 is adding DT binding schema for the MIP interrupt > controller, patch 2 is adding relevant changes for PCIe > bcm2712 in yaml. Patch 3 adds MIP intterrupt cotroller driver. > Patches 4 and 5 are preparations for adding bcm2712 support in 6. > The last patch updates bcm2712 .dsti by adding pcie DT nodes. > > Few concerns about the implementation: > - the connection between MIP interrupt-controller and PCIe RC is > done through BAR1. The PCIe driver is parsing the msi_parent > DT property in order to obtain few private DT properties like > "brcm,msi-pci-addr" and "reg". IMO this looks hackish but I failed > to find something better. Ideas? > > - in downstream RPi kernel "ranges" and "dma-ranges" DT properties > are under an axi {} simple-bus node even that PCIe block is on CPU > MMIO bus. I tried to merge axi {} in soc {} and the result could be > seen on the last patch in this series, but I'm still not sure that > it looks good enough. > > This series has been functionally tested on OpenSUSE Tumbleweed with > downstream RP1 south-bridge PCIe endpoint driver implementation as > MFD by using ethernet which is part of it. > > The series is based on Andrea's "Add minimal boot support for Raspberry Pi 5" > series. > > Comments are welcome! We are just about submitting support for 7712 which is the sister chip of 2712 and requires similar, if not identical types of changes to pcie-brcmstb.c, would you mind reviewing that patch series when it gets posted by Jim in the next few days, and base yours upon that one? It does separate changes in a more atomic and a more reviewer friendly rather than having one big commit modifying pcie-brcmstb.c Thanks.
On 26/06/2024 11:45, Stanimir Varbanov wrote: > Adds DT bindings for bcm2712 MSI-X interrupt peripheral controller. > > Signed-off-by: Stanimir Varbanov <svarbanov@suse.de> > --- > .../brcm,bcm2712-msix.yaml | 74 +++++++++++++++++++ > 1 file changed, 74 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml > new file mode 100644 > index 000000000000..ca610e4467d9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml > @@ -0,0 +1,74 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/brcm,bcm2712-msix.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Broadcom bcm2712 MSI-X Interrupt Peripheral support > + > +maintainers: > + - Stanimir Varbanov <svarbanov@suse.de> > + > +description: > > + This interrupt controller is used to provide intterupt vectors to the > + generic interrupt controller (GIC) on bcm2712. It will be used as > + external MSI-X controller for PCIe root complex. > + > +allOf: > + - $ref: /schemas/interrupt-controller/msi-controller.yaml# > + > +properties: > + compatible: > + items: > + - enum: > + - "brcm,bcm2712-mip-intc" > + reg: > + maxItems: 1 > + description: > > + Specifies the base physical address and size of the registers > + > + interrupt-controller: true > + > + "#interrupt-cells": > + const: 2 Should we have some sort of an interrupt-map or interrupt-map-mask property that defines the "linkage" between the inputs and the outputs? This controller does not really sit at the top-level of the interrupt tree as it feeds the ARM GIC, unfortunately this is not captured at all, and it seems to require ad-hoc properties to establish the mapping, that does not seem ideal.