2015-05-19 21:15:52

by Kevin Hilman

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

On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <[email protected]> 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 <[email protected]>
> Acked-by: Pavel Machek <[email protected]>

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


2015-05-19 21:40:21

by Thierry Reding

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

On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <[email protected]> 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 <[email protected]>
> > Acked-by: Pavel Machek <[email protected]>
>
> 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;


Attachments:
(No filename) (1.69 kB)
(No filename) (819.00 B)
Download all attachments

2015-05-19 21:45:25

by Kevin Hilman

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

On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
<[email protected]> 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 <[email protected]> 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 <[email protected]>
>> > Acked-by: Pavel Machek <[email protected]>
>>
>> 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 <[email protected]>

Kevin

2015-05-19 21:52:38

by Thierry Reding

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

On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >> > Acked-by: Pavel Machek <[email protected]>
> >>
> >> 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 <[email protected]>

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


Attachments:
(No filename) (2.47 kB)
(No filename) (819.00 B)
Download all attachments

2015-05-19 23:41:22

by Greg Kroah-Hartman

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

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
> > <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > >> > Acked-by: Pavel Machek <[email protected]>
> > >>
> > >> 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 <[email protected]>
>
> 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

2015-05-20 00:37:12

by Scot Doyle

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

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
> > <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>

2015-05-20 12:36:25

by Thierry Reding

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

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
> > > <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > >> > Acked-by: Pavel Machek <[email protected]>
> > > >>
> > > >> 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 <[email protected]>
> >
> > 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


Attachments:
(No filename) (0.00 B)
(No filename) (819.00 B)
Download all attachments

2015-05-21 04:26:43

by Greg Kroah-Hartman

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

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
> > > > <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > >> > Acked-by: Pavel Machek <[email protected]>
> > > > >>
> > > > >> 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 <[email protected]>
> > >
> > > 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

2015-05-21 08:00:59

by Thierry Reding

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

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
> > > > > <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > > >> > Acked-by: Pavel Machek <[email protected]>
> > > > > >>
> > > > > >> 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 <[email protected]>
> > > >
> > > > 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


Attachments:
(No filename) (3.47 kB)
(No filename) (819.00 B)
Download all attachments

2015-05-22 02:00:38

by Greg Kroah-Hartman

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

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
> > > > > > <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > > > >> > Acked-by: Pavel Machek <[email protected]>
> > > > > > >>
> > > > > > >> 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 <[email protected]>
> > > > >
> > > > > 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

2015-05-22 10:00:11

by Thierry Reding

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

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
> > > > > > > <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > > > > >> > Acked-by: Pavel Machek <[email protected]>
> > > > > > > >>
> > > > > > > >> 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 <[email protected]>
> > > > > >
> > > > > > 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


Attachments:
(No filename) (4.47 kB)
(No filename) (819.00 B)
Download all attachments

2015-05-22 10:36:35

by Arnd Bergmann

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

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

2015-05-22 11:50:06

by Thierry Reding

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

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


Attachments:
(No filename) (1.03 kB)
(No filename) (819.00 B)
Download all attachments

2015-05-22 13:07:34

by Silvan Jegen

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

On Fri, May 22, 2015 at 1:49 PM, Thierry Reding
<[email protected]> 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.

2015-05-22 13:27:26

by Arnd Bergmann

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

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

2015-05-22 14:32:10

by Greg Kroah-Hartman

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

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.

2015-05-22 14:41:08

by Thierry Reding

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

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

2015-05-22 14:44:17

by Thierry Reding

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

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


Attachments:
(No filename) (1.91 kB)
(No filename) (819.00 B)
Download all attachments