Re: [PATCH 2/3] dt-bindings: mailbox: Add Apple mailbox bindings

[Sven Peter]

On Tue, Sep 7, 2021, at 22:48, Alyssa Rosenzweig wrote:
> > > > +      - description:
> > > > +          M3 mailboxes are an older variant with a slightly different MMIO
> > > > +          interface still found on the M1.
> > > > +        items:
> > > > +          - const: apple,t8103-m3-mailbox
> > > 
> > > Would be nice to document an example of where an M3 mailbox is found.
> > 
> > Sure, I can add a comment that this is used for the coprocessor controlling Thunderbolt.
> 
> That raises another issue ... how do we know the M3 code works at all
> without TB support yet? It "looks" correct but some of the IRQ handling
> stuff is nontrivial.

Enabling the mailbox interface just requires a few clocks to be ungated.
Then I injected messages manually to verify that the code works.
In addition I also brought up parts of the Thunderbolt controller which
then allowed the co-processor on the other end of the mailbox to boot.
After that I was also able to successfully talk to that processor using
the same protocol used by most other processors.


> 
> > > > +  interrupts:
> > > > +    minItems: 4
> > > > +    items:
> > > > +      - description: send fifo is empty interrupt
> > > > +      - description: send fifo is not empty interrupt
> > > > +      - description: receive fifo is empty interrupt
> > > > +      - description: receive fifo is not empty interrupt
> > > > +
> > > > +  interrupt-names:
> > > > +    minItems: 4
> > > > +    items:
> > > > +      - const: send-empty
> > > > +      - const: send-not-empty
> > > > +      - const: recv-empty
> > > > +      - const: recv-not-empty
> > > 
> > > If the names became not-constant the asprintf thing goes away, not sure
> > > that's better or worse.
> > 
> > I'm not sure I understand your comment here. This property just gives a name
> > to the interrupts so that they can be referenced by that instead of a magic
> > number between 0 and 4 in the driver.
> 
> D'oh, right, retracted. (Both this comment and the corresponding comment
> on the driver itself). Sorry about that.
> 
> > > > +  clocks:
> > > > +    description:
> > > > +      Reference to the clock gate phandle(s) if required for this mailbox.
> > > > +      Optional since not all mailboxes are attached to a clock gate.
> > > 
> > > Do we do anything with the clocks at this point?
> > > 
> > 
> > The device tree bindings describe the hardware (as best as we can without proper
> > documentation) and some of these mailboxes have clock gates which need to be turned
> > on before accessing their MMIO. This driver already tries to do that and works fine
> > with the downstream clock driver(s) we have.
> 
> Good enough for me, thanks for clarifying 👍
> 
> Commit r-b, though Rob will surely point out problems and I'll need to
> rereview 😉
> 

Thanks!



Sven