Thread (8 messages) 8 messages, 3 authors, 2010-04-29

RE: [Linux-fbdev-devel] [PATCH] added S6E63M0 AMOLED LCD Panel driver.

flat view

From: H Hartley Sweeten <hidden>
Date: 2010-03-30 23:26:20
Also in: lkml

On Tuesday, March 30, 2010 4:03 PM, Andrew Morton wrote:
On Fri, 26 Mar 2010 12:24:24 +0900
InKi Dae [off-list ref] wrote:
quoted
+static int s6e63m0_ldi_init(struct s6e63m0 *lcd)
+{
+	int ret;
+
+	ret = s6e63m0_panel_send_sequence(lcd, SEQ_PANEL_CONDITION_SET);
+	ret |= s6e63m0_panel_send_sequence(lcd, SEQ_DISPLAY_CONDITION_SET);
+	ret |= s6e63m0_panel_send_sequence(lcd, SEQ_GAMMA_SETTING);
+	ret |= s6e63m0_panel_send_sequence(lcd, SEQ_ETC_CONDITION_SET);
+	ret |= s6e63m0_panel_send_sequence(lcd, SEQ_ACL_ON);
+	ret |= s6e63m0_panel_send_sequence(lcd, SEQ_ELVSS_ON);
+
+	return ret;
+}
Well.  If one call to s6e63m0_panel_send_sequence() returns -ENOMEM and
another call returns -EIO (for example), this function will return some
other, incorrect errno.

Which is a rather minor problem, unless some caller is explicitly
looking for some particular error code, which doesn't happen often.
Why not handle the calls with a loop?

+static int s6e63m0_ldi_init(struct s6e63m0 *lcd)
+{
+	const unsigned short *init_seq[] = {
+		SEQ_PANEL_CONDITION_SET,
+		SEQ_DISPLAY_CONDITION_SET,
+		SEQ_GAMMA_SETTING,
+		SEQ_ETC_CONDITION_SET,
+		SEQ_ACL_ON,
+		SEQ_ELVSS_ON,
+	};
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
+		ret = s6e63m0_panel_send_sequence(lcd, init_seq[i]);
+		if (ret)
+			break;
+	}
+	return ret;
+}

Note that _s6e63m0_gamma_ctl has the same issue.  Actually, the whole
driver has issues with returning errors properly.

Regards,
Hartley
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help