* Re: [PATCH v3 1/4] pci: add pci_ioremap_wc_bar()
From: Bjorn Helgaas @ 2015-05-20 21:02 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: mst, plagnioj, tomi.valkeinen, linux-fbdev, luto, cocci,
Luis R. Rodriguez, Toshi Kani, Suresh Siddha, Ingo Molnar,
Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
Antonino Daplas, Ville Syrjälä, Mel Gorman,
Vlastimil Babka, Borislav Petkov, Davidlohr Bueso, linux-kernel
In-Reply-To: <1429647634-17169-2-git-send-email-mcgrof@do-not-panic.com>
On Tue, Apr 21, 2015 at 01:20:31PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> This lets drivers take advantage of PAT when available. This
> should help with the transition of converting video drivers over
> to ioremap_wc() to help with the goal of eventually using
> _PAGE_CACHE_UC over _PAGE_CACHE_UC_MINUS on x86 on
> ioremap_nocache() (de33c442e titled "x86 PAT: fix performance
> drop for glx, use UC minus for ioremap(), ioremap_nocache() and
> pci_mmap_page_range()")
>
> Cc: Toshi Kani <toshi.kani@hp.com>
> Cc: Bjorn Helgaas <bhelgaas@google.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: Ville Syrjälä <syrjala@sci.fi>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Davidlohr Bueso <dbueso@suse.de>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
I don't have any objection to this, and I can apply the follow-on patches
as well, given the appropriate acks and prerequisite patches (ioremap_wc()
implementation).
> ---
> drivers/pci/pci.c | 14 ++++++++++++++
> include/linux/pci.h | 1 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 687af72..fd14f10 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -138,6 +138,20 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
> return ioremap_nocache(res->start, resource_size(res));
> }
> EXPORT_SYMBOL_GPL(pci_ioremap_bar);
> +
> +void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar)
> +{
> + /*
> + * Make sure the BAR is actually a memory resource, not an IO resource
> + */
> + if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
> + WARN_ON(1);
> + return NULL;
> + }
> + return ioremap_wc(pci_resource_start(pdev, bar),
> + pci_resource_len(pdev, bar));
> +}
> +EXPORT_SYMBOL_GPL(pci_ioremap_wc_bar);
> #endif
>
> #define PCI_FIND_CAP_TTL 48
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index bc50bb0..490ca41 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1667,6 +1667,7 @@ static inline void pci_mmcfg_late_init(void) { }
> int pci_ext_cfg_avail(void);
>
> void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar);
> +void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar);
>
> #ifdef CONFIG_PCI_IOV
> int pci_iov_virtfn_bus(struct pci_dev *dev, int id);
> --
> 2.3.2.209.gd67f9d5.dirty
>
^ permalink raw reply
* Re: [PATCH v3 1/4] pci: add pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-05-20 21:06 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Michael S. Tsirkin, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, Andy Lutomirski,
cocci@systeme.lip6.fr, Toshi Kani, Suresh Siddha, Ingo Molnar,
Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
Antonino Daplas, Ville Syrjälä, Mel Gorman,
Vlastimil Babka, Borislav Petkov, Davidlohr Bueso,
linux-kernel@vger.kernel.org
In-Reply-To: <20150520210234.GD32152@google.com>
On Wed, May 20, 2015 at 2:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Tue, Apr 21, 2015 at 01:20:31PM -0700, Luis R. Rodriguez wrote:
>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>
>> This lets drivers take advantage of PAT when available. This
>> should help with the transition of converting video drivers over
>> to ioremap_wc() to help with the goal of eventually using
>> _PAGE_CACHE_UC over _PAGE_CACHE_UC_MINUS on x86 on
>> ioremap_nocache() (de33c442e titled "x86 PAT: fix performance
>> drop for glx, use UC minus for ioremap(), ioremap_nocache() and
>> pci_mmap_page_range()")
>>
>> Cc: Toshi Kani <toshi.kani@hp.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.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: Ville Syrjälä <syrjala@sci.fi>
>> Cc: Mel Gorman <mgorman@suse.de>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: Davidlohr Bueso <dbueso@suse.de>
>> Cc: linux-fbdev@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
>
> I don't have any objection to this, and I can apply the follow-on patches
> as well, given the appropriate acks and prerequisite patches (ioremap_wc()
> implementation).
OK great, ioremap_wc() has been upstream for a long time. So I'll wait
for Tomi's review of the fbdev changes. Once I get those I can resend
in a series bundled together with the other pending pci / fbdev
changes we just discussed to make things clear.
Luis
^ permalink raw reply
* [PATCH v6 0/4] pci: add and use 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
From: "Luis R. Rodriguez" <mcgrof@suse.com>
Bjorn, this v6 respins the introduction of pci_iomap_wc() and
its users onto Bjorn's pci tree [0] on the next branch. It goes
with your own Acked-by on the PCI part as well as the framebuffer
subsystem maintainer's own Acked-bys for the framebuffer driver
specific changes.
The driver changes are being submitted through the PCI tree
to make the caller have users as soon as its merged otherwise
we'd have to wait 2 release before full integration.
Bjorn, if you rather pull instead of apply these patches one
by one I've put these changes up on a branch on a tree based
on yours to pull from.
The following changes since commit 9b08ae0dde00c20a75cf15636a08e0901c6fe482:
Merge branch 'pci/msi' into next (2015-05-08 15:05:10 -0500)
are available in the git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-pci.git pci_iomap_wc-v6-20150520
for you to fetch changes up to 480d550fa62bdad1c8f25b16d27d2035f36a283b:
video: fbdev: vt8623fb: use arch_phys_wc_add() and pci_iomap_wc() (2015-05-20 15:12:04 -0700)
[0] https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/
Luis R. Rodriguez (4):
pci: add pci_iomap_wc() variants
video: fbdev: arkfb: use arch_phys_wc_add() and pci_iomap_wc()
video: fbdev: s3fb: use arch_phys_wc_add() and pci_iomap_wc()
video: fbdev: vt8623fb: use arch_phys_wc_add() and pci_iomap_wc()
drivers/video/fbdev/arkfb.c | 36 ++++--------------------
drivers/video/fbdev/s3fb.c | 35 ++++-------------------
drivers/video/fbdev/vt8623fb.c | 31 ++++-----------------
include/asm-generic/pci_iomap.h | 14 ++++++++++
lib/pci_iomap.c | 61 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 92 insertions(+), 85 deletions(-)
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply
* [PATCH v6 1/4] pci: add pci_iomap_wc() variants
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, 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-1-git-send-email-mcgrof@do-not-panic.com>
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
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Andy Lutomirski <luto@amacapital.net>
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: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: venkatesh.pallipadi@intel.com
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
include/asm-generic/pci_iomap.h | 14 ++++++++++
lib/pci_iomap.c | 61 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h
index 7389c87..b1e17fc 100644
--- a/include/asm-generic/pci_iomap.h
+++ b/include/asm-generic/pci_iomap.h
@@ -15,9 +15,13 @@ struct pci_dev;
#ifdef CONFIG_PCI
/* Create a virtual mapping cookie for a PCI BAR (memory or IO) */
extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
+extern void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max);
extern void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
unsigned long offset,
unsigned long maxlen);
+extern void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
+ unsigned long offset,
+ unsigned long maxlen);
/* Create a virtual mapping cookie for a port on a given PCI device.
* Do not call this directly, it exists to make it easier for architectures
* to override */
@@ -34,12 +38,22 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lon
return NULL;
}
+static inline void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max)
+{
+ return NULL;
+}
static inline void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
unsigned long offset,
unsigned long maxlen)
{
return NULL;
}
+static inline void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
+ unsigned long offset,
+ unsigned long maxlen)
+{
+ return NULL;
+}
#endif
#endif /* __ASM_GENERIC_IO_H */
diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c
index bcce5f1..9604dcb 100644
--- 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);
#endif /* CONFIG_PCI */
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* [PATCH v6 2/4] video: fbdev: arkfb: 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, Laurent Pinchart,
Geert Uytterhoeven, 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>
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: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
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/arkfb.c | 36 +++++-------------------------------
1 file changed, 5 insertions(+), 31 deletions(-)
diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index b305a1e..6a317de 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.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 arkfb_info {
int mclk_freq;
- int mtrr_reg;
+ int wc_cookie;
struct dac_info *dac;
struct vgastate state;
@@ -102,10 +98,6 @@ static const struct svga_timing_regs ark_timing_regs = {
static char *mode_option = "640x480-8@60";
-#ifdef CONFIG_MTRR
-static int mtrr = 1;
-#endif
-
MODULE_AUTHOR("(c) 2007 Ondrej Zajicek <santiago@crfreenet.org>");
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("fbdev driver for ARK 2000PV");
@@ -115,11 +107,6 @@ MODULE_PARM_DESC(mode_option, "Default video mode ('640x480-8@60', etc)");
module_param_named(mode, mode_option, charp, 0444);
MODULE_PARM_DESC(mode, "Default video mode ('640x480-8@60', etc) (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
-
static int threshold = 4;
module_param(threshold, int, 0644);
@@ -1002,7 +989,7 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
info->fix.smem_len = pci_resource_len(dev, 0);
/* 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");
@@ -1057,14 +1044,8 @@ static int ark_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
-
+ par->wc_cookie = arch_phys_wc_add(info->fix.smem_start,
+ info->fix.smem_len);
return 0;
/* Error handling */
@@ -1092,14 +1073,7 @@ static void ark_pci_remove(struct pci_dev *dev)
if (info) {
struct arkfb_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);
dac_release(par->dac);
unregister_framebuffer(info);
fb_dealloc_cmap(&info->cmap);
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* [PATCH v6 3/4] video: fbdev: s3fb: 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,
Jean-Christophe Plagniol-Villard, Jingoo Han, Geert Uytterhoeven,
Daniel Vetter, Lad, Prabhakar, Rickard Strandqvist, Suresh Siddha,
Ingo Molnar, Thomas Gleixner, Juergen Gross, Dave Airlie,
Antonino Daplas
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: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
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: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.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/s3fb.c | 35 ++++++-----------------------------
1 file changed, 6 insertions(+), 29 deletions(-)
diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index f0ae61a..13b1090 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -28,13 +28,9 @@
#include <linux/i2c.h>
#include <linux/i2c-algo-bit.h>
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
-
struct s3fb_info {
int chip, rev, mclk_freq;
- int mtrr_reg;
+ int wc_cookie;
struct vgastate state;
struct mutex open_lock;
unsigned int ref_count;
@@ -154,11 +150,7 @@ static const struct svga_timing_regs s3_timing_regs = {
static char *mode_option;
-
-#ifdef CONFIG_MTRR
static int mtrr = 1;
-#endif
-
static int fasttext = 1;
@@ -170,11 +162,8 @@ module_param(mode_option, charp, 0444);
MODULE_PARM_DESC(mode_option, "Default video mode ('640x480-8@60', etc)");
module_param_named(mode, mode_option, charp, 0444);
MODULE_PARM_DESC(mode, "Default video mode ('640x480-8@60', etc) (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
module_param(fasttext, int, 0644);
MODULE_PARM_DESC(fasttext, "Enable S3 fast text mode (1=enable, 0=disable, default=1)");
@@ -1168,7 +1157,7 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
info->fix.smem_len = pci_resource_len(dev, 0);
/* 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");
@@ -1365,12 +1354,9 @@ static int s3_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;
@@ -1405,14 +1391,7 @@ static void s3_pci_remove(struct pci_dev *dev)
if (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);
@@ -1551,10 +1530,8 @@ static int __init s3fb_setup(char *options)
if (!*opt)
continue;
-#ifdef CONFIG_MTRR
else if (!strncmp(opt, "mtrr:", 5))
mtrr = simple_strtoul(opt + 5, NULL, 0);
-#endif
else if (!strncmp(opt, "fasttext:", 9))
fasttext = simple_strtoul(opt + 9, NULL, 0);
else
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* [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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox