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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 08 Sep 2021 17:36:30 +0200
From:   "Sven Peter" <sven@...npeter.dev>
To:     "Alyssa Rosenzweig" <alyssa@...enzweig.io>
Cc:     "Jassi Brar" <jassisinghbrar@...il.com>,
        "Rob Herring" <robh+dt@...nel.org>,
        "Mark Kettenis" <mark.kettenis@...all.nl>,
        "Hector Martin" <marcan@...can.st>,
        "Mohamed Mediouni" <mohamed.mediouni@...amail.com>,
        "Stan Skowronek" <stan@...ellium.com>, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: mailbox: Add Apple mailbox bindings



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