linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Heikki Orsila <heikki.orsila@iki.fi>
To: Tomi Valkeinen <tomi.valkeinen@nokia.com>
Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org
Subject: [RFC PATCH] OMAP: DSS2: RFBI driver: Cleanup "kfifo is full" test
Date: Mon, 14 Dec 2009 03:18:16 +0000	[thread overview]
Message-ID: <20091214031816.GA1829@zakalwe.fi> (raw)

RFBI_CMD_FIFO_LEN_BYTES might not be a power-of-two,
but kfifo() allocates the size up to the next power of two,
which makes (RFBI_CMD_FIFO_LEN_BYTES - kfifo_len())
look very dubious (possibly a negative value!).

Notes:

[1] Not tested nor compiled here

[2] The removed code compares int with a size_t.
It looks buggy, because if the full allocated
size of the fifo is used, the integer comparison
leads to "always empty space in the fifo".
Fortunately, the code can never go there, because
available = 0 will come before negative integer
values.

[3] Do we want to use the whole allocated space?
In that case the change should be:

-               available = RFBI_CMD_FIFO_LEN_BYTES -
-                       __kfifo_len(rfbi.cmd_fifo);
+               available = rfbi.cmd_fifo->size - __kfifo_len(rfbi.cmd_fifo);

Signed-off-by: Heikki Orsila <heikki.orsila@iki.fi>
---
 drivers/video/omap2/dss/rfbi.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index d0b3006..1dbe993 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -1050,14 +1050,11 @@ static void rfbi_push_cmd(struct update_param *p)
 
 	while (1) {
 		unsigned long flags;
-		int available;
 
 		spin_lock_irqsave(rfbi.cmd_fifo->lock, flags);
-		available = RFBI_CMD_FIFO_LEN_BYTES -
-			__kfifo_len(rfbi.cmd_fifo);
 
 /*		DSSDBG("%d bytes left in fifo\n", available); */
-		if (available < sizeof(struct update_param)) {
+		if (__kfifo_len(rfbi.cmd_fifo) >= RFBI_CMD_FIFO_LEN_BYTES) {
 			DSSDBG("Going to wait because FIFO FULL..\n");
 			spin_unlock_irqrestore(rfbi.cmd_fifo->lock, flags);
 			atomic_inc(&rfbi.cmd_fifo_full);
-- 
1.6.4.4

             reply	other threads:[~2009-12-14  3:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-14  3:18 Heikki Orsila [this message]
2010-01-07 12:06 ` [RFC PATCH] OMAP: DSS2: RFBI driver: Cleanup "kfifo is full" Tomi Valkeinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091214031816.GA1829@zakalwe.fi \
    --to=heikki.orsila@iki.fi \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=tomi.valkeinen@nokia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).