* [PATCH] fbcon: Avoid corrupting active workqueue entries
From: Chris Wilson @ 2013-12-17 14:27 UTC (permalink / raw)
To: intel-gfx
Cc: Chris Wilson, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
linux-fbdev, dri-devel, stable
We attempt to reschedule an active work which can itself corrupt the
workqueue lists, and we may then free the work item whilst the task is
still pending. Both of these may lead to a system deadlock and an
unresponsive machine without any outputs for a panic to even be shown.
[ 7.372961] WARNING: at kernel/workqueue.c:1365 __queue_work+0x216/0x292()
[ 7.372964] Modules linked in: coretemp arc4 kvm_intel kvm iwldvm crc32c_intel mac80211 ghash_clmulni_intel cryptd joydev hid_lenovo_tpkbd lib80211 iTCO_wdt iwlwifi iTCO_vendor_support i915(+) btusb snd_hda_codec_hdmi bluetooth evdev snd_hda_intel usbhid snd_hda_codec pcspkr hid cfg80211 microcode snd_hwdep i2c_i801 snd_pcm drm_kms_helper lpc_ich drm mfd_core snd_page_alloc rfkill snd_timer snd soundcore mei_me i2c_algo_bit video mei acpi_cpufreq mperf i2c_core button processor ext4 crc16 jbd2 mbcache sg sd_mod crc_t10dif ahci libahci libata scsi_mod thermal fan ehci_pci ehci_hcd thermal_sys usbcore usb_common
[ 7.373068] CPU: 0 PID: 660 Comm: ps Not tainted 3.10.9+ #55
[ 7.373071] Hardware name: /D33217CK, BIOS GKPPT10H.86A.0025.2012.1011.1534 10/11/2012
[ 7.373075] ffffffff81596a1e ffff88045f203d38 ffffffff813eaef6 ffff88045f203d78
[ 7.373083] ffffffff81041027 ffff88045f203d78 0000000000000000 ffff88045f217f00
[ 7.373091] ffff88044a89c800 ffff88042b473aa0 0000000000000000 ffff88045f203d88
[ 7.373098] Call Trace:
[ 7.373101] <IRQ> [<ffffffff813eaef6>] dump_stack+0x19/0x1b
[ 7.373115] [<ffffffff81041027>] warn_slowpath_common+0x62/0x7b
[ 7.373121] [<ffffffff81041055>] warn_slowpath_null+0x15/0x17
[ 7.373127] [<ffffffff8105aa82>] __queue_work+0x216/0x292
[ 7.373133] [<ffffffff8105ab65>] queue_work_on+0x4c/0x7c
[ 7.373140] [<ffffffff8123cebb>] ? fbcon_add_cursor_timer+0xfb/0xfb
[ 7.373146] [<ffffffff8123cee1>] cursor_timer_handler+0x26/0x42
[ 7.373153] [<ffffffff8104ee1f>] call_timer_fn+0xcc/0x1ea
[ 7.373160] [<ffffffff8104ed53>] ? detach_if_pending+0x7a/0x7a
[ 7.373166] [<ffffffff8123cebb>] ? fbcon_add_cursor_timer+0xfb/0xfb
[ 7.373172] [<ffffffff8104f27b>] run_timer_softirq+0x19c/0x1e4
[ 7.373178] [<ffffffff8104874e>] ? __do_softirq+0x9e/0x2a7
[ 7.373183] [<ffffffff810487e9>] __do_softirq+0x139/0x2a7
[ 7.373189] [<ffffffff81048a7a>] irq_exit+0x56/0x9b
[ 7.373196] [<ffffffff8102af31>] smp_apic_timer_interrupt+0x77/0x85
[ 7.373203] [<ffffffff813f5ff2>] apic_timer_interrupt+0x72/0x80
[ 7.373206] <EOI> [<ffffffff8113ea70>] ? spin_lock+0x9/0xb
[ 7.373217] [<ffffffff8120d8c1>] ? do_raw_spin_trylock+0x42/0x42
[ 7.373223] [<ffffffff813ef2e0>] ? _raw_spin_unlock+0x23/0x36
[ 7.373229] [<ffffffff8113ea7b>] spin_unlock+0x9/0xb
[ 7.373235] [<ffffffff8113fd25>] dput+0xd9/0xf8
[ 7.373241] [<ffffffff8113685e>] path_put+0x13/0x20
[ 7.373247] [<ffffffff8113a6f3>] do_last+0x925/0xa0d
[ 7.373253] [<ffffffff81137fa4>] ? inode_permission+0x40/0x42
[ 7.373259] [<ffffffff8113a89c>] path_openat+0xc1/0x325
[ 7.373265] [<ffffffff8113ae0c>] do_filp_open+0x33/0x81
[ 7.373271] [<ffffffff811455bd>] ? __alloc_fd+0x169/0x17b
[ 7.373279] [<ffffffff8112d78f>] do_sys_open+0x67/0xf4
[ 7.373285] [<ffffffff8112d839>] SyS_open+0x1d/0x1f
[ 7.373290] [<ffffffff813f5369>] system_call_fastpath+0x16/0x1b
[ 7.373294] ---[ end trace 78bba0b9776072a9 ]---
[ 7.538936] fbcon: inteldrmfb (fb0) is primary device
[ 7.539446] Console: switching consoles 2-6 to frame buffer device
[ 7.539463] i915 0000:00:02.0: fb0: inteldrmfb frame buffer device
[ 7.539468] i915 0000:00:02.0: registered panic notifier
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?idr765
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: stable@vger.kernel.org
---
drivers/video/console/fbcon.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index cd8a8027f8ae..f474976f6b34 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -397,6 +397,8 @@ static void fb_flashcursor(struct work_struct *work)
ops->cursor(vc, info, mode, softback_lines, get_color(vc, info, c, 1),
get_color(vc, info, c, 0));
console_unlock();
+
+ mod_timer(&ops->cursor_timer, jiffies + HZ/5);
}
static void cursor_timer_handler(unsigned long dev_addr)
@@ -405,7 +407,6 @@ static void cursor_timer_handler(unsigned long dev_addr)
struct fbcon_ops *ops = info->fbcon_par;
queue_work(system_power_efficient_wq, &info->queue);
- mod_timer(&ops->cursor_timer, jiffies + HZ/5);
}
static void fbcon_add_cursor_timer(struct fb_info *info)
@@ -417,6 +418,8 @@ static void fbcon_add_cursor_timer(struct fb_info *info)
!fbcon_cursor_noblink) {
if (!info->queue.func)
INIT_WORK(&info->queue, fb_flashcursor);
+ else
+ flush_work(&info->queue);
init_timer(&ops->cursor_timer);
ops->cursor_timer.function = cursor_timer_handler;
@@ -433,6 +436,7 @@ static void fbcon_del_cursor_timer(struct fb_info *info)
if (info->queue.func = fb_flashcursor &&
ops->flags & FBCON_FLAGS_CURSOR_TIMER) {
+ flush_work(&info->queue);
del_timer_sync(&ops->cursor_timer);
ops->flags &= ~FBCON_FLAGS_CURSOR_TIMER;
}
--
1.8.5.1
^ permalink raw reply related
* Re: [Xen-devel] [PATCH v3 1/2] xen/pvhvm: If xen_platform_pci=0 is set don't blow up (v3).
From: Konrad Rzeszutek Wilk @ 2013-12-17 14:51 UTC (permalink / raw)
To: Fabio Fantoni
Cc: axboe, stefano.stabellini, ian.campbell, xen-devel, linux-kernel,
boris.ostrovsky, david.vrabel, leosilva, ashley, peterhuewe, mail,
tpmdd, dmitry.torokhov, bhelgaas, plagnioj, tomi.valkeinen,
tpmdd-devel, linux-input, netdev, linux-pci, linux-fbdev
In-Reply-To: <52B01F67.1030108@m2r.biz>
On Tue, Dec 17, 2013 at 10:54:47AM +0100, Fabio Fantoni wrote:
> Il 16/12/2013 16:04, Konrad Rzeszutek Wilk ha scritto:
> >The user has the option of disabling the platform driver:
> >00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
> >
> >which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
> >and allow the PV drivers to take over. If the user wishes
> >to disable that they can set:
> >
> > xen_platform_pci=0
> > (in the guest config file)
> >
> >or
> > xen_emul_unplug=never
> > (on the Linux command line)
> >
> >except it does not work properly. The PV drivers still try to
> >load and since the Xen platform driver is not run - and it
> >has not initialized the grant tables, most of the PV drivers
> >stumble upon:
> >
> >input: Xen Virtual Keyboard as /devices/virtual/input/input5
> >input: Xen Virtual Pointer as /devices/virtual/input/input6M
> >------------[ cut here ]------------
> >kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
> >invalid opcode: 0000 [#1] SMP
> >Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
> >CPU: 6 PID: 1389 Comm: modprobe Not tainted 3.13.0-rc1upstream-00021-ga6c892b-dirty #1
> >Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
> >RIP: 0010:[<ffffffff813ddc40>] [<ffffffff813ddc40>] get_free_entries+0x2e0/0x300
> >Call Trace:
> > [<ffffffff8150d9a3>] ? evdev_connect+0x1e3/0x240
> > [<ffffffff813ddd0e>] gnttab_grant_foreign_access+0x2e/0x70
> > [<ffffffffa0010081>] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
> > [<ffffffffa0010a12>] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
> > [<ffffffff813e5757>] xenbus_dev_probe+0x77/0x130
> > [<ffffffff813e7217>] xenbus_frontend_dev_probe+0x47/0x50
> > [<ffffffff8145e9a9>] driver_probe_device+0x89/0x230
> > [<ffffffff8145ebeb>] __driver_attach+0x9b/0xa0
> > [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
> > [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
> > [<ffffffff8145cf1c>] bus_for_each_dev+0x8c/0xb0
> > [<ffffffff8145e7d9>] driver_attach+0x19/0x20
> > [<ffffffff8145e260>] bus_add_driver+0x1a0/0x220
> > [<ffffffff8145f1ff>] driver_register+0x5f/0xf0
> > [<ffffffff813e55c5>] xenbus_register_driver_common+0x15/0x20
> > [<ffffffff813e76b3>] xenbus_register_frontend+0x23/0x40
> > [<ffffffffa0015000>] ? 0xffffffffa0014fff
> > [<ffffffffa001502b>] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
> > [<ffffffff81002049>] do_one_initcall+0x49/0x170
> >
> >.. snip..
> >
> >which is hardly nice. This patch fixes this by having each
> >PV driver check for:
> > - if running in PV, then it is fine to execute (as that is their
> > native environment).
> > - if running in HVM, check if user wanted 'xen_emul_unplug=never',
> > in which case bail out and don't load any PV drivers.
> > - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
> > does not exist, then bail out and not load PV drivers.
> > - (v2) if running in HVM, and if the user wanted 'xen_emul_unplug=disks',
> > then bail out for all PV devices _except_ the block one.
> > Ditto for the network one ('nics').
> > - (v2) if running in HVM, and if the user wanted 'xen_emul_unplug=unnecessary'
> > then load block PV driver, and also setup the legacy IDE paths.
> > In (v3) make it actually load PV drivers.
> >
> >Reported-by: Sander Eikelenboom <linux@eikelenboom.it
> >Reported-by: Anthony PERARD <anthony.perard@citrix.com>
> >Reported-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> >Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >[v2: Add extra logic to handle the myrid ways 'xen_emul_unplug'
> >can be used per Ian and Stefano suggestion]
> >[v3: Make the unnecessary case work properly]
> >Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> I tested this patch with all possible cases that I know, no crash or
> calltrace found.
>
> I don't understand the utility of 'xen_emul_unplug=unnecessary' but
> probably it is working correctly, it shows both pv and not pv for
> blocks and nics.
Great!
>
> About 'xen_emul_unplug=disks' and 'xen_emul_unplug=nics' probably
> there is something wrong.
> With 'xen_emul_unplug=nics' it shows pv nic (this should be correct)
> and about disks it shows the same disk twice, as pv and not pv (xvda
> and sda) with different device number.
Good.
> With 'xen_emul_unplug=disks' it shows pv block (this should be
> correct) and it seems to show the pv nic too (this should be wrong).
Correct. You should see only the PV disk and all other PV devices
should not function. Let me dig in this and see whether there is
another bug. Thank you!
>
> Thanks for any reply.
>
> >---
> > arch/x86/xen/platform-pci-unplug.c | 74 ++++++++++++++++++++++++++++++
> > drivers/block/xen-blkfront.c | 4 +-
> > drivers/char/tpm/xen-tpmfront.c | 4 ++
> > drivers/input/misc/xen-kbdfront.c | 4 ++
> > drivers/net/xen-netfront.c | 2 +-
> > drivers/pci/xen-pcifront.c | 4 ++
> > drivers/video/xen-fbfront.c | 4 ++
> > drivers/xen/xenbus/xenbus_probe_frontend.c | 2 +-
> > include/xen/platform_pci.h | 23 ++++++++++
> > 9 files changed, 117 insertions(+), 4 deletions(-)
> >
> >diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
> >index 0a78524..ab84ac1 100644
> >--- a/arch/x86/xen/platform-pci-unplug.c
> >+++ b/arch/x86/xen/platform-pci-unplug.c
> >@@ -69,6 +69,80 @@ static int check_platform_magic(void)
> > return 0;
> > }
> >+bool xen_has_pv_devices()
> >+{
> >+ if (!xen_domain())
> >+ return false;
> >+
> >+ /* PV domains always have them. */
> >+ if (xen_pv_domain())
> >+ return true;
> >+
> >+ /* And user has xen_platform_pci=0 set in guest config as
> >+ * driver did not modify the value. */
> >+ if (xen_platform_pci_unplug = 0)
> >+ return false;
> >+
> >+ if (xen_platform_pci_unplug & XEN_UNPLUG_NEVER)
> >+ return false;
> >+
> >+ if (xen_platform_pci_unplug & XEN_UNPLUG_ALL)
> >+ return true;
> >+
> >+ /* This is an odd one - we are going to run legacy
> >+ * and PV drivers at the same time. */
> >+ if (xen_platform_pci_unplug & XEN_UNPLUG_UNNECESSARY)
> >+ return true;
> >+
> >+ /* And the caller has to follow with xen_pv_{disk,nic}_devices
> >+ * to be certain which driver can load. */
> >+ return false;
> >+}
> >+EXPORT_SYMBOL_GPL(xen_has_pv_devices);
> >+
> >+static bool __xen_has_pv_device(int state)
> >+{
> >+ /* HVM domains might or might not */
> >+ if (xen_hvm_domain() && (xen_platform_pci_unplug & state))
> >+ return true;
> >+
> >+ return xen_has_pv_devices();
> >+}
> >+
> >+bool xen_has_pv_nic_devices(void)
> >+{
> >+ return __xen_has_pv_device(XEN_UNPLUG_ALL_NICS | XEN_UNPLUG_ALL);
> >+}
> >+EXPORT_SYMBOL_GPL(xen_has_pv_nic_devices);
> >+
> >+bool xen_has_pv_disk_devices(void)
> >+{
> >+ return __xen_has_pv_device(XEN_UNPLUG_ALL_IDE_DISKS |
> >+ XEN_UNPLUG_AUX_IDE_DISKS | XEN_UNPLUG_ALL);
> >+}
> >+EXPORT_SYMBOL_GPL(xen_has_pv_disk_devices);
> >+
> >+/*
> >+ * This one is odd - it determines whether you want to run PV _and_
> >+ * legacy (IDE) drivers together. This combination is only possible
> >+ * under HVM.
> >+ */
> >+bool xen_has_pv_and_legacy_disk_devices(void)
> >+{
> >+ if (!xen_domain())
> >+ return false;
> >+
> >+ /* N.B. This is only ever used in HVM mode */
> >+ if (xen_pv_domain())
> >+ return false;
> >+
> >+ if (xen_platform_pci_unplug & XEN_UNPLUG_UNNECESSARY)
> >+ return true;
> >+
> >+ return false;
> >+}
> >+EXPORT_SYMBOL_GPL(xen_has_pv_and_legacy_disk_devices);
> >+
> > void xen_unplug_emulated_devices(void)
> > {
> > int r;
> >diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> >index a4660bb..ed88b3c 100644
> >--- a/drivers/block/xen-blkfront.c
> >+++ b/drivers/block/xen-blkfront.c
> >@@ -1278,7 +1278,7 @@ static int blkfront_probe(struct xenbus_device *dev,
> > char *type;
> > int len;
> > /* no unplug has been done: do not hook devices != xen vbds */
> >- if (xen_platform_pci_unplug & XEN_UNPLUG_UNNECESSARY) {
> >+ if (xen_has_pv_and_legacy_disk_devices()) {
> > int major;
> > if (!VDEV_IS_EXTENDED(vdevice))
> >@@ -2022,7 +2022,7 @@ static int __init xlblk_init(void)
> > if (!xen_domain())
> > return -ENODEV;
> >- if (xen_hvm_domain() && !xen_platform_pci_unplug)
> >+ if (!xen_has_pv_disk_devices())
> > return -ENODEV;
> > if (register_blkdev(XENVBD_MAJOR, DEV_NAME)) {
> >diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> >index 06189e5..9c2cbd1 100644
> >--- a/drivers/char/tpm/xen-tpmfront.c
> >+++ b/drivers/char/tpm/xen-tpmfront.c
> >@@ -16,6 +16,7 @@
> > #include <xen/xenbus.h>
> > #include <xen/page.h>
> > #include "tpm.h"
> >+#include <xen/platform_pci.h>
> > struct tpm_private {
> > struct tpm_chip *chip;
> >@@ -422,6 +423,9 @@ static int __init xen_tpmfront_init(void)
> > if (!xen_domain())
> > return -ENODEV;
> >+ if (!xen_has_pv_devices())
> >+ return -ENODEV;
> >+
> > return xenbus_register_frontend(&tpmfront_driver);
> > }
> > module_init(xen_tpmfront_init);
> >diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> >index e21c181..fbfdc10 100644
> >--- a/drivers/input/misc/xen-kbdfront.c
> >+++ b/drivers/input/misc/xen-kbdfront.c
> >@@ -29,6 +29,7 @@
> > #include <xen/interface/io/fbif.h>
> > #include <xen/interface/io/kbdif.h>
> > #include <xen/xenbus.h>
> >+#include <xen/platform_pci.h>
> > struct xenkbd_info {
> > struct input_dev *kbd;
> >@@ -380,6 +381,9 @@ static int __init xenkbd_init(void)
> > if (xen_initial_domain())
> > return -ENODEV;
> >+ if (!xen_has_pv_devices())
> >+ return -ENODEV;
> >+
> > return xenbus_register_frontend(&xenkbd_driver);
> > }
> >diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> >index 36808bf..eea2392 100644
> >--- a/drivers/net/xen-netfront.c
> >+++ b/drivers/net/xen-netfront.c
> >@@ -2106,7 +2106,7 @@ static int __init netif_init(void)
> > if (!xen_domain())
> > return -ENODEV;
> >- if (xen_hvm_domain() && !xen_platform_pci_unplug)
> >+ if (!xen_has_pv_nic_devices())
> > return -ENODEV;
> > pr_info("Initialising Xen virtual ethernet driver\n");
> >diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> >index f7197a7..eae7cd9 100644
> >--- a/drivers/pci/xen-pcifront.c
> >+++ b/drivers/pci/xen-pcifront.c
> >@@ -20,6 +20,7 @@
> > #include <linux/workqueue.h>
> > #include <linux/bitops.h>
> > #include <linux/time.h>
> >+#include <xen/platform_pci.h>
> > #include <asm/xen/swiotlb-xen.h>
> > #define INVALID_GRANT_REF (0)
> >@@ -1138,6 +1139,9 @@ static int __init pcifront_init(void)
> > if (!xen_pv_domain() || xen_initial_domain())
> > return -ENODEV;
> >+ if (!xen_has_pv_devices())
> >+ return -ENODEV;
> >+
> > pci_frontend_registrar(1 /* enable */);
> > return xenbus_register_frontend(&xenpci_driver);
> >diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
> >index cd005c2..4b2d3ab 100644
> >--- a/drivers/video/xen-fbfront.c
> >+++ b/drivers/video/xen-fbfront.c
> >@@ -35,6 +35,7 @@
> > #include <xen/interface/io/fbif.h>
> > #include <xen/interface/io/protocols.h>
> > #include <xen/xenbus.h>
> >+#include <xen/platform_pci.h>
> > struct xenfb_info {
> > unsigned char *fb;
> >@@ -699,6 +700,9 @@ static int __init xenfb_init(void)
> > if (xen_initial_domain())
> > return -ENODEV;
> >+ if (!xen_has_pv_devices())
> >+ return -ENODEV;
> >+
> > return xenbus_register_frontend(&xenfb_driver);
> > }
> >diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
> >index 34b20bf..6244f9c 100644
> >--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
> >+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
> >@@ -496,7 +496,7 @@ subsys_initcall(xenbus_probe_frontend_init);
> > #ifndef MODULE
> > static int __init boot_wait_for_devices(void)
> > {
> >- if (xen_hvm_domain() && !xen_platform_pci_unplug)
> >+ if (!xen_has_pv_devices())
> > return -ENODEV;
> > ready_to_wait_for_devices = 1;
> >diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h
> >index 438c256..b49eeab 100644
> >--- a/include/xen/platform_pci.h
> >+++ b/include/xen/platform_pci.h
> >@@ -48,4 +48,27 @@ static inline int xen_must_unplug_disks(void) {
> > extern int xen_platform_pci_unplug;
> >+#if defined(CONFIG_XEN_PVHVM)
> >+extern bool xen_has_pv_devices(void);
> >+extern bool xen_has_pv_disk_devices(void);
> >+extern bool xen_has_pv_nic_devices(void);
> >+extern bool xen_has_pv_and_legacy_disk_devices(void);
> >+#else
> >+static inline bool xen_has_pv_devices(void)
> >+{
> >+ return IS_ENABLED(CONFIG_XEN);
> >+}
> >+static inline bool xen_has_pv_disk_devices(void)
> >+{
> >+ return IS_ENABLED(CONFIG_XEN);
> >+}
> >+static inline bool xen_has_pv_nic_devices(void)
> >+{
> >+ return IS_ENABLED(CONFIG_XEN);
> >+}
> >+static inline bool xen_has_pv_and_legacy_disk_devices(void)
> >+{
> >+ return false;
> >+}
> >+#endif
> > #endif /* _XEN_PLATFORM_PCI_H */
>
^ permalink raw reply
* Re: [PATCHv2 23/27] OMAPDSS: connector-dvi: Add DT support
From: Laurent Pinchart @ 2013-12-17 15:15 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Sascha Hauer, linux-omap, linux-fbdev, devicetree, Tony Lindgren
In-Reply-To: <52AFFA2E.5050702@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2606 bytes --]
Hi Tomi,
On Tuesday 17 December 2013 09:15:58 Tomi Valkeinen wrote:
> On 2013-12-17 09:05, Sascha Hauer wrote:
> > On Mon, Dec 16, 2013 at 04:56:30PM +0200, Tomi Valkeinen wrote:
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >> ---
> >>
> >> drivers/video/omap2/displays-new/connector-dvi.c | 43 ++++++++++++++++++
> >> 1 file changed, 43 insertions(+)
> >>
> >> diff --git a/drivers/video/omap2/displays-new/connector-dvi.c
> >> b/drivers/video/omap2/displays-new/connector-dvi.c index
> >> b6c50904038e..d1204b1c5182 100644
> >>
> >> +static const struct of_device_id dvic_of_match[] = {
> >> + { .compatible = "dvi-connector", },
> >
> > Either the driver is too specific or the binding is too generic, but
> > having such a generic name for an omap specific driver seems wrong. Same
> > for panel-dpi, svideo-connector, composite-video-connector and
> > hdmi-connector,
>
> Hmm. Good point. I was thinking that the driver is only used on OMAP, but of
> course that's not true, the driver is there for all platforms if the kernel
> just happens to be compiled with the driver.
>
> And it's not just about those drivers you mention. The same issue is there
> for, say, "ti,tpd12s015". I have an omapdss specific driver for that, but if
> some other platform uses the same chip, they'll have a driver for it also...
>
> Sigh. I wonder how this should be handled... The only solution that comes to
> my mind is to have all the compatible strings as "ti,...". But that's not
> correct, as they are not TI components, but some are generic ones and some
> from another vendor.
>
> And even "ti,..." is not good, as there are other TI SoCs with other display
> drivers. So I'd need to prepend the compatible strings with "omapdss,...",
> making the hardware components driver specific.
>
> The long term plan is to make the drivers generic (or implement the same
> driver for common display framework). But for now I need to have future
> proof DT bindings with omapdss specific drivers...
I'll refrain from mentioning that the problem has been identified more than a
year ago. Oh, wait, I've metioned it, sorry :-D
We really want to make drivers generic enough to be shared by multiple display
controllers. I would vote for making the DT bindings generic now already,
which would be enough to buy some time to fix the problem properly. If we
start prepending driver-specific prefixes such as "omapdss," to compatible
string we'll just make the mess even larger and reduce the incentive to fix
it. It would be the worst decision we could make.
--
Regards,
Laurent Pinchart
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCHv2 01/27] ARM: OMAP: remove DSS DT hack
From: Tony Lindgren @ 2013-12-17 15:30 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, devicetree
In-Reply-To: <52AFF31C.6010100@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [131216 22:47]:
> On 2013-12-16 20:41, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [131216 06:59]:
> >> As a temporary solution to enable DSS for selected boards when booting
> >> with DT, a hack was added to board-generic.c in
> >> 63d5fc0c2f748e20f38a0a0ec1c8494bddf5c288 (OMAP: board-generic: enable
> >> DSS for panda & sdp boards).
> >>
> >> We're now adding proper DT support, so the hack can be removed.
> >
> > I guess this patch should be merged later on after we have the DT support
> > working?
>
> I'll move this patch also to the end of the series.
>
> "Merged later" sounds like you mean this could be merged in a separate
> series. I think this and the other removal can be in this series, they
> just need to be at the end.
Well yeah let's keep those separate still as at least Russell needed
some more time with the legacy booting. The point we can drop the
legacy booting for omap3 may still need to wait a bit, maybe even
until v3.15 to keep things working.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] video: amba-clcd: Make CLCD driver available on more platforms
From: Catalin Marinas @ 2013-12-17 16:27 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1386868390-12231-1-git-send-email-broonie@kernel.org>
On 12 December 2013 17:13, Mark Brown <broonie@kernel.org> wrote:
> From: Mark Brown <broonie@linaro.org>
>
> The CLCD driver is used on ARM reference models for ARMv8 so add ARM64
> to the list of dependencies. The driver also has no build time dependencies
> on ARM (stubs are provided for ARM-specific DMA functions in the code) so
> make it available with COMPILE_TEST in order to maximise build coverage.
I haven't followed other CLCD patches but for arm64 it requires
support for DT. I have been carrying some patches (done by Linaro) for
a long time and Pawel @ ARM has another set of patches. Do you know
what the status for those is before enabling CLCD on arm64? That's
unless you are only looking at compile-testing.
--
Catalin
^ permalink raw reply
* [PATCH] A patch to viafbdev.c
From: objectkuan @ 2013-12-17 16:57 UTC (permalink / raw)
To: linux-fbdev
There is a potential integer overflow in viafb_ioctl() if
users pass an unexpected value to viafb_second_size. An if
statement might be a simple solution to this issue.
Signed-off-by: objectkuan <objectkuan@gmail.com>
---
drivers/video/via/viafbdev.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 325c43c..382368e 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -463,6 +463,10 @@ static int viafb_ioctl(struct fb_info *info, u_int cmd, u_long arg)
u.viasamm.size_prim = viaparinfo->fbmem_free;
u.viasamm.size_sec = viaparinfo1->fbmem_free;
} else {
+ if (0 > viafb_second_size
+ || viafb_second_size > INT_MAX / 1024 / 1024) {
+ return -EFAULT;
+ }
if (viafb_second_size) {
u.viasamm.size_prim viaparinfo->fbmem_free -
@@ -1819,6 +1822,10 @@ int via_fb_pci_probe(struct viafb_dev *vdev)
default_var.accel_flags = 0;
}
+ if (0 > viafb_second_size
+ || viafb_second_size > INT_MAX / 1024 / 1024) {
+ return -EFAULT;
+ }
if (viafb_second_size && (viafb_second_size < 8)) {
viafb_second_offset = viaparinfo->fbmem_free -
viafb_second_size * 1024 * 1024;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 0/4] OMAPDSS: DT support for N900 panel
From: Sebastian Reichel @ 2013-12-17 17:14 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Benoît Cousson, Tony Lindgren, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
linux-omap, linux-fbdev, devicetree
In-Reply-To: <52AFFF3F.7020807@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]
On Tue, Dec 17, 2013 at 09:37:35AM +0200, Tomi Valkeinen wrote:
> Hi,
>
> On 2013-12-13 20:17, Sebastian Reichel wrote:
> > Hi,
> >
> > This patchset adds DT support for the N900 panel. The patchset is based on
> > Tomi's work/dss-dt-2 branch [0]. I suggest to send the DT changes through
> > Benoits queue, since I have more N900 DT changes for 3.14. Also the patch
> > editing the rx51 boardcode can be dropped, since the file is removed in 3.14.
> > I included those two with this patchset, since they are needed to test the
> > other two patches.
> >
> > I did not include a documentation for the DT API, since the omapdss
> > documentation is still missing.
> >
> > I have successfully tested this on the N900.
> >
> > [0] https://git.kernel.org/cgit/linux/kernel/git/tomba/linux.git/log/?h=work/dss-dt-2
>
> I added N900 display DT support on top of my v2 series, including
> pinmuxing. Can you check if it looks right and works?
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git work/dss-dt
I just tried it and it does not work. On a first look the pinmuxing
looks fishy: 0x0d4 is muxed two times.
I will have a more detailed look into this later.
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCHv2 17/27] ARM: omap3-tobi.dts: add lcd (TEST)
From: Florian Vaussard @ 2013-12-17 17:17 UTC (permalink / raw)
To: Tomi Valkeinen, linux-omap, linux-fbdev, devicetree
In-Reply-To: <1387205794-32246-18-git-send-email-tomi.valkeinen@ti.com>
Hello
On 12/16/2013 03:56 PM, Tomi Valkeinen wrote:
> This is a test for Overo with Palo43 expansion, _not_ Tobi. Palo43
> doesn't have a dts, but seems to work ok with omap3-tobi.dts, so I used
> it as a test.
>
> Not to be merged.
>
FYI, I got the DVI (HDMI connector) on the Overo/Tobi to work with the patch
below (not to be merged, and probably corrupted by our mail server), until a
framebuffer console. So for me:
Tested-by: Florian Vaussard <florian.vaussard@epfl.ch>
Regards,
Florian
8< --------------------------------
From 29237c38d35b833efe304b1a58463127555c4748 Mon Sep 17 00:00:00 2001
From: Florian Vaussard <florian.vaussard@epfl.ch>
Date: Tue, 17 Dec 2013 17:47:04 +0100
Subject: [PATCH] ARM: omap3-tobi.dts: add dvi output
Add DVI output to Overo/Tobi board.
Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
arch/arm/boot/dts/omap3-overo.dtsi | 33 +++++++++++++++++++++++
arch/arm/boot/dts/omap3-tobi.dts | 54
++++++++++++++++++++++++++++++++++++++
2 files changed, 87 insertions(+)
diff --git a/arch/arm/boot/dts/omap3-overo.dtsi
b/arch/arm/boot/dts/omap3-overo.dtsi
index a461d2f..90b08f6 100644
--- a/arch/arm/boot/dts/omap3-overo.dtsi
+++ b/arch/arm/boot/dts/omap3-overo.dtsi
@@ -89,6 +89,39 @@
0x170 (PIN_OUTPUT | MUX_MODE0) /*
uart3_tx_irtx.uart3_tx_irtx */
>;
};
+
+ dss_dpi_pins: pinmux_dss_dpi_pins {
+ pinctrl-single,pins = <
+ 0x0a4 (PIN_OUTPUT | MUX_MODE0) /*
dss_pclk.dss_pclk */
+ 0x0a6 (PIN_OUTPUT | MUX_MODE0) /*
dss_hsync.dss_hsync */
+ 0x0a8 (PIN_OUTPUT | MUX_MODE0) /*
dss_vsync.dss_vsync */
+ 0x0aa (PIN_OUTPUT | MUX_MODE0) /*
dss_acbias.dss_acbias */
+ 0x0ac (PIN_OUTPUT | MUX_MODE0) /*
dss_data0.dss_data0 */
+ 0x0ae (PIN_OUTPUT | MUX_MODE0) /*
dss_data1.dss_data1 */
+ 0x0b0 (PIN_OUTPUT | MUX_MODE0) /*
dss_data2.dss_data2 */
+ 0x0b2 (PIN_OUTPUT | MUX_MODE0) /*
dss_data3.dss_data3 */
+ 0x0b4 (PIN_OUTPUT | MUX_MODE0) /*
dss_data4.dss_data4 */
+ 0x0b6 (PIN_OUTPUT | MUX_MODE0) /*
dss_data5.dss_data5 */
+ 0x0b8 (PIN_OUTPUT | MUX_MODE0) /*
dss_data6.dss_data6 */
+ 0x0ba (PIN_OUTPUT | MUX_MODE0) /*
dss_data7.dss_data7 */
+ 0x0bc (PIN_OUTPUT | MUX_MODE0) /*
dss_data8.dss_data8 */
+ 0x0be (PIN_OUTPUT | MUX_MODE0) /*
dss_data9.dss_data9 */
+ 0x0c0 (PIN_OUTPUT | MUX_MODE0) /*
dss_data10.dss_data10 */
+ 0x0c2 (PIN_OUTPUT | MUX_MODE0) /*
dss_data11.dss_data11 */
+ 0x0c4 (PIN_OUTPUT | MUX_MODE0) /*
dss_data12.dss_data12 */
+ 0x0c6 (PIN_OUTPUT | MUX_MODE0) /*
dss_data13.dss_data13 */
+ 0x0c8 (PIN_OUTPUT | MUX_MODE0) /*
dss_data14.dss_data14 */
+ 0x0ca (PIN_OUTPUT | MUX_MODE0) /*
dss_data15.dss_data15 */
+ 0x0cc (PIN_OUTPUT | MUX_MODE0) /*
dss_data16.dss_data16 */
+ 0x0ce (PIN_OUTPUT | MUX_MODE0) /*
dss_data17.dss_data17 */
+ 0x0d0 (PIN_OUTPUT | MUX_MODE0) /*
dss_data18.dss_data18 */
+ 0x0d2 (PIN_OUTPUT | MUX_MODE0) /*
dss_data19.dss_data19 */
+ 0x0d4 (PIN_OUTPUT | MUX_MODE0) /*
dss_data20.dss_data20 */
+ 0x0d6 (PIN_OUTPUT | MUX_MODE0) /*
dss_data21.dss_data21 */
+ 0x0d8 (PIN_OUTPUT | MUX_MODE0) /*
dss_data22.dss_data22 */
+ 0x0da (PIN_OUTPUT | MUX_MODE0) /*
dss_data23.dss_data23 */
+ >;
+ };
};
&uart3 {
diff --git a/arch/arm/boot/dts/omap3-tobi.dts
b/arch/arm/boot/dts/omap3-tobi.dts
index 7e4ad2a..bfb1a1d 100644
--- a/arch/arm/boot/dts/omap3-tobi.dts
+++ b/arch/arm/boot/dts/omap3-tobi.dts
@@ -81,3 +81,57 @@
&mmc3 {
status = "disabled";
};
+
+&dss {
+ pinctrl-names = "default";
+ pinctrl-0 = <&dss_dpi_pins>;
+
+ vdds_dsi-supply = <&vpll2>;
+
+ dpi_out: endpoint {
+ remote-endpoint = <&tfp410_in>;
+ data-lines = <24>;
+ };
+};
+
+/ {
+ aliases {
+ display0 = &dvi0;
+ };
+
+ tfp410: encoder@0 {
+ compatible = "ti,tfp410";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ tfp410_in: endpoint@0 {
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ tfp410_out: endpoint@0 {
+ remote-endpoint <&dvi_connector_in>;
+ };
+ };
+ };
+ };
+
+ dvi0: connector@0 {
+ compatible = "dvi-connector";
+ label = "dvi";
+
+ i2c-bus = <&i2c3>;
+
+ dvi_connector_in: endpoint {
+ remote-endpoint = <&tfp410_out>;
+ };
+ };
+};
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH 0/4] OMAPDSS: DT support for N900 panel
From: Tomi Valkeinen @ 2013-12-17 17:29 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Benoît Cousson, Tony Lindgren, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
linux-omap, linux-fbdev, devicetree
In-Reply-To: <20131217171412.GA10442@earth.universe>
[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]
On 2013-12-17 19:14, Sebastian Reichel wrote:
> On Tue, Dec 17, 2013 at 09:37:35AM +0200, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 2013-12-13 20:17, Sebastian Reichel wrote:
>>> Hi,
>>>
>>> This patchset adds DT support for the N900 panel. The patchset is based on
>>> Tomi's work/dss-dt-2 branch [0]. I suggest to send the DT changes through
>>> Benoits queue, since I have more N900 DT changes for 3.14. Also the patch
>>> editing the rx51 boardcode can be dropped, since the file is removed in 3.14.
>>> I included those two with this patchset, since they are needed to test the
>>> other two patches.
>>>
>>> I did not include a documentation for the DT API, since the omapdss
>>> documentation is still missing.
>>>
>>> I have successfully tested this on the N900.
>>>
>>> [0] https://git.kernel.org/cgit/linux/kernel/git/tomba/linux.git/log/?h=work/dss-dt-2
>>
>> I added N900 display DT support on top of my v2 series, including
>> pinmuxing. Can you check if it looks right and works?
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git work/dss-dt
>
> I just tried it and it does not work. On a first look the pinmuxing
> looks fishy: 0x0d4 is muxed two times.
Hmm, so it is.
I'm not really familiar with SDI, I just muxed all the SDI pins, except
datapair3. I previously thought that there's only the data and clock
pairs for SDI, but the TRM revealed more sdi pins, so I included them.
It is well possible that these can be removed:
0x0d0 (PIN_OUTPUT | MUX_MODE1) /* dss_data18.sdi_vsync */
0x0d2 (PIN_OUTPUT | MUX_MODE1) /* dss_data19.sdi_hsync */
0x0d4 (PIN_OUTPUT | MUX_MODE1) /* dss_data20.sdi_den */
0x0d6 (PIN_OUTPUT | MUX_MODE1) /* dss_data21.sdi_stp */
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH] video: amba-clcd: Make CLCD driver available on more platforms
From: Mark Brown @ 2013-12-17 18:26 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1386868390-12231-1-git-send-email-broonie@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]
On Tue, Dec 17, 2013 at 04:27:34PM +0000, Catalin Marinas wrote:
> On 12 December 2013 17:13, Mark Brown <broonie@kernel.org> wrote:
> > The CLCD driver is used on ARM reference models for ARMv8 so add ARM64
> > to the list of dependencies. The driver also has no build time dependencies
> > on ARM (stubs are provided for ARM-specific DMA functions in the code) so
> > make it available with COMPILE_TEST in order to maximise build coverage.
> I haven't followed other CLCD patches but for arm64 it requires
> support for DT. I have been carrying some patches (done by Linaro) for
> a long time and Pawel @ ARM has another set of patches. Do you know
> what the status for those is before enabling CLCD on arm64? That's
> unless you are only looking at compile-testing.
Pawel's patches are going nowhere slowly as far as I can tell due to a
slow feedback loop and extensive discussion of the DT bindings.
I think this patch is an orthogonal thing, though - as you say it's
useful for compile testing on its own and it means the driver will load
as soon as the DT bindings are supported. It's one less patch to carry
out of tree and one less iteration when debugging why the framebuffer
isn't supported when testing.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [Xen-devel] [PATCH v3 1/2] xen/pvhvm: If xen_platform_pci=0 is set don't blow up (v3).
From: Konrad Rzeszutek Wilk @ 2013-12-17 21:23 UTC (permalink / raw)
To: Fabio Fantoni
Cc: axboe, stefano.stabellini, ian.campbell, xen-devel, linux-kernel,
boris.ostrovsky, david.vrabel, leosilva, ashley, peterhuewe, mail,
tpmdd, dmitry.torokhov, bhelgaas, plagnioj, tomi.valkeinen,
tpmdd-devel, linux-input, netdev, linux-pci, linux-fbdev
In-Reply-To: <20131217145150.GC4683@phenom.dumpdata.com>
On Tue, Dec 17, 2013 at 09:51:50AM -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Dec 17, 2013 at 10:54:47AM +0100, Fabio Fantoni wrote:
> > Il 16/12/2013 16:04, Konrad Rzeszutek Wilk ha scritto:
> > >The user has the option of disabling the platform driver:
> > >00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
> > >
> > >which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
> > >and allow the PV drivers to take over. If the user wishes
> > >to disable that they can set:
> > >
> > > xen_platform_pci=0
> > > (in the guest config file)
> > >
> > >or
> > > xen_emul_unplug=never
> > > (on the Linux command line)
> > >
> > >except it does not work properly. The PV drivers still try to
> > >load and since the Xen platform driver is not run - and it
> > >has not initialized the grant tables, most of the PV drivers
> > >stumble upon:
> > >
> > >input: Xen Virtual Keyboard as /devices/virtual/input/input5
> > >input: Xen Virtual Pointer as /devices/virtual/input/input6M
> > >------------[ cut here ]------------
> > >kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
> > >invalid opcode: 0000 [#1] SMP
> > >Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
> > >CPU: 6 PID: 1389 Comm: modprobe Not tainted 3.13.0-rc1upstream-00021-ga6c892b-dirty #1
> > >Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
> > >RIP: 0010:[<ffffffff813ddc40>] [<ffffffff813ddc40>] get_free_entries+0x2e0/0x300
> > >Call Trace:
> > > [<ffffffff8150d9a3>] ? evdev_connect+0x1e3/0x240
> > > [<ffffffff813ddd0e>] gnttab_grant_foreign_access+0x2e/0x70
> > > [<ffffffffa0010081>] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
> > > [<ffffffffa0010a12>] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
> > > [<ffffffff813e5757>] xenbus_dev_probe+0x77/0x130
> > > [<ffffffff813e7217>] xenbus_frontend_dev_probe+0x47/0x50
> > > [<ffffffff8145e9a9>] driver_probe_device+0x89/0x230
> > > [<ffffffff8145ebeb>] __driver_attach+0x9b/0xa0
> > > [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
> > > [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
> > > [<ffffffff8145cf1c>] bus_for_each_dev+0x8c/0xb0
> > > [<ffffffff8145e7d9>] driver_attach+0x19/0x20
> > > [<ffffffff8145e260>] bus_add_driver+0x1a0/0x220
> > > [<ffffffff8145f1ff>] driver_register+0x5f/0xf0
> > > [<ffffffff813e55c5>] xenbus_register_driver_common+0x15/0x20
> > > [<ffffffff813e76b3>] xenbus_register_frontend+0x23/0x40
> > > [<ffffffffa0015000>] ? 0xffffffffa0014fff
> > > [<ffffffffa001502b>] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
> > > [<ffffffff81002049>] do_one_initcall+0x49/0x170
> > >
> > >.. snip..
> > >
> > >which is hardly nice. This patch fixes this by having each
> > >PV driver check for:
> > > - if running in PV, then it is fine to execute (as that is their
> > > native environment).
> > > - if running in HVM, check if user wanted 'xen_emul_unplug=never',
> > > in which case bail out and don't load any PV drivers.
> > > - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
> > > does not exist, then bail out and not load PV drivers.
> > > - (v2) if running in HVM, and if the user wanted 'xen_emul_unplug=disks',
> > > then bail out for all PV devices _except_ the block one.
> > > Ditto for the network one ('nics').
> > > - (v2) if running in HVM, and if the user wanted 'xen_emul_unplug=unnecessary'
> > > then load block PV driver, and also setup the legacy IDE paths.
> > > In (v3) make it actually load PV drivers.
> > >
> > >Reported-by: Sander Eikelenboom <linux@eikelenboom.it
> > >Reported-by: Anthony PERARD <anthony.perard@citrix.com>
> > >Reported-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> > >Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > >[v2: Add extra logic to handle the myrid ways 'xen_emul_unplug'
> > >can be used per Ian and Stefano suggestion]
> > >[v3: Make the unnecessary case work properly]
> > >Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >
> > I tested this patch with all possible cases that I know, no crash or
> > calltrace found.
> >
> > I don't understand the utility of 'xen_emul_unplug=unnecessary' but
> > probably it is working correctly, it shows both pv and not pv for
> > blocks and nics.
>
> Great!
> >
> > About 'xen_emul_unplug=disks' and 'xen_emul_unplug=nics' probably
> > there is something wrong.
> > With 'xen_emul_unplug=nics' it shows pv nic (this should be correct)
> > and about disks it shows the same disk twice, as pv and not pv (xvda
> > and sda) with different device number.
>
> Good.
> > With 'xen_emul_unplug=disks' it shows pv block (this should be
> > correct) and it seems to show the pv nic too (this should be wrong).
>
> Correct. You should see only the PV disk and all other PV devices
> should not function. Let me dig in this and see whether there is
> another bug. Thank you!
That is because 'disks' is incorrect. It should have been 'ide-disks'
[ 0.000000] unrecognised option 'disks' in parameter 'xen_emul_unplug'
With the 'ide-disks' it should work. I will update the description to
mention 'ide-disks' instead of 'disks'. Thank you for finding this!
^ permalink raw reply
* [PATCH 05/11] fbcon: queue work on power efficient wq
From: Mark Brown @ 2013-12-17 21:56 UTC (permalink / raw)
To: linux-fbdev
From: Viresh Kumar <viresh.kumar@linaro.org>
fbcon uses workqueues and it has no real dependency of scheduling these on the
cpu which scheduled them.
On a idle system, it is observed that and idle cpu wakes up many times just to
service this work. It would be better if we can schedule it on a cpu which the
scheduler believes to be the most appropriate one.
This patch replaces system_wq with system_power_efficient_wq.
Cc: Dave Airlie <airlied@redhat.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
(cherry picked from commit a85f1a41f020bc2c97611060bcfae6f48a1db28d)
Signed-off-by: Mark Brown <broonie@linaro.org>
---
drivers/video/console/fbcon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index a92783e480e6..0d8f98c79a6c 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -404,7 +404,7 @@ static void cursor_timer_handler(unsigned long dev_addr)
struct fb_info *info = (struct fb_info *) dev_addr;
struct fbcon_ops *ops = info->fbcon_par;
- schedule_work(&info->queue);
+ queue_work(system_power_efficient_wq, &info->queue);
mod_timer(&ops->cursor_timer, jiffies + HZ/5);
}
--
1.8.5.1
^ permalink raw reply related
* [PATCH] video: vgacon: Don't build on arm64
From: Mark Brown @ 2013-12-17 23:37 UTC (permalink / raw)
To: linux-fbdev
From: Mark Brown <broonie@linaro.org>
arm64 is unlikely to have a VGA console and does not export screen_info
causing build failures if the driver is build, for example in all*config.
Add a dependency on !ARM64 to prevent this.
This list is getting quite long, it may be easier to depend on a symbol
which architectures that do support the driver can select.
Signed-off-by: Mark Brown <broonie@linaro.org>
---
drivers/video/console/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 846caab75a46..c39d6c42c3ef 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -8,7 +8,8 @@ config VGA_CONSOLE
bool "VGA text console" if EXPERT || !X86
depends on !4xx && !8xx && !SPARC && !M68K && !PARISC && !FRV && \
!SUPERH && !BLACKFIN && !AVR32 && !MN10300 && !CRIS && \
- (!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER)
+ (!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER) \
+ && !ARM64
default y
help
Saying Y here will allow you to use Linux in text mode through a
--
1.8.5.1
^ permalink raw reply related
* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Tony Lindgren @ 2013-12-18 0:30 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Laurent Pinchart, linux-omap, linux-fbdev, devicetree,
Archit Taneja, Darren Etheridge, Stefan Roese, Sebastian Reichel,
Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <52AEAAB7.7010308@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [131215 23:26]:
> On 2013-12-14 16:09, Tony Lindgren wrote:
>
> >> Why was e30b06f4d5f000c31a7747a7e7ada78a5fd419a1 merged into -rc2? It's
> >> not a fix, just a cleanup.
> >
> > Hmm OK sorry looks like I removed some non-legacy mux framework code
> > by accident. The omap_mux_* parts clearly are dead code for omap4 as
> > it's been DT only since v3.10, but the omap4_dsi_mux_pads() is not
> > using omap_mux_* functions.
> >
> > Yes, let's revert e30b06f4d5f0 except for the dead code parts, which
> > seems to be omap4_tpd12s015_mux_pads(), right?
>
> Yes. I tested the below patch on 4430SDP, both DSI and HDMI works.
OK thanks applying for the -rc cycle.
Tony
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 1/2] video/fb: Propagate error code from failing to unregister conflicting fb
From: Dave Airlie @ 2013-12-18 0:56 UTC (permalink / raw)
To: Chris Wilson, Jani Nikula, intel-gfx@lists.freedesktop.org,
Linux Fbdev development list, Tomi Valkeinen,
Jean-Christophe Plagniol-Villard, dri-devel
In-Reply-To: <20131217121435.GF22448@nuc-i3427.alporthouse.com>
On Tue, Dec 17, 2013 at 10:14 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Dec 17, 2013 at 09:33:08AM +0200, Jani Nikula wrote:
>> On Mon, 16 Dec 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > If we fail to remove a conflicting fb driver, we need to abort the
>> > loading of the second driver to avoid likely kernel panics.
>> >
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> > Cc: linux-fbdev@vger.kernel.org
>> > Cc: dri-devel@lists.freedesktop.org
I'll merge this via the drm tree,
Dave.
^ permalink raw reply
* Re: [PATCHv2 01/27] ARM: OMAP: remove DSS DT hack
From: Tomi Valkeinen @ 2013-12-18 7:12 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap, linux-fbdev, devicetree
In-Reply-To: <20131217153001.GN12324@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]
On 2013-12-17 17:30, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [131216 22:47]:
>> On 2013-12-16 20:41, Tony Lindgren wrote:
>>> * Tomi Valkeinen <tomi.valkeinen@ti.com> [131216 06:59]:
>>>> As a temporary solution to enable DSS for selected boards when booting
>>>> with DT, a hack was added to board-generic.c in
>>>> 63d5fc0c2f748e20f38a0a0ec1c8494bddf5c288 (OMAP: board-generic: enable
>>>> DSS for panda & sdp boards).
>>>>
>>>> We're now adding proper DT support, so the hack can be removed.
>>>
>>> I guess this patch should be merged later on after we have the DT support
>>> working?
>>
>> I'll move this patch also to the end of the series.
>>
>> "Merged later" sounds like you mean this could be merged in a separate
>> series. I think this and the other removal can be in this series, they
>> just need to be at the end.
>
> Well yeah let's keep those separate still as at least Russell needed
> some more time with the legacy booting. The point we can drop the
> legacy booting for omap3 may still need to wait a bit, maybe even
> until v3.15 to keep things working.
They can't be separate. Once I add DT support for a board, I have to
remove the legacy support for that board. This patch removes legacy
support for the boards that are converted in the series.
If I don't remove the legacy support, both DT and legacy side will be
ran, and both create the DSS devices...
But, it's true that this patch removes the whole file, as all the boards
currently there are converted. But if new boards are added to the DSS
quirks, then I can't remove the file. So I'll change this patch to only
remove the parts for the converted boards, not the whole file.
But but... If I understand right, the plan is to remove all omap3 board
files for the next merge window. I'm not totally familiar with the
quirks system, but how should this be handled:
omap3.dtsi will contain the SoC's DSS nodes. This means that DSS devices
are created via DT code. But if the display (i.e. panels) for a
particular omap3 board has not been converted to DT, we should still use
the legacy DSS initialization. But the DSS is already initialized via DT.
I guess I can set the status for all the DSS nodes to "disabled" in
omap3.dtsi, and that should prevent DT code from creating the DSS
devices. Then, each omap3-board.dts that has been converted to DSS DT,
can override those to "enabled".
That way the DT code should not create DSS devices by default, and the
quirks system would probably work fine.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* cirrusdrmfb broken with simplefb
From: Takashi Iwai @ 2013-12-18 7:42 UTC (permalink / raw)
To: David Herrmann; +Cc: Stephen Warren, linux-fbdev, x86, linux-kernel
Hi,
with the recent enablement of simplefb on x86, cirrusdrmfb on QEMU/KVM
gets broken now, as reported at:
https://bugzilla.novell.com/show_bug.cgi?id…5821
The cirrus VGA resource is reserved at first as "BOOTFB" in
arch/x86/kernel/sysfb_simplefb.c, which is taken by simplefb platform
device. This resource is, however, never released until the platform
device is destroyed, and the framebuffer switching doesn't trigger
it. It calls fb's destroy callback, at most. Then, cirrus driver
tries to assign the resource, fails and gives up, resulting in a
complete blank screen.
The same problem should exist on other KMS drivers like mgag200 or
ast, not only cirrus. Intel graphics doesn't hit this problem just
because the reserved iomem by BOOTFB isn't required by i915 driver.
The patch below is a quick attempt to solve the issue. It adds a new
API function for releasing resources of platform_device, and call it
in destroy op of simplefb. But, forcibly releasing resources of a
parent device doesn't sound like a correct design. We may take such
as a band aid, but definitely need a more fundamental fix.
Any thoughts?
thanks,
Takashi
---
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 3a94b79..f939236 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -267,6 +267,23 @@ int platform_device_add_data(struct platform_device *pdev, const void *data,
}
EXPORT_SYMBOL_GPL(platform_device_add_data);
+static void do_release_resources(struct platform_device *pdev, int nums)
+{
+ int i;
+
+ for (i = 0; i < nums; i++) {
+ struct resource *r = &pdev->resource[i];
+ unsigned long type = resource_type(r);
+
+ if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
+ release_resource(r);
+ }
+
+ kfree(pdev->resource);
+ pdev->resource = NULL;
+ pdev->num_resources = 0;
+}
+
/**
* platform_device_add - add a platform device to device hierarchy
* @pdev: platform device we're adding
@@ -342,13 +359,7 @@ int platform_device_add(struct platform_device *pdev)
pdev->id = PLATFORM_DEVID_AUTO;
}
- while (--i >= 0) {
- struct resource *r = &pdev->resource[i];
- unsigned long type = resource_type(r);
-
- if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
- release_resource(r);
- }
+ do_release_resources(pdev, i - 1);
err_out:
return ret;
@@ -365,8 +376,6 @@ EXPORT_SYMBOL_GPL(platform_device_add);
*/
void platform_device_del(struct platform_device *pdev)
{
- int i;
-
if (pdev) {
device_del(&pdev->dev);
@@ -375,17 +384,17 @@ void platform_device_del(struct platform_device *pdev)
pdev->id = PLATFORM_DEVID_AUTO;
}
- for (i = 0; i < pdev->num_resources; i++) {
- struct resource *r = &pdev->resource[i];
- unsigned long type = resource_type(r);
-
- if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
- release_resource(r);
- }
+ do_release_resources(pdev, pdev->num_resources);
}
}
EXPORT_SYMBOL_GPL(platform_device_del);
+void platform_device_release_resources(struct platform_device *pdev)
+{
+ do_release_resources(pdev, pdev->num_resources);
+}
+EXPORT_SYMBOL_GPL(platform_device_release_resources);
+
/**
* platform_device_register - add a platform-level device
* @pdev: platform device we're adding
diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
index 210f3a0..fbf5e89 100644
--- a/drivers/video/simplefb.c
+++ b/drivers/video/simplefb.c
@@ -70,6 +70,7 @@ static void simplefb_destroy(struct fb_info *info)
{
if (info->screen_base)
iounmap(info->screen_base);
+ platform_device_release_resources(to_platform_device(info->device));
}
static struct fb_ops simplefb_ops = {
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 16f6654..7cc1f54 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -42,6 +42,7 @@ struct platform_device {
extern int platform_device_register(struct platform_device *);
extern void platform_device_unregister(struct platform_device *);
+extern void platform_device_release_resources(struct platform_device *pdev);
extern struct bus_type platform_bus_type;
extern struct device platform_bus;
^ permalink raw reply related
* Re: cirrusdrmfb broken with simplefb
From: David Herrmann @ 2013-12-18 8:21 UTC (permalink / raw)
To: Takashi Iwai
Cc: Stephen Warren, linux-fbdev@vger.kernel.org,
the arch/x86 maintainers, linux-kernel
In-Reply-To: <s5hlhzik3kk.wl%tiwai@suse.de>
Hi
On Wed, Dec 18, 2013 at 8:42 AM, Takashi Iwai <tiwai@suse.de> wrote:
> Hi,
>
> with the recent enablement of simplefb on x86, cirrusdrmfb on QEMU/KVM
> gets broken now, as reported at:
> https://bugzilla.novell.com/show_bug.cgi?id…5821
>
> The cirrus VGA resource is reserved at first as "BOOTFB" in
> arch/x86/kernel/sysfb_simplefb.c, which is taken by simplefb platform
> device. This resource is, however, never released until the platform
> device is destroyed, and the framebuffer switching doesn't trigger
> it. It calls fb's destroy callback, at most. Then, cirrus driver
> tries to assign the resource, fails and gives up, resulting in a
> complete blank screen.
>
> The same problem should exist on other KMS drivers like mgag200 or
> ast, not only cirrus. Intel graphics doesn't hit this problem just
> because the reserved iomem by BOOTFB isn't required by i915 driver.
>
> The patch below is a quick attempt to solve the issue. It adds a new
> API function for releasing resources of platform_device, and call it
> in destroy op of simplefb. But, forcibly releasing resources of a
> parent device doesn't sound like a correct design. We may take such
> as a band aid, but definitely need a more fundamental fix.
>
> Any thoughts?
That bug always existed, simplefb is just the first driver to hit it
(vesafb/efifb didn't use resources). I'm aware of the issue but as a
workaround you can simply disable CONFIG_X86_SYSFB. That restores the
old behavior.
As a proper fix, I'd propose something like:
dev = platform_find_device("platform-framebuffer");
platform_remove_device(dev);
And we wrap this as:
sysfb_remove_framebuffers()
in arch/x86/kernel/sysfb.c
This should cause a ->remove() event for the platform-driver and
correctly release the resources. Comments?
I will try to write a patch and send it later.
Thanks
David
>
> thanks,
>
> Takashi
>
> ---
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 3a94b79..f939236 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -267,6 +267,23 @@ int platform_device_add_data(struct platform_device *pdev, const void *data,
> }
> EXPORT_SYMBOL_GPL(platform_device_add_data);
>
> +static void do_release_resources(struct platform_device *pdev, int nums)
> +{
> + int i;
> +
> + for (i = 0; i < nums; i++) {
> + struct resource *r = &pdev->resource[i];
> + unsigned long type = resource_type(r);
> +
> + if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
> + release_resource(r);
> + }
> +
> + kfree(pdev->resource);
> + pdev->resource = NULL;
> + pdev->num_resources = 0;
> +}
> +
> /**
> * platform_device_add - add a platform device to device hierarchy
> * @pdev: platform device we're adding
> @@ -342,13 +359,7 @@ int platform_device_add(struct platform_device *pdev)
> pdev->id = PLATFORM_DEVID_AUTO;
> }
>
> - while (--i >= 0) {
> - struct resource *r = &pdev->resource[i];
> - unsigned long type = resource_type(r);
> -
> - if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
> - release_resource(r);
> - }
> + do_release_resources(pdev, i - 1);
>
> err_out:
> return ret;
> @@ -365,8 +376,6 @@ EXPORT_SYMBOL_GPL(platform_device_add);
> */
> void platform_device_del(struct platform_device *pdev)
> {
> - int i;
> -
> if (pdev) {
> device_del(&pdev->dev);
>
> @@ -375,17 +384,17 @@ void platform_device_del(struct platform_device *pdev)
> pdev->id = PLATFORM_DEVID_AUTO;
> }
>
> - for (i = 0; i < pdev->num_resources; i++) {
> - struct resource *r = &pdev->resource[i];
> - unsigned long type = resource_type(r);
> -
> - if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
> - release_resource(r);
> - }
> + do_release_resources(pdev, pdev->num_resources);
> }
> }
> EXPORT_SYMBOL_GPL(platform_device_del);
>
> +void platform_device_release_resources(struct platform_device *pdev)
> +{
> + do_release_resources(pdev, pdev->num_resources);
> +}
> +EXPORT_SYMBOL_GPL(platform_device_release_resources);
> +
> /**
> * platform_device_register - add a platform-level device
> * @pdev: platform device we're adding
> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
> index 210f3a0..fbf5e89 100644
> --- a/drivers/video/simplefb.c
> +++ b/drivers/video/simplefb.c
> @@ -70,6 +70,7 @@ static void simplefb_destroy(struct fb_info *info)
> {
> if (info->screen_base)
> iounmap(info->screen_base);
> + platform_device_release_resources(to_platform_device(info->device));
> }
>
> static struct fb_ops simplefb_ops = {
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> index 16f6654..7cc1f54 100644
> --- a/include/linux/platform_device.h
> +++ b/include/linux/platform_device.h
> @@ -42,6 +42,7 @@ struct platform_device {
>
> extern int platform_device_register(struct platform_device *);
> extern void platform_device_unregister(struct platform_device *);
> +extern void platform_device_release_resources(struct platform_device *pdev);
>
> extern struct bus_type platform_bus_type;
> extern struct device platform_bus;
^ permalink raw reply
* Re: cirrusdrmfb broken with simplefb
From: Takashi Iwai @ 2013-12-18 8:44 UTC (permalink / raw)
To: David Herrmann
Cc: Stephen Warren, linux-fbdev@vger.kernel.org,
the arch/x86 maintainers, linux-kernel
In-Reply-To: <CANq1E4TFKDduy71jCXy+8CMS1_OFxWEpsHY3AA0RUJPW4VE8hg@mail.gmail.com>
At Wed, 18 Dec 2013 09:21:28 +0100,
David Herrmann wrote:
>
> Hi
>
> On Wed, Dec 18, 2013 at 8:42 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > Hi,
> >
> > with the recent enablement of simplefb on x86, cirrusdrmfb on QEMU/KVM
> > gets broken now, as reported at:
> > https://bugzilla.novell.com/show_bug.cgi?id…5821
> >
> > The cirrus VGA resource is reserved at first as "BOOTFB" in
> > arch/x86/kernel/sysfb_simplefb.c, which is taken by simplefb platform
> > device. This resource is, however, never released until the platform
> > device is destroyed, and the framebuffer switching doesn't trigger
> > it. It calls fb's destroy callback, at most. Then, cirrus driver
> > tries to assign the resource, fails and gives up, resulting in a
> > complete blank screen.
> >
> > The same problem should exist on other KMS drivers like mgag200 or
> > ast, not only cirrus. Intel graphics doesn't hit this problem just
> > because the reserved iomem by BOOTFB isn't required by i915 driver.
> >
> > The patch below is a quick attempt to solve the issue. It adds a new
> > API function for releasing resources of platform_device, and call it
> > in destroy op of simplefb. But, forcibly releasing resources of a
> > parent device doesn't sound like a correct design. We may take such
> > as a band aid, but definitely need a more fundamental fix.
> >
> > Any thoughts?
>
> That bug always existed, simplefb is just the first driver to hit it
> (vesafb/efifb didn't use resources).
Heh, the bug didn't "exist" because no other grabbed the resource
before. The way the cirrus driver allocates the resource is no bug,
per se. But the proper resource takeover is missing.
> I'm aware of the issue but as a
> workaround you can simply disable CONFIG_X86_SYSFB. That restores the
> old behavior.
Yes, but CONFIG_X86_SYSFB breaks things as of now. Shouldn't it be
mentioned or give some kconfig hints instead of silently giving a
broken output, if any quick fix isn't possible?
> As a proper fix, I'd propose something like:
> dev = platform_find_device("platform-framebuffer");
> platform_remove_device(dev);
>
> And we wrap this as:
> sysfb_remove_framebuffers()
> in arch/x86/kernel/sysfb.c
>
> This should cause a ->remove() event for the platform-driver and
> correctly release the resources. Comments?
Who will call this? From destroy callback?
Or can we simply do put_device() the bound platform device instead?
> I will try to write a patch and send it later.
Thanks!
Takashi
>
> Thanks
> David
>
> >
> > thanks,
> >
> > Takashi
> >
> > ---
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 3a94b79..f939236 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -267,6 +267,23 @@ int platform_device_add_data(struct platform_device *pdev, const void *data,
> > }
> > EXPORT_SYMBOL_GPL(platform_device_add_data);
> >
> > +static void do_release_resources(struct platform_device *pdev, int nums)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < nums; i++) {
> > + struct resource *r = &pdev->resource[i];
> > + unsigned long type = resource_type(r);
> > +
> > + if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
> > + release_resource(r);
> > + }
> > +
> > + kfree(pdev->resource);
> > + pdev->resource = NULL;
> > + pdev->num_resources = 0;
> > +}
> > +
> > /**
> > * platform_device_add - add a platform device to device hierarchy
> > * @pdev: platform device we're adding
> > @@ -342,13 +359,7 @@ int platform_device_add(struct platform_device *pdev)
> > pdev->id = PLATFORM_DEVID_AUTO;
> > }
> >
> > - while (--i >= 0) {
> > - struct resource *r = &pdev->resource[i];
> > - unsigned long type = resource_type(r);
> > -
> > - if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
> > - release_resource(r);
> > - }
> > + do_release_resources(pdev, i - 1);
> >
> > err_out:
> > return ret;
> > @@ -365,8 +376,6 @@ EXPORT_SYMBOL_GPL(platform_device_add);
> > */
> > void platform_device_del(struct platform_device *pdev)
> > {
> > - int i;
> > -
> > if (pdev) {
> > device_del(&pdev->dev);
> >
> > @@ -375,17 +384,17 @@ void platform_device_del(struct platform_device *pdev)
> > pdev->id = PLATFORM_DEVID_AUTO;
> > }
> >
> > - for (i = 0; i < pdev->num_resources; i++) {
> > - struct resource *r = &pdev->resource[i];
> > - unsigned long type = resource_type(r);
> > -
> > - if (type = IORESOURCE_MEM || type = IORESOURCE_IO)
> > - release_resource(r);
> > - }
> > + do_release_resources(pdev, pdev->num_resources);
> > }
> > }
> > EXPORT_SYMBOL_GPL(platform_device_del);
> >
> > +void platform_device_release_resources(struct platform_device *pdev)
> > +{
> > + do_release_resources(pdev, pdev->num_resources);
> > +}
> > +EXPORT_SYMBOL_GPL(platform_device_release_resources);
> > +
> > /**
> > * platform_device_register - add a platform-level device
> > * @pdev: platform device we're adding
> > diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
> > index 210f3a0..fbf5e89 100644
> > --- a/drivers/video/simplefb.c
> > +++ b/drivers/video/simplefb.c
> > @@ -70,6 +70,7 @@ static void simplefb_destroy(struct fb_info *info)
> > {
> > if (info->screen_base)
> > iounmap(info->screen_base);
> > + platform_device_release_resources(to_platform_device(info->device));
> > }
> >
> > static struct fb_ops simplefb_ops = {
> > diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> > index 16f6654..7cc1f54 100644
> > --- a/include/linux/platform_device.h
> > +++ b/include/linux/platform_device.h
> > @@ -42,6 +42,7 @@ struct platform_device {
> >
> > extern int platform_device_register(struct platform_device *);
> > extern void platform_device_unregister(struct platform_device *);
> > +extern void platform_device_release_resources(struct platform_device *pdev);
> >
> > extern struct bus_type platform_bus_type;
> > extern struct device platform_bus;
>
^ permalink raw reply
* Re: cirrusdrmfb broken with simplefb
From: Geert Uytterhoeven @ 2013-12-18 8:48 UTC (permalink / raw)
To: Takashi Iwai
Cc: David Herrmann, Stephen Warren, linux-fbdev@vger.kernel.org,
the arch/x86 maintainers, linux-kernel
In-Reply-To: <s5hfvpqk0p8.wl%tiwai@suse.de>
On Wed, Dec 18, 2013 at 9:44 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> That bug always existed, simplefb is just the first driver to hit it
>> (vesafb/efifb didn't use resources).
>
> Heh, the bug didn't "exist" because no other grabbed the resource
> before. The way the cirrus driver allocates the resource is no bug,
> per se. But the proper resource takeover is missing.
>
>> I'm aware of the issue but as a
>> workaround you can simply disable CONFIG_X86_SYSFB. That restores the
>> old behavior.
>
> Yes, but CONFIG_X86_SYSFB breaks things as of now. Shouldn't it be
> mentioned or give some kconfig hints instead of silently giving a
> broken output, if any quick fix isn't possible?
Indeed. That's called a "regression" in distro/allmodconfig kernels.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: cirrusdrmfb broken with simplefb
From: Ingo Molnar @ 2013-12-18 9:29 UTC (permalink / raw)
To: David Herrmann
Cc: Takashi Iwai, Stephen Warren, linux-fbdev@vger.kernel.org,
the arch/x86 maintainers, linux-kernel
In-Reply-To: <CANq1E4TFKDduy71jCXy+8CMS1_OFxWEpsHY3AA0RUJPW4VE8hg@mail.gmail.com>
* David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Wed, Dec 18, 2013 at 8:42 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > Hi,
> >
> > with the recent enablement of simplefb on x86, cirrusdrmfb on QEMU/KVM
> > gets broken now, as reported at:
> > https://bugzilla.novell.com/show_bug.cgi?id…5821
> >
> > The cirrus VGA resource is reserved at first as "BOOTFB" in
> > arch/x86/kernel/sysfb_simplefb.c, which is taken by simplefb platform
> > device. This resource is, however, never released until the platform
> > device is destroyed, and the framebuffer switching doesn't trigger
> > it. It calls fb's destroy callback, at most. Then, cirrus driver
> > tries to assign the resource, fails and gives up, resulting in a
> > complete blank screen.
> >
> > The same problem should exist on other KMS drivers like mgag200 or
> > ast, not only cirrus. Intel graphics doesn't hit this problem just
> > because the reserved iomem by BOOTFB isn't required by i915 driver.
> >
> > The patch below is a quick attempt to solve the issue. It adds a new
> > API function for releasing resources of platform_device, and call it
> > in destroy op of simplefb. But, forcibly releasing resources of a
> > parent device doesn't sound like a correct design. We may take such
> > as a band aid, but definitely need a more fundamental fix.
> >
> > Any thoughts?
>
> That bug always existed, simplefb is just the first driver to hit it
> (vesafb/efifb didn't use resources). I'm aware of the issue but as a
> workaround you can simply disable CONFIG_X86_SYSFB. That restores
> the old behavior.
This looks like a regression, so we'll either need a fix or we'll have
to mark CONFIG_X86_SYSFB as CONFIG_BROKEN.
> As a proper fix, I'd propose something like:
> dev = platform_find_device("platform-framebuffer");
> platform_remove_device(dev);
>
> And we wrap this as:
> sysfb_remove_framebuffers()
> in arch/x86/kernel/sysfb.c
>
> This should cause a ->remove() event for the platform-driver and
> correctly release the resources. Comments?
> I will try to write a patch and send it later.
A fix would be awesome.
Thanks,
Ingo
^ permalink raw reply
* Re: cirrusdrmfb broken with simplefb
From: Takashi Iwai @ 2013-12-18 10:17 UTC (permalink / raw)
To: David Herrmann
Cc: Stephen Warren, linux-fbdev@vger.kernel.org,
the arch/x86 maintainers, linux-kernel
In-Reply-To: <s5hfvpqk0p8.wl%tiwai@suse.de>
At Wed, 18 Dec 2013 09:44:35 +0100,
Takashi Iwai wrote:
>
> At Wed, 18 Dec 2013 09:21:28 +0100,
> David Herrmann wrote:
> >
> > Hi
> >
> > On Wed, Dec 18, 2013 at 8:42 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > > Hi,
> > >
> > > with the recent enablement of simplefb on x86, cirrusdrmfb on QEMU/KVM
> > > gets broken now, as reported at:
> > > https://bugzilla.novell.com/show_bug.cgi?id…5821
> > >
> > > The cirrus VGA resource is reserved at first as "BOOTFB" in
> > > arch/x86/kernel/sysfb_simplefb.c, which is taken by simplefb platform
> > > device. This resource is, however, never released until the platform
> > > device is destroyed, and the framebuffer switching doesn't trigger
> > > it. It calls fb's destroy callback, at most. Then, cirrus driver
> > > tries to assign the resource, fails and gives up, resulting in a
> > > complete blank screen.
> > >
> > > The same problem should exist on other KMS drivers like mgag200 or
> > > ast, not only cirrus. Intel graphics doesn't hit this problem just
> > > because the reserved iomem by BOOTFB isn't required by i915 driver.
> > >
> > > The patch below is a quick attempt to solve the issue. It adds a new
> > > API function for releasing resources of platform_device, and call it
> > > in destroy op of simplefb. But, forcibly releasing resources of a
> > > parent device doesn't sound like a correct design. We may take such
> > > as a band aid, but definitely need a more fundamental fix.
> > >
> > > Any thoughts?
> >
> > That bug always existed, simplefb is just the first driver to hit it
> > (vesafb/efifb didn't use resources).
>
> Heh, the bug didn't "exist" because no other grabbed the resource
> before. The way the cirrus driver allocates the resource is no bug,
> per se. But the proper resource takeover is missing.
>
> > I'm aware of the issue but as a
> > workaround you can simply disable CONFIG_X86_SYSFB. That restores the
> > old behavior.
>
> Yes, but CONFIG_X86_SYSFB breaks things as of now. Shouldn't it be
> mentioned or give some kconfig hints instead of silently giving a
> broken output, if any quick fix isn't possible?
>
> > As a proper fix, I'd propose something like:
> > dev = platform_find_device("platform-framebuffer");
> > platform_remove_device(dev);
> >
> > And we wrap this as:
> > sysfb_remove_framebuffers()
> > in arch/x86/kernel/sysfb.c
> >
> > This should cause a ->remove() event for the platform-driver and
> > correctly release the resources. Comments?
>
> Who will call this? From destroy callback?
> Or can we simply do put_device() the bound platform device instead?
Just tested, put_device() doesn't work since the refcount isn't 1, so
if any, we need to call platform_device_unregister() to release
forcibly.
The patch below seems working at a quick test. But I still have
some bad feeling for unregistering the parent device from the child's
destroy callback...
thanks,
Takashi
---
diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
index 210f3a02121a..196360c54157 100644
--- a/drivers/video/simplefb.c
+++ b/drivers/video/simplefb.c
@@ -41,6 +41,11 @@ static struct fb_var_screeninfo simplefb_var = {
.vmode = FB_VMODE_NONINTERLACED,
};
+struct simplefb_priv {
+ u32 pseudo_palette[16];
+ struct platform_device *pdev;
+};
+
static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
u_int transp, struct fb_info *info)
{
@@ -68,8 +73,19 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
static void simplefb_destroy(struct fb_info *info)
{
+ struct simplefb_priv *priv = info->par;
+ struct platform_device *pdev = priv->pdev;
+
if (info->screen_base)
iounmap(info->screen_base);
+
+ /* release the bound platform device when called from
+ * remove_conflicting_framebuffers()
+ */
+ if (pdev) {
+ priv->pdev = NULL; /* to avoid double-free */
+ platform_device_unregister(pdev);
+ }
}
static struct fb_ops simplefb_ops = {
@@ -169,6 +185,7 @@ static int simplefb_probe(struct platform_device *pdev)
struct simplefb_params params;
struct fb_info *info;
struct resource *mem;
+ struct simplefb_priv *priv;
if (fb_get_options("simplefb", NULL))
return -ENODEV;
@@ -188,7 +205,7 @@ static int simplefb_probe(struct platform_device *pdev)
return -EINVAL;
}
- info = framebuffer_alloc(sizeof(u32) * 16, &pdev->dev);
+ info = framebuffer_alloc(sizeof(struct simplefb_priv), &pdev->dev);
if (!info)
return -ENOMEM;
platform_set_drvdata(pdev, info);
@@ -225,7 +242,9 @@ static int simplefb_probe(struct platform_device *pdev)
framebuffer_release(info);
return -ENODEV;
}
- info->pseudo_palette = (void *)(info + 1);
+
+ priv = info->par;
+ info->pseudo_palette = priv->pseudo_palette;
dev_info(&pdev->dev, "framebuffer at 0x%lx, 0x%x bytes, mapped to 0x%p\n",
info->fix.smem_start, info->fix.smem_len,
@@ -243,6 +262,7 @@ static int simplefb_probe(struct platform_device *pdev)
return ret;
}
+ priv->pdev = pdev;
dev_info(&pdev->dev, "fb%d: simplefb registered!\n", info->node);
return 0;
@@ -251,8 +271,13 @@ static int simplefb_probe(struct platform_device *pdev)
static int simplefb_remove(struct platform_device *pdev)
{
struct fb_info *info = platform_get_drvdata(pdev);
+ struct simplefb_priv *priv = info->par;
+
+ if (priv->pdev) {
+ priv->pdev = NULL; /* to avoid double-free */
+ unregister_framebuffer(info);
+ }
- unregister_framebuffer(info);
framebuffer_release(info);
return 0;
^ permalink raw reply related
* Re: [PATCHv2 01/27] ARM: OMAP: remove DSS DT hack
From: Tomi Valkeinen @ 2013-12-18 10:21 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap, linux-fbdev, devicetree
In-Reply-To: <52B14AEE.8090807@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2247 bytes --]
On 2013-12-18 09:12, Tomi Valkeinen wrote:
>> Well yeah let's keep those separate still as at least Russell needed
>> some more time with the legacy booting. The point we can drop the
>> legacy booting for omap3 may still need to wait a bit, maybe even
>> until v3.15 to keep things working.
>
> They can't be separate. Once I add DT support for a board, I have to
> remove the legacy support for that board. This patch removes legacy
> support for the boards that are converted in the series.
>
> If I don't remove the legacy support, both DT and legacy side will be
> ran, and both create the DSS devices...
>
> But, it's true that this patch removes the whole file, as all the boards
> currently there are converted. But if new boards are added to the DSS
> quirks, then I can't remove the file. So I'll change this patch to only
> remove the parts for the converted boards, not the whole file.
>
> But but... If I understand right, the plan is to remove all omap3 board
> files for the next merge window. I'm not totally familiar with the
> quirks system, but how should this be handled:
>
> omap3.dtsi will contain the SoC's DSS nodes. This means that DSS devices
> are created via DT code. But if the display (i.e. panels) for a
> particular omap3 board has not been converted to DT, we should still use
> the legacy DSS initialization. But the DSS is already initialized via DT.
>
> I guess I can set the status for all the DSS nodes to "disabled" in
> omap3.dtsi, and that should prevent DT code from creating the DSS
> devices. Then, each omap3-board.dts that has been converted to DSS DT,
> can override those to "enabled".
>
> That way the DT code should not create DSS devices by default, and the
> quirks system would probably work fine.
I changed the DSS DT nodes to be disabled by default, and I think this
works nicely. It's actually better this way in any case, as we can leave
blocks like DSI out for boards that don't need it.
Also, I now remove the quirks only for the boards that are converted to
DT, not the whole dss-common.c file. So I think we can now add new
boards to dss-common.c, if and when there are ones that cannot be
converted to use DSS DT yet.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: cirrusdrmfb broken with simplefb
From: David Herrmann @ 2013-12-18 10:21 UTC (permalink / raw)
To: Takashi Iwai
Cc: Stephen Warren, linux-fbdev@vger.kernel.org,
the arch/x86 maintainers, linux-kernel
In-Reply-To: <s5hppouv4y4.wl%tiwai@suse.de>
Hi
On Wed, Dec 18, 2013 at 11:17 AM, Takashi Iwai <tiwai@suse.de> wrote:
> At Wed, 18 Dec 2013 09:44:35 +0100,
> Takashi Iwai wrote:
>>
>> At Wed, 18 Dec 2013 09:21:28 +0100,
>> David Herrmann wrote:
>> >
>> > Hi
>> >
>> > On Wed, Dec 18, 2013 at 8:42 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> > > Hi,
>> > >
>> > > with the recent enablement of simplefb on x86, cirrusdrmfb on QEMU/KVM
>> > > gets broken now, as reported at:
>> > > https://bugzilla.novell.com/show_bug.cgi?id…5821
>> > >
>> > > The cirrus VGA resource is reserved at first as "BOOTFB" in
>> > > arch/x86/kernel/sysfb_simplefb.c, which is taken by simplefb platform
>> > > device. This resource is, however, never released until the platform
>> > > device is destroyed, and the framebuffer switching doesn't trigger
>> > > it. It calls fb's destroy callback, at most. Then, cirrus driver
>> > > tries to assign the resource, fails and gives up, resulting in a
>> > > complete blank screen.
>> > >
>> > > The same problem should exist on other KMS drivers like mgag200 or
>> > > ast, not only cirrus. Intel graphics doesn't hit this problem just
>> > > because the reserved iomem by BOOTFB isn't required by i915 driver.
>> > >
>> > > The patch below is a quick attempt to solve the issue. It adds a new
>> > > API function for releasing resources of platform_device, and call it
>> > > in destroy op of simplefb. But, forcibly releasing resources of a
>> > > parent device doesn't sound like a correct design. We may take such
>> > > as a band aid, but definitely need a more fundamental fix.
>> > >
>> > > Any thoughts?
>> >
>> > That bug always existed, simplefb is just the first driver to hit it
>> > (vesafb/efifb didn't use resources).
>>
>> Heh, the bug didn't "exist" because no other grabbed the resource
>> before. The way the cirrus driver allocates the resource is no bug,
>> per se. But the proper resource takeover is missing.
>>
>> > I'm aware of the issue but as a
>> > workaround you can simply disable CONFIG_X86_SYSFB. That restores the
>> > old behavior.
>>
>> Yes, but CONFIG_X86_SYSFB breaks things as of now. Shouldn't it be
>> mentioned or give some kconfig hints instead of silently giving a
>> broken output, if any quick fix isn't possible?
>>
>> > As a proper fix, I'd propose something like:
>> > dev = platform_find_device("platform-framebuffer");
>> > platform_remove_device(dev);
>> >
>> > And we wrap this as:
>> > sysfb_remove_framebuffers()
>> > in arch/x86/kernel/sysfb.c
>> >
>> > This should cause a ->remove() event for the platform-driver and
>> > correctly release the resources. Comments?
>>
>> Who will call this? From destroy callback?
>> Or can we simply do put_device() the bound platform device instead?
>
> Just tested, put_device() doesn't work since the refcount isn't 1, so
> if any, we need to call platform_device_unregister() to release
> forcibly.
>
> The patch below seems working at a quick test. But I still have
> some bad feeling for unregistering the parent device from the child's
> destroy callback...
The idea is right, but call it from
"remove_conflicting_framebuffers()". The problem is, if we load a
driver on top of a dummy like vesafb/efifb/simplefb, we never removed
them properly. Instead, we only unregistered their fb driver. But with
the driver-model, that's just removing the driver, not the dummy
device.
So we'd just call platform_device_unregister() in
remove_conflicting_framebuffers() instead of unregister_framebuffer()
on such devices. I'm now at home and will have a look.
Thanks
David
>
> thanks,
>
> Takashi
>
> ---
> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
> index 210f3a02121a..196360c54157 100644
> --- a/drivers/video/simplefb.c
> +++ b/drivers/video/simplefb.c
> @@ -41,6 +41,11 @@ static struct fb_var_screeninfo simplefb_var = {
> .vmode = FB_VMODE_NONINTERLACED,
> };
>
> +struct simplefb_priv {
> + u32 pseudo_palette[16];
> + struct platform_device *pdev;
> +};
> +
> static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
> u_int transp, struct fb_info *info)
> {
> @@ -68,8 +73,19 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
>
> static void simplefb_destroy(struct fb_info *info)
> {
> + struct simplefb_priv *priv = info->par;
> + struct platform_device *pdev = priv->pdev;
> +
> if (info->screen_base)
> iounmap(info->screen_base);
> +
> + /* release the bound platform device when called from
> + * remove_conflicting_framebuffers()
> + */
> + if (pdev) {
> + priv->pdev = NULL; /* to avoid double-free */
> + platform_device_unregister(pdev);
> + }
> }
>
> static struct fb_ops simplefb_ops = {
> @@ -169,6 +185,7 @@ static int simplefb_probe(struct platform_device *pdev)
> struct simplefb_params params;
> struct fb_info *info;
> struct resource *mem;
> + struct simplefb_priv *priv;
>
> if (fb_get_options("simplefb", NULL))
> return -ENODEV;
> @@ -188,7 +205,7 @@ static int simplefb_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - info = framebuffer_alloc(sizeof(u32) * 16, &pdev->dev);
> + info = framebuffer_alloc(sizeof(struct simplefb_priv), &pdev->dev);
> if (!info)
> return -ENOMEM;
> platform_set_drvdata(pdev, info);
> @@ -225,7 +242,9 @@ static int simplefb_probe(struct platform_device *pdev)
> framebuffer_release(info);
> return -ENODEV;
> }
> - info->pseudo_palette = (void *)(info + 1);
> +
> + priv = info->par;
> + info->pseudo_palette = priv->pseudo_palette;
>
> dev_info(&pdev->dev, "framebuffer at 0x%lx, 0x%x bytes, mapped to 0x%p\n",
> info->fix.smem_start, info->fix.smem_len,
> @@ -243,6 +262,7 @@ static int simplefb_probe(struct platform_device *pdev)
> return ret;
> }
>
> + priv->pdev = pdev;
> dev_info(&pdev->dev, "fb%d: simplefb registered!\n", info->node);
>
> return 0;
> @@ -251,8 +271,13 @@ static int simplefb_probe(struct platform_device *pdev)
> static int simplefb_remove(struct platform_device *pdev)
> {
> struct fb_info *info = platform_get_drvdata(pdev);
> + struct simplefb_priv *priv = info->par;
> +
> + if (priv->pdev) {
> + priv->pdev = NULL; /* to avoid double-free */
> + unregister_framebuffer(info);
> + }
>
> - unregister_framebuffer(info);
> framebuffer_release(info);
>
> return 0;
^ permalink raw reply
* Re: [PATCHv2 23/27] OMAPDSS: connector-dvi: Add DT support
From: Tomi Valkeinen @ 2013-12-18 10:41 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sascha Hauer, linux-omap, linux-fbdev, devicetree, Tony Lindgren
In-Reply-To: <1651476.iaILztiDEi@avalon>
[-- Attachment #1: Type: text/plain, Size: 3952 bytes --]
On 2013-12-17 17:15, Laurent Pinchart wrote:
>>> Either the driver is too specific or the binding is too generic, but
>>> having such a generic name for an omap specific driver seems wrong. Same
>>> for panel-dpi, svideo-connector, composite-video-connector and
>>> hdmi-connector,
>>
>> Hmm. Good point. I was thinking that the driver is only used on OMAP, but of
>> course that's not true, the driver is there for all platforms if the kernel
>> just happens to be compiled with the driver.
>>
>> And it's not just about those drivers you mention. The same issue is there
>> for, say, "ti,tpd12s015". I have an omapdss specific driver for that, but if
>> some other platform uses the same chip, they'll have a driver for it also...
>>
>> Sigh. I wonder how this should be handled... The only solution that comes to
>> my mind is to have all the compatible strings as "ti,...". But that's not
>> correct, as they are not TI components, but some are generic ones and some
>> from another vendor.
>>
>> And even "ti,..." is not good, as there are other TI SoCs with other display
>> drivers. So I'd need to prepend the compatible strings with "omapdss,...",
>> making the hardware components driver specific.
>>
>> The long term plan is to make the drivers generic (or implement the same
>> driver for common display framework). But for now I need to have future
>> proof DT bindings with omapdss specific drivers...
>
> I'll refrain from mentioning that the problem has been identified more than a
> year ago. Oh, wait, I've metioned it, sorry :-D
>
> We really want to make drivers generic enough to be shared by multiple display
> controllers. I would vote for making the DT bindings generic now already,
> which would be enough to buy some time to fix the problem properly. If we
> start prepending driver-specific prefixes such as "omapdss," to compatible
> string we'll just make the mess even larger and reduce the incentive to fix
> it. It would be the worst decision we could make.
Well... I think there are no good options here. I see the following options:
1. Add "omapdss," or similar to compat strings in DT, and the same for
the drivers. This solves the issue, but then we'll have bad DT data,
although I see similar method already being used in some places. When we
have common drivers, we can remove the "omapdss," strings from DT, but
we still need to keep them in the drivers to have backward compatibility.
2. Keep the compat strings generic in DT. This way we'll have correct DT
data, but if anyone happens to create a new driver with the same compat
string, things will break. omapdss can only be compiled for a kernel
with OMAP and ARM support, so it narrows the problematic drivers a bit.
3. Have correct DT data, but at init time, in omap arch code, go through
the DT data and change the compat strings for the display nodes to
include "omapdss,". This way the drivers would only work for omap
platforms. Like a combination of 1. and 2. I'm not sure if the DT-code
allows this at the moment, though.
We could also now select option 2., and go for 3. later if someone else
creates a driver with the same compat string.
While I'm obviously not very impartial here, I do think that using
generic bindings for omapdss is not the worst possible case, as:
I'm very much dedicated to get CDF merged at some point, and I already
have been working on it for each revision.
I also think that even if the omap panel drivers are currently omap
specific, they are not very much so. I have been changing them over the
years to be more and more generic, and I have used them as a base for
CDF panel drivers. If some platform specific driver may have a generic
compat string, omap panel drivers are not the worst option.
I will look at the option 3., hopefully that will be possible and
everybody will be happy. Any other ideas appreciated.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
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