[v3,1/4] x86/of: Convert Intel's APIC bindings to YAML schema

Message ID 0cf089495a422b945ac4fc9c980ddb5429a711c3.1669100394.git.rtanwar@maxlinear.com
State New
Headers
Series x86/of: Fix a bug in x86 arch OF support |

Commit Message

Rahul Tanwar Nov. 22, 2022, 7:39 a.m. UTC
  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
  

Comments

Andy Shevchenko Nov. 22, 2022, 9:10 a.m. UTC | #1
On Tue, Nov 22, 2022 at 03:39:07PM +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.

...

> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/interrupt-controller/intel,ce4100-ioapic.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Intel I/O Advanced Programmable Interrupt Controller (IO APIC)

> +maintainers:
> +  - Sebastian Andrzej Siewior <bigeasy@linutronix.de>

I'm not sure, you need to have a confirmation before putting someone's name here.
Yours is easier to add.

> +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 https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf

Dunno if schema has special format for data sheet links...

> +  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. See bindings/x86/ce4100.txt for more

Shouldn't you change this?

> +  details on it.
> +
> +  This schema defines bindings for I/O APIC interrupt controller.

...

> +maintainers:
> +  - Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> +
> +
> +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 https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf
> +  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. See bindings/x86/ce4100.txt for more
> +  details on it.
> +
> +  This schema defines bindings for local APIC interrupt controller.

Same two comments as per above.
  
Rahul Tanwar Nov. 22, 2022, 9:43 a.m. UTC | #2
On 22/11/2022 5:11 pm, Andy Shevchenko wrote:
> This email was sent from outside of MaxLinear.
> 
> On Tue, Nov 22, 2022 at 03:39:07PM +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.
> 
> ...
> 
>  > +# 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:
>  > + - Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> I'm not sure, you need to have a confirmation before putting someone's 
> name here.
> Yours is easier to add.
>

Well noted, will update.



>  > +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 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>
> 
> Dunno if schema has special format for data sheet links...
> 


Example-schema says this is the place to put URL's..



>  > + 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. See bindings/x86/ce4100.txt for more
> 
> Shouldn't you change this?
> 


Do you mean change compatibility property prefix from 
"intel,ce4100-ioapic" to "intel,ioapic"? If yes, then i totally agree 
and i will change it (including new file names & all other references to 
ce4100). If not, please clarify more..


>  > + details on it.
>  > +
>  > + This schema defines bindings for I/O APIC interrupt controller.
> 
> ...
> 
>  > +maintainers:
>  > + - Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>  > +
>  > +
>  > +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 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>
>  > + 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. See bindings/x86/ce4100.txt for more
>  > + details on it.
>  > +
>  > + This schema defines bindings for local APIC interrupt controller.
> 
> Same two comments as per above.
>


Well noted.


> -- 
> With Best Regards,
> Andy Shevchenko
>
  
Andy Shevchenko Nov. 22, 2022, 10:09 a.m. UTC | #3
On Tue, Nov 22, 2022 at 09:43:12AM +0000, Rahul Tanwar wrote:
> On 22/11/2022 5:11 pm, Andy Shevchenko wrote:
> > On Tue, Nov 22, 2022 at 03:39:07PM +0800, Rahul Tanwar wrote:

...

> >  > + first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
> > 
> > Shouldn't you change this?
> 
> Do you mean change compatibility property prefix from 
> "intel,ce4100-ioapic" to "intel,ioapic"? If yes, then i totally agree 
> and i will change it (including new file names & all other references to 
> ce4100). If not, please clarify more..

I specifically emphasized a single line (by putting blank lines around).
For your convenience I removed the unneeded parts of the context, so you can
see better what I meant.

...

> >  > + first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more


Ditto.
  
Rahul Tanwar Nov. 22, 2022, 10:37 a.m. UTC | #4
On 22/11/2022 6:09 pm, Andy Shevchenko wrote:
> This email was sent from outside of MaxLinear.
> 
> 
> On Tue, Nov 22, 2022 at 09:43:12AM +0000, Rahul Tanwar wrote:
>> On 22/11/2022 5:11 pm, Andy Shevchenko wrote:
>>> On Tue, Nov 22, 2022 at 03:39:07PM +0800, Rahul Tanwar wrote:
> 
> ...
> 
>>>   > + first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
>>>
>>> Shouldn't you change this?
>>
>> Do you mean change compatibility property prefix from
>> "intel,ce4100-ioapic" to "intel,ioapic"? If yes, then i totally agree
>> and i will change it (including new file names & all other references to
>> ce4100). If not, please clarify more..
> 
> I specifically emphasized a single line (by putting blank lines around).
> For your convenience I removed the unneeded parts of the context, so you can
> see better what I meant.
>


Got it. I will remove the mention of "See bindings/x86/ce4100.txt" from 
here.



> ...
> 
>>>   > + first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
> 
> 
> Ditto.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 
>
  
Krzysztof Kozlowski Nov. 23, 2022, 4:02 p.m. UTC | #5
On 22/11/2022 08:39, 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
> 

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.

You miss not only people but also lists, meaning this will not be
automatically tested.

So: No.

Best regards,
Krzysztof
  
Andy Shevchenko Nov. 23, 2022, 5:24 p.m. UTC | #6
On Wed, Nov 23, 2022 at 05:02:33PM +0100, Krzysztof Kozlowski wrote:
> On 22/11/2022 08:39, Rahul Tanwar wrote:

...

> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC.  It might happen, that command when run on an older
> kernel, gives you outdated entries.  Therefore please be sure you base
> your patches on recent Linux kernel.
> 
> You miss not only people but also lists, meaning this will not be
> automatically tested.

It seems that v4 manages to get the testing (and it's a good thing since
it found some issues).
  
Rahul Tanwar Nov. 24, 2022, 8:33 a.m. UTC | #7
On 24/11/2022 12:08 am, Krzysztof Kozlowski wrote:
> This email was sent from outside of MaxLinear.
> 
> 
> On 22/11/2022 08:39, 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
>>
> 
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC.  It might happen, that command when run on an older
> kernel, gives you outdated entries.  Therefore please be sure you base
> your patches on recent Linux kernel.
> 
> You miss not only people but also lists, meaning this will not be
> automatically tested.
> 
> So: No.
> 


Agree that i made mistakes in email list earlier. But i fixed that 
problem from v4 onwards thanks to Andy. From v4 onwards, To & Cc should 
be correct. Thanks.

Regards,
Rahul


> Best regards,
> Krzysztof
> 
>
  

Patch

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..da966287eec2
--- /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#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
+
+maintainers:
+  - Sebastian Andrzej Siewior <bigeasy@linutronix.de>
+
+
+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 https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf
+  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. See bindings/x86/ce4100.txt for more
+  details on it.
+
+  This schema defines bindings for I/O APIC interrupt controller.
+
+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..d4b99bf7bf6e
--- /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#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Intel Local Advanced Programmable Interrupt Controller (LAPIC)
+
+maintainers:
+  - Sebastian Andrzej Siewior <bigeasy@linutronix.de>
+
+
+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 https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf
+  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. See bindings/x86/ce4100.txt for more
+  details on it.
+
+  This schema defines bindings for local APIC interrupt controller.
+
+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>;
+    };