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

[Rahul Tanwar]

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@xxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxx>
>  > +
>  > +
>  > +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
>