Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH v6 4/4] video: fbdev: vt8623fb: use arch_phys_wc_add() and pci_iomap_wc()
From: Luis R. Rodriguez @ 2015-05-20 23:08 UTC (permalink / raw)
  To: bhelgaas
  Cc: tomi.valkeinen, airlied, linux-fbdev, luto, linux-kernel,
	linux-pci, xen-devel, Luis R. Rodriguez, Rob Clark,
	Laurent Pinchart, Jingoo Han, Lad, Prabhakar, Suresh Siddha,
	Ingo Molnar, Thomas Gleixner, Juergen Gross, Daniel Vetter,
	Dave Airlie, Antonino Daplas, Jean-Christophe Plagniol-Villard
In-Reply-To: <1432163293-20965-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This driver uses the same area for MTRR as for the ioremap().
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.

There are a few motivations for this:

a) Take advantage of PAT when available

b) Help bury MTRR code away, MTRR is architecture specific and on
   x86 its replaced by PAT

c) Help with the goal of eventually using _PAGE_CACHE_UC over
   _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
   de33c442e titled "x86 PAT: fix performance drop for glx,
   use UC minus for ioremap(), ioremap_nocache() and
   pci_mmap_page_range()")

The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.

@ mtrr_found @
expression index, base, size;
@@

-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);

@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@

-mtrr_del(index, base, size);
+arch_phys_wc_del(index);

@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@

-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);

@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@

-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);

@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);

@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);

Generated-by: Coccinelle SmPL
Cc: Rob Clark <robdclark@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/vt8623fb.c | 31 ++++++-------------------------
 1 file changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
index ea7f056..60f24828 100644
--- a/drivers/video/fbdev/vt8623fb.c
+++ b/drivers/video/fbdev/vt8623fb.c
@@ -26,13 +26,9 @@
 #include <linux/console.h> /* Why should fb driver call console functions? because console_lock() */
 #include <video/vga.h>
 
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
-
 struct vt8623fb_info {
 	char __iomem *mmio_base;
-	int mtrr_reg;
+	int wc_cookie;
 	struct vgastate state;
 	struct mutex open_lock;
 	unsigned int ref_count;
@@ -99,10 +95,7 @@ static struct svga_timing_regs vt8623_timing_regs     = {
 /* Module parameters */
 
 static char *mode_option = "640x480-8@60";
-
-#ifdef CONFIG_MTRR
 static int mtrr = 1;
-#endif
 
 MODULE_AUTHOR("(c) 2006 Ondrej Zajicek <santiago@crfreenet.org>");
 MODULE_LICENSE("GPL");
@@ -112,11 +105,8 @@ module_param(mode_option, charp, 0644);
 MODULE_PARM_DESC(mode_option, "Default video mode ('640x480-8@60', etc)");
 module_param_named(mode, mode_option, charp, 0);
 MODULE_PARM_DESC(mode, "Default video mode e.g. '648x480-8@60' (deprecated)");
-
-#ifdef CONFIG_MTRR
 module_param(mtrr, int, 0444);
 MODULE_PARM_DESC(mtrr, "Enable write-combining with MTRR (1=enable, 0=disable, default=1)");
-#endif
 
 
 /* ------------------------------------------------------------------------- */
@@ -710,7 +700,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	info->fix.mmio_len = pci_resource_len(dev, 1);
 
 	/* Map physical IO memory address into kernel space */
-	info->screen_base = pci_iomap(dev, 0, 0);
+	info->screen_base = pci_iomap_wc(dev, 0, 0);
 	if (! info->screen_base) {
 		rc = -ENOMEM;
 		dev_err(info->device, "iomap for framebuffer failed\n");
@@ -781,12 +771,9 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	/* Record a reference to the driver data */
 	pci_set_drvdata(dev, info);
 
-#ifdef CONFIG_MTRR
-	if (mtrr) {
-		par->mtrr_reg = -1;
-		par->mtrr_reg = mtrr_add(info->fix.smem_start, info->fix.smem_len, MTRR_TYPE_WRCOMB, 1);
-	}
-#endif
+	if (mtrr)
+		par->wc_cookie = arch_phys_wc_add(info->fix.smem_start,
+						  info->fix.smem_len);
 
 	return 0;
 
@@ -816,13 +803,7 @@ static void vt8623_pci_remove(struct pci_dev *dev)
 	if (info) {
 		struct vt8623fb_info *par = info->par;
 
-#ifdef CONFIG_MTRR
-		if (par->mtrr_reg >= 0) {
-			mtrr_del(par->mtrr_reg, 0, 0);
-			par->mtrr_reg = -1;
-		}
-#endif
-
+		arch_phys_wc_del(par->wc_cookie);
 		unregister_framebuffer(info);
 		fb_dealloc_cmap(&info->cmap);
 
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Greg Kroah-Hartman @ 2015-05-21  4:26 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Kevin Hilman, Scot Doyle, Tomi Valkeinen, Michael Kerrisk,
	Jiri Slaby, Jean-Christophe Plagniol-Villard, Pavel Machek,
	Geert Uytterhoeven, lkml, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker, Olof Johansson,
	Daniel Stone, Arnd Bergmann
In-Reply-To: <20150520123615.GA24016-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>

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.

thanks,

greg k-h

^ permalink raw reply

* [PATCH] fbcon: Avoid deleting a timer in IRQ context
From: Thierry Reding @ 2015-05-21  7:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kevin Hilman, Scot Doyle, Daniel Stone,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
	linux-kernel

From: Thierry Reding <treding@nvidia.com>

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@fooishbar.org>
Reported-by: Kevin Hilman <khilman@kernel.org>
Tested-by: Kevin Hilman <khilman@linaro.org>
Tested-by: Scot Doyle <lkml14@scotdoyle.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 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


^ permalink raw reply related

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Thierry Reding @ 2015-05-21  8:00 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kevin Hilman, Scot Doyle, Tomi Valkeinen, Michael Kerrisk,
	Jiri Slaby, Jean-Christophe Plagniol-Villard, Pavel Machek,
	Geert Uytterhoeven, lkml, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker, Olof Johansson,
	Daniel Stone, Arnd Bergmann
In-Reply-To: <20150521042638.GB22632-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 3624 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@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.

Thierry

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

^ permalink raw reply

* Re: [Cocci] [PATCH v3 0/4] pci: add and use pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-05-21 21:23 UTC (permalink / raw)
  To: cocci
In-Reply-To: <CAB=NE6UsMMoPhkKof+VN7ZUCd81VxrwP9nqHDUj5Lr3DJLm3bA@mail.gmail.com>

On Wed, May 20, 2015 at 1:54 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Tue, May 19, 2015 at 10:53 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> On Wed, Apr 29, 2015 at 02:08:47PM -0700, Luis R. Rodriguez wrote:
>>> On Tue, Apr 21, 2015 at 1:20 PM, Luis R. Rodriguez
>>> <mcgrof@do-not-panic.com> wrote:
>>> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>> >
>>> > This series adds pci_ioremap_wc_bar() and makes use of it
>>> > on a few framebuffer device drivers.
>>> >
>>> > Luis R. Rodriguez (4):
>>> >   pci: add pci_ioremap_wc_bar()
>>> >   video: fbdev: i740fb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
>>> >   video: fbdev: kyrofb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
>>> >   video: fbdev: gxt4500: use pci_ioremap_wc_bar() for framebuffer
>>>
>>> Bjorn,
>>>
>>> any feedback on this series?
>>
>> Hey Bjorn, any feedback on this series ?
>
> I meant this series. Also Tomi, do I get your Acks for these as well?
> Since they depend on pci parts do you mind if we route them through
> the PCI tree? I'll poke you soon about another similar type of patch
> which has dependencies on another development tree.

Bjorn, Tomi, please let me know if this series is OK, if so I'll
resubmit rebased onto Bjorn's pci tree.

 Luis

^ permalink raw reply

* Re: [PATCH v6 1/4] pci: add pci_iomap_wc() variants
From: Bjorn Helgaas @ 2015-05-21 22:26 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: tomi.valkeinen, airlied, linux-fbdev, luto, linux-kernel,
	linux-pci, xen-devel, Luis R. Rodriguez, Toshi Kani,
	Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
	Daniel Vetter, Dave Airlie, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Dave Hansen, Arnd Bergmann,
	Michael S. Tsirkin, venkatesh.pallipadi, Stefan Bader,
	Ville Syrjälä, Mel Gorman, Vlastimil Babka,
	Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
	david.vrabel, jbeulich, Roger Pau Monné
In-Reply-To: <1432163293-20965-2-git-send-email-mcgrof@do-not-panic.com>

On Wed, May 20, 2015 at 04:08:10PM -0700, Luis R. Rodriguez wrote:
> ...
> --- a/lib/pci_iomap.c
> +++ b/lib/pci_iomap.c
> @@ -52,6 +52,46 @@ void __iomem *pci_iomap_range(struct pci_dev *dev,
>  EXPORT_SYMBOL(pci_iomap_range);
>  
>  /**
> + * pci_iomap_wc_range - create a virtual WC mapping cookie for a PCI BAR
> + * @dev: PCI device that owns the BAR
> + * @bar: BAR number
> + * @offset: map memory at the given offset in BAR
> + * @maxlen: max length of the memory to map
> + *
> + * Using this function you will get a __iomem address to your device BAR.
> + * You can access it using ioread*() and iowrite*(). These functions hide
> + * the details if this is a MMIO or PIO address space and will just do what
> + * you expect from them in the correct way. When possible write combining
> + * is used.
> + *
> + * @maxlen specifies the maximum length to map. If you want to get access to
> + * the complete BAR from offset to the end, pass %0 here.
> + * */
> +void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
> +				 int bar,
> +				 unsigned long offset,
> +				 unsigned long maxlen)
> +{
> +	resource_size_t start = pci_resource_start(dev, bar);
> +	resource_size_t len = pci_resource_len(dev, bar);
> +	unsigned long flags = pci_resource_flags(dev, bar);
> +
> +	if (len <= offset || !start)
> +		return NULL;
> +	len -= offset;
> +	start += offset;
> +	if (maxlen && len > maxlen)
> +		len = maxlen;
> +	if (flags & IORESOURCE_IO)
> +		return NULL;
> +	if (flags & IORESOURCE_MEM)
> +		return ioremap_wc(start, len);
> +	/* What? */
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(pci_iomap_wc_range);
> +
> +/**
>   * pci_iomap - create a virtual mapping cookie for a PCI BAR
>   * @dev: PCI device that owns the BAR
>   * @bar: BAR number
> @@ -70,4 +110,25 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
>  	return pci_iomap_range(dev, bar, 0, maxlen);
>  }
>  EXPORT_SYMBOL(pci_iomap);
> +
> +/**
> + * pci_iomap_wc - create a virtual WC mapping cookie for a PCI BAR
> + * @dev: PCI device that owns the BAR
> + * @bar: BAR number
> + * @maxlen: length of the memory to map
> + *
> + * Using this function you will get a __iomem address to your device BAR.
> + * You can access it using ioread*() and iowrite*(). These functions hide
> + * the details if this is a MMIO or PIO address space and will just do what
> + * you expect from them in the correct way. When possible write combining
> + * is used.
> + *
> + * @maxlen specifies the maximum length to map. If you want to get access to
> + * the complete BAR without checking for its length first, pass %0 here.
> + * */
> +void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen)
> +{
> +	return pci_iomap_wc_range(dev, bar, 0, maxlen);
> +}
> +EXPORT_SYMBOL_GPL(pci_iomap_wc);

Huh.  So you let me talk about marking the unused pcim_iomap_wc()
EXPORT_SYMBOL_GPL(), but didn't remind me that you also proposed to mark
the symbol you really care about, the one you already have a use for, as
EXPORT_SYMBOL_GPL().  Sigh.

In my opinion, if we're going to use EXPORT_SYMBOL_GPL() at all, we should
use it consistently and based on technical considerations.  I base this on
statements like the following:

  - "[EXPORT_SYMBOL_GPL()] implies that the function is considered an
    internal implementation issue, and not really an interface." [Rusty
    Russell, 1]

  - "... using the xxx_GPL() version to show that it's an internal
    interface ..." [Linus Torvalds, 2]

  - "Anything exported via EXPORT_SYMBOL_GPL() is considered by the author
    to be so fundamental to the kernel that using it would be impossible
    without creating a derivative work." [Matthew Garrett, 3]

  - "Linus's initial point for [_GPL symbols] has been so diluted by random
    lobby groups asking for every symbol to be _GPL that they are becoming
    effectively pointless now." [Dave Airlie, 4]

Existing interfaces like these are exported with EXPORT_SYMBOL():

  ioremap()
  ioremap_wc()
  ioremap_prot()
  pci_iomap()
  pci_map_rom()

I would argue that pci_iomap_wc() is similar in spirit and is no more an
internal implementation issue than they are, and should be exported
similarly.

So my *advice* is to use EXPORT_SYMBOL() in this case, because that's a
choice you can defend on technical grounds.  I think it's hard to argue
that pci_iomap_wc() is so fundamental or unique to Linux that a caller
would automatically be a derivative work.

Will I still merge it as EXPORT_SYMBOL_GPL()?  Maybe.  I don't feel *good*
about it because the only explanation I can give is "the author wanted it
that way," and that's unsatisfying.  But I did already ack it (before I
noticed the _GPL() issue), and I won't try to retract that and prevent
somebody else from merging it.  And maybe your proposal to clarify the
kernel-hacking.tmpl language will convince me.

Bjorn

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id¶c17ea4eff3
[2] http://lkml.kernel.org/r/Pine.LNX.4.64.0510050742550.31407@g5.osdl.org
[3] http://mjg59.dreamwidth.org/31357.html
[4] http://lkml.kernel.org/r/CAPM=9tzsT+nah2P-qZ8iKW=aTZJzYgm18mMWyy2-RVkoOSwyjg@mail.gmail.com

^ permalink raw reply

* Re: [PATCH v6 1/4] pci: add pci_iomap_wc() variants
From: Bjorn Helgaas @ 2015-05-21 22:33 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: tomi.valkeinen, airlied, linux-fbdev, luto, linux-kernel,
	linux-pci, xen-devel, Luis R. Rodriguez, Toshi Kani,
	Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
	Daniel Vetter, Dave Airlie, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Dave Hansen, Arnd Bergmann,
	Michael S. Tsirkin, venkatesh.pallipadi, Stefan Bader,
	Ville Syrjälä, Mel Gorman, Vlastimil Babka,
	Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
	david.vrabel, jbeulich, Roger Pau Monné
In-Reply-To: <1432163293-20965-2-git-send-email-mcgrof@do-not-panic.com>

On Wed, May 20, 2015 at 04:08:10PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> This allows drivers to take advantage of write-combining
> when possible. The PCI specification does not allow for us
> to automatically identify a memory region which needs
> write-combining so drivers have to identify these areas
> on their own. There is IORESOURCE_PREFETCH but as clarified
> by Michael and confirmed later by Bjorn, PCI prefetch bit
> merely means bridges can combine writes and prefetch reads.
> Prefetch does not affect ordering rules and does not allow
> writes to be collapsed [0]. WC is stronger, it allows collapsing
> and changes ordering rules. WC can also hurt latency as small
> writes are buffered. Because of all this drivers needs to
> know what they are doing, we can't set a write-combining
> preference flag in the pci core automatically for drivers.
> 
> Lastly although there is also arch_phys_wc_add() this makes
> use of architecture specific write-combining *hacks* and
> the only one currently defined and used is MTRR for x86.
> MTRRs are legacy, limited in number, have restrictive size
> constraints, and are known to interact pooly with the BIOS.
> MTRRs should only really be considered on old video framebuffer
> drivers. If we made ioremap_wc() and similar calls start
> automatically adding MTRRs, then performance will vary wildly
> with the order of driver loading because we'll run out of MTRRs
> part-way through bootup.
> 
> There are a few motivations for phasing out of MTRR and
> helping driver change over to use write-combining with PAT:
> 
> a) Take advantage of PAT when available
> 
> b) Help bury MTRR code away, MTRR is architecture specific and on
>    x86 its replaced by PAT
> 
> c) Help with the goal of eventually using _PAGE_CACHE_UC over
>    _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
>    de33c442e titled "x86 PAT: fix performance drop for glx,
>    use UC minus for ioremap(), ioremap_nocache() and
>    pci_mmap_page_range()")
> 
> [0] https://lkml.org/lkml/2015/4/21/714

I tentatively put this (and the rest of the series) on a pci/resource
branch.  I'm hoping you'll propose some clarification about
EXPORT_SYMBOL_GPL().

In the meantime, I tried to make the changelog a bit more concise, as
below.  Let me know if I omitted something essential.  We still have URLs
for the discussion for the fine points.

commit 75387ae87b7aebc2a5b447f4d1b8b17a23c15b08
Author: Luis R. Rodriguez <mcgrof@suse.com>
Date:   Wed May 20 16:08:10 2015 -0700

    PCI: Add pci_iomap_wc() variants
    
    PCI BARs tell us whether prefetching is safe, but they don't say anything
    about write combining (WC).  WC changes ordering rules and allows writes to
    be collapsed, so it's not safe in general to use it on a prefetchable
    region.
    
    Add pci_iomap_wc() and pci_iomap_wc_range() so drivers can take advantage
    of write combining when they know it's safe.
    
    On architectures that don't fully support WC, e.g., x86 without PAT,
    drivers for legacy framebuffers may get some of the benefit by using
    arch_phys_wc_add() in addition to pci_iomap_wc().  But arch_phys_wc_add()
    is unreliable and should be avoided in general.  On x86, it uses MTRRs,
    which are limited in number and size, so the results will vary based on
    driver loading order.
    
    The goals of adding pci_iomap_wc() are to:
    
      - Give drivers an architecture-independent way to use WC so they can stop
        using interfaces like mtrr_add() (on x86, pci_iomap_wc() uses PAT when
        available)
    
      - Move toward using _PAGE_CACHE_MODE_UC, not _PAGE_CACHE_MODE_UC_MINUS,
        on x86 on ioremap_nocache() (see de33c442ed2a ("x86 PAT: fix
        performance drop for glx, use UC minus for ioremap(), ioremap_nocache()
        and pci_mmap_page_range()")
    
    [bhelgaas: changelog]
    Link: http://lkml.kernel.org/r/1426893517-2511-6-git-send-email-mcgrof@do-not-panic.com
    Original-posting: http://lkml.kernel.org/r/1432163293-20965-1-git-send-email-mcgrof@do-not-panic.com
    Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

^ permalink raw reply

* Re: [PATCH v6 1/4] pci: add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-05-22  0:23 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Luis R. Rodriguez, tomi.valkeinen, airlied, linux-fbdev, luto,
	linux-kernel, linux-pci, xen-devel, Toshi Kani, Suresh Siddha,
	Ingo Molnar, Thomas Gleixner, Juergen Gross, Daniel Vetter,
	Dave Airlie, Antonino Daplas, Jean-Christophe Plagniol-Villard,
	Dave Hansen, Arnd Bergmann, Michael S. Tsirkin,
	venkatesh.pallipadi, Stefan Bader, Ville Syrjälä,
	Mel Gorman, Vlastimil Babka, Borislav Petkov, Davidlohr Bueso,
	konrad.wilk, ville.syrjala, david.vrabel, jbeulich,
	Roger Pau Monné
In-Reply-To: <20150521223321.GG32152@google.com>

On Thu, May 21, 2015 at 05:33:21PM -0500, Bjorn Helgaas wrote:
> 
> I tentatively put this (and the rest of the series) on a pci/resource
> branch.  I'm hoping you'll propose some clarification about
> EXPORT_SYMBOL_GPL().

EXPORT_SYMBOL_GPL() also serves to ensure only GPL modules can
only run that code. So for instance although we have "Dual BSD/GPL"
tags for modules pure "BSD" tags do not exist for module tags and
cannot run EXPORT_SYMBOL_GPL() code [0]. Also there is some folks
who do believe tha at run time all kernel modules are GPL [1] [2].
And to be precise even though the FSF may claim a list of licenses
are GPL-compatible we cannot rely on this list alone for our own
goals and if folks want to use our EXPORT_SYMBOL_GPL()s they must
discuss this on lkml [2].

In the past when I've tried to try to clarify EXPORT_SYMBOL_GPL()
requirements, implications, its been said that its best to leave
some things as-is [3] and let attorneys figure things out. In so
far as to what exactly it is and can be used for requires legal
attorney review, but the question of derivative work certainly
comes up [4]. Now folks companies seem to want to obviously use
and abuse our symbols despite of all the things above, for instance
Red Hat once tried to change an EXPORT_SYMBOL_GPL() to
EXPORT_SYMBOL() [5]. Obviously that didn't go so well, and some
folks went off on a good rant about this [6].

What developers do and accept varies, I'm not going into pointing
out specifics and I do not wnat to do homework for folks who wish
to abuse things further, but by no means should a developer be
nack'd entry to new code if their functionality is not replacing
old one [9]. In this case this is new functionality. Also in terms
of preference:

"nobody has said that symbols exported with plain EXPORT_SYMBOL() can be freely
used by proprietary code; indeed, a number of developers claim that all (or
nearly all) loadable modules are derived products of the kernel regardless of
whether they use GPL-only symbols or not" [7].

And spot on:

"In general, the kernel community has long worked to maintain a vague and scary
ambiguity around the legal status of proprietary modules while being unwilling
to attempt to ban such modules outright." [7].

Now, a few maintainers insist on tons of new symbols to be EXPORT_SYMBOL_GPL()
though proactively [8] [9] and the reasons vary, I just happen to also write my
code to be perfectly clear with my goals and intent and you are the first to
ask me to reconsider this, even if you do make me use EXPORT_SYMBOL() my intent
and goal does not change, as with others. No code I ever write should be used
by proprietary shit, and I hope to convince others of the importance to do this
as well.

> In the meantime, I tried to make the changelog a bit more concise, as
> below.  Let me know if I omitted something essential.  We still have URLs
> for the discussion for the fine points.

Looks good.

[0] https://lkml.org/lkml/2012/4/7/102
[1] https://lkml.org/lkml/2012/4/8/71
[2] https://lkml.org/lkml/2012/4/8/71
[3] https://lkml.org/lkml/2009/6/1/385
[4] https://lkml.org/lkml/2009/6/1/376
[5] https://lkml.org/lkml/2012/4/20/328
[6] https://plus.google.com/+AlanCoxLinux/posts/D2feRNc6R4d
[7] https://lwn.net/Articles/603131/
[8] https://lwn.net/Articles/603139/
[9] https://lkml.org/lkml/2015/2/26/379

  Luis

^ permalink raw reply

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Greg Kroah-Hartman @ 2015-05-22  2:00 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Kevin Hilman, Scot Doyle, Tomi Valkeinen, Michael Kerrisk,
	Jiri Slaby, Jean-Christophe Plagniol-Villard, Pavel Machek,
	Geert Uytterhoeven, lkml, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker, Olof Johansson,
	Daniel Stone, Arnd Bergmann
In-Reply-To: <20150521080049.GA25079@ulmo>

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.

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

^ permalink raw reply

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Thierry Reding @ 2015-05-22 10:00 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kevin Hilman, Scot Doyle, Tomi Valkeinen, Michael Kerrisk,
	Jiri Slaby, Jean-Christophe Plagniol-Villard, Pavel Machek,
	Geert Uytterhoeven, lkml, linux-fbdev, linux-man, linux-api,
	Tyler Baker, Olof Johansson, Daniel Stone, Arnd Bergmann
In-Reply-To: <20150522020031.GA3516@kroah.com>

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

^ permalink raw reply

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Arnd Bergmann @ 2015-05-22 10:33 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Greg Kroah-Hartman, Kevin Hilman, Scot Doyle, Tomi Valkeinen,
	Michael Kerrisk, Jiri Slaby, Jean-Christophe Plagniol-Villard,
	Pavel Machek, Geert Uytterhoeven, lkml,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker, Olof Johansson,
	Daniel Stone
In-Reply-To: <20150522100002.GB16507@ulmo>

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

^ permalink raw reply

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Thierry Reding @ 2015-05-22 11:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Greg Kroah-Hartman, Kevin Hilman, Scot Doyle, Tomi Valkeinen,
	Michael Kerrisk, Jiri Slaby, Jean-Christophe Plagniol-Villard,
	Pavel Machek, Geert Uytterhoeven, lkml,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker, Olof Johansson,
	Daniel Stone
In-Reply-To: <3661627.JqpS87pBks@wuerfel>

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

^ permalink raw reply

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Silvan Jegen @ 2015-05-22 13:07 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Kevin Hilman, Scot Doyle,
	Tomi Valkeinen, Michael Kerrisk, Jiri Slaby,
	Jean-Christophe Plagniol-Villard, Pavel Machek,
	Geert Uytterhoeven, lkml, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man, linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker,
	Olof Johansson, Daniel Stone
In-Reply-To: <20150522114957.GF16507@ulmo>

On Fri, May 22, 2015 at 1:49 PM, Thierry Reding
<thierry.reding@gmail.com> 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.

^ permalink raw reply

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Arnd Bergmann @ 2015-05-22 13:24 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Greg Kroah-Hartman, Kevin Hilman, Scot Doyle, Tomi Valkeinen,
	Michael Kerrisk, Jiri Slaby, Jean-Christophe Plagniol-Villard,
	Pavel Machek, Geert Uytterhoeven, lkml,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker, Olof Johansson,
	Daniel Stone
In-Reply-To: <20150522114957.GF16507@ulmo>

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

^ permalink raw reply

* [PATCH] video: omap/h3: fix tps65010 dependency
From: Arnd Bergmann @ 2015-05-22 14:22 UTC (permalink / raw)
  To: linux-arm-kernel

The h3 LCD driver fails to link when tps65010 is configured
as a loadable module:

drivers/built-in.o: In function `h3_panel_disable':
debugfs.c:(.text+0x206ac): undefined reference to `tps65010_set_gpio_out_value'
debugfs.c:(.text+0x206cc): undefined reference to `tps65010_set_gpio_out_value'
drivers/built-in.o: In function `h3_panel_enable':
debugfs.c:(.text+0x206e0): undefined reference to `tps65010_set_gpio_out_value'
debugfs.c:(.text+0x20704): undefined reference to `tps65010_set_gpio_out_value'

This clarifies the dependency so we can only select it if
the dependnecy is built-in.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/video/fbdev/omap/Kconfig b/drivers/video/fbdev/omap/Kconfig
index 18c4cb0d5690..29d250da8a3e 100644
--- a/drivers/video/fbdev/omap/Kconfig
+++ b/drivers/video/fbdev/omap/Kconfig
@@ -42,7 +42,7 @@ config FB_OMAP_LCD_MIPID
 config FB_OMAP_LCD_H3
 	bool "TPS65010 LCD controller on OMAP-H3"
 	depends on MACH_OMAP_H3
-	depends on TPS65010
+	depends on TPS65010=y
 	default y
 	help
 	  Say Y here if you want to have support for the LCD on the


^ permalink raw reply related

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Greg Kroah-Hartman @ 2015-05-22 14:32 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Arnd Bergmann, Kevin Hilman, Scot Doyle, Tomi Valkeinen,
	Michael Kerrisk, Jiri Slaby, Jean-Christophe Plagniol-Villard,
	Pavel Machek, Geert Uytterhoeven, lkml,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker, Olof Johansson,
	Daniel Stone
In-Reply-To: <20150522114957.GF16507@ulmo>

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.


^ permalink raw reply

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Thierry Reding @ 2015-05-22 14:41 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Greg Kroah-Hartman, Kevin Hilman, Scot Doyle, Tomi Valkeinen,
	Michael Kerrisk, Jiri Slaby, Jean-Christophe Plagniol-Villard,
	Pavel Machek, Geert Uytterhoeven, lkml,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker, Olof Johansson,
	Daniel Stone
In-Reply-To: <2820529.TShzski8m6@wuerfel>

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

^ permalink raw reply

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Thierry Reding @ 2015-05-22 14:44 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Arnd Bergmann, Kevin Hilman, Scot Doyle, Tomi Valkeinen,
	Michael Kerrisk, Jiri Slaby, Jean-Christophe Plagniol-Villard,
	Pavel Machek, Geert Uytterhoeven, lkml,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker, Olof Johansson,
	Daniel Stone
In-Reply-To: <20150522143205.GA6508-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>

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

^ permalink raw reply

* [patch] fbdev: ssd1307fb: potential ERR_PTR dereference
From: Dan Carpenter @ 2015-05-23 17:32 UTC (permalink / raw)
  To: linux-fbdev

The error handling got shifted down a few lines from where it was
supposed to be for some reason.

Fixes: a14a7ba8cb0f ('fbdev: ssd1307fb: add backlight controls for setting the contrast')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 8fc224c..9c28a77 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -655,15 +655,16 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	snprintf(bl_name, sizeof(bl_name), "ssd1307fb%d", info->node);
 	bl = backlight_device_register(bl_name, &client->dev, par,
 				       &ssd1307fb_bl_ops, NULL);
-	bl->props.brightness = par->contrast;
-	bl->props.max_brightness = MAX_CONTRAST;
-	info->bl_dev = bl;
-
 	if (IS_ERR(bl)) {
 		dev_err(&client->dev, "unable to register backlight device: %ld\n",
 			PTR_ERR(bl));
 		goto bl_init_error;
 	}
+
+	bl->props.brightness = par->contrast;
+	bl->props.max_brightness = MAX_CONTRAST;
+	info->bl_dev = bl;
+
 	dev_info(&client->dev, "fb%d: %s framebuffer device registered, using %d bytes of video memory\n", info->node, info->fix.id, vmem_size);
 
 	return 0;

^ permalink raw reply related

* re: fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT
From: Dan Carpenter @ 2015-05-23 17:32 UTC (permalink / raw)
  To: linux-fbdev

Hello Thomas Niederprüm,

The patch a3998fe03e87: "fbdev: ssd1307fb: Unify init code and obtain
hw specific bits from DT" from Mar 31, 2015, leads to the following
static checker warning:

	drivers/video/fbdev/ssd1307fb.c:371 ssd1307fb_init()
	warn: add some parenthesis here?

drivers/video/fbdev/ssd1307fb.c
   366          /* Set COM pins configuration */
   367          ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_COM_PINS_CONFIG);
   368          if (ret < 0)
   369                  return ret;
   370  
   371          compins = 0x02 | (!par->com_seq & 0x1) << 4
   372                                     | (par->com_lrremap & 0x1) << 5;

Smatch is complaining because it's normally  "!par->com_seq & 0x1" is
a bug and "!(par->com_seq & 0x1)" is intended.  I don't know what was
intended here though.  If the current code is correct, you can silence
the static checker warning by writing it as "(!par->com_seq) & 0x1".

But I also have a hard time remembering if | or << is higher precedence
so that might be clearer with parenthesis as well even though the code
is clearly correct when I google for "order of operations" in C.

   373          ret = ssd1307fb_write_cmd(par->client, compins);
   374          if (ret < 0)
   375                  return ret;

regards,
dan carpenter

^ permalink raw reply

* Re: [patch] fbdev: ssd1307fb: potential ERR_PTR dereference
From: Maxime Ripard @ 2015-05-24  7:19 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150523173235.GA31595@mwanda>

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

On Sat, May 23, 2015 at 08:32:35PM +0300, Dan Carpenter wrote:
> The error handling got shifted down a few lines from where it was
> supposed to be for some reason.
> 
> Fixes: a14a7ba8cb0f ('fbdev: ssd1307fb: add backlight controls for setting the contrast')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

^ permalink raw reply

* [PATCH] fbdev: radeon: Remove 'struct timeval' usage
From: Tina Ruchandani @ 2015-05-25  4:19 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: y2038, Benjamin Herrenschmidt, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, linux-fbdev, linux-kernel

'struct timeval' uses a 32-bit representation for the
seconds field which will overflow in the year 2038 and beyond.
This patch replaces the usage of 'struct timeval' with
ktime_t which uses a 64-bit time representation and does not
suffer from the y2038 problem. This patch is part of a larger
effort to remove all instances of 'struct timeval', 'struct
timespec', time_t and other 32-bit timekeeping variables
from the kernel.
The patch also replaces the use of real time (do_gettimeofday)
with monotonic time (ktime_get).

Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
---
 drivers/video/fbdev/aty/radeon_base.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index 01237c8..9747e9e 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -64,6 +64,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/time.h>
+#include <linux/ktime.h>
 #include <linux/fb.h>
 #include <linux/ioport.h>
 #include <linux/init.h>
@@ -461,8 +462,8 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
 	int hTotal, vTotal, num, denom, m, n;
 	unsigned long long hz, vclk;
 	long xtal;
-	struct timeval start_tv, stop_tv;
-	long total_secs, total_usecs;
+	ktime_t start, stop;
+	s64 delta;
 	int i;
 
 	/* Ugh, we cut interrupts, bad bad bad, but we want some precision
@@ -478,7 +479,7 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
 		if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) = 0)
 			break;
 
-	do_gettimeofday(&start_tv);
+	start = ktime_get();
 
 	for(i=0; i<1000000; i++)
 		if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) != 0)
@@ -487,20 +488,18 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
 	for(i=0; i<1000000; i++)
 		if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) = 0)
 			break;
-	
-	do_gettimeofday(&stop_tv);
-	
+
+	stop = ktime_get();
+
 	local_irq_enable();
 
-	total_secs = stop_tv.tv_sec - start_tv.tv_sec;
-	if (total_secs > 10)
+	delta = ktime_us_delta(stop, start);
+
+	/* Return -1 if more than 10 seconds have elapsed */
+	if (delta > (10*1000000))
 		return -1;
-	total_usecs = stop_tv.tv_usec - start_tv.tv_usec;
-	total_usecs += total_secs * 1000000;
-	if (total_usecs < 0)
-		total_usecs = -total_usecs;
-	hz = 1000000/total_usecs;
- 
+	hz = 1000000/delta;
+
 	hTotal = ((INREG(CRTC_H_TOTAL_DISP) & 0x1ff) + 1) * 8;
 	vTotal = ((INREG(CRTC_V_TOTAL_DISP) & 0x3ff) + 1);
 	vclk = (long long)hTotal * (long long)vTotal * hz;
@@ -548,7 +547,7 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
 		denom *= 3;
 		break;
 	case 6:
-		denom *= 6;   
+		denom *= 6;
 		break;
 	case 7:
 		denom *= 12;
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related

* Re: fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT
From: Sudip Mukherjee @ 2015-05-25  5:12 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150523173245.GA31663@mwanda>

On Sat, May 23, 2015 at 08:32:45PM +0300, Dan Carpenter wrote:
> Hello Thomas Niederprüm,

> But I also have a hard time remembering if | or << is higher precedence

man page of operator (man operator) will show the precedence.
<< is higher precendence than |

regards
sudip

^ permalink raw reply

* Re: [patch] fbdev: ssd1307fb: potential ERR_PTR dereference
From: Tomi Valkeinen @ 2015-05-25  5:40 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150523173235.GA31595@mwanda>

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



On 23/05/15 20:32, Dan Carpenter wrote:
> The error handling got shifted down a few lines from where it was
> supposed to be for some reason.
> 
> Fixes: a14a7ba8cb0f ('fbdev: ssd1307fb: add backlight controls for setting the contrast')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 8fc224c..9c28a77 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -655,15 +655,16 @@ static int ssd1307fb_probe(struct i2c_client *client,
>  	snprintf(bl_name, sizeof(bl_name), "ssd1307fb%d", info->node);
>  	bl = backlight_device_register(bl_name, &client->dev, par,
>  				       &ssd1307fb_bl_ops, NULL);
> -	bl->props.brightness = par->contrast;
> -	bl->props.max_brightness = MAX_CONTRAST;
> -	info->bl_dev = bl;
> -
>  	if (IS_ERR(bl)) {
>  		dev_err(&client->dev, "unable to register backlight device: %ld\n",
>  			PTR_ERR(bl));
>  		goto bl_init_error;
>  	}
> +
> +	bl->props.brightness = par->contrast;
> +	bl->props.max_brightness = MAX_CONTRAST;
> +	info->bl_dev = bl;
> +
>  	dev_info(&client->dev, "fb%d: %s framebuffer device registered, using %d bytes of video memory\n", info->node, info->fix.id, vmem_size);
>  
>  	return 0;
> 

Thanks, queued for 4.2.

 Tomi


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

^ permalink raw reply

* Re: [PATCH] video: omap/h3: fix tps65010 dependency
From: Tomi Valkeinen @ 2015-05-25  5:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4416278.oVFvT3Eyqf@wuerfel>

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



On 22/05/15 17:22, Arnd Bergmann wrote:
> The h3 LCD driver fails to link when tps65010 is configured
> as a loadable module:
> 
> drivers/built-in.o: In function `h3_panel_disable':
> debugfs.c:(.text+0x206ac): undefined reference to `tps65010_set_gpio_out_value'
> debugfs.c:(.text+0x206cc): undefined reference to `tps65010_set_gpio_out_value'
> drivers/built-in.o: In function `h3_panel_enable':
> debugfs.c:(.text+0x206e0): undefined reference to `tps65010_set_gpio_out_value'
> debugfs.c:(.text+0x20704): undefined reference to `tps65010_set_gpio_out_value'
> 
> This clarifies the dependency so we can only select it if
> the dependnecy is built-in.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/drivers/video/fbdev/omap/Kconfig b/drivers/video/fbdev/omap/Kconfig
> index 18c4cb0d5690..29d250da8a3e 100644
> --- a/drivers/video/fbdev/omap/Kconfig
> +++ b/drivers/video/fbdev/omap/Kconfig
> @@ -42,7 +42,7 @@ config FB_OMAP_LCD_MIPID
>  config FB_OMAP_LCD_H3
>  	bool "TPS65010 LCD controller on OMAP-H3"
>  	depends on MACH_OMAP_H3
> -	depends on TPS65010
> +	depends on TPS65010=y
>  	default y
>  	help
>  	  Say Y here if you want to have support for the LCD on the
> 

Thanks, queued for 4.2.

 Tomi


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

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox