[PATCH] staging: panel: fix regression in lcd_write

[Bastien Armand]

This patch fix a regression in lcd_write caused by commit
70a8c3eb8546cefe40fb0bc7991e8899b7b91075

Signed-off-by: Bastien Armand 
---
 drivers/staging/panel/panel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 136671a..acd4b8e 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -1324,7 +1324,7 @@ static ssize_t lcd_write(struct file *file,
   that need some CPU */
schedule();

-   if (get_user(c, buf))
+   if (get_user(c, tmp))
return -EFAULT;

lcd_write_char(c);
--
1.8.5.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 2/2] staging: panel: fix sparse warnings in lcd_write

[Bastien Armand]

On Tue, Apr 22, 2014 at 01:01:45PM +0300, Dan Carpenter wrote:
> Out of curiosity, have you tested this patch?
> 
> On Fri, Apr 18, 2014 at 06:10:57PM +0200, Bastien Armand wrote:
> > This patch fixes two sparse warnings related to lcd_write :
> > warning: incorrect type in argument 1 (different address spaces)
> > warning: incorrect type in initializer (incompatible argument 2 
> > (different address spaces))
> > 
> > lcd_write can be used from kernel space (in panel_lcd_print) or from user
> > space. So we introduce the lcd_write_char function that will write a char to
> > the device. We modify lcd_write and panel_lcd_print to use it. Finally we 
> > add
> > __user annotation missing in lcd_write.
> > 
> > 
> > Signed-off-by: Bastien Armand 
> > ---
> >  drivers/staging/panel/panel.c |  205 
> > ++---
> >  1 file changed, 109 insertions(+), 96 deletions(-)
> > 
> > diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
> > index 1569e26..dc34254 100644
> > --- a/drivers/staging/panel/panel.c
> > +++ b/drivers/staging/panel/panel.c
> > @@ -1217,111 +1217,113 @@ static inline int handle_lcd_special_code(void)
> > return processed;
> >  }
> >  ...
> 
> This is buggy.  You are getting the first character over and over.  You
> should be doing:
> 
>   if (get_user(c, tmp))
>   return -EFAULT;
> 
Hi,

You're totally right. I am sorry to have missed that... As the patch has
been accepted in linux-next. I will send a correction right away.

> Btw, this whole function is terrible.  It should be reading larger
> chunks at once instead of get_user() for each character.
> 
> 

The aim of my patch was basically to add __user annotation. I tried to
keep the change minimal to lessen the risk of regression (it seems I have
failed here...). Perhaps, I can write an other patch to change that.

> > ...
> >  static void panel_lcd_print(const char *s)
> >  {
> > -   if (lcd_enabled && lcd_initialized)
> > -   lcd_write(NULL, s, strlen(s), NULL);
> > +   const char *tmp = s;
> > +   int count = strlen(s);
> > +
> > +   if (lcd_enabled && lcd_initialized) {
> > +   for (; count-- > 0; tmp++) {
> > +   if (!in_interrupt() && (((count + 1) & 0x1f) == 0))
> > +   /* let's be a little nice with other processes
> > +  that need some CPU */
> > +   schedule();
> 
> This schedule() isn't needed.  It just prints a line or two at start up
> and some other debug output or something.  Small small.
> 

I hesitated to remove it. I leave it here as it was allready in lcd_write.
Perhaps, I could send another patch to remove it.

> regards,
> dan carpenter
> 

Thanks for your review. It was really appreciated.

Regards,
bastien armand

> > +
> > +   lcd_write_char(*tmp);
> > +   }
> > +   }
> >  }
> > 
> >  /* initialize the LCD driver */
> > --
> > 1.7.10.4
> > 
> > ___
> > devel mailing list
> > de...@linuxdriverproject.org
> > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/2] staging: panel: fix sparse warnings in lcd_write

[Bastien Armand]

This patch fixes two sparse warnings related to lcd_write :
warning: incorrect type in argument 1 (different address spaces)
warning: incorrect type in initializer (incompatible argument 2 
(different address spaces))

lcd_write can be used from kernel space (in panel_lcd_print) or from user
space. So we introduce the lcd_write_char function that will write a char to
the device. We modify lcd_write and panel_lcd_print to use it. Finally we add
__user annotation missing in lcd_write.


Signed-off-by: Bastien Armand 
---
 drivers/staging/panel/panel.c |  205 ++---
 1 file changed, 109 insertions(+), 96 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 1569e26..dc34254 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -1217,111 +1217,113 @@ static inline int handle_lcd_special_code(void)
return processed;
 }

+static void lcd_write_char(char c)
+{
+   /* first, we'll test if we're in escape mode */
+   if ((c != '\n') && lcd_escape_len >= 0) {
+   /* yes, let's add this char to the buffer */
+   lcd_escape[lcd_escape_len++] = c;
+   lcd_escape[lcd_escape_len] = 0;
+   } else {
+   /* aborts any previous escape sequence */
+   lcd_escape_len = -1;
+
+   switch (c) {
+   case LCD_ESCAPE_CHAR:
+   /* start of an escape sequence */
+   lcd_escape_len = 0;
+   lcd_escape[lcd_escape_len] = 0;
+   break;
+   case '\b':
+   /* go back one char and clear it */
+   if (lcd_addr_x > 0) {
+   /* check if we're not at the
+  end of the line */
+   if (lcd_addr_x < lcd_bwidth)
+   /* back one char */
+   lcd_write_cmd(0x10);
+   lcd_addr_x--;
+   }
+   /* replace with a space */
+   lcd_write_data(' ');
+   /* back one char again */
+   lcd_write_cmd(0x10);
+   break;
+   case '\014':
+   /* quickly clear the display */
+   lcd_clear_fast();
+   break;
+   case '\n':
+   /* flush the remainder of the current line and
+  go to the beginning of the next line */
+   for (; lcd_addr_x < lcd_bwidth; lcd_addr_x++)
+   lcd_write_data(' ');
+   lcd_addr_x = 0;
+   lcd_addr_y = (lcd_addr_y + 1) % lcd_height;
+   lcd_gotoxy();
+   break;
+   case '\r':
+   /* go to the beginning of the same line */
+   lcd_addr_x = 0;
+   lcd_gotoxy();
+   break;
+   case '\t':
+   /* print a space instead of the tab */
+   lcd_print(' ');
+   break;
+   default:
+   /* simply print this char */
+   lcd_print(c);
+   break;
+   }
+   }
+
+   /* now we'll see if we're in an escape mode and if the current
+  escape sequence can be understood. */
+   if (lcd_escape_len >= 2) {
+   int processed = 0;
+
+   if (!strcmp(lcd_escape, "[2J")) {
+   /* clear the display */
+   lcd_clear_fast();
+   processed = 1;
+   } else if (!strcmp(lcd_escape, "[H")) {
+   /* cursor to home */
+   lcd_addr_x = lcd_addr_y = 0;
+   lcd_gotoxy();
+   processed = 1;
+   }
+   /* codes starting with ^[[L */
+   else if ((lcd_escape_len >= 3) &&
+(lcd_escape[0] == '[') &&
+(lcd_escape[1] == 'L')) {
+   processed = handle_lcd_special_code();
+   }
+
+   /* LCD special escape codes */
+   /* flush the escape sequence if it's been processed
+  or if it is getting too long. */
+   if (processed || (lcd_escape_len >= LCD_ESCAPE_LEN))
+   lcd_escape_len = -1;
+   } /* escape codes */
+}
+
 static ssize_t lcd_write(struct file *file,
-   

[PATCH v2 1/2] staging: panel: fix sparse warnings in keypad_read

[Bastien Armand]

This patch fixes two sparse warnings related to keypad_read :
warning: incorrect type in argument 1 (different address spaces)
warning: incorrect type in initializer (incompatible argument 2 (different 
address spaces))

Signed-off-by: Bastien Armand 
---
 drivers/staging/panel/panel.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 08f9a48..1569e26 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -1571,11 +1571,11 @@ static void lcd_init(void)
  */

 static ssize_t keypad_read(struct file *file,
-  char *buf, size_t count, loff_t *ppos)
+  char __user *buf, size_t count, loff_t *ppos)
 {

unsigned i = *ppos;
-   char *tmp = buf;
+   char __user *tmp = buf;

if (keypad_buflen == 0) {
if (file->f_flags & O_NONBLOCK)
--
1.7.10.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 0/2] staging: panel: fix sparse warnings.

[Bastien Armand]

This serie of two patch fix the following sparse warnings in panel.c :
panel.c:1235:26: warning: incorrect type in argument 1 (different address 
spaces)
panel.c:1235:26:expected void const volatile [noderef] *
panel.c:1235:26:got char const *tmp
panel.c:1353:20: warning: incorrect type in initializer (incompatible argument 
2 (different address spaces))
panel.c:1353:20:expected int ( *write )( ... )
panel.c:1353:20:got int ( static [toplevel] * )( ... )
panel.c:1591:17: warning: incorrect type in argument 1 (different address 
spaces)
panel.c:1591:17:expected void const volatile [noderef] *
panel.c:1591:17:got char *tmp
panel.c:1620:20: warning: incorrect type in initializer (incompatible argument 
2 (different address spaces))
panel.c:1620:20:expected int ( *read )( ... )
panel.c:1620:20:got int ( static [toplevel] * )( ... )

Changes since v1 :
- splitted patch in two

Bastien Armand (2):
  staging: panel: fix sparse warnings in keypad_read
  staging: panel: fix sparse warnings in lcd_write

 drivers/staging/panel/panel.c |  209 ++---
 1 file changed, 111 insertions(+), 98 deletions(-)

--
1.7.10.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: panel: fix sparse warnings

[Bastien Armand]

On Thu, Apr 17, 2014 at 11:15:10PM -0700, Greg Kroah-Hartman wrote:
> On Fri, Apr 18, 2014 at 08:01:22AM +0200, Bastien Armand wrote:
> > This patch fixes sparse warnings in panel.c.
> 
> What sparse warnings were fixed here?
> 
> It looks like you did a lot of different things, can you please break
> this out into "one patch per logical change" so it can be reviewed
> easier?
> 
> thanks,
> 
> greg k-h

Hi,

Thank you for this return. I will split this patch, document it a little
more and submit it again.

take care.
bastien
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: panel: fix sparse warnings

[Bastien Armand]

This patch fixes sparse warnings in panel.c.


Signed-off-by: Bastien Armand 
---
 drivers/staging/panel/panel.c |  209 ++---
 1 file changed, 111 insertions(+), 98 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 08f9a48..dc34254 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -1217,111 +1217,113 @@ static inline int handle_lcd_special_code(void)
return processed;
 }

+static void lcd_write_char(char c)
+{
+   /* first, we'll test if we're in escape mode */
+   if ((c != '\n') && lcd_escape_len >= 0) {
+   /* yes, let's add this char to the buffer */
+   lcd_escape[lcd_escape_len++] = c;
+   lcd_escape[lcd_escape_len] = 0;
+   } else {
+   /* aborts any previous escape sequence */
+   lcd_escape_len = -1;
+
+   switch (c) {
+   case LCD_ESCAPE_CHAR:
+   /* start of an escape sequence */
+   lcd_escape_len = 0;
+   lcd_escape[lcd_escape_len] = 0;
+   break;
+   case '\b':
+   /* go back one char and clear it */
+   if (lcd_addr_x > 0) {
+   /* check if we're not at the
+  end of the line */
+   if (lcd_addr_x < lcd_bwidth)
+   /* back one char */
+   lcd_write_cmd(0x10);
+   lcd_addr_x--;
+   }
+   /* replace with a space */
+   lcd_write_data(' ');
+   /* back one char again */
+   lcd_write_cmd(0x10);
+   break;
+   case '\014':
+   /* quickly clear the display */
+   lcd_clear_fast();
+   break;
+   case '\n':
+   /* flush the remainder of the current line and
+  go to the beginning of the next line */
+   for (; lcd_addr_x < lcd_bwidth; lcd_addr_x++)
+   lcd_write_data(' ');
+   lcd_addr_x = 0;
+   lcd_addr_y = (lcd_addr_y + 1) % lcd_height;
+   lcd_gotoxy();
+   break;
+   case '\r':
+   /* go to the beginning of the same line */
+   lcd_addr_x = 0;
+   lcd_gotoxy();
+   break;
+   case '\t':
+   /* print a space instead of the tab */
+   lcd_print(' ');
+   break;
+   default:
+   /* simply print this char */
+   lcd_print(c);
+   break;
+   }
+   }
+
+   /* now we'll see if we're in an escape mode and if the current
+  escape sequence can be understood. */
+   if (lcd_escape_len >= 2) {
+   int processed = 0;
+
+   if (!strcmp(lcd_escape, "[2J")) {
+   /* clear the display */
+   lcd_clear_fast();
+   processed = 1;
+   } else if (!strcmp(lcd_escape, "[H")) {
+   /* cursor to home */
+   lcd_addr_x = lcd_addr_y = 0;
+   lcd_gotoxy();
+   processed = 1;
+   }
+   /* codes starting with ^[[L */
+   else if ((lcd_escape_len >= 3) &&
+(lcd_escape[0] == '[') &&
+(lcd_escape[1] == 'L')) {
+   processed = handle_lcd_special_code();
+   }
+
+   /* LCD special escape codes */
+   /* flush the escape sequence if it's been processed
+  or if it is getting too long. */
+   if (processed || (lcd_escape_len >= LCD_ESCAPE_LEN))
+   lcd_escape_len = -1;
+   } /* escape codes */
+}
+
 static ssize_t lcd_write(struct file *file,
-const char *buf, size_t count, loff_t *ppos)
+   const char __user *buf, size_t count, loff_t *ppos)
 {
-   const char *tmp = buf;
+   const char __user *tmp = buf;
char c;

-   for (; count-- > 0; (ppos ? (*ppos)++ : 0), ++tmp) {
+   for (; count-- > 0; (*ppos)++, tmp++) {
if (!in_interrupt() && (((count + 1) & 0x1f) == 0))
/* let's be a lit

[PATCH v2] staging: panel: add blank lines after declarations

[Bastien Armand]

This patch fixes "Missing a blank line after declarations" checkpatch warnings 
in panel.c.

Signed-off-by: Bastien Armand 
---
 drivers/staging/panel/panel.c |   15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 08f9a48..7b390ea 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -867,6 +867,7 @@ static void lcd_print(char c)
 static void lcd_clear_fast_s(void)
 {
int pos;
+
lcd_addr_x = lcd_addr_y = 0;
lcd_gotoxy();

@@ -887,6 +888,7 @@ static void lcd_clear_fast_s(void)
 static void lcd_clear_fast_p8(void)
 {
int pos;
+
lcd_addr_x = lcd_addr_y = 0;
lcd_gotoxy();

@@ -922,6 +924,7 @@ static void lcd_clear_fast_p8(void)
 static void lcd_clear_fast_tilcd(void)
 {
int pos;
+
lcd_addr_x = lcd_addr_y = 0;
lcd_gotoxy();

@@ -1092,6 +1095,7 @@ static inline int handle_lcd_special_code(void)
break;
case 'k': { /* kill end of line */
int x;
+
for (x = lcd_addr_x; x < lcd_bwidth; x++)
lcd_write_data(' ');

@@ -1747,16 +1751,20 @@ static inline int input_state_high(struct logical_input 
*input)

if (input->high_timer == 0) {
char *press_str = input->u.kbd.press_str;
+
if (press_str[0]) {
int s = sizeof(input->u.kbd.press_str);
+
keypad_send_key(press_str, s);
}
}

if (input->u.kbd.repeat_str[0]) {
char *repeat_str = input->u.kbd.repeat_str;
+
if (input->high_timer >= KEYPAD_REP_START) {
int s = sizeof(input->u.kbd.repeat_str);
+
input->high_timer -= KEYPAD_REP_DELAY;
keypad_send_key(repeat_str, s);
}
@@ -1794,8 +1802,10 @@ static inline void input_state_falling(struct 
logical_input *input)

if (input->u.kbd.repeat_str[0]) {
char *repeat_str = input->u.kbd.repeat_str;
+
if (input->high_timer >= KEYPAD_REP_START) {
int s = sizeof(input->u.kbd.repeat_str);
+
input->high_timer -= KEYPAD_REP_DELAY;
keypad_send_key(repeat_str, s);
}
@@ -1811,12 +1821,15 @@ static inline void input_state_falling(struct 
logical_input *input)
/* call release event */
if (input->type == INPUT_TYPE_STD) {
void (*release_fct)(int) = input->u.std.release_fct;
+
if (release_fct != NULL)
release_fct(input->u.std.release_data);
} else if (input->type == INPUT_TYPE_KBD) {
char *release_str = input->u.kbd.release_str;
+
if (release_str[0]) {
int s = sizeof(input->u.kbd.release_str);
+
keypad_send_key(release_str, s);
}
}
@@ -1933,6 +1946,7 @@ static int input_name2mask(const char *name, pmask_t 
*mask, pmask_t *value,
om = im = m = v = 0ULL;
while (*name) {
int in, out, bit, neg;
+
for (in = 0; (in < sizeof(sigtab)) &&
 (sigtab[in] != *name); in++)
;
@@ -2040,6 +2054,7 @@ static struct logical_input *panel_bind_callback(char 
*name,
 static void keypad_init(void)
 {
int keynum;
+
init_waitqueue_head(&keypad_read_wait);
keypad_buflen = 0;  /* flushes any eventual noisy keystroke */

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel