2022-11-22 07:42:15

by Rahul Tanwar

[permalink] [raw]
Subject: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema

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 <[email protected]>
Signed-off-by: Rahul Tanwar <[email protected]>
---
.../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..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 <[email protected]>
+
+
+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 <[email protected]>
+
+
+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>;
+ };
--
2.17.1


2022-11-22 10:01:28

by Rahul Tanwar

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema

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 <[email protected]>
>
> 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 <[email protected]>
> > +
> > +
> > +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
>

2022-11-22 10:16:52

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema

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.

--
With Best Regards,
Andy Shevchenko


2022-11-22 10:40:58

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema

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 <[email protected]>

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 <[email protected]>
> +
> +
> +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.

--
With Best Regards,
Andy Shevchenko


2022-11-22 11:14:29

by Rahul Tanwar

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema

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
>
>
>

2022-11-23 17:00:03

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema

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 <[email protected]>
> Signed-off-by: Rahul Tanwar <[email protected]>
> ---
> .../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

2022-11-23 17:58:33

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema

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).

--
With Best Regards,
Andy Shevchenko


2022-11-24 09:09:25

by Rahul Tanwar

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema

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 <[email protected]>
>> Signed-off-by: Rahul Tanwar <[email protected]>
>> ---
>> .../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
>
>