Thread (3 messages) 3 messages, 2 authors, 2008-06-25

Re: [PATCH][1/2] add new Cobalt LCD framebuffer driver

From: Yoichi Yuasa <hidden>
Date: 2008-06-25 13:47:38
Also in: linux-mips

On Tue, 24 Jun 2008 14:15:15 -0700
Andrew Morton [off-list ref] wrote:
On Tue, 24 Jun 2008 22:46:54 +0900
Yoichi Yuasa [off-list ref] wrote:
quoted
Add new Cobalt LCD framebuffer driver.

Signed-off-by: Yoichi Yuasa <redacted>


...

+static ssize_t cobalt_lcdfb_read(struct fb_info *info, char __user *buf,
+				 size_t count, loff_t *ppos)
+{
+	char src[LCD_CHARS_MAX];
+	unsigned long pos;
+	int len, retval;
+
+	pos = *ppos;
+	if (pos >= LCD_CHARS_MAX)
+		return 0;
+
+	if (pos + count >= LCD_CHARS_MAX)
+		count = LCD_CHARS_MAX - pos;
I think if sizeof(pos) == sizeof(count), and `count' is sufficiently
large (eg: 0xffffffff) then bad things will happen in this function.
quoted
+	for (len = 0; len < count; len++) {
+		retval = lcd_busy_wait(info);
+		if (retval < 0)
+			break;
+
+		lcd_write_control(info, LCD_TEXT_POS(pos));
+
+		retval = lcd_busy_wait(info);
+		if (retval < 0)
+			break;
+
+		src[len] = lcd_read_data(info);
+		if (pos == 0x0f)
+			pos = 0x40;
+		else
+			pos++;
+	}
+
+	if (copy_to_user(buf, src, len))
+		return -EFAULT;
+
+	*ppos += len;
+
+	return len;
+}
+
+static ssize_t cobalt_lcdfb_write(struct fb_info *info, const char __user *buf,
+				  size_t count, loff_t *ppos)
+{
+	char dst[LCD_CHARS_MAX];
+	unsigned long pos;
+	int len, retval;
+
+	pos = *ppos;
+	if (pos >= LCD_CHARS_MAX)
+		return 0;
+
+	if (pos + count >= LCD_CHARS_MAX)
+		count = LCD_CHARS_MAX - pos;
Ditto.
quoted
+	if (copy_from_user(dst, buf, count))
+		return -EFAULT;
+
+	for (len = 0; len < count; len++) {
+		retval = lcd_busy_wait(info);
+		if (retval < 0)
+			break;
+
+		lcd_write_control(info, LCD_TEXT_POS(pos));
+
+		retval = lcd_busy_wait(info);
+		if (retval < 0)
+			break;
+
+		lcd_write_data(info, dst[len]);
+		if (pos == 0x0f)
+			pos = 0x40;
+		else
+			pos++;
+	}
+
+	*ppos += len;
+
+	return len;
+}
Is there any real benefit in this handling of signal_pending()?  afaict
it is done correctly, but why did we bother doing it?
Thank you for your comments.
I'll update it.

Yoichi

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help