From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754831AbbE1Vqu (ORCPT ); Thu, 28 May 2015 17:46:50 -0400 Received: from gabe.freedesktop.org ([131.252.210.177]:49529 "EHLO gabe.freedesktop.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751812AbbE1Vqs (ORCPT ); Thu, 28 May 2015 17:46:48 -0400 From: Eric Anholt To: Stephen Warren Cc: linux-kernel@vger.kernel.org, Jassi Brar , linux-rpi-kernel@lists.infradead.org Subject: Re: [PATCH] mailbox/bcm2835: Fix mailbox full detection. In-Reply-To: <55677E6F.7070504@wwwdotorg.org> References: <1431547832-32430-1-git-send-email-eric@anholt.net> <55677E6F.7070504@wwwdotorg.org> User-Agent: Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Thu, 28 May 2015 14:46:46 -0700 Message-ID: <878uc8pe7t.fsf@eliezer.anholt.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Stephen Warren writes: > On 05/13/2015 02:10 PM, Eric Anholt wrote: >> With the VC reader blocked and the ARM writing, MAIL0_STA reads empty >> permanently while MAIL1_STA goes from empty (0x40000000) to non-empty >> (0x00000001-0x00000007) to full (0x80000008). >> >> This bug ended up having no effect on us, because all of our >> transactions in the client driver were synchronous and under a mutex. > > If you could get someone at the RPi Foundation or Broadcom to update the= =20 > register descriptions and example code at the following URLs, that would= =20 > be rather useful. Otherwise, this code will appear incorrect when=20 > compared against the documentation: > > https://github.com/raspberrypi/firmware/wiki/Mailboxes > ("Mailbox registers" at the bottom) > > https://github.com/raspberrypi/firmware/wiki/Accessing-mailboxes > ("Sample code") Since it's a wiki, I went ahead and edited the first one. Hopefully that clarifies how the c++ in the other page is supposed to be used. >> diff --git a/drivers/mailbox/bcm2835-mailbox.c b/drivers/mailbox/bcm2835= -mailbox.c > >> @@ -117,7 +118,7 @@ static bool bcm2835_last_tx_done(struct mbox_chan *l= ink) >> bool ret; >> >> spin_lock(&mbox->lock); >> - ret =3D !(readl(mbox->regs + MAIL0_STA) & ARM_MS_FULL); >> + ret =3D !(readl(mbox->regs + MAIL1_STA) & ARM_MS_FULL); > > What does "tx done" mean semantically? > > If "tx done" means "remote side received all our messages", then surely=20 > this should check MAIL1_STA for emptiness, which is different to the=20 > "not full" check implemented here? > > If "tx done" means "there's space to transmit more messages", then=20 > consider this: The mailbox core appears to use this hook as "there's space to transmit more messages." The name does seem really confusing. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJVZ4zHAAoJELXWKTbR/J7oIKIQALDiPwNDxfD9MjYiX18/P0ln l8MT9icclC8IVgIQawHkKKjnOrs3/bcxojfoMT58E8x38GsQIYnThE5PkBtf0IvD +2YUxf71EfnyVPNZDb8ZqFHucPQbJYVb7j8SlXdQJ3gjctYduroLJ4ofBDOd783x NUWyU1ULPEU0cHpPo4OeJnzuxzqLogBT9E3q85yUO2u6b8awxghciO3BXqoHB2Yr 9pxzXO0Zqni/i7rr+6D4hLlOrRnBb1/gXdl/zAgiZKonWr3VWPKQ9rn991iw9Emy CiiU+rNdPPM1qv6dy5TLo1cNet7pncl/Qv0aHJFPZBj/S40YdnTu1mHmQBfKR+IV pn5X69plyUn+YIRtiFPwTjSwC66X2zgF7Z1FMTCHvaJkGDren1tN8gtzRbuU7n/N u9MvwHAdLVLkmj4TI6xHwAn6X4S5zfN5goOFi8qqWMPcrMaxVVyXgpJ+/70si+by oAxyucHkPd5cIwunQKtGmlMcMNz49I92hXL+OpkK0x392HyIzLPo5+zKbmAGm5+2 7UWTaduYUx/EpUJzUyDlUzMPuR/9EabdzGaf91zr4+va51UC7SHQT+9Up2xOPiyu pu+pAeFEYc+cnwZTU5puqod0ZUc1KEN7doW2e7M3FV85DUB38RXJm7icmZYCx3hu SMVug5CnOdGteavb+IlH =VyKw -----END PGP SIGNATURE----- --=-=-=--