diff mbox series

[09/17] dt-bindings: interrupt-controller: Add support for Microchip LAN966x OIC

Message ID 20240430083730.134918-10-herve.codina@bootlin.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for the LAN966x PCI device using a DT overlay | expand

Checks

Context Check Description
netdev/series_format fail Series longer than 15 patches
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 932 this patch: 932
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 938 this patch: 938
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 944 this patch: 944
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Herve Codina April 30, 2024, 8:37 a.m. UTC 
The Microchip LAN966x outband interrupt controller (OIC) maps the
internal interrupt sources of the LAN966x device to an external
interrupt.
When the LAN966x device is used as a PCI device, the external interrupt
is routed to the PCI interrupt.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 .../microchip,lan966x-oic.yaml                | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml

Comments

Rob Herring (Arm) May 7, 2024, 3:28 p.m. UTC | #1 
On Tue, Apr 30, 2024 at 10:37:18AM +0200, Herve Codina wrote:
> The Microchip LAN966x outband interrupt controller (OIC) maps the
> internal interrupt sources of the LAN966x device to an external
> interrupt.
> When the LAN966x device is used as a PCI device, the external interrupt
> is routed to the PCI interrupt.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  .../microchip,lan966x-oic.yaml                | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml b/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
> new file mode 100644
> index 000000000000..b2adc7174177
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/microchip,lan966x-oic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip LAN966x outband interrupt controller
> +
> +maintainers:
> +  - Herve Codina <herve.codina@bootlin.com>
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +description: |
> +  The Microchip LAN966x outband interrupt controller (OIC) maps the internal
> +  interrupt sources of the LAN966x device to an external interrupt.
> +  When the LAN966x device is used as a PCI device, the external interrupt is
> +  routed to the PCI interrupt.
> +
> +properties:
> +  compatible:
> +    const: microchip,lan966x-oic
> +
> +  '#interrupt-cells':
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - '#interrupt-cells'
> +  - interrupt-controller
> +  - interrupts
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    interrupt-controller@e00c0120 {
> +        compatible = "microchip,lan966x-oic";
> +        reg = <0xe00c0120 0x190>;

Looks like this is part of some larger block?

> +        #interrupt-cells = <2>;
> +        interrupt-controller;
> +        interrupts = <0>;
> +        interrupt-parent = <&intc>;
> +    };
> +...
> -- 
> 2.44.0
>
Herve Codina May 13, 2024, 12:37 p.m. UTC | #2 
Hi Rob,

On Tue, 7 May 2024 10:28:06 -0500
Rob Herring <robh@kernel.org> wrote:

...
> > +examples:
> > +  - |
> > +    interrupt-controller@e00c0120 {
> > +        compatible = "microchip,lan966x-oic";
> > +        reg = <0xe00c0120 0x190>;  
> 
> Looks like this is part of some larger block?
> 

According to the registers information document:
  https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,intr

The interrupt controller is mapped at offset 0x48 (offset in number of
32bit words).
-> Address offset: 0x48 * 4 = 0x120
-> size: (0x63 + 1) *  4 = 0x190

IMHO, the reg property value looks correct.

Best regards,
Hervé
Rob Herring (Arm) May 13, 2024, 2:53 p.m. UTC | #3 
On Mon, May 13, 2024 at 02:37:20PM +0200, Herve Codina wrote:
> Hi Rob,
> 
> On Tue, 7 May 2024 10:28:06 -0500
> Rob Herring <robh@kernel.org> wrote:
> 
> ...
> > > +examples:
> > > +  - |
> > > +    interrupt-controller@e00c0120 {
> > > +        compatible = "microchip,lan966x-oic";
> > > +        reg = <0xe00c0120 0x190>;  
> > 
> > Looks like this is part of some larger block?
> > 
> 
> According to the registers information document:
>   https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,intr
> 
> The interrupt controller is mapped at offset 0x48 (offset in number of
> 32bit words).
> -> Address offset: 0x48 * 4 = 0x120
> -> size: (0x63 + 1) *  4 = 0x190
> 
> IMHO, the reg property value looks correct.

What I mean is h/w blocks don't just start at some address with small 
alignment. That wouldn't work from a physical design standpoint. The 
larger block here is "CPU System Regs". The block as a whole should be 
documented, but maybe that ship already sailed.

Also, here you call it the OIC, but the link above calls it the VCore 
interrupt controller.

Rob
Herve Codina May 13, 2024, 5:04 p.m. UTC | #4 
Hi Rob,

On Mon, 13 May 2024 09:53:58 -0500
Rob Herring <robh@kernel.org> wrote:

> On Mon, May 13, 2024 at 02:37:20PM +0200, Herve Codina wrote:
> > Hi Rob,
> > 
> > On Tue, 7 May 2024 10:28:06 -0500
> > Rob Herring <robh@kernel.org> wrote:
> > 
> > ...  
> > > > +examples:
> > > > +  - |
> > > > +    interrupt-controller@e00c0120 {
> > > > +        compatible = "microchip,lan966x-oic";
> > > > +        reg = <0xe00c0120 0x190>;    
> > > 
> > > Looks like this is part of some larger block?
> > >   
> > 
> > According to the registers information document:
> >   https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,intr
> > 
> > The interrupt controller is mapped at offset 0x48 (offset in number of
> > 32bit words).  
> > -> Address offset: 0x48 * 4 = 0x120
> > -> size: (0x63 + 1) *  4 = 0x190  
> > 
> > IMHO, the reg property value looks correct.  
> 
> What I mean is h/w blocks don't just start at some address with small 
> alignment. That wouldn't work from a physical design standpoint. The 
> larger block here is "CPU System Regs". The block as a whole should be 
> documented, but maybe that ship already sailed.

The clock controller, also part of the "CPU System Regs" is already defined
and used without the larger block
  Documentation/devicetree/bindings/clock/microchip,lan966x-gck.yaml

IMHO, the binding related to the interrupt controller should be consistent
with the one related to the clock controller.


> 
> Also, here you call it the OIC, but the link above calls it the VCore 
> interrupt controller.

Yes, I call it OIC (Outband Interrupt Controller) as it is its name in the
datasheet explaining how it works.
The datasheet I have is not publicly available and so, I can point only to
the register map (url provided).

I think it would be better to keep "Outband Interrupt Controller" as
mentioned in the datasheet.

Best regards,
Hervé
Rob Herring (Arm) May 22, 2024, 2:24 p.m. UTC | #5 
On Tue, 30 Apr 2024 10:37:18 +0200, Herve Codina wrote:
> The Microchip LAN966x outband interrupt controller (OIC) maps the
> internal interrupt sources of the LAN966x device to an external
> interrupt.
> When the LAN966x device is used as a PCI device, the external interrupt
> is routed to the PCI interrupt.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  .../microchip,lan966x-oic.yaml                | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml b/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
new file mode 100644
index 000000000000..b2adc7174177
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
@@ -0,0 +1,55 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/microchip,lan966x-oic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip LAN966x outband interrupt controller
+
+maintainers:
+  - Herve Codina <herve.codina@bootlin.com>
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+description: |
+  The Microchip LAN966x outband interrupt controller (OIC) maps the internal
+  interrupt sources of the LAN966x device to an external interrupt.
+  When the LAN966x device is used as a PCI device, the external interrupt is
+  routed to the PCI interrupt.
+
+properties:
+  compatible:
+    const: microchip,lan966x-oic
+
+  '#interrupt-cells':
+    const: 2
+
+  interrupt-controller: true
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - '#interrupt-cells'
+  - interrupt-controller
+  - interrupts
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    interrupt-controller@e00c0120 {
+        compatible = "microchip,lan966x-oic";
+        reg = <0xe00c0120 0x190>;
+        #interrupt-cells = <2>;
+        interrupt-controller;
+        interrupts = <0>;
+        interrupt-parent = <&intc>;
+    };
+...