Re: [PATCH 2/2] fbcon: expose cursor blink interval via sysfs

Re: [PATCH 2/2] fbcon: expose cursor blink interval via sysfs

[Scot Doyle]

On Wed, 25 Feb 2015, Pavel Machek wrote:
> On Mon 2015-01-26 20:41:53, Scot Doyle wrote:
> > The fbcon cursor, when set to blink, is hardcoded to toggle display state
> > five times per second. Expose this setting via
> > /sys/class/graphics/fbcon/cursor_blink_ms
> > 
> > Values written to the interface set the approximate time interval in
> > milliseconds between cursor toggles, from 1 to 32767. Since the interval
> > is stored internally as a number of jiffies, the millisecond value read
> > from the interface may not exactly match the entered value.
> > 
> > An outstanding blink timer is reset after a new value is entered.
> > 
> > If the cursor blink is disabled, either via the 'cursor_blink' boolean
> > setting or some other mechanism, the 'cursor_blink_ms' setting may still
> > be modified. The new value will be used if the blink is reactivated.
> > 
> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> 
> Normally, this would be set by ansi escape sequences, no? We can hide
> cursor using them, set its appearance.. makes sense to change timing
> value there, too....
> 									Pavel

Hi Pavel, what about something like this? For example,
"echo -e '\033[16;500]' would set the blink interval to 500 milliseconds.

The duration is stored twice to avoid locking the console in
cursor_timer_handler().


diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 6e00572..f117966 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -135,6 +135,7 @@ const struct consw *conswitchp;
  */
 #define DEFAULT_BELL_PITCH	750
 #define DEFAULT_BELL_DURATION	(HZ/8)
+#define DEFAULT_CURSOR_BLINK_MS	200
 
 struct vc vc_cons [MAX_NR_CONSOLES];
 
@@ -1590,6 +1591,13 @@ static void setterm_command(struct vc_data *vc)
 		case 15: /* activate the previous console */
 			set_console(last_console);
 			break;
+		case 16: /* set cursor blink duration in msec */
+			if (vc->vc_npar >= 1 && vc->vc_par[1] > 0 &&
+					vc->vc_par[1] <= USHRT_MAX)
+				vc->vc_cur_blink_ms = vc->vc_par[1];
+			else
+				vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
+			break;
 	}
 }
 
@@ -1717,6 +1725,7 @@ static void reset_terminal(struct vc_data *vc, int do_clear)
 
 	vc->vc_bell_pitch = DEFAULT_BELL_PITCH;
 	vc->vc_bell_duration = DEFAULT_BELL_DURATION;
+	vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
 
 	gotoxy(vc, 0, 0);
 	save_cur(vc);
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index b972106..05b1d1a 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -402,7 +402,7 @@ static void cursor_timer_handler(unsigned long dev_addr)
 	struct fbcon_ops *ops = info->fbcon_par;
 
 	queue_work(system_power_efficient_wq, &info->queue);
-	mod_timer(&ops->cursor_timer, jiffies + HZ/5);
+	mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
 }
 
 static void fbcon_add_cursor_timer(struct fb_info *info)
@@ -417,7 +417,7 @@ static void fbcon_add_cursor_timer(struct fb_info *info)
 
 		init_timer(&ops->cursor_timer);
 		ops->cursor_timer.function = cursor_timer_handler;
-		ops->cursor_timer.expires = jiffies + HZ / 5;
+		ops->cursor_timer.expires = jiffies + ops->cur_blink_jiffies;
 		ops->cursor_timer.data = (unsigned long ) info;
 		add_timer(&ops->cursor_timer);
 		ops->flags |= FBCON_FLAGS_CURSOR_TIMER;
@@ -1309,9 +1309,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
 	if (fbcon_is_inactive(vc, info) || vc->vc_deccm != 1)
 		return;
 
-	if (vc->vc_cursor_type & 0x10)
-		fbcon_del_cursor_timer(info);
-	else
+	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
+	fbcon_del_cursor_timer(info);
+	if (!(vc->vc_cursor_type & 0x10))
 		fbcon_add_cursor_timer(info);
 
 	ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
diff --git a/drivers/video/console/fbcon.h b/drivers/video/console/fbcon.h
index 6bd2e0c..7aaa4ea 100644
--- a/drivers/video/console/fbcon.h
+++ b/drivers/video/console/fbcon.h
@@ -70,6 +70,7 @@ struct fbcon_ops {
 	struct fb_cursor cursor_state;
 	struct display *p;
         int    currcon;	                /* Current VC. */
+	int    cur_blink_jiffies;
 	int    cursor_flash;
 	int    cursor_reset;
 	int    blank_state;
diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index e859c98..e329ee2 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -104,6 +104,7 @@ struct vc_data {
 	unsigned int    vc_resize_user;         /* resize request from user */
 	unsigned int	vc_bell_pitch;		/* Console bell pitch */
 	unsigned int	vc_bell_duration;	/* Console bell duration */
+	unsigned short	vc_cur_blink_ms;	/* Cursor blink duration */
 	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
 	struct uni_pagedir *vc_uni_pagedir;
 	struct uni_pagedir **vc_uni_pagedir_loc; /* [!] Location of uni_pagedir variable for this console */

Re: [PATCH 2/2] fbcon: expose cursor blink interval via sysfs

[Pavel Machek]

On Wed 2015-02-25 23:32:00, Scot Doyle wrote:
> On Wed, 25 Feb 2015, Pavel Machek wrote:
> > On Mon 2015-01-26 20:41:53, Scot Doyle wrote:
> > > The fbcon cursor, when set to blink, is hardcoded to toggle display state
> > > five times per second. Expose this setting via
> > > /sys/class/graphics/fbcon/cursor_blink_ms
> > > 
> > > Values written to the interface set the approximate time interval in
> > > milliseconds between cursor toggles, from 1 to 32767. Since the interval
> > > is stored internally as a number of jiffies, the millisecond value read
> > > from the interface may not exactly match the entered value.
> > > 
> > > An outstanding blink timer is reset after a new value is entered.
> > > 
> > > If the cursor blink is disabled, either via the 'cursor_blink' boolean
> > > setting or some other mechanism, the 'cursor_blink_ms' setting may still
> > > be modified. The new value will be used if the blink is reactivated.
> > > 
> > > Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> > 
> > Normally, this would be set by ansi escape sequences, no? We can hide
> > cursor using them, set its appearance.. makes sense to change timing
> > value there, too....
> > 									Pavel
> 
> Hi Pavel, what about something like this? For example,
> "echo -e '\033[16;500]' would set the blink interval to 500 milliseconds.
> 
> The duration is stored twice to avoid locking the console in
> cursor_timer_handler().

Yes, I'd say this matches the existing code better.

Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

> +		case 16: /* set cursor blink duration in msec */
> +			if (vc->vc_npar >= 1 && vc->vc_par[1] > 0 &&
> +					vc->vc_par[1] <= USHRT_MAX)
> +				vc->vc_cur_blink_ms = vc->vc_par[1];
> +			else

Actually, vc_cur_blink_ms less then about 50 probably does not make
sense (and may overload the system). Should that be checked?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 0/2] add cursor blink interval terminal escape sequence

[Scot Doyle]

Greg, the first patch of this series is for the tty tree.

Tomi, the second patch of this series is for your tree, but it depends on
the first patch. Also, will you remove these two previously queued patches?
"fbcon: store cursor blink interval in fbcon_ops"
"fbcon: expose cursor blink interval via sysfs"

Michael, I plan to send a documentation patch if these are accepted.

This patch series adds an escape sequence to specify the current console's
cursor blink interval. The default interval is set to fbcon's currently 
hardcoded 200 msecs.

Scot Doyle (2):
  vt: add cursor blink interval escape sequence
  fbcon: use the cursor blink interval provided by vt

 drivers/tty/vt/vt.c            |  9 +++++++++
 drivers/video/console/fbcon.c  | 10 +++++-----
 drivers/video/console/fbcon.h  |  1 +
 include/linux/console_struct.h |  1 +
 4 files changed, 16 insertions(+), 5 deletions(-)

-- 
2.3.0

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] vt: add cursor blink interval escape sequence

[Scot Doyle]

Add an escape sequence to specify the current console's cursor blink
interval. The interval is specified as a number of milliseconds until
the next cursor display state toggle, from 50 to 65535. /proc/loadavg
did not show a difference with a one msec interval, but the lower
bound is set to 50 msecs since slower hardware wasn't tested.

Store the interval in the vc_data structure for later access by fbcon,
initializing the value to fbcon's current hardcoded value of 200 msecs.

Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/tty/vt/vt.c            | 9 +++++++++
 include/linux/console_struct.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 6e00572..ab1f173 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -135,6 +135,7 @@ const struct consw *conswitchp;
  */
 #define DEFAULT_BELL_PITCH	750
 #define DEFAULT_BELL_DURATION	(HZ/8)
+#define DEFAULT_CURSOR_BLINK_MS	200
 
 struct vc vc_cons [MAX_NR_CONSOLES];
 
@@ -1590,6 +1591,13 @@ static void setterm_command(struct vc_data *vc)
 		case 15: /* activate the previous console */
 			set_console(last_console);
 			break;
+		case 16: /* set cursor blink duration in msec */
+			if (vc->vc_npar >= 1 && vc->vc_par[1] >= 50 &&
+					vc->vc_par[1] <= USHRT_MAX)
+				vc->vc_cur_blink_ms = vc->vc_par[1];
+			else
+				vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
+			break;
 	}
 }
 
@@ -1717,6 +1725,7 @@ static void reset_terminal(struct vc_data *vc, int do_clear)
 
 	vc->vc_bell_pitch = DEFAULT_BELL_PITCH;
 	vc->vc_bell_duration = DEFAULT_BELL_DURATION;
+	vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
 
 	gotoxy(vc, 0, 0);
 	save_cur(vc);
diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index e859c98..e329ee2 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -104,6 +104,7 @@ struct vc_data {
 	unsigned int    vc_resize_user;         /* resize request from user */
 	unsigned int	vc_bell_pitch;		/* Console bell pitch */
 	unsigned int	vc_bell_duration;	/* Console bell duration */
+	unsigned short	vc_cur_blink_ms;	/* Cursor blink duration */
 	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
 	struct uni_pagedir *vc_uni_pagedir;
 	struct uni_pagedir **vc_uni_pagedir_loc; /* [!] Location of uni_pagedir variable for this console */
-- 
2.3.0

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] vt: add cursor blink interval escape sequence

[Scot Doyle]

On Fri, 27 Feb 2015, Scot Doyle wrote:
> Add an escape sequence to specify the current console's cursor blink
> interval. The interval is specified as a number of milliseconds until
> the next cursor display state toggle, from 50 to 65535. /proc/loadavg
> did not show a difference with a one msec interval, but the lower
> bound is set to 50 msecs since slower hardware wasn't tested.
> 
> Store the interval in the vc_data structure for later access by fbcon,
> initializing the value to fbcon's current hardcoded value of 200 msecs.
> 
> Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

Hi Greg, sorry about your backlog. Is it too soon for a ping?

> ---
>  drivers/tty/vt/vt.c            | 9 +++++++++
>  include/linux/console_struct.h | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 6e00572..ab1f173 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -135,6 +135,7 @@ const struct consw *conswitchp;
>   */
>  #define DEFAULT_BELL_PITCH	750
>  #define DEFAULT_BELL_DURATION	(HZ/8)
> +#define DEFAULT_CURSOR_BLINK_MS	200
>  
>  struct vc vc_cons [MAX_NR_CONSOLES];
>  
> @@ -1590,6 +1591,13 @@ static void setterm_command(struct vc_data *vc)
>  		case 15: /* activate the previous console */
>  			set_console(last_console);
>  			break;
> +		case 16: /* set cursor blink duration in msec */
> +			if (vc->vc_npar >= 1 && vc->vc_par[1] >= 50 &&
> +					vc->vc_par[1] <= USHRT_MAX)
> +				vc->vc_cur_blink_ms = vc->vc_par[1];
> +			else
> +				vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
> +			break;
>  	}
>  }
>  
> @@ -1717,6 +1725,7 @@ static void reset_terminal(struct vc_data *vc, int do_clear)
>  
>  	vc->vc_bell_pitch = DEFAULT_BELL_PITCH;
>  	vc->vc_bell_duration = DEFAULT_BELL_DURATION;
> +	vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
>  
>  	gotoxy(vc, 0, 0);
>  	save_cur(vc);
> diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
> index e859c98..e329ee2 100644
> --- a/include/linux/console_struct.h
> +++ b/include/linux/console_struct.h
> @@ -104,6 +104,7 @@ struct vc_data {
>  	unsigned int    vc_resize_user;         /* resize request from user */
>  	unsigned int	vc_bell_pitch;		/* Console bell pitch */
>  	unsigned int	vc_bell_duration;	/* Console bell duration */
> +	unsigned short	vc_cur_blink_ms;	/* Cursor blink duration */
>  	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
>  	struct uni_pagedir *vc_uni_pagedir;
>  	struct uni_pagedir **vc_uni_pagedir_loc; /* [!] Location of uni_pagedir variable for this console */
> -- 
> 2.3.0
> 

Re: [PATCH 1/2] vt: add cursor blink interval escape sequence

[Greg Kroah-Hartman]

On Fri, Feb 27, 2015 at 07:13:48PM +0000, Scot Doyle wrote:
> Add an escape sequence to specify the current console's cursor blink
> interval. The interval is specified as a number of milliseconds until
> the next cursor display state toggle, from 50 to 65535. /proc/loadavg
> did not show a difference with a one msec interval, but the lower
> bound is set to 50 msecs since slower hardware wasn't tested.
> 
> Store the interval in the vc_data structure for later access by fbcon,
> initializing the value to fbcon's current hardcoded value of 200 msecs.
> 
> Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
>  drivers/tty/vt/vt.c            | 9 +++++++++
>  include/linux/console_struct.h | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 6e00572..ab1f173 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -135,6 +135,7 @@ const struct consw *conswitchp;
>   */
>  #define DEFAULT_BELL_PITCH	750
>  #define DEFAULT_BELL_DURATION	(HZ/8)
> +#define DEFAULT_CURSOR_BLINK_MS	200
>  
>  struct vc vc_cons [MAX_NR_CONSOLES];
>  
> @@ -1590,6 +1591,13 @@ static void setterm_command(struct vc_data *vc)
>  		case 15: /* activate the previous console */
>  			set_console(last_console);
>  			break;
> +		case 16: /* set cursor blink duration in msec */

Where is this now documented?  Is this a "standard" ASCII command
somewhere?  Adding new userspace apis have to be documented properly.

Without that, I can't take this series, sorry.

greg k-h

[PATCH v2 0/3] add cursor blink interval terminal escape sequence

[Scot Doyle]

v2: Add documentation to console_codes man page (man-pages repo)

This patch series adds an escape sequence to specify the current console's
cursor blink interval. The default interval is set to fbcon's currently 
hardcoded 200 msecs.

Scot Doyle (3):
  vt: add cursor blink interval escape sequence
  fbcon: use the cursor blink interval provided by vt
  console_codes.4: Add CSI sequence for cursor blink interval

 drivers/tty/vt/vt.c            |  9 +++++++++
 drivers/video/console/fbcon.c  | 10 +++++-----
 drivers/video/console/fbcon.h  |  1 +
 include/linux/console_struct.h |  1 +
 man4/console_codes.4           |  1 +
 5 files changed, 17 insertions(+), 5 deletions(-)

-- 
2.1.0

[PATCH v2 1/3] vt: add cursor blink interval escape sequence

[Scot Doyle]

Add an escape sequence to specify the current console's cursor blink
interval. The interval is specified as a number of milliseconds until
the next cursor display state toggle, from 50 to 65535. /proc/loadavg
did not show a difference with a one msec interval, but the lower
bound is set to 50 msecs since slower hardware wasn't tested.

Store the interval in the vc_data structure for later access by fbcon,
initializing the value to fbcon's current hardcoded value of 200 msecs.

Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/tty/vt/vt.c            | 9 +++++++++
 include/linux/console_struct.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 6e00572..ab1f173 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -135,6 +135,7 @@ const struct consw *conswitchp;
  */
 #define DEFAULT_BELL_PITCH	750
 #define DEFAULT_BELL_DURATION	(HZ/8)
+#define DEFAULT_CURSOR_BLINK_MS	200
 
 struct vc vc_cons [MAX_NR_CONSOLES];
 
@@ -1590,6 +1591,13 @@ static void setterm_command(struct vc_data *vc)
 		case 15: /* activate the previous console */
 			set_console(last_console);
 			break;
+		case 16: /* set cursor blink duration in msec */
+			if (vc->vc_npar >= 1 && vc->vc_par[1] >= 50 &&
+					vc->vc_par[1] <= USHRT_MAX)
+				vc->vc_cur_blink_ms = vc->vc_par[1];
+			else
+				vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
+			break;
 	}
 }
 
@@ -1717,6 +1725,7 @@ static void reset_terminal(struct vc_data *vc, int do_clear)
 
 	vc->vc_bell_pitch = DEFAULT_BELL_PITCH;
 	vc->vc_bell_duration = DEFAULT_BELL_DURATION;
+	vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
 
 	gotoxy(vc, 0, 0);
 	save_cur(vc);
diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index e859c98..e329ee2 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -104,6 +104,7 @@ struct vc_data {
 	unsigned int    vc_resize_user;         /* resize request from user */
 	unsigned int	vc_bell_pitch;		/* Console bell pitch */
 	unsigned int	vc_bell_duration;	/* Console bell duration */
+	unsigned short	vc_cur_blink_ms;	/* Cursor blink duration */
 	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
 	struct uni_pagedir *vc_uni_pagedir;
 	struct uni_pagedir **vc_uni_pagedir_loc; /* [!] Location of uni_pagedir variable for this console */
-- 
2.1.0

Re: [PATCH v2 1/3] vt: add cursor blink interval escape sequence

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

On 26 Mar 2015 13:54, Scot Doyle wrote:
> Add an escape sequence to specify the current console's cursor blink
> interval. The interval is specified as a number of milliseconds until
> the next cursor display state toggle, from 50 to 65535. /proc/loadavg
> did not show a difference with a one msec interval, but the lower
> bound is set to 50 msecs since slower hardware wasn't tested.

if they want to be crazy, why not let them ?  it's not like we generally prevent 
the user from destroying their machine.  i.e. just require the value to be > 0 
(unless you want to let 0 disable things).
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 1/3] vt: add cursor blink interval escape sequence

[Pavel Machek]

On Fri 2015-03-27 20:35:03, Mike Frysinger wrote:
> On 26 Mar 2015 13:54, Scot Doyle wrote:
> > Add an escape sequence to specify the current console's cursor blink
> > interval. The interval is specified as a number of milliseconds until
> > the next cursor display state toggle, from 50 to 65535. /proc/loadavg
> > did not show a difference with a one msec interval, but the lower
> > bound is set to 50 msecs since slower hardware wasn't tested.
> 
> if they want to be crazy, why not let them ?  it's not like we generally prevent 
> the user from destroying their machine.  i.e. just require the value to be > 0 
> (unless you want to let 0 disable things).

Because anyone with access to console can do this, and we only allow root
users to destroy the machine.
									Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Scot Doyle]

vt now provides a cursor blink interval via vc_data. Use this
interval instead of the currently hardcoded 200 msecs. Store it in
fbcon_ops to avoid locking the console in cursor_timer_handler().

Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/video/console/fbcon.c | 10 +++++-----
 drivers/video/console/fbcon.h |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index b972106..05b1d1a 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -402,7 +402,7 @@ static void cursor_timer_handler(unsigned long dev_addr)
 	struct fbcon_ops *ops = info->fbcon_par;
 
 	queue_work(system_power_efficient_wq, &info->queue);
-	mod_timer(&ops->cursor_timer, jiffies + HZ/5);
+	mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
 }
 
 static void fbcon_add_cursor_timer(struct fb_info *info)
@@ -417,7 +417,7 @@ static void fbcon_add_cursor_timer(struct fb_info *info)
 
 		init_timer(&ops->cursor_timer);
 		ops->cursor_timer.function = cursor_timer_handler;
-		ops->cursor_timer.expires = jiffies + HZ / 5;
+		ops->cursor_timer.expires = jiffies + ops->cur_blink_jiffies;
 		ops->cursor_timer.data = (unsigned long ) info;
 		add_timer(&ops->cursor_timer);
 		ops->flags |= FBCON_FLAGS_CURSOR_TIMER;
@@ -1309,9 +1309,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
 	if (fbcon_is_inactive(vc, info) || vc->vc_deccm != 1)
 		return;
 
-	if (vc->vc_cursor_type & 0x10)
-		fbcon_del_cursor_timer(info);
-	else
+	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
+	fbcon_del_cursor_timer(info);
+	if (!(vc->vc_cursor_type & 0x10))
 		fbcon_add_cursor_timer(info);
 
 	ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
diff --git a/drivers/video/console/fbcon.h b/drivers/video/console/fbcon.h
index 6bd2e0c..7aaa4ea 100644
--- a/drivers/video/console/fbcon.h
+++ b/drivers/video/console/fbcon.h
@@ -70,6 +70,7 @@ struct fbcon_ops {
 	struct fb_cursor cursor_state;
 	struct display *p;
         int    currcon;	                /* Current VC. */
+	int    cur_blink_jiffies;
 	int    cursor_flash;
 	int    cursor_reset;
 	int    blank_state;
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Kevin Hilman]

On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org> wrote:
> vt now provides a cursor blink interval via vc_data. Use this
> interval instead of the currently hardcoded 200 msecs. Store it in
> fbcon_ops to avoid locking the console in cursor_timer_handler().
>
> Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

This patch hit next-20150519 in the form of commit 27a4c827c34a
(fbcon: use the cursor blink interval provided by vt) and has caused
boot failure on a handful of ARM platforms when booting a MMC root
filesystem.  This error was spotted by the kernelci.org bot on
exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
some tegra platforms too.

Thierry spotted this commit as a potential cause, and both Daniel and
I have reverted and boot tested on exynos5 and tegra respectively and
the boot panics disappear.

Kevin

[1] http://storage.kernelci.org/next/next-20150519/arm-exynos_defconfig/lab-khilman/boot-exynos5800-peach-pi_rootfs:mmc.html

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1845 bytes --]

On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org> wrote:
> > vt now provides a cursor blink interval via vc_data. Use this
> > interval instead of the currently hardcoded 200 msecs. Store it in
> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> >
> > Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> 
> This patch hit next-20150519 in the form of commit 27a4c827c34a
> (fbcon: use the cursor blink interval provided by vt) and has caused
> boot failure on a handful of ARM platforms when booting a MMC root
> filesystem.  This error was spotted by the kernelci.org bot on
> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> some tegra platforms too.
> 
> Thierry spotted this commit as a potential cause, and both Daniel and
> I have reverted and boot tested on exynos5 and tegra respectively and
> the boot panics disappear.

FWIW, if I apply the below on top of next-20150519 things seem to be
back to normal as well:

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 05b1d1a71ef9..658c34bb9076 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
                return;
 
        ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-       fbcon_del_cursor_timer(info);
-       if (!(vc->vc_cursor_type & 0x10))
+       if (vc->vc_cursor_type & 0x10)
+               fbcon_del_cursor_timer(info);
+       else
                fbcon_add_cursor_timer(info);
 
        ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Kevin Hilman]

On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
>> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14@scotdoyle.com> wrote:
>> > vt now provides a cursor blink interval via vc_data. Use this
>> > interval instead of the currently hardcoded 200 msecs. Store it in
>> > fbcon_ops to avoid locking the console in cursor_timer_handler().
>> >
>> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
>> > Acked-by: Pavel Machek <pavel@ucw.cz>
>>
>> This patch hit next-20150519 in the form of commit 27a4c827c34a
>> (fbcon: use the cursor blink interval provided by vt) and has caused
>> boot failure on a handful of ARM platforms when booting a MMC root
>> filesystem.  This error was spotted by the kernelci.org bot on
>> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
>> some tegra platforms too.
>>
>> Thierry spotted this commit as a potential cause, and both Daniel and
>> I have reverted and boot tested on exynos5 and tegra respectively and
>> the boot panics disappear.
>
> FWIW, if I apply the below on top of next-20150519 things seem to be
> back to normal as well:
>
> diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> index 05b1d1a71ef9..658c34bb9076 100644
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
>                 return;
>
>         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> -       fbcon_del_cursor_timer(info);
> -       if (!(vc->vc_cursor_type & 0x10))
> +       if (vc->vc_cursor_type & 0x10)
> +               fbcon_del_cursor_timer(info);
> +       else
>                 fbcon_add_cursor_timer(info);
>
>         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;

Applying this on next-20150519 makes my exynos board happily boot again as well.

Tested-by: Kevin Hilman <khilman@linaro.org>

Kevin

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2722 bytes --]

On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> >> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org> wrote:
> >> > vt now provides a cursor blink interval via vc_data. Use this
> >> > interval instead of the currently hardcoded 200 msecs. Store it in
> >> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> >> >
> >> > Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> >> > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> >>
> >> This patch hit next-20150519 in the form of commit 27a4c827c34a
> >> (fbcon: use the cursor blink interval provided by vt) and has caused
> >> boot failure on a handful of ARM platforms when booting a MMC root
> >> filesystem.  This error was spotted by the kernelci.org bot on
> >> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> >> some tegra platforms too.
> >>
> >> Thierry spotted this commit as a potential cause, and both Daniel and
> >> I have reverted and boot tested on exynos5 and tegra respectively and
> >> the boot panics disappear.
> >
> > FWIW, if I apply the below on top of next-20150519 things seem to be
> > back to normal as well:
> >
> > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > index 05b1d1a71ef9..658c34bb9076 100644
> > --- a/drivers/video/console/fbcon.c
> > +++ b/drivers/video/console/fbcon.c
> > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> >                 return;
> >
> >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > -       fbcon_del_cursor_timer(info);
> > -       if (!(vc->vc_cursor_type & 0x10))
> > +       if (vc->vc_cursor_type & 0x10)
> > +               fbcon_del_cursor_timer(info);
> > +       else
> >                 fbcon_add_cursor_timer(info);
> >
> >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> 
> Applying this on next-20150519 makes my exynos board happily boot again as well.
> 
> Tested-by: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Excellent. Greg, Scot, any opinions on whether or not this is the right
thing to do? It restores a bit that looks suspiciously like it snuck in
in the original (at least it isn't documented in the commit message).

Greg, feel free to squash this in if everybody agrees this is good to
go. If you prefer a patch on top let me know and I'll come up with a
proper commit message.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Greg Kroah-Hartman]

On Tue, May 19, 2015 at 11:52:29PM +0200, Thierry Reding wrote:
> On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > <thierry.reding@gmail.com> wrote:
> > > On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> > >> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14@scotdoyle.com> wrote:
> > >> > vt now provides a cursor blink interval via vc_data. Use this
> > >> > interval instead of the currently hardcoded 200 msecs. Store it in
> > >> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> > >> >
> > >> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> > >> > Acked-by: Pavel Machek <pavel@ucw.cz>
> > >>
> > >> This patch hit next-20150519 in the form of commit 27a4c827c34a
> > >> (fbcon: use the cursor blink interval provided by vt) and has caused
> > >> boot failure on a handful of ARM platforms when booting a MMC root
> > >> filesystem.  This error was spotted by the kernelci.org bot on
> > >> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> > >> some tegra platforms too.
> > >>
> > >> Thierry spotted this commit as a potential cause, and both Daniel and
> > >> I have reverted and boot tested on exynos5 and tegra respectively and
> > >> the boot panics disappear.
> > >
> > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > back to normal as well:
> > >
> > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > index 05b1d1a71ef9..658c34bb9076 100644
> > > --- a/drivers/video/console/fbcon.c
> > > +++ b/drivers/video/console/fbcon.c
> > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > >                 return;
> > >
> > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > -       fbcon_del_cursor_timer(info);
> > > -       if (!(vc->vc_cursor_type & 0x10))
> > > +       if (vc->vc_cursor_type & 0x10)
> > > +               fbcon_del_cursor_timer(info);
> > > +       else
> > >                 fbcon_add_cursor_timer(info);
> > >
> > >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> > 
> > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > 
> > Tested-by: Kevin Hilman <khilman@linaro.org>
> 
> Excellent. Greg, Scot, any opinions on whether or not this is the right
> thing to do? It restores a bit that looks suspiciously like it snuck in
> in the original (at least it isn't documented in the commit message).
> 
> Greg, feel free to squash this in if everybody agrees this is good to
> go. If you prefer a patch on top let me know and I'll come up with a
> proper commit message.

Please send a real patch and I'll apply it on top, as I can't rebase my
public tree.

thanks,

greg k-h

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 3182 bytes --]

On Tue, May 19, 2015 at 04:41:12PM -0700, Greg Kroah-Hartman wrote:
> On Tue, May 19, 2015 at 11:52:29PM +0200, Thierry Reding wrote:
> > On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > > <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > > On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> > > >> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org> wrote:
> > > >> > vt now provides a cursor blink interval via vc_data. Use this
> > > >> > interval instead of the currently hardcoded 200 msecs. Store it in
> > > >> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> > > >> >
> > > >> > Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> > > >> > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> > > >>
> > > >> This patch hit next-20150519 in the form of commit 27a4c827c34a
> > > >> (fbcon: use the cursor blink interval provided by vt) and has caused
> > > >> boot failure on a handful of ARM platforms when booting a MMC root
> > > >> filesystem.  This error was spotted by the kernelci.org bot on
> > > >> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> > > >> some tegra platforms too.
> > > >>
> > > >> Thierry spotted this commit as a potential cause, and both Daniel and
> > > >> I have reverted and boot tested on exynos5 and tegra respectively and
> > > >> the boot panics disappear.
> > > >
> > > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > > back to normal as well:
> > > >
> > > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > > index 05b1d1a71ef9..658c34bb9076 100644
> > > > --- a/drivers/video/console/fbcon.c
> > > > +++ b/drivers/video/console/fbcon.c
> > > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > > >                 return;
> > > >
> > > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > > -       fbcon_del_cursor_timer(info);
> > > > -       if (!(vc->vc_cursor_type & 0x10))
> > > > +       if (vc->vc_cursor_type & 0x10)
> > > > +               fbcon_del_cursor_timer(info);
> > > > +       else
> > > >                 fbcon_add_cursor_timer(info);
> > > >
> > > >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> > > 
> > > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > > 
> > > Tested-by: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > 
> > Excellent. Greg, Scot, any opinions on whether or not this is the right
> > thing to do? It restores a bit that looks suspiciously like it snuck in
> > in the original (at least it isn't documented in the commit message).
> > 
> > Greg, feel free to squash this in if everybody agrees this is good to
> > go. If you prefer a patch on top let me know and I'll come up with a
> > proper commit message.
> 
> Please send a real patch and I'll apply it on top, as I can't rebase my
> public tree.

Attached.

Thierry

[-- Attachment #1.2: 0001-fbcon-Avoid-deleting-a-timer-in-IRQ-context.patch --]
[-- Type: text/x-diff, Size: 3447 bytes --]

From 4f2f70dbbe9de54c0da9b03a1f384e1464755eab Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Date: Wed, 20 May 2015 13:41:52 +0200
Subject: [PATCH] fbcon: Avoid deleting a timer in IRQ context

Commit 27a4c827c34a ("fbcon: use the cursor blink interval provided by
vt") unconditionally removes the cursor blink timer. Unfortunately that
wreaks havoc under some circumstances. An easily reproducible way is to
use both the framebuffer console and a debug serial port as the console
output for kernel messages (e.g. "console=ttyS0 console=tty1" on the
kernel command-line. Upon boot this triggers a warning from within the
del_timer_sync() function because it is called from IRQ context:

	[    5.070096] ------------[ cut here ]------------
	[    5.070110] WARNING: CPU: 0 PID: 0 at ../kernel/time/timer.c:1098 del_timer_sync+0x4c/0x54()
	[    5.070115] Modules linked in:
	[    5.070120] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.1.0-rc4-next-20150519 #1
	[    5.070123] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
	[    5.070142] [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
	[    5.070156] [] (show_stack) from [] (dump_stack+0x70/0xbc)
	[    5.070164] [] (dump_stack) from [] (warn_slowpath_common+0x74/0xb0)
	[    5.070169] [] (warn_slowpath_common) from [] (warn_slowpath_null+0x1c/0x24)
	[    5.070174] [] (warn_slowpath_null) from [] (del_timer_sync+0x4c/0x54)
	[    5.070183] [] (del_timer_sync) from [] (fbcon_del_cursor_timer+0x2c/0x40)
	[    5.070190] [] (fbcon_del_cursor_timer) from [] (fbcon_cursor+0x9c/0x180)
	[    5.070198] [] (fbcon_cursor) from [] (hide_cursor+0x30/0x98)
	[    5.070204] [] (hide_cursor) from [] (vt_console_print+0x2a8/0x340)
	[    5.070212] [] (vt_console_print) from [] (call_console_drivers.constprop.23+0xc8/0xec)
	[    5.070218] [] (call_console_drivers.constprop.23) from [] (console_unlock+0x498/0x4f0)
	[    5.070223] [] (console_unlock) from [] (vprintk_emit+0x1f0/0x508)
	[    5.070228] [] (vprintk_emit) from [] (vprintk_default+0x24/0x2c)
	[    5.070234] [] (vprintk_default) from [] (printk+0x70/0x88)

After which the system starts spewing all kinds of weird and seemingly
unrelated error messages.

This commit fixes this by restoring the condition under which the call
to fbcon_del_cursor_timer() happens.

Reported-by: Daniel Stone <daniel-rLtY4a/8tF1rovVCs/uTlw@public.gmane.org>
Reported-by: Kevin Hilman <khilman-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Tested-by: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Tested-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/video/console/fbcon.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 05b1d1a71ef9..658c34bb9076 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
 		return;
 
 	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-	fbcon_del_cursor_timer(info);
-	if (!(vc->vc_cursor_type & 0x10))
+	if (vc->vc_cursor_type & 0x10)
+		fbcon_del_cursor_timer(info);
+	else
 		fbcon_add_cursor_timer(info);
 
 	ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
-- 
2.4.1


[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Greg Kroah-Hartman]

On Wed, May 20, 2015 at 02:36:17PM +0200, Thierry Reding wrote:
> On Tue, May 19, 2015 at 04:41:12PM -0700, Greg Kroah-Hartman wrote:
> > On Tue, May 19, 2015 at 11:52:29PM +0200, Thierry Reding wrote:
> > > On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > > > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > > > <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > > > On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> > > > >> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org> wrote:
> > > > >> > vt now provides a cursor blink interval via vc_data. Use this
> > > > >> > interval instead of the currently hardcoded 200 msecs. Store it in
> > > > >> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> > > > >> >
> > > > >> > Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> > > > >> > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> > > > >>
> > > > >> This patch hit next-20150519 in the form of commit 27a4c827c34a
> > > > >> (fbcon: use the cursor blink interval provided by vt) and has caused
> > > > >> boot failure on a handful of ARM platforms when booting a MMC root
> > > > >> filesystem.  This error was spotted by the kernelci.org bot on
> > > > >> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> > > > >> some tegra platforms too.
> > > > >>
> > > > >> Thierry spotted this commit as a potential cause, and both Daniel and
> > > > >> I have reverted and boot tested on exynos5 and tegra respectively and
> > > > >> the boot panics disappear.
> > > > >
> > > > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > > > back to normal as well:
> > > > >
> > > > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > > > index 05b1d1a71ef9..658c34bb9076 100644
> > > > > --- a/drivers/video/console/fbcon.c
> > > > > +++ b/drivers/video/console/fbcon.c
> > > > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > > > >                 return;
> > > > >
> > > > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > > > -       fbcon_del_cursor_timer(info);
> > > > > -       if (!(vc->vc_cursor_type & 0x10))
> > > > > +       if (vc->vc_cursor_type & 0x10)
> > > > > +               fbcon_del_cursor_timer(info);
> > > > > +       else
> > > > >                 fbcon_add_cursor_timer(info);
> > > > >
> > > > >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> > > > 
> > > > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > > > 
> > > > Tested-by: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > 
> > > Excellent. Greg, Scot, any opinions on whether or not this is the right
> > > thing to do? It restores a bit that looks suspiciously like it snuck in
> > > in the original (at least it isn't documented in the commit message).
> > > 
> > > Greg, feel free to squash this in if everybody agrees this is good to
> > > go. If you prefer a patch on top let me know and I'll come up with a
> > > proper commit message.
> > 
> > Please send a real patch and I'll apply it on top, as I can't rebase my
> > public tree.
> 
> Attached.

Ugh, no, please resend it as a stand-alone patch, I can't easily apply
attachments.

thanks,

greg k-h

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3731 bytes --]

On Wed, May 20, 2015 at 09:26:38PM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 20, 2015 at 02:36:17PM +0200, Thierry Reding wrote:
> > On Tue, May 19, 2015 at 04:41:12PM -0700, Greg Kroah-Hartman wrote:
> > > On Tue, May 19, 2015 at 11:52:29PM +0200, Thierry Reding wrote:
> > > > On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > > > > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > > > > <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > > > > On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> > > > > >> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14@scotdoyle.com> wrote:
> > > > > >> > vt now provides a cursor blink interval via vc_data. Use this
> > > > > >> > interval instead of the currently hardcoded 200 msecs. Store it in
> > > > > >> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> > > > > >> >
> > > > > >> > Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> > > > > >> > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> > > > > >>
> > > > > >> This patch hit next-20150519 in the form of commit 27a4c827c34a
> > > > > >> (fbcon: use the cursor blink interval provided by vt) and has caused
> > > > > >> boot failure on a handful of ARM platforms when booting a MMC root
> > > > > >> filesystem.  This error was spotted by the kernelci.org bot on
> > > > > >> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> > > > > >> some tegra platforms too.
> > > > > >>
> > > > > >> Thierry spotted this commit as a potential cause, and both Daniel and
> > > > > >> I have reverted and boot tested on exynos5 and tegra respectively and
> > > > > >> the boot panics disappear.
> > > > > >
> > > > > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > > > > back to normal as well:
> > > > > >
> > > > > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > > > > index 05b1d1a71ef9..658c34bb9076 100644
> > > > > > --- a/drivers/video/console/fbcon.c
> > > > > > +++ b/drivers/video/console/fbcon.c
> > > > > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > > > > >                 return;
> > > > > >
> > > > > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > > > > -       fbcon_del_cursor_timer(info);
> > > > > > -       if (!(vc->vc_cursor_type & 0x10))
> > > > > > +       if (vc->vc_cursor_type & 0x10)
> > > > > > +               fbcon_del_cursor_timer(info);
> > > > > > +       else
> > > > > >                 fbcon_add_cursor_timer(info);
> > > > > >
> > > > > >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> > > > > 
> > > > > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > > > > 
> > > > > Tested-by: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > > 
> > > > Excellent. Greg, Scot, any opinions on whether or not this is the right
> > > > thing to do? It restores a bit that looks suspiciously like it snuck in
> > > > in the original (at least it isn't documented in the commit message).
> > > > 
> > > > Greg, feel free to squash this in if everybody agrees this is good to
> > > > go. If you prefer a patch on top let me know and I'll come up with a
> > > > proper commit message.
> > > 
> > > Please send a real patch and I'll apply it on top, as I can't rebase my
> > > public tree.
> > 
> > Attached.
> 
> Ugh, no, please resend it as a stand-alone patch, I can't easily apply
> attachments.

Really? Your MUA can't dissect multipart messages? Anyway, sent
separately for your convenience.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Greg Kroah-Hartman]

On Thu, May 21, 2015 at 10:00:50AM +0200, Thierry Reding wrote:
> On Wed, May 20, 2015 at 09:26:38PM -0700, Greg Kroah-Hartman wrote:
> > On Wed, May 20, 2015 at 02:36:17PM +0200, Thierry Reding wrote:
> > > On Tue, May 19, 2015 at 04:41:12PM -0700, Greg Kroah-Hartman wrote:
> > > > On Tue, May 19, 2015 at 11:52:29PM +0200, Thierry Reding wrote:
> > > > > On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > > > > > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > > > > > <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > > > > > On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> > > > > > >> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org> wrote:
> > > > > > >> > vt now provides a cursor blink interval via vc_data. Use this
> > > > > > >> > interval instead of the currently hardcoded 200 msecs. Store it in
> > > > > > >> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> > > > > > >> >
> > > > > > >> > Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> > > > > > >> > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> > > > > > >>
> > > > > > >> This patch hit next-20150519 in the form of commit 27a4c827c34a
> > > > > > >> (fbcon: use the cursor blink interval provided by vt) and has caused
> > > > > > >> boot failure on a handful of ARM platforms when booting a MMC root
> > > > > > >> filesystem.  This error was spotted by the kernelci.org bot on
> > > > > > >> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> > > > > > >> some tegra platforms too.
> > > > > > >>
> > > > > > >> Thierry spotted this commit as a potential cause, and both Daniel and
> > > > > > >> I have reverted and boot tested on exynos5 and tegra respectively and
> > > > > > >> the boot panics disappear.
> > > > > > >
> > > > > > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > > > > > back to normal as well:
> > > > > > >
> > > > > > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > > > > > index 05b1d1a71ef9..658c34bb9076 100644
> > > > > > > --- a/drivers/video/console/fbcon.c
> > > > > > > +++ b/drivers/video/console/fbcon.c
> > > > > > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > > > > > >                 return;
> > > > > > >
> > > > > > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > > > > > -       fbcon_del_cursor_timer(info);
> > > > > > > -       if (!(vc->vc_cursor_type & 0x10))
> > > > > > > +       if (vc->vc_cursor_type & 0x10)
> > > > > > > +               fbcon_del_cursor_timer(info);
> > > > > > > +       else
> > > > > > >                 fbcon_add_cursor_timer(info);
> > > > > > >
> > > > > > >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> > > > > > 
> > > > > > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > > > > > 
> > > > > > Tested-by: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > > > 
> > > > > Excellent. Greg, Scot, any opinions on whether or not this is the right
> > > > > thing to do? It restores a bit that looks suspiciously like it snuck in
> > > > > in the original (at least it isn't documented in the commit message).
> > > > > 
> > > > > Greg, feel free to squash this in if everybody agrees this is good to
> > > > > go. If you prefer a patch on top let me know and I'll come up with a
> > > > > proper commit message.
> > > > 
> > > > Please send a real patch and I'll apply it on top, as I can't rebase my
> > > > public tree.
> > > 
> > > Attached.
> > 
> > Ugh, no, please resend it as a stand-alone patch, I can't easily apply
> > attachments.
> 
> Really? Your MUA can't dissect multipart messages? Anyway, sent
> separately for your convenience.

"git am" doesn't do that.  I apply patches in huge chunks of mbox files.

Remember, if I have to hand-edit, or do something special with your
patch, I will not do it, you need to do it correctly to make
maintainer's lives easier, not harder, given that maintainers are the
limited resouce, not developers.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4661 bytes --]

On Thu, May 21, 2015 at 07:00:31PM -0700, Greg Kroah-Hartman wrote:
> On Thu, May 21, 2015 at 10:00:50AM +0200, Thierry Reding wrote:
> > On Wed, May 20, 2015 at 09:26:38PM -0700, Greg Kroah-Hartman wrote:
> > > On Wed, May 20, 2015 at 02:36:17PM +0200, Thierry Reding wrote:
> > > > On Tue, May 19, 2015 at 04:41:12PM -0700, Greg Kroah-Hartman wrote:
> > > > > On Tue, May 19, 2015 at 11:52:29PM +0200, Thierry Reding wrote:
> > > > > > On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > > > > > > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > > > > > > <thierry.reding@gmail.com> wrote:
> > > > > > > > On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> > > > > > > >> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14@scotdoyle.com> wrote:
> > > > > > > >> > vt now provides a cursor blink interval via vc_data. Use this
> > > > > > > >> > interval instead of the currently hardcoded 200 msecs. Store it in
> > > > > > > >> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> > > > > > > >> >
> > > > > > > >> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> > > > > > > >> > Acked-by: Pavel Machek <pavel@ucw.cz>
> > > > > > > >>
> > > > > > > >> This patch hit next-20150519 in the form of commit 27a4c827c34a
> > > > > > > >> (fbcon: use the cursor blink interval provided by vt) and has caused
> > > > > > > >> boot failure on a handful of ARM platforms when booting a MMC root
> > > > > > > >> filesystem.  This error was spotted by the kernelci.org bot on
> > > > > > > >> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> > > > > > > >> some tegra platforms too.
> > > > > > > >>
> > > > > > > >> Thierry spotted this commit as a potential cause, and both Daniel and
> > > > > > > >> I have reverted and boot tested on exynos5 and tegra respectively and
> > > > > > > >> the boot panics disappear.
> > > > > > > >
> > > > > > > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > > > > > > back to normal as well:
> > > > > > > >
> > > > > > > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > > > > > > index 05b1d1a71ef9..658c34bb9076 100644
> > > > > > > > --- a/drivers/video/console/fbcon.c
> > > > > > > > +++ b/drivers/video/console/fbcon.c
> > > > > > > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > > > > > > >                 return;
> > > > > > > >
> > > > > > > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > > > > > > -       fbcon_del_cursor_timer(info);
> > > > > > > > -       if (!(vc->vc_cursor_type & 0x10))
> > > > > > > > +       if (vc->vc_cursor_type & 0x10)
> > > > > > > > +               fbcon_del_cursor_timer(info);
> > > > > > > > +       else
> > > > > > > >                 fbcon_add_cursor_timer(info);
> > > > > > > >
> > > > > > > >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> > > > > > > 
> > > > > > > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > > > > > > 
> > > > > > > Tested-by: Kevin Hilman <khilman@linaro.org>
> > > > > > 
> > > > > > Excellent. Greg, Scot, any opinions on whether or not this is the right
> > > > > > thing to do? It restores a bit that looks suspiciously like it snuck in
> > > > > > in the original (at least it isn't documented in the commit message).
> > > > > > 
> > > > > > Greg, feel free to squash this in if everybody agrees this is good to
> > > > > > go. If you prefer a patch on top let me know and I'll come up with a
> > > > > > proper commit message.
> > > > > 
> > > > > Please send a real patch and I'll apply it on top, as I can't rebase my
> > > > > public tree.
> > > > 
> > > > Attached.
> > > 
> > > Ugh, no, please resend it as a stand-alone patch, I can't easily apply
> > > attachments.
> > 
> > Really? Your MUA can't dissect multipart messages? Anyway, sent
> > separately for your convenience.
> 
> "git am" doesn't do that.  I apply patches in huge chunks of mbox files.

What I frequently end up doing is apply patches straight from mutt by
piping the mail or an attached patch to git am. I guess I had expected
that you'd have something similar to simplify applying patches.

> Remember, if I have to hand-edit, or do something special with your
> patch, I will not do it, you need to do it correctly to make
> maintainer's lives easier, not harder, given that maintainers are the
> limited resouce, not developers.

I understand. I'll make a mental note to never send you patches as
attachment again.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Arnd Bergmann]

On Friday 22 May 2015 12:00:03 Thierry Reding wrote:
> 
> > Remember, if I have to hand-edit, or do something special with your
> > patch, I will not do it, you need to do it correctly to make
> > maintainer's lives easier, not harder, given that maintainers are the
> > limited resouce, not developers.
> 
> I understand. I'll make a mental note to never send you patches as
> attachment again.
> 

Better make that a general rule. My workflow is different from Greg's
but also doesn't cope well with attachments. A lot of people in turn
have problems quoting from an attachment when replying to the patch,
which happens to work for me.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]

On Fri, May 22, 2015 at 12:33:42PM +0200, Arnd Bergmann wrote:
> On Friday 22 May 2015 12:00:03 Thierry Reding wrote:
> > 
> > > Remember, if I have to hand-edit, or do something special with your
> > > patch, I will not do it, you need to do it correctly to make
> > > maintainer's lives easier, not harder, given that maintainers are the
> > > limited resouce, not developers.
> > 
> > I understand. I'll make a mental note to never send you patches as
> > attachment again.
> > 
> 
> Better make that a general rule. My workflow is different from Greg's
> but also doesn't cope well with attachments. A lot of people in turn
> have problems quoting from an attachment when replying to the patch,
> which happens to work for me.

Okay. Any hints on how to simplify sending out such patches with the
same list of recipients? I find it very annoying to have to manually
copy each recipient into the git send-email command-line, but I don't
know of a better way to do it. Replying to an email from the MUA will
at least do that automatically.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Silvan Jegen]

On Fri, May 22, 2015 at 1:49 PM, Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, May 22, 2015 at 12:33:42PM +0200, Arnd Bergmann wrote:
>> On Friday 22 May 2015 12:00:03 Thierry Reding wrote:
>> >
>> > > Remember, if I have to hand-edit, or do something special with your
>> > > patch, I will not do it, you need to do it correctly to make
>> > > maintainer's lives easier, not harder, given that maintainers are the
>> > > limited resouce, not developers.
>> >
>> > I understand. I'll make a mental note to never send you patches as
>> > attachment again.
>> >
>>
>> Better make that a general rule. My workflow is different from Greg's
>> but also doesn't cope well with attachments. A lot of people in turn
>> have problems quoting from an attachment when replying to the patch,
>> which happens to work for me.
>
> Okay. Any hints on how to simplify sending out such patches with the
> same list of recipients? I find it very annoying to have to manually
> copy each recipient into the git send-email command-line, but I don't
> know of a better way to do it. Replying to an email from the MUA will
> at least do that automatically.

You can write your recipients to the "To:" header of the patch you
generated using git format-patch. git send-email will pick all
recipients thus specified up automatically. In later iterations you
can just copy the "To:" line.
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Arnd Bergmann]

On Friday 22 May 2015 13:49:58 Thierry Reding wrote:
> On Fri, May 22, 2015 at 12:33:42PM +0200, Arnd Bergmann wrote:
> > On Friday 22 May 2015 12:00:03 Thierry Reding wrote:
> > > 
> > > > Remember, if I have to hand-edit, or do something special with your
> > > > patch, I will not do it, you need to do it correctly to make
> > > > maintainer's lives easier, not harder, given that maintainers are the
> > > > limited resouce, not developers.
> > > 
> > > I understand. I'll make a mental note to never send you patches as
> > > attachment again.
> > > 
> > 
> > Better make that a general rule. My workflow is different from Greg's
> > but also doesn't cope well with attachments. A lot of people in turn
> > have problems quoting from an attachment when replying to the patch,
> > which happens to work for me.
> 
> Okay. Any hints on how to simplify sending out such patches with the
> same list of recipients? I find it very annoying to have to manually
> copy each recipient into the git send-email command-line, but I don't
> know of a better way to do it. Replying to an email from the MUA will
> at least do that automatically.

You can have a line starting with 8<------ (the scissors  symbol) after
your reply, and then paste the patch below.

I usually use 'git show --format=email | xclip' to copy the patch into
the X clipboard and paste it into the email window from there.

	Arnd

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Thierry Reding]

On Fri, May 22, 2015 at 03:24:30PM +0200, Arnd Bergmann wrote:
> On Friday 22 May 2015 13:49:58 Thierry Reding wrote:
> > On Fri, May 22, 2015 at 12:33:42PM +0200, Arnd Bergmann wrote:
> > > On Friday 22 May 2015 12:00:03 Thierry Reding wrote:
> > > > 
> > > > > Remember, if I have to hand-edit, or do something special with your
> > > > > patch, I will not do it, you need to do it correctly to make
> > > > > maintainer's lives easier, not harder, given that maintainers are the
> > > > > limited resouce, not developers.
> > > > 
> > > > I understand. I'll make a mental note to never send you patches as
> > > > attachment again.
> > > > 
> > > 
> > > Better make that a general rule. My workflow is different from Greg's
> > > but also doesn't cope well with attachments. A lot of people in turn
> > > have problems quoting from an attachment when replying to the patch,
> > > which happens to work for me.
> > 
> > Okay. Any hints on how to simplify sending out such patches with the
> > same list of recipients? I find it very annoying to have to manually
> > copy each recipient into the git send-email command-line, but I don't
> > know of a better way to do it. Replying to an email from the MUA will
> > at least do that automatically.
> 
> You can have a line starting with 8<------ (the scissors  symbol) after
> your reply, and then paste the patch below.
> 
> I usually use 'git show --format=email | xclip' to copy the patch into
> the X clipboard and paste it into the email window from there.

Cool, that's pretty useful. I should be able to do that without going
through the X clipboard with mutt/vim even.

Thanks,
Thierry

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Greg Kroah-Hartman]

On Fri, May 22, 2015 at 01:49:58PM +0200, Thierry Reding wrote:
> On Fri, May 22, 2015 at 12:33:42PM +0200, Arnd Bergmann wrote:
> > On Friday 22 May 2015 12:00:03 Thierry Reding wrote:
> > > 
> > > > Remember, if I have to hand-edit, or do something special with your
> > > > patch, I will not do it, you need to do it correctly to make
> > > > maintainer's lives easier, not harder, given that maintainers are the
> > > > limited resouce, not developers.
> > > 
> > > I understand. I'll make a mental note to never send you patches as
> > > attachment again.
> > > 
> > 
> > Better make that a general rule. My workflow is different from Greg's
> > but also doesn't cope well with attachments. A lot of people in turn
> > have problems quoting from an attachment when replying to the patch,
> > which happens to work for me.
> 
> Okay. Any hints on how to simplify sending out such patches with the
> same list of recipients? I find it very annoying to have to manually
> copy each recipient into the git send-email command-line, but I don't
> know of a better way to do it. Replying to an email from the MUA will
> at least do that automatically.

Reply from the MUA and then just put the patch in the email body.  If
you have a good MUA it should be trivial to do[1]

thanks,

greg k-h

1) mutt drops you to your editor, and then you can just read in the
   patch file directly to that buffer.

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2000 bytes --]

On Fri, May 22, 2015 at 07:32:05AM -0700, Greg Kroah-Hartman wrote:
> On Fri, May 22, 2015 at 01:49:58PM +0200, Thierry Reding wrote:
> > On Fri, May 22, 2015 at 12:33:42PM +0200, Arnd Bergmann wrote:
> > > On Friday 22 May 2015 12:00:03 Thierry Reding wrote:
> > > > 
> > > > > Remember, if I have to hand-edit, or do something special with your
> > > > > patch, I will not do it, you need to do it correctly to make
> > > > > maintainer's lives easier, not harder, given that maintainers are the
> > > > > limited resouce, not developers.
> > > > 
> > > > I understand. I'll make a mental note to never send you patches as
> > > > attachment again.
> > > > 
> > > 
> > > Better make that a general rule. My workflow is different from Greg's
> > > but also doesn't cope well with attachments. A lot of people in turn
> > > have problems quoting from an attachment when replying to the patch,
> > > which happens to work for me.
> > 
> > Okay. Any hints on how to simplify sending out such patches with the
> > same list of recipients? I find it very annoying to have to manually
> > copy each recipient into the git send-email command-line, but I don't
> > know of a better way to do it. Replying to an email from the MUA will
> > at least do that automatically.
> 
> Reply from the MUA and then just put the patch in the email body.  If
> you have a good MUA it should be trivial to do[1]
> 
> thanks,
> 
> greg k-h
> 
> 1) mutt drops you to your editor, and then you can just read in the
>    patch file directly to that buffer.

Indeed, that should work. As I understand it, I wouldn't even have to
further edit the email (except strip the reply) because git am prefers
headers in the patch to headers in the message (it certainly does that
for From:, so I suspect it would do it for Date: and Subject: as well).
Or if that doesn't work, Arnd's suggestion to use a scissors line is a
good alternative as well.

Thanks guys for the suggestions,
Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt

[Scot Doyle]

On Tue, 19 May 2015, Thierry Reding wrote:
> On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > <thierry.reding@gmail.com> wrote:

...

> > >
> > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > back to normal as well:
> > >
> > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > index 05b1d1a71ef9..658c34bb9076 100644
> > > --- a/drivers/video/console/fbcon.c
> > > +++ b/drivers/video/console/fbcon.c
> > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > >                 return;
> > >
> > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > -       fbcon_del_cursor_timer(info);
> > > -       if (!(vc->vc_cursor_type & 0x10))
> > > +       if (vc->vc_cursor_type & 0x10)
> > > +               fbcon_del_cursor_timer(info);
> > > +       else
> > >                 fbcon_add_cursor_timer(info);
> > >
> > >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> > 
> > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > 
> > Tested-by: Kevin Hilman <khilman@linaro.org>
> 
> Excellent. Greg, Scot, any opinions on whether or not this is the right
> thing to do? It restores a bit that looks suspiciously like it snuck in
> in the original (at least it isn't documented in the commit message).
> 
> Greg, feel free to squash this in if everybody agrees this is good to
> go. If you prefer a patch on top let me know and I'll come up with a
> proper commit message.
> 
> Thierry

Hi all, sorry for the trouble.

The timer delete was to prevent blink stutter when updating the interval. 
Since the stutter isn't so noticable when changing from the default 200ms, 
and since most people seem to prefer leaving the fbcon code alone if 
possible, I agree with Thierry's approach.

Tested-by: Scot Doyle <lkml14@scotdoyle.com>

[PATCH v2 3/3] console_codes.4: Add CSI sequence for cursor blink interval

[Scot Doyle]

Add a Console Private CSI sequence to specify the current console's
cursor blink interval. The interval is specified as a number of
milliseconds until the next cursor display state toggle, from 50 to
65535.

Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
---
 man4/console_codes.4 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/man4/console_codes.4 b/man4/console_codes.4
index 34f7535..7d05076 100644
--- a/man4/console_codes.4
+++ b/man4/console_codes.4
@@ -377,6 +377,7 @@ ESC [ 15 ]      	T{
 Bring the previous console to the front
 (since Linux 2.6.0).
 T}
+ESC [ 16 ; \fIn\fP ]   	Set the cursor blink interval in milliseconds.
 .TE
 .SS Character sets
 The kernel knows about 4 translations of bytes into console-screen
-- 
2.1.0

Re: [PATCH v2 3/3] console_codes.4: Add CSI sequence for cursor blink interval

[Pavel Machek]

On Thu 2015-03-26 13:57:44, Scot Doyle wrote:
> Add a Console Private CSI sequence to specify the current console's
> cursor blink interval. The interval is specified as a number of
> milliseconds until the next cursor display state toggle, from 50 to
> 65535.
> 
> Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>

Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH v2 3/3] console_codes.4: Add CSI sequence for cursor blink interval

[Scot Doyle]

On Thu, 26 Mar 2015, Scot Doyle wrote:
> Add a Console Private CSI sequence to specify the current console's
> cursor blink interval. The interval is specified as a number of
> milliseconds until the next cursor display state toggle, from 50 to
> 65535.
> 
> Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> ---
>  man4/console_codes.4 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/man4/console_codes.4 b/man4/console_codes.4
> index 34f7535..7d05076 100644
> --- a/man4/console_codes.4
> +++ b/man4/console_codes.4
> @@ -377,6 +377,7 @@ ESC [ 15 ]      	T{
>  Bring the previous console to the front
>  (since Linux 2.6.0).
>  T}
> +ESC [ 16 ; \fIn\fP ]   	Set the cursor blink interval in milliseconds.
>  .TE
>  .SS Character sets
>  The kernel knows about 4 translations of bytes into console-screen
> -- 
> 2.1.0
> 

Hi Michael,

Will you apply now that Linus has pulled the rest?
(see bd63364caa8df38bad2b25b11b2a1b849475cce5)

Thank you,
Scot
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] console_codes.4: Add CSI sequence for cursor blink interval

[Michael Kerrisk (man-pages)]

Hello Scot,

On 5 July 2015 at 19:41, Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org> wrote:
> On Thu, 26 Mar 2015, Scot Doyle wrote:
>> Add a Console Private CSI sequence to specify the current console's
>> cursor blink interval. The interval is specified as a number of
>> milliseconds until the next cursor display state toggle, from 50 to
>> 65535.
>>
>> Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>

I've applied this, adding Pavel's Acked-by.

I also added some text to note that this appeared in Linux 4.2. Okay?

Cheers,

Michael


>> ---
>>  man4/console_codes.4 | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/man4/console_codes.4 b/man4/console_codes.4
>> index 34f7535..7d05076 100644
>> --- a/man4/console_codes.4
>> +++ b/man4/console_codes.4
>> @@ -377,6 +377,7 @@ ESC [ 15 ]        T{
>>  Bring the previous console to the front
>>  (since Linux 2.6.0).
>>  T}
>> +ESC [ 16 ; \fIn\fP ]         Set the cursor blink interval in milliseconds.
>>  .TE
>>  .SS Character sets
>>  The kernel knows about 4 translations of bytes into console-screen
>> --
>> 2.1.0
>>
>
> Hi Michael,
>
> Will you apply now that Linus has pulled the rest?
> (see bd63364caa8df38bad2b25b11b2a1b849475cce5)
>
> Thank you,
> Scot



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 3/3] console_codes.4: Add CSI sequence for cursor blink interval

[Scot Doyle]

On Tue, 21 Jul 2015, Michael Kerrisk (man-pages) wrote:
> On 5 July 2015 at 19:41, Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org> wrote:
> > On Thu, 26 Mar 2015, Scot Doyle wrote:
> >> Add a Console Private CSI sequence to specify the current console's
> >> cursor blink interval. The interval is specified as a number of
> >> milliseconds until the next cursor display state toggle, from 50 to
> >> 65535.
> >>
> >> Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
> 
> I've applied this, adding Pavel's Acked-by.
> 
> I also added some text to note that this appeared in Linux 4.2. Okay?

Yes, thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] add cursor blink interval terminal escape sequence

[Pavel Machek]

On Thu 2015-03-26 13:51:04, Scot Doyle wrote:
> v2: Add documentation to console_codes man page (man-pages repo)
> 
> This patch series adds an escape sequence to specify the current console's
> cursor blink interval. The default interval is set to fbcon's currently 
> hardcoded 200 msecs.

Actually... Greg, can you import console_codes.4 into kernel tree somehow?

This will bring documentation for a bunch of currently undocumented kernel
interfaces into the kernel tree... where it belongs.

Thanks,
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH v2 0/3] add cursor blink interval terminal escape sequence

[Greg Kroah-Hartman]

On Sat, Mar 28, 2015 at 08:54:43AM +0100, Pavel Machek wrote:
> On Thu 2015-03-26 13:51:04, Scot Doyle wrote:
> > v2: Add documentation to console_codes man page (man-pages repo)
> > 
> > This patch series adds an escape sequence to specify the current console's
> > cursor blink interval. The default interval is set to fbcon's currently 
> > hardcoded 200 msecs.
> 
> Actually... Greg, can you import console_codes.4 into kernel tree somehow?

Is that file part of the manpages project?  If so, it's fine where it
is, otherwise feel free to send a patch.

thanks,

greg k-h

[PATCH 2/2] fbcon: use the cursor blink interval provided by vt

[Scot Doyle]

vt now provides a cursor blink interval via vc_data. Use this
interval instead of the currently hardcoded 200 msecs. Store it in
fbcon_ops to avoid locking the console in cursor_timer_handler().

Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/video/console/fbcon.c | 10 +++++-----
 drivers/video/console/fbcon.h |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index b972106..05b1d1a 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -402,7 +402,7 @@ static void cursor_timer_handler(unsigned long dev_addr)
 	struct fbcon_ops *ops = info->fbcon_par;
 
 	queue_work(system_power_efficient_wq, &info->queue);
-	mod_timer(&ops->cursor_timer, jiffies + HZ/5);
+	mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
 }
 
 static void fbcon_add_cursor_timer(struct fb_info *info)
@@ -417,7 +417,7 @@ static void fbcon_add_cursor_timer(struct fb_info *info)
 
 		init_timer(&ops->cursor_timer);
 		ops->cursor_timer.function = cursor_timer_handler;
-		ops->cursor_timer.expires = jiffies + HZ / 5;
+		ops->cursor_timer.expires = jiffies + ops->cur_blink_jiffies;
 		ops->cursor_timer.data = (unsigned long ) info;
 		add_timer(&ops->cursor_timer);
 		ops->flags |= FBCON_FLAGS_CURSOR_TIMER;
@@ -1309,9 +1309,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
 	if (fbcon_is_inactive(vc, info) || vc->vc_deccm != 1)
 		return;
 
-	if (vc->vc_cursor_type & 0x10)
-		fbcon_del_cursor_timer(info);
-	else
+	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
+	fbcon_del_cursor_timer(info);
+	if (!(vc->vc_cursor_type & 0x10))
 		fbcon_add_cursor_timer(info);
 
 	ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
diff --git a/drivers/video/console/fbcon.h b/drivers/video/console/fbcon.h
index 6bd2e0c..7aaa4ea 100644
--- a/drivers/video/console/fbcon.h
+++ b/drivers/video/console/fbcon.h
@@ -70,6 +70,7 @@ struct fbcon_ops {
 	struct fb_cursor cursor_state;
 	struct display *p;
         int    currcon;	                /* Current VC. */
+	int    cur_blink_jiffies;
 	int    cursor_flash;
 	int    cursor_reset;
 	int    blank_state;
-- 
2.3.0

Re: [PATCH 2/2] fbcon: use the cursor blink interval provided by vt

[Andrey Wagin]

2015-02-27 22:15 GMT+03:00 Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>:
> vt now provides a cursor blink interval via vc_data. Use this
> interval instead of the currently hardcoded 200 msecs. Store it in
> fbcon_ops to avoid locking the console in cursor_timer_handler().

I regularly execute criu tests on linux-next. For this, I use virtual
machine from the digitalocean clould. The current version of
linux-next hangs after a few seconds. I use git bisect to find the
commit where the problem is appeaed. And it looks like the problem is
in this patch.

When the kernel hangs, it doesn't report anything on the screen and
there is nothing suspicious in logs after reboot.

I will try to reproduce the problem in my local enviroment to get more
information.

There is my config file:
https://github.com/avagin/criu-jenkins-digitalocean/blob/d95d9e30a7da8755c47b290630bac7ee1fe7132d/jenkins-scripts/config

Thanks,
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] fbcon: use the cursor blink interval provided by vt

[Scot Doyle]

On Wed, 27 May 2015, Andrey Wagin wrote:
...
> I regularly execute criu tests on linux-next. For this, I use virtual
> machine from the digitalocean clould. The current version of
> linux-next hangs after a few seconds. I use git bisect to find the
> commit where the problem is appeaed. And it looks like the problem is
> in this patch.
...
> Thanks,
> Andrew

Perhaps this pending patch will help: 
http://marc.info/?l=linux-kernel&m=143219509918969&w=2

Thanks,
Scot

Re: [PATCH 2/2] fbcon: use the cursor blink interval provided by vt

[Andrey Wagin]

2015-05-27 10:52 GMT+03:00 Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>:
> On Wed, 27 May 2015, Andrey Wagin wrote:
> ...
>> I regularly execute criu tests on linux-next. For this, I use virtual
>> machine from the digitalocean clould. The current version of
>> linux-next hangs after a few seconds. I use git bisect to find the
>> commit where the problem is appeaed. And it looks like the problem is
>> in this patch.
> ...
>> Thanks,
>> Andrew
>
> Perhaps this pending patch will help:
> http://marc.info/?l=linux-kernel&m=143219509918969&w=2

Yes, it helps. Thanks.
>
> Thanks,
> Scot
>

Re: [PATCH 0/2] add cursor blink interval terminal escape sequence

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 512 bytes --]

On 27/02/15 21:10, Scot Doyle wrote:
> Greg, the first patch of this series is for the tty tree.
> 
> Tomi, the second patch of this series is for your tree, but it depends on
> the first patch. Also, will you remove these two previously queued patches?
> "fbcon: store cursor blink interval in fbcon_ops"
> "fbcon: expose cursor blink interval via sysfs"

I have dropped those two patches from fbdev tree.

I don't know much about the console side, so I can't comment anything on
that.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]