From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Fri, 13 May 2016 15:26:13 -0600 Subject: [U-Boot] [PATCH] Add a mailbox driver framework/uclass In-Reply-To: References: <1463092066-10478-1-git-send-email-swarren@wwwdotorg.org> Message-ID: <57364675.2090504@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 05/13/2016 02:05 PM, Simon Glass wrote: > Hi Stephen, > > On 12 May 2016 at 16:27, Stephen Warren wrote: >> From: Stephen Warren >> >> A mailbox is a hardware mechanism for transferring small message and/or >> notifications between the CPU on which U-Boot runs and some other device >> such as an auxilliary CPU running firmware or a hardware module. >> >> This patch defines a standard API that connects mailbox clients to mailbox >> providers (drivers). Initially, DT is the only supported method for >> connecting the two. >> >> The DT binding specification (mailbox.txt) was taken from Linux kernel >> v4.5's Documentation/devicetree/bindings/mailbox/mailbox.txt. >> >> Signed-off-by: Stephen Warren > Also see the remoteproc uclass. I'd just like to make sure that these > should not be combined. I think that's quite a different API; more to do with loading firmware(?)/booting/resetting remote processors than communicating with them once they're booted. FWIW, the Linux kernel has separate mailbox and remoteproc APIs too. >> diff --git a/drivers/mailbox/mailbox-uclass.c b/drivers/mailbox/mailbox-uclass.c >> +int mbox_send(struct mbox_chan *chan, const void *data) > > Is there no length on the data? The length is implicit; each mailbox implementation defines (the HW) defines the exact message size it transfers. Clients would always use this exact size. >> +int mbox_recv(struct mbox_chan *chan, void *data, ulong timeout_us) >> + start_time = get_ticks(); >> + /* >> + * Account for partial us ticks, but if timeout_us is 0, ensure we >> + * still don't wait at all. >> + */ >> + if (timeout_us) >> + timeout_us++; > > This seems a bit picky - it is only a microsecond! Is there anything wrong with being correct? Equally, "only a microsecond" admittedly isn't relevant if timeout_us is 1000 * 1000, but is a massive percentage of the delay if timeout_us is something tiny like 1 or 2.