Re: [PATCH 6/9] dt-bindings: Add RISC-V advanced PLIC bindings

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4020 bytes --]

On Mon, Jan 02, 2023 at 10:20:48PM +0530, Anup Patel wrote:
> On Sun, Nov 13, 2022 at 9:14 PM Conor Dooley <conor@kernel.org> wrote:

> > > +  domain.
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/interrupt-controller.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +          - vendor,chip-aplic
> >
> > Same comment here about the validity of this placeholder.
> 
> Okay, I will add "riscv,qemu-aplic" as QEMU specific compatible string.

Ah neat. I think that's a fair compromise.

> > > +      - const: riscv,aplic

> > > +  msi-parent:
> > > +    description:
> > > +      The presence of this property implies that given APLIC domain forwards
> > > +      wired interrupts as MSIs to a AIA incoming message signaled interrupt
> > > +      controller (IMSIC). This property should be considered only when the
> > > +      interrupts-extended property is absent.
> >
> > This mutual exclusion can be represented, can't it?
> > IIRC it is some sort of oneOf thing, somewhat like below:
> > oneOf:
> >   - required:
> >       - msi-parent
> >   - required:
> >       - interrupts-extended
> >
> > AFAIR from doing the i2c ocores binding, this will force the addition of
> > one, but not both, to a node.
> >
> > Or is this not actually mutually exclusive & the msi-parent property is
> > permitted but just left unused if interrupts-extended is present?
> 
> If both are present then interrupts-extended is preferred.

Perhaps I am making a fool of myself here, but why would someone include
both of them at once, if only one is going to be used?
It would appear that making them explicitly mutually exclusive would
make the binding easier to understand.
What am I missing?

> > > +  riscv,children:
> > > +    $ref: '/schemas/types.yaml#/definitions/phandle-array'
> > > +    minItems: 1
> > > +    maxItems: 1024
> > > +    description:
> > > +      This property represents a list of child APLIC domains for the given
> > > +      APLIC domain. Each child APLIC domain is assigned child index in
> > > +      increasing order with the first child APLIC domain assigned child
> > > +      index 0. The APLIC domain child index is used by firmware to delegate
> > > +      interrupts from the given APLIC domain to a particular child APLIC
> > > +      domain.
> > > +
> > > +  riscv,delegate:
> > > +    $ref: '/schemas/types.yaml#/definitions/phandle-array'
> > > +    minItems: 1
> > > +    maxItems: 1024
> > > +    description:
> > > +      This property represents a interrupt delegation list where each entry
> > > +      is a triple consisting of child APLIC domain phandle, first interrupt
> > > +      number, and last interrupt number. The firmware will configure interrupt
> > > +      delegation registers based on interrupt delegation list.
> >
> > What is the inter dependence of the children and delegate?
> > Is it valid to have a delegate property without children?
> > Can the firmware delegate interrupts without the delegation list, based
> > on the children property alone? Or is it effectively useless without a
> > children property?
> 
> Both properties convey different information. The "riscv,childen" describes
> the association of child indexes with child APLIC domains whereas the
> "riscv,delegate" describes the interrupt delegation to few of the child
> APLIC domains.
> 
> 
> >
> > In your examples, the second has msi-parent but neither of these custom
> > properties. Do the children/delegate properties have a meaning in the
> > msi-parent case?
> 
> The "riscv,childern" and "riscv,delegate" are only useful when we have
> hierarchy of multiple APLIC domains. The second example only has
> one APLIC domain hence these custom properties are absent.

It'd be great if you could include an example that explains the
difference as, IIRC, both Rob and I both were kinda confused as to how
the properties differ.

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]