Re: [PATCH v4 1/4] dt-bindings: x86: apic: Convert Intel's APIC bindings to YAML schema

[Rahul Tanwar <rtanwar@maxlinear.com>]

On 24/11/2022 5:28 am, Rob Herring wrote:
> This email was sent from outside of MaxLinear.
> 
> On Wed, Nov 23, 2022 at 06:08:47PM +0800, Rahul Tanwar wrote:
>  > Intel's APIC family of interrupt controllers support local APIC
>  > (lapic) & I/O APIC (ioapic). Convert existing bindings for lapic
>  > & ioapic from text to YAML schema. Separate lapic & ioapic schemas.
>  > Addditionally, add description which was missing in text file and
>  > add few more required standard properties which were also missing
>  > in text file.
>  >
>  > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>  > Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
>  > ---
>  > .../intel,ce4100-ioapic.txt | 26 --------
>  > .../intel,ce4100-ioapic.yaml | 62 +++++++++++++++++++
>  > .../intel,ce4100-lapic.yaml | 49 +++++++++++++++
>  > 3 files changed, 111 insertions(+), 26 deletions(-)
>  > delete mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt
>  > create mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml
>  > create mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
>  >
>  > diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt
>  > deleted file mode 100644
>  > index 7d19f494f19a..000000000000
>  > --- 
> a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt
>  > +++ /dev/null
>  > @@ -1,26 +0,0 @@
>  > -Interrupt chips
>  > ----------------
>  > -
>  > -* Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
>  > -
>  > - Required properties:
>  > - --------------------
>  > - compatible = "intel,ce4100-ioapic";
>  > - #interrupt-cells = <2>;
>  > -
>  > - Device's interrupt property:
>  > -
>  > - interrupts = <P S>;
>  > -
>  > - The first number (P) represents the interrupt pin which is wired to the
>  > - IO APIC. The second number (S) represents the sense of interrupt which
>  > - should be configured and can be one of:
>  > - 0 - Edge Rising
>  > - 1 - Level Low
>  > - 2 - Level High
>  > - 3 - Edge Falling
>  > -
>  > -* Local APIC
>  > - Required property:
>  > -
>  > - compatible = "intel,ce4100-lapic";
>  > diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml
>  > new file mode 100644
>  > index 000000000000..25d549220c2a
>  > --- /dev/null
>  > +++ 
> b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml
>  > @@ -0,0 +1,62 @@
>  > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>  > +%YAML 1.2
>  > +---
>  > +$id: 
> "http://devicetree.org/schemas/interrupt-controller/intel,ce4100-ioapic.yaml# <http://devicetree.org/schemas/interrupt-controller/intel,ce4100-ioapic.yaml#>"
>  > +$schema: "http://devicetree.org/meta-schemas/core.yaml# 
> <http://devicetree.org/meta-schemas/core.yaml#>"
>  > +
>  > +title: Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
>  > +
>  > +maintainers:
>  > + - Rahul Tanwar <rtanwar@maxlinear.com>
>  > +
>  > +
> 
> One blank line.

Well noted.

> 
>  > +description: |
>  > + Intel's Advanced Programmable Interrupt Controller (APIC) is a
>  > + family of interrupt controllers. The APIC is a split
>  > + architecture design, with a local component (LAPIC) integrated
>  > + into the processor itself and an external I/O APIC. Local APIC
>  > + (lapic) receives interrupts from the processor's interrupt pins,
>  > + from internal sources and from an external I/O APIC (ioapic).
>  > + And it sends these to the processor core for handling.
>  > + See [1] Chapter 8 for more details.
>  > +
>  > + Many of the Intel's generic devices like hpet, ioapic, lapic have
>  > + the ce4100 name in their compatible property names because they
>  > + first appeared in CE4100 SoC.
>  > +
>  > + This schema defines bindings for I/O APIC interrupt controller.
>  > +
>  > + [1] https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf 
> <https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf>
>  > +
>  > +properties:
>  > + compatible:
>  > + const: intel,ce4100-ioapic
>  > +
>  > + reg:
>  > + maxItems: 1
>  > +
>  > + interrupt-controller: true
>  > +
>  > + '#interrupt-cells':
>  > + const: 2
>  > +
>  > + interrupts:
>  > + maxItems: 1
>  > +
>  > +required:
>  > + - compatible
>  > + - reg
>  > + - interrupt-controller
>  > + - '#interrupt-cells'
>  > +
>  > +additionalProperties: false
>  > +
>  > +examples:
>  > + - |
>  > + ioapic1: interrupt-controller@fec00000 {
>  > + compatible = "intel,ce4100-ioapic";
>  > + reg = <0xfec00000 0x1000>;
>  > + #interrupt-cells = <2>;
>  > + #address-cells = <0>;
>  > + interrupt-controller;
>  > + };
>  > diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
>  > new file mode 100644
>  > index 000000000000..88f320ef4616
>  > --- /dev/null
>  > +++ 
> b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
>  > @@ -0,0 +1,49 @@
>  > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>  > +%YAML 1.2
>  > +---
>  > +$id: 
> "http://devicetree.org/schemas/interrupt-controller/intel,ce4100-lapic.yaml# <http://devicetree.org/schemas/interrupt-controller/intel,ce4100-lapic.yaml#>"
>  > +$schema: "http://devicetree.org/meta-schemas/core.yaml# 
> <http://devicetree.org/meta-schemas/core.yaml#>"
>  > +
>  > +title: Intel Local Advanced Programmable Interrupt Controller (LAPIC)
>  > +
>  > +maintainers:
>  > + - Rahul Tanwar <rtanwar@maxlinear.com>
>  > +
>  > +
>  > +description: |
>  > + Intel's Advanced Programmable Interrupt Controller (APIC) is a
>  > + family of interrupt controllers. The APIC is a split
>  > + architecture design, with a local component (LAPIC) integrated
>  > + into the processor itself and an external I/O APIC. Local APIC
>  > + (lapic) receives interrupts from the processor's interrupt pins,
>  > + from internal sources and from an external I/O APIC (ioapic).
>  > + And it sends these to the processor core for handling.
>  > + See [1] Chapter 8 for more details.
>  > +
>  > + Many of the Intel's generic devices like hpet, ioapic, lapic have
>  > + the ce4100 name in their compatible property names because they
>  > + first appeared in CE4100 SoC.
>  > +
>  > + This schema defines bindings for local APIC interrupt controller.
>  > +
>  > + [1] https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf 
> <https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf>
>  > +
>  > +properties:
>  > + compatible:
>  > + const: intel,ce4100-lapic
>  > +
>  > + reg:
>  > + maxItems: 1
>  > +
>  > +required:
>  > + - compatible
>  > + - reg
>  > +
>  > +additionalProperties: false
>  > +
>  > +examples:
>  > + - |
>  > + lapic0: interrupt-controller@fee00000 {
>  > + compatible = "intel,ce4100-lapic";
>  > + reg = <0xfee00000 0x1000>;
> 
> What about interrupt-controller and #interrupt-cells properties?


Thanks for pointing it out. Yes, interrupt-controller & #interrupt-cells 
properties rightfully belong here. Will update in v5. Thanks.

Regards,
Rahul


> 
>  > + };
>  > --
>  > 2.17.1
>  >
>  >