* Re: [RFC PATCH RESEND] fb: reorder the lock sequence to fix a potential lockdep
From: Gu Zheng @ 2013-10-30 3:18 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen
Cc: Linux Fbdev development list, linux-kernel, guz.fnst
In-Reply-To: <526DFDA8.7010806@cn.fujitsu.com>
Hi Tomi, Jean,
How about this patch?
Thanks,
Gu
On 10/28/2013 02:01 PM, Gu Zheng wrote:
> Following commits:
> 50e244cc79 fb: rework locking to fix lock ordering on takeover
> e93a9a8687 fb: Yet another band-aid for fixing lockdep mess
> 054430e773 fbcon: fix locking harder
>
> reworked locking to fix related lock ordering on takeover, and introduced console_lock
> into fbmem, but it seems that the new lock sequence(fb_info->lock ---> console_lock)
> is against with the one in console_callback(console_lock ---> fb_info->lock), and leads to
> a potential deadlock as following:
> [ 601.079000] ===========================
> [ 601.079000] [ INFO: possible circular locking dependency detected ]
> [ 601.079000] 3.11.0 #189 Not tainted
> [ 601.079000] -------------------------------------------------------
> [ 601.079000] kworker/0:3/619 is trying to acquire lock:
> [ 601.079000] (&fb_info->lock){+.+.+.}, at: [<ffffffff81397566>] lock_fb_info+0x26/0x60
> [ 601.079000]
> but task is already holding lock:
> [ 601.079000] (console_lock){+.+.+.}, at: [<ffffffff8141aae3>] console_callback+0x13/0x160
> [ 601.079000]
> which lock already depends on the new lock.
>
> [ 601.079000]
> the existing dependency chain (in reverse order) is:
> [ 601.079000]
> -> #1 (console_lock){+.+.+.}:
> [ 601.079000] [<ffffffff810dc971>] lock_acquire+0xa1/0x140
> [ 601.079000] [<ffffffff810c6267>] console_lock+0x77/0x80
> [ 601.079000] [<ffffffff81399448>] register_framebuffer+0x1d8/0x320
> [ 601.079000] [<ffffffff81cfb4c8>] efifb_probe+0x408/0x48f
> [ 601.079000] [<ffffffff8144a963>] platform_drv_probe+0x43/0x80
> [ 601.079000] [<ffffffff8144853b>] driver_probe_device+0x8b/0x390
> [ 601.079000] [<ffffffff814488eb>] __driver_attach+0xab/0xb0
> [ 601.079000] [<ffffffff814463bd>] bus_for_each_dev+0x5d/0xa0
> [ 601.079000] [<ffffffff81447e6e>] driver_attach+0x1e/0x20
> [ 601.079000] [<ffffffff81447a07>] bus_add_driver+0x117/0x290
> [ 601.079000] [<ffffffff81448fea>] driver_register+0x7a/0x170
> [ 601.079000] [<ffffffff8144a10a>] __platform_driver_register+0x4a/0x50
> [ 601.079000] [<ffffffff8144a12d>] platform_driver_probe+0x1d/0xb0
> [ 601.079000] [<ffffffff81cfb0a1>] efifb_init+0x273/0x292
> [ 601.079000] [<ffffffff81002132>] do_one_initcall+0x102/0x1c0
> [ 601.079000] [<ffffffff81cb80a6>] kernel_init_freeable+0x15d/0x1ef
> [ 601.079000] [<ffffffff8166d2de>] kernel_init+0xe/0xf0
> [ 601.079000] [<ffffffff816914ec>] ret_from_fork+0x7c/0xb0
> [ 601.079000]
> -> #0 (&fb_info->lock){+.+.+.}:
> [ 601.079000] [<ffffffff810dc1d8>] __lock_acquire+0x1e18/0x1f10
> [ 601.079000] [<ffffffff810dc971>] lock_acquire+0xa1/0x140
> [ 601.079000] [<ffffffff816835ca>] mutex_lock_nested+0x7a/0x3b0
> [ 601.079000] [<ffffffff81397566>] lock_fb_info+0x26/0x60
> [ 601.079000] [<ffffffff813a4aeb>] fbcon_blank+0x29b/0x2e0
> [ 601.079000] [<ffffffff81418658>] do_blank_screen+0x1d8/0x280
> [ 601.079000] [<ffffffff8141ab34>] console_callback+0x64/0x160
> [ 601.079000] [<ffffffff8108d855>] process_one_work+0x1f5/0x540
> [ 601.079000] [<ffffffff8108e04c>] worker_thread+0x11c/0x370
> [ 601.079000] [<ffffffff81095fbd>] kthread+0xed/0x100
> [ 601.079000] [<ffffffff816914ec>] ret_from_fork+0x7c/0xb0
> [ 601.079000]
> other info that might help us debug this:
>
> [ 601.079000] Possible unsafe locking scenario:
>
> [ 601.079000] CPU0 CPU1
> [ 601.079000] ---- ----
> [ 601.079000] lock(console_lock);
> [ 601.079000] lock(&fb_info->lock);
> [ 601.079000] lock(console_lock);
> [ 601.079000] lock(&fb_info->lock);
> [ 601.079000]
> *** DEADLOCK ***
>
> so we reorder the lock sequence the same as it in console_callback() to
> avoid this issue.
> Not very sure this change is suitable, any comments is welcome.
>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
> drivers/video/fbmem.c | 50 +++++++++++++++++++++++++++++++-----------------
> 1 files changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index dacaf74..010d191 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1108,14 +1108,16 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> case FBIOPUT_VSCREENINFO:
> if (copy_from_user(&var, argp, sizeof(var)))
> return -EFAULT;
> - if (!lock_fb_info(info))
> - return -ENODEV;
> console_lock();
> + if (!lock_fb_info(info)) {
> + console_unlock();
> + return -ENODEV;
> + }
> info->flags |= FBINFO_MISC_USEREVENT;
> ret = fb_set_var(info, &var);
> info->flags &= ~FBINFO_MISC_USEREVENT;
> - console_unlock();
> unlock_fb_info(info);
> + console_unlock();
> if (!ret && copy_to_user(argp, &var, sizeof(var)))
> ret = -EFAULT;
> break;
> @@ -1144,12 +1146,14 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> case FBIOPAN_DISPLAY:
> if (copy_from_user(&var, argp, sizeof(var)))
> return -EFAULT;
> - if (!lock_fb_info(info))
> - return -ENODEV;
> console_lock();
> + if (!lock_fb_info(info)) {
> + console_unlock();
> + return -ENODEV;
> + }
> ret = fb_pan_display(info, &var);
> - console_unlock();
> unlock_fb_info(info);
> + console_unlock();
> if (ret = 0 && copy_to_user(argp, &var, sizeof(var)))
> return -EFAULT;
> break;
> @@ -1184,23 +1188,27 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> break;
> }
> event.data = &con2fb;
> - if (!lock_fb_info(info))
> - return -ENODEV;
> console_lock();
> + if (!lock_fb_info(info)) {
> + console_unlock();
> + return -ENODEV;
> + }
> event.info = info;
> ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event);
> - console_unlock();
> unlock_fb_info(info);
> + console_unlock();
> break;
> case FBIOBLANK:
> - if (!lock_fb_info(info))
> - return -ENODEV;
> console_lock();
> + if (!lock_fb_info(info)) {
> + console_unlock();
> + return -ENODEV;
> + }
> info->flags |= FBINFO_MISC_USEREVENT;
> ret = fb_blank(info, arg);
> info->flags &= ~FBINFO_MISC_USEREVENT;
> - console_unlock();
> unlock_fb_info(info);
> + console_unlock();
> break;
> default:
> if (!lock_fb_info(info))
> @@ -1660,12 +1668,15 @@ static int do_register_framebuffer(struct fb_info *fb_info)
> registered_fb[i] = fb_info;
>
> event.info = fb_info;
> - if (!lock_fb_info(fb_info))
> - return -ENODEV;
> console_lock();
> + if (!lock_fb_info(fb_info)) {
> + console_unlock();
> + return -ENODEV;
> + }
> +
> fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
> - console_unlock();
> unlock_fb_info(fb_info);
> + console_unlock();
> return 0;
> }
>
> @@ -1678,13 +1689,16 @@ static int do_unregister_framebuffer(struct fb_info *fb_info)
> if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
> return -EINVAL;
>
> - if (!lock_fb_info(fb_info))
> - return -ENODEV;
> console_lock();
> + if (!lock_fb_info(fb_info)) {
> + console_unlock();
> + return -ENODEV;
> + }
> +
> event.info = fb_info;
> ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
> - console_unlock();
> unlock_fb_info(fb_info);
> + console_unlock();
>
> if (ret)
> return -EINVAL;
^ permalink raw reply
* Re: [PATCH 1/1] video: exynos_mipi_dsi: Remove unused variable
From: Sachin Kamat @ 2013-10-30 3:24 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1382333533-32740-1-git-send-email-sachin.kamat@linaro.org>
On 30 October 2013 04:30, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Fri, Oct 25, 2013 at 10:51:48AM +0530, Sachin Kamat wrote:
>> On 24 October 2013 21:25, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> > On Monday 21 October 2013 11:02 AM, Sachin Kamat wrote:
>> >> 'pdev' is not used anymore (Removed vide commit 7e0be9f9 "video:
>> >> exynos_mipi_dsim: Use the generic PHY driver"). Remove it and
>> >> silence the following compilation warning:
>> >> drivers/video/exynos/exynos_mipi_dsi.c:144:26: warning:
>> >> unused variable ‘pdev’ [-Wunused-variable]
>> >>
>> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> > Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com>
>>
>> Thanks Kishon.
>>
>> I believe this patch should go through Greg's tree as your other
>> patches are lined up there?
>
> I don't take drivers/video/* patches now, am I?
Actually the patch that introduced this warning is lined up in your
usb-next tree [1]
as part of the phy series patches sent by Kishon. That is the reason I
thought this
one should go though your (usb) tree as well for now. Alternatively we
can wait for
rc-1 and then send it through the video tree if you prefer it that way.
[1] https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id~0be9f9f7cba3356f75b86737dbe3a005da067e
--
With warm regards,
Sachin
^ permalink raw reply
* Re: [PATCH 1/2] simplefb: fix unmapping fb during destruction
From: David Herrmann @ 2013-10-30 7:48 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-fbdev@vger.kernel.org, Stephen Warren, linux-kernel,
dri-devel@lists.freedesktop.org, Alexandre Courbot,
Jean-Christophe Plagniol-Villard
In-Reply-To: <CANq1E4TuzskdyXVza3YRbR7GmGzHnm9jFmeHmyO2ghuC9u_5WA@mail.gmail.com>
Hi Tomi
Could we get this in -next before the merge-window starts? Stephen
already ack'ed it.
Thanks
David
On Wed, Oct 2, 2013 at 6:23 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Wed, Oct 2, 2013 at 6:16 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/02/2013 08:58 AM, David Herrmann wrote:
>>> Unfortunately, fbdev does not create its own "struct device" for
>>> framebuffers. Instead, it attaches to the device of the parent layer. This
>>> has the side-effect that devm_* managed resources are not cleaned up on
>>> framebuffer-destruction but rather during destruction of the
>>> parent-device. In case of fbdev this might be too late, though.
>>> remove_conflicting_framebuffer() may remove fbdev devices but keep the
>>> parent device as it is.
>>>
>>> Therefore, we now use plain ioremap() and unmap the framebuffer in the
>>> fb_destroy() callback. Note that we must not free the device here as this
>>> might race with the parent-device removal. Instead, we rely on
>>> unregister_framebuffer() as barrier and we're safe.
>>
>> So, once the .fb_destroy callback has been executed, there's no other
>> callback to resurrect it? The framebuffer itself is still registered
>> until the device's remove, yet after .fb_destroy, the memory is
>> unmapped, which would be dangerous if the FB can be re-started.
>
> fbdev lifetime tracking is weird.. ->fb_destroy() is called by
> unregister_framebuffer() _after_ it got unlinked. So no, the
> framebuffer is gone and cannot be resurrected. However, the
> unregistered/dead fbdev object itself stays around until you call
> framebuffer_release(). We cannot call it from fb_destroy(), though as
> the platform_data in your platform device still points to the fbdev
> device. Therefore we keep it and wait for the platform-driver to be
> removed which then again calls unregister_framebuffer() (which will
> immediately return as the fbdev device is not registered) and then you
> can finally call framebuffer_release().
>
> Note that even though there's fb_get_info() and fb_put_info(), both
> are not exported and never used. So there is *no* fbdev ref-counting
> (which would be horribly broken anyway) and the driver is the sole
> owner of the fbdev object.
>
>> If that's not an issue, this patch seems fine, so
>> Acked-by: Stephen Warren <swarren@nvidia.com>
>
> Thanks!
>
>>> I know that simplefb was supposed to stay "as simple as possible" but I really
>>> think this series is the last set of fixes I have. Unfortunately framebuffer DRM
>>> handover is mandatory so we cannot ignore it in simplefb.
>>
>> I don't think this patch adds any significant complexity, so I'm not
>> worried at least.
>
> Good to hear!
>
> Thanks
> David
^ permalink raw reply
* Re: [PATCH 2/2] simplefb: use write-combined remapping
From: David Herrmann @ 2013-10-30 7:49 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-fbdev@vger.kernel.org, Stephen Warren, linux-kernel,
dri-devel@lists.freedesktop.org, Alexandre Courbot,
Jean-Christophe Plagniol-Villard
In-Reply-To: <1380725919-1961-2-git-send-email-dh.herrmann@gmail.com>
Hi Tomi
Ping?
Thanks
David
On Wed, Oct 2, 2013 at 4:58 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Framebuffers shouldn't be cached and it is usually very uncommon to read
> them. Therefore, use ioremap_wc() to get significant speed improvements on
> systems which provide it. On all other systems it's aliased to
> ioremap_nocache() which is also fine.
>
> Reported-by: Tom Gundersen <teg@jklm.no>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> Tested-by: Tom Gundersen <teg@jklm.no>
> Tested-by: Alexandre Courbot <acourbot@nvidia.com>
> Tested-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
> drivers/video/simplefb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
> index 74b016c..64db54a 100644
> --- a/drivers/video/simplefb.c
> +++ b/drivers/video/simplefb.c
> @@ -219,8 +219,8 @@ static int simplefb_probe(struct platform_device *pdev)
>
> info->fbops = &simplefb_ops;
> info->flags = FBINFO_DEFAULT | FBINFO_MISC_FIRMWARE;
> - info->screen_base = ioremap(info->fix.smem_start,
> - info->fix.smem_len);
> + info->screen_base = ioremap_wc(info->fix.smem_start,
> + info->fix.smem_len);
> if (!info->screen_base) {
> framebuffer_release(info);
> return -ENODEV;
> --
> 1.8.4
>
^ permalink raw reply
* Re: [PATCH 2/2] simplefb: use write-combined remapping
From: Tomi Valkeinen @ 2013-10-30 10:54 UTC (permalink / raw)
To: David Herrmann
Cc: linux-kernel, Tom Gundersen, Alexandre Courbot, Stephen Warren,
dri-devel@lists.freedesktop.org, Jean-Christophe Plagniol-Villard,
linux-fbdev@vger.kernel.org
In-Reply-To: <CANq1E4SRWsKf4uqF01Rju-uSNF-gj9zL9_+0zAqsgPMjv8Q3Uw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1627 bytes --]
On 2013-10-30 09:49, David Herrmann wrote:
> Hi Tomi
>
> Ping?
Thanks, queued this and the 1/2 patch for 3.13.
Tomi
>
> Thanks
> David
>
> On Wed, Oct 2, 2013 at 4:58 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> Framebuffers shouldn't be cached and it is usually very uncommon to read
>> them. Therefore, use ioremap_wc() to get significant speed improvements on
>> systems which provide it. On all other systems it's aliased to
>> ioremap_nocache() which is also fine.
>>
>> Reported-by: Tom Gundersen <teg@jklm.no>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> Tested-by: Tom Gundersen <teg@jklm.no>
>> Tested-by: Alexandre Courbot <acourbot@nvidia.com>
>> Tested-by: Stephen Warren <swarren@wwwdotorg.org>
>> ---
>> drivers/video/simplefb.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
>> index 74b016c..64db54a 100644
>> --- a/drivers/video/simplefb.c
>> +++ b/drivers/video/simplefb.c
>> @@ -219,8 +219,8 @@ static int simplefb_probe(struct platform_device *pdev)
>>
>> info->fbops = &simplefb_ops;
>> info->flags = FBINFO_DEFAULT | FBINFO_MISC_FIRMWARE;
>> - info->screen_base = ioremap(info->fix.smem_start,
>> - info->fix.smem_len);
>> + info->screen_base = ioremap_wc(info->fix.smem_start,
>> + info->fix.smem_len);
>> if (!info->screen_base) {
>> framebuffer_release(info);
>> return -ENODEV;
>> --
>> 1.8.4
>>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: outcome of DRM/KMS DT bindings session
From: Thierry Reding @ 2013-10-30 11:19 UTC (permalink / raw)
To: Dave Airlie; +Cc: ksummit-2013-discuss, Linux Fbdev development list, dri-devel
In-Reply-To: <CAPM=9twObPRhPavm9xD+7=JDPEVD+K_WFJxMGG=f5eXR12PvLw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3718 bytes --]
On Tue, Oct 29, 2013 at 01:52:57PM +1000, Dave Airlie wrote:
> So we had a sessions at kernel summit to discuss the driver model and
> DT interactions for a display pipeline,
>
> we had good attendance from a few sides and I hope to summarise the
> recommendations below,
>
> a) Device Tree bindings
>
> We should create a top-level virtual device binding that a top level
> driver can bind to, like alsa asoc does.
>
> We should separate the CDF device tree model from CDF as a starting
> point and refine it outside of CDF, and produce a set of bindings that
> cover the current drivers we have, exynos, imx, tegra, msm, armada
> etc. This set of bindings should not be tied on CDF being merged or
> anything else.
>
> Display pipelines should be modelered in the device tree, but the
> level of detail required for links between objects may be left up to
> the SoC developer, esp wrt tightly coupled SoCs.
>
> Externally linked devices like bridges and panels should be explicitly linked.
According to the above, the device tree bindings for simple panels that
I proposed earlier should be fine. However there was so much controversy
involved that I've decided not to make that part of my pull request this
cycle. Also they haven't been reviewed by DT bindings maintainers yes,
so according to our new rules they cannot be merged.
I think Laurent was more or less fine with them too, although he had
some objections to how DSI panels were represented and wanted those to
be sub-nodes of the DSI controller. I'll see if I can come up with
something to address that.
We should probably aim for a common binding for things like DSI.
> b) Driver Model
>
> The big thing here is that the device tree description we use should
> not dictate the driver model we use. This is the biggest thing I
> learned, so what does it mean?
>
> We aren't required to write a device driver per device tree object.
>
> We shouldn't be writing device drivers per device tree object.
I may remember this wrongly, but that's the opposite recommendation that
I got back when I started to work on Tegra DRM.
> For tightly-coupled SoCs where the blocks come from one vendor and are
> reused a lot, a top level driver should use the DT as configuration
> information source for the list of blocks it needs to initialise on
> the card, not as a list of separate drivers. There may be some
> external drivers required and the code should deal with this, like how
> alsa asoc does.
>
> To share code between layers we should refactor it into a helper
> library not a separate driver, the kms/v4l/fbdev can use the library.
>
> This should allow us to move forward a bit clearer esp with new
> drivers and following these recommendations, and I think porting
> current drivers to a sane model, especially exynos and imx.
>
> Now I saw we here but I'm only going to be donating my use of a big
> stick and review abilities to making this happen, but I'm quite
> willing to enforce some of these rules going forward as I think it
> will make life easier.
>
> After looking at some of the ordering issues we've had with x86 GPUs
> (which are really just a tightly coupled SoC) I don't want separate
> drivers all having their own init, suspend/resume paths in them as I
> know we'll have to start making special vtable entry points etc to
> solve some random ordering issues that crop up.
Where does that leave the Tegra driver? I've spent a significant amount
of time to get it to some sane state where having multiple subdrivers
are handled fairly nicely (in my opinion). Rewriting all of it isn't
something that I look forward to at all.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC PATCH RESEND] fb: reorder the lock sequence to fix a potential lockdep
From: Tomi Valkeinen @ 2013-10-30 11:20 UTC (permalink / raw)
To: Gu Zheng, Jean-Christophe PLAGNIOL-VILLARD
Cc: Linux Fbdev development list, linux-kernel
In-Reply-To: <526DFDA8.7010806@cn.fujitsu.com>
[-- Attachment #1: Type: text/plain, Size: 714 bytes --]
Hi,
On 2013-10-28 08:01, Gu Zheng wrote:
> Following commits:
> 50e244cc79 fb: rework locking to fix lock ordering on takeover
> e93a9a8687 fb: Yet another band-aid for fixing lockdep mess
> 054430e773 fbcon: fix locking harder
>
> reworked locking to fix related lock ordering on takeover, and introduced console_lock
> into fbmem, but it seems that the new lock sequence(fb_info->lock ---> console_lock)
> is against with the one in console_callback(console_lock ---> fb_info->lock), and leads to
> a potential deadlock as following:
A quick grep shows that there are other places than fbmem.c which use
lock_fb_info and console_lock, for example drivers/video/sh_mobile_lcdcfb.c.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [Trivial PATCH] video: Remove unnecessary semicolons
From: Tomi Valkeinen @ 2013-10-30 11:35 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-fbdev, LKML
In-Reply-To: <1381274604.23937.15.camel@joe-AO722>
[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]
On 2013-10-09 02:23, Joe Perches wrote:
> These aren't necessary after switch, for, and if blocks.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/video/cfbimgblt.c | 2 +-
> drivers/video/cg14.c | 4 ++--
> drivers/video/cg6.c | 2 +-
> drivers/video/exynos/exynos_mipi_dsi_common.c | 2 +-
> drivers/video/fsl-diu-fb.c | 2 +-
> drivers/video/leo.c | 2 +-
> drivers/video/matrox/matroxfb_DAC1064.c | 4 ++--
> drivers/video/matrox/matroxfb_Ti3026.c | 2 +-
> drivers/video/nvidia/nv_hw.c | 2 +-
> drivers/video/omap2/displays-new/panel-dsi-cm.c | 2 +-
> drivers/video/omap2/dss/dispc.c | 2 +-
> drivers/video/omap2/dss/dsi.c | 2 +-
> drivers/video/sbuslib.c | 2 +-
> drivers/video/sysimgblt.c | 2 +-
> drivers/video/tcx.c | 4 ++--
> drivers/video/vt8500lcdfb.c | 2 +-
> 16 files changed, 19 insertions(+), 19 deletions(-)
Thanks, queued for 3.13.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] video: exynos_mipi_dsi: Unlock the mutex before returning
From: Tomi Valkeinen @ 2013-10-30 11:53 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1382677964-15298-1-git-send-email-sachin.kamat@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 847 bytes --]
On 2013-10-25 08:12, Sachin Kamat wrote:
> Mutex should be unlocked before returning. Fixes mutex lock-unlock
> imbalance issue.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> drivers/video/exynos/exynos_mipi_dsi_common.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.c b/drivers/video/exynos/exynos_mipi_dsi_common.c
> index 520fc9b..649d9e6 100644
> --- a/drivers/video/exynos/exynos_mipi_dsi_common.c
> +++ b/drivers/video/exynos/exynos_mipi_dsi_common.c
> @@ -376,6 +376,7 @@ int exynos_mipi_dsi_rd_data(struct mipi_dsim_device *dsim, unsigned int data_id,
> "data id %x is not supported current DSI spec.\n",
> data_id);
>
> + mutex_unlock(&dsim->lock);
> return -EINVAL;
> }
>
>
Thanks, queued for 3.13.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: outcome of DRM/KMS DT bindings session
From: Sascha Hauer @ 2013-10-30 12:02 UTC (permalink / raw)
To: Dave Airlie; +Cc: ksummit-2013-discuss, Linux Fbdev development list, dri-devel
In-Reply-To: <CAPM=9twObPRhPavm9xD+7=JDPEVD+K_WFJxMGG=f5eXR12PvLw@mail.gmail.com>
On Tue, Oct 29, 2013 at 01:52:57PM +1000, Dave Airlie wrote:
> So we had a sessions at kernel summit to discuss the driver model and
> DT interactions for a display pipeline,
>
> we had good attendance from a few sides and I hope to summarise the
> recommendations below,
>
> a) Device Tree bindings
>
> We should create a top-level virtual device binding that a top level
> driver can bind to, like alsa asoc does.
>
> We should separate the CDF device tree model from CDF as a starting
> point and refine it outside of CDF, and produce a set of bindings that
> cover the current drivers we have, exynos, imx, tegra, msm, armada
> etc. This set of bindings should not be tied on CDF being merged or
> anything else.
>
> Display pipelines should be modelered in the device tree, but the
> level of detail required for links between objects may be left up to
> the SoC developer, esp wrt tightly coupled SoCs.
>
> Externally linked devices like bridges and panels should be explicitly linked.
>
> b) Driver Model
>
> The big thing here is that the device tree description we use should
> not dictate the driver model we use. This is the biggest thing I
> learned, so what does it mean?
>
> We aren't required to write a device driver per device tree object.
>
> We shouldn't be writing device drivers per device tree object.
>
> For tightly-coupled SoCs where the blocks come from one vendor and are
> reused a lot, a top level driver should use the DT as configuration
> information source for the list of blocks it needs to initialise on
> the card, not as a list of separate drivers. There may be some
> external drivers required and the code should deal with this, like how
> alsa asoc does.
>
> To share code between layers we should refactor it into a helper
> library not a separate driver, the kms/v4l/fbdev can use the library.
>
> This should allow us to move forward a bit clearer esp with new
> drivers and following these recommendations, and I think porting
> current drivers to a sane model, especially exynos and imx.
>
> Now I saw we here but I'm only going to be donating my use of a big
> stick and review abilities to making this happen, but I'm quite
> willing to enforce some of these rules going forward as I think it
> will make life easier.
>
> After looking at some of the ordering issues we've had with x86 GPUs
> (which are really just a tightly coupled SoC) I don't want separate
> drivers all having their own init, suspend/resume paths in them as I
> know we'll have to start making special vtable entry points etc to
> solve some random ordering issues that crop up.
The DRM device has to be initialized/suspended/resumed as a whole, no
doubt about that. If that's not the case you indeed open up the door for
all kinds of ordering issues.
Still the different components can be multiple devices, just initialize
the drm device once all components are probed. Remove it again once a
component is removed. Handle suspend in the DRM device, not in
the individual component drivers. The suspend in the component drivers
would only be called after the DRM device is completely quiesced.
Similarly the resume in the component drivers would not reenable the
components, this instead would be done in the DRM device when all
components are there again.
This way all components could be proper (driver model)devices with
proper drivers without DRM even noticing that multiple components are
involved.
Side note: We have no choice anyway. All SoCs can (sometimes must)
be extended with external I2C devices. On every SoC the I2C bus master
is a separate device, so we have a multicomponent device (in the sense
of driver model) already in many cases.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 1/1] video: exynos_mipi_dsi: Remove unused variable
From: Greg Kroah-Hartman @ 2013-10-30 17:50 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1382333533-32740-1-git-send-email-sachin.kamat@linaro.org>
On Wed, Oct 30, 2013 at 08:42:12AM +0530, Sachin Kamat wrote:
> On 30 October 2013 04:30, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > On Fri, Oct 25, 2013 at 10:51:48AM +0530, Sachin Kamat wrote:
> >> On 24 October 2013 21:25, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> >> > On Monday 21 October 2013 11:02 AM, Sachin Kamat wrote:
> >> >> 'pdev' is not used anymore (Removed vide commit 7e0be9f9 "video:
> >> >> exynos_mipi_dsim: Use the generic PHY driver"). Remove it and
> >> >> silence the following compilation warning:
> >> >> drivers/video/exynos/exynos_mipi_dsi.c:144:26: warning:
> >> >> unused variable ‘pdev’ [-Wunused-variable]
> >> >>
> >> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> >> > Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com>
> >>
> >> Thanks Kishon.
> >>
> >> I believe this patch should go through Greg's tree as your other
> >> patches are lined up there?
> >
> > I don't take drivers/video/* patches now, am I?
>
> Actually the patch that introduced this warning is lined up in your
> usb-next tree [1]
> as part of the phy series patches sent by Kishon. That is the reason I
> thought this
> one should go though your (usb) tree as well for now. Alternatively we
> can wait for
> rc-1 and then send it through the video tree if you prefer it that way.
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id~0be9f9f7cba3356f75b86737dbe3a005da067e
Please send it after 3.13-rc1 is out, as this isn't a big deal. That
way you can go through the "normal" tree.
thanks,
greg k-h
^ permalink raw reply
* Re: outcome of DRM/KMS DT bindings session
From: Tomasz Figa @ 2013-10-30 18:26 UTC (permalink / raw)
To: dri-devel; +Cc: ksummit-2013-discuss, Linux Fbdev development list
In-Reply-To: <20131030120229.GF24559@pengutronix.de>
On Wednesday 30 of October 2013 13:02:29 Sascha Hauer wrote:
> On Tue, Oct 29, 2013 at 01:52:57PM +1000, Dave Airlie wrote:
> > So we had a sessions at kernel summit to discuss the driver model and
> > DT interactions for a display pipeline,
> >
> > we had good attendance from a few sides and I hope to summarise the
> > recommendations below,
> >
> > a) Device Tree bindings
> >
> > We should create a top-level virtual device binding that a top level
> > driver can bind to, like alsa asoc does.
> >
> > We should separate the CDF device tree model from CDF as a starting
> > point and refine it outside of CDF, and produce a set of bindings that
> > cover the current drivers we have, exynos, imx, tegra, msm, armada
> > etc. This set of bindings should not be tied on CDF being merged or
> > anything else.
> >
> > Display pipelines should be modelered in the device tree, but the
> > level of detail required for links between objects may be left up to
> > the SoC developer, esp wrt tightly coupled SoCs.
> >
> > Externally linked devices like bridges and panels should be explicitly
> > linked.
> >
> > b) Driver Model
> >
> > The big thing here is that the device tree description we use should
> > not dictate the driver model we use. This is the biggest thing I
> > learned, so what does it mean?
> >
> > We aren't required to write a device driver per device tree object.
> >
> > We shouldn't be writing device drivers per device tree object.
> >
> > For tightly-coupled SoCs where the blocks come from one vendor and are
> > reused a lot, a top level driver should use the DT as configuration
> > information source for the list of blocks it needs to initialise on
> > the card, not as a list of separate drivers. There may be some
> > external drivers required and the code should deal with this, like how
> > alsa asoc does.
> >
> > To share code between layers we should refactor it into a helper
> > library not a separate driver, the kms/v4l/fbdev can use the library.
> >
> > This should allow us to move forward a bit clearer esp with new
> > drivers and following these recommendations, and I think porting
> > current drivers to a sane model, especially exynos and imx.
> >
> > Now I saw we here but I'm only going to be donating my use of a big
> > stick and review abilities to making this happen, but I'm quite
> > willing to enforce some of these rules going forward as I think it
> > will make life easier.
> >
> > After looking at some of the ordering issues we've had with x86 GPUs
> > (which are really just a tightly coupled SoC) I don't want separate
> > drivers all having their own init, suspend/resume paths in them as I
> > know we'll have to start making special vtable entry points etc to
> > solve some random ordering issues that crop up.
>
> The DRM device has to be initialized/suspended/resumed as a whole, no
> doubt about that. If that's not the case you indeed open up the door for
> all kinds of ordering issues.
>
> Still the different components can be multiple devices, just initialize
> the drm device once all components are probed. Remove it again once a
> component is removed. Handle suspend in the DRM device, not in
> the individual component drivers. The suspend in the component drivers
> would only be called after the DRM device is completely quiesced.
> Similarly the resume in the component drivers would not reenable the
> components, this instead would be done in the DRM device when all
> components are there again.
>
> This way all components could be proper (driver model)devices with
> proper drivers without DRM even noticing that multiple components are
> involved.
>
> Side note: We have no choice anyway. All SoCs can (sometimes must)
> be extended with external I2C devices. On every SoC the I2C bus master
> is a separate device, so we have a multicomponent device (in the sense
> of driver model) already in many cases.
+1
Best regards,
Tomasz
^ permalink raw reply
* Re: [PATCH 9/9] backlight: atmel-pwm-bl: use gpio_request_one
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-30 19:30 UTC (permalink / raw)
To: Johan Hovold
Cc: Richard Purdie, Jingoo Han, Nicolas Ferre, Tomi Valkeinen,
Andrew Morton, linux-fbdev, linux-kernel
In-Reply-To: <20131029132018.GC29615@localhost>
On 14:20 Tue 29 Oct , Johan Hovold wrote:
> On Fri, Oct 25, 2013 at 01:15:43PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 11:55 Wed 23 Oct , Johan Hovold wrote:
> > > Use devm_gpio_request_one rather than requesting and setting direction
> > > in two calls.
> >
> > this is the same I do not see any advantage
>
> It removes one error path.
>
> > and as I said for ather backligth It's wrong to enable or disable it at probe
> > as the bootloader might have already enable it for splash screen
>
> I agree with you on that, and it's actually the reason for the below
> clean up. I use a second patch:
>
> if (gpio_is_valid(pwmbl->gpio_on)) {
> - /* Turn display off by default. */
> - if (pdata->on_active_low)
> + /* Turn display off unless already enabled. */
> + if (pdata->gpio_on_enabled ^ pdata->on_active_low)
> flags = GPIOF_OUT_INIT_HIGH;
> else
> flags = GPIOF_OUT_INIT_LOW;
>
> to make sure the driver does not temporarily disable the backlight at
> probe.
>
> Decided not to submit it as part of this series when I realised that
> several other backlight drivers did the same, but perhaps I should?
I'm not happy with this idea to play with enable at probe time
we need to detect the current status if possible and only enable or disable
when requiered
Best Regards,
J.
>
> Thanks,
> Johan
>
> > >
> > > Acked-by: Jingoo Han <jg1.han@samsung.com>:w
> > > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> > > ---
> > > drivers/video/backlight/atmel-pwm-bl.c | 15 ++++++++-------
> > > 1 file changed, 8 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
> > > index 1277e0c..5ea2517 100644
> > > --- a/drivers/video/backlight/atmel-pwm-bl.c
> > > +++ b/drivers/video/backlight/atmel-pwm-bl.c
> > > @@ -124,6 +124,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
> > > const struct atmel_pwm_bl_platform_data *pdata;
> > > struct backlight_device *bldev;
> > > struct atmel_pwm_bl *pwmbl;
> > > + int flags;
> > > int retval;
> > >
> > > pdata = dev_get_platdata(&pdev->dev);
> > > @@ -149,14 +150,14 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
> > > return retval;
> > >
> > > if (gpio_is_valid(pwmbl->gpio_on)) {
> > > - retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
> > > - "gpio_atmel_pwm_bl");
> > > - if (retval)
> > > - goto err_free_pwm;
> > > -
> > > /* Turn display off by default. */
> > > - retval = gpio_direction_output(pwmbl->gpio_on,
> > > - 0 ^ pdata->on_active_low);
> > > + if (pdata->on_active_low)
> > > + flags = GPIOF_OUT_INIT_HIGH;
> > > + else
> > > + flags = GPIOF_OUT_INIT_LOW;
> > > +
> > > + retval = devm_gpio_request_one(&pdev->dev, pwmbl->gpio_on,
> > > + flags, "gpio_atmel_pwm_bl");
> > > if (retval)
> > > goto err_free_pwm;
> > > }
> > > --
> > > 1.8.4
> > >
^ permalink raw reply
* Re: [PATCHv8][ 1/6] video: imxfb: Introduce regulator support.
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-30 19:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1383061860-25035-1-git-send-email-denis@eukrea.com>
On 16:50 Tue 29 Oct , Denis Carikli wrote:
> This commit is based on the following commit by Fabio Estevam:
> 4344429 video: mxsfb: Introduce regulator support
>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
> drivers/video/imxfb.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index 44ee678..4bf3837 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -28,6 +28,7 @@
> #include <linux/cpufreq.h>
> #include <linux/clk.h>
> #include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/dma-mapping.h>
> #include <linux/io.h>
> #include <linux/math64.h>
> @@ -145,6 +146,7 @@ struct imxfb_info {
> struct clk *clk_ipg;
> struct clk *clk_ahb;
> struct clk *clk_per;
> + struct regulator *reg_lcd;
> enum imxfb_type devtype;
> bool enabled;
>
> @@ -563,12 +565,23 @@ static void imxfb_exit_backlight(struct imxfb_info *fbi)
>
> static void imxfb_enable_controller(struct imxfb_info *fbi)
> {
> + int ret;
return the error so you can return it in imxfb_blank
>
> if (fbi->enabled)
> return;
>
> pr_debug("Enabling LCD controller\n");
>
> + if (fbi->reg_lcd) {
> + ret = regulator_enable(fbi->reg_lcd);
> + if (ret) {
> + dev_err(&fbi->pdev->dev,
> + "lcd regulator enable failed with error: %d\n",
> + ret);
> + return;
> + }
> + }
> +
> writel(fbi->screen_dma, fbi->regs + LCDC_SSA);
>
> /* panning offset 0 (0 pixel offset) */
> @@ -597,6 +610,8 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
>
> static void imxfb_disable_controller(struct imxfb_info *fbi)
> {
> + int ret;
> +
> if (!fbi->enabled)
> return;
>
> @@ -613,6 +628,14 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
> fbi->enabled = false;
>
> writel(0, fbi->regs + LCDC_RMCR);
> +
> + if (fbi->reg_lcd) {
> + ret = regulator_disable(fbi->reg_lcd);
> + if (ret)
> + dev_err(&fbi->pdev->dev,
> + "lcd regulator disable failed with error: %d\n",
> + ret);
> + }
ditto here
> }
>
> static int imxfb_blank(int blank, struct fb_info *info)
> @@ -1020,6 +1043,12 @@ static int imxfb_probe(struct platform_device *pdev)
> goto failed_register;
> }
>
> + fbi->reg_lcd = devm_regulator_get(&pdev->dev, "lcd");
> + if (IS_ERR(fbi->reg_lcd)) {
> + dev_info(&pdev->dev, "No lcd regulator used.\n");
> + fbi->reg_lcd = NULL;
> + }
> +
> imxfb_enable_controller(fbi);
> fbi->pdev = pdev;
> #ifdef PWMR_BACKLIGHT_AVAILABLE
> --
> 1.7.9.5
>
^ permalink raw reply
* Re: [PATCHv8][ 3/6] video: Kconfig: Allow more broad selection of the imxfb framebuffer driver.
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-30 19:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1383061860-25035-3-git-send-email-denis@eukrea.com>
On 16:50 Tue 29 Oct , Denis Carikli wrote:
> Without that patch, a user can't select the imxfb driver when the i.MX25 and/or
> the i.MX27 device tree board are selected and that no boards that selects
> IMX_HAVE_PLATFORM_IMX_FB are compiled in.
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Eric Bénard <eric@eukrea.com>
>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
> ChangeLog v7->v8:
> - Added ACKs.
> - Add some CC(Framebuffer maintainers and mailing lists).
> - Improved commit message summary.
> ---
> drivers/video/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 4f2e1b3..22adaee 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -363,7 +363,7 @@ config FB_SA1100
>
> config FB_IMX
> tristate "Freescale i.MX1/21/25/27 LCD support"
> - depends on FB && IMX_HAVE_PLATFORM_IMX_FB
> + depends on FB && ARCH_MXC
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> --
> 1.7.9.5
>
^ permalink raw reply
* Re: [RFC PATCH RESEND] fb: reorder the lock sequence to fix a potential lockdep
From: Gu Zheng @ 2013-10-31 1:17 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jean-Christophe PLAGNIOL-VILLARD, Linux Fbdev development list,
linux-kernel
In-Reply-To: <5270EB85.1030103@ti.com>
Hi Tomi,
On 10/30/2013 07:20 PM, Tomi Valkeinen wrote:
> Hi,
>
> On 2013-10-28 08:01, Gu Zheng wrote:
>> Following commits:
>> 50e244cc79 fb: rework locking to fix lock ordering on takeover
>> e93a9a8687 fb: Yet another band-aid for fixing lockdep mess
>> 054430e773 fbcon: fix locking harder
>>
>> reworked locking to fix related lock ordering on takeover, and introduced console_lock
>> into fbmem, but it seems that the new lock sequence(fb_info->lock ---> console_lock)
>> is against with the one in console_callback(console_lock ---> fb_info->lock), and leads to
>> a potential deadlock as following:
>
> A quick grep shows that there are other places than fbmem.c which use
> lock_fb_info and console_lock, for example drivers/video/sh_mobile_lcdcfb.c.
Yes, thanks for your reminder, I'll fix them all in the next version.
Regards,
Gu
>
> Tomi
>
>
^ permalink raw reply
* [PATCHv9][ 1/6] video: imxfb: Introduce regulator support.
From: Denis Carikli @ 2013-10-31 9:15 UTC (permalink / raw)
To: linux-arm-kernel
This commit is based on the following commit by Fabio Estevam:
4344429 video: mxsfb: Introduce regulator support
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v8->v9:
- return an error if regulator_{enable,disable} fails in
imxfb_{enable,disable}_controller, and use it.
---
drivers/video/imxfb.c | 53 ++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 41 insertions(+), 12 deletions(-)
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 44ee678..322b358 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -28,6 +28,7 @@
#include <linux/cpufreq.h>
#include <linux/clk.h>
#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
#include <linux/dma-mapping.h>
#include <linux/io.h>
#include <linux/math64.h>
@@ -145,6 +146,7 @@ struct imxfb_info {
struct clk *clk_ipg;
struct clk *clk_ahb;
struct clk *clk_per;
+ struct regulator *reg_lcd;
enum imxfb_type devtype;
bool enabled;
@@ -561,14 +563,25 @@ static void imxfb_exit_backlight(struct imxfb_info *fbi)
}
#endif
-static void imxfb_enable_controller(struct imxfb_info *fbi)
+static int imxfb_enable_controller(struct imxfb_info *fbi)
{
+ int ret;
if (fbi->enabled)
- return;
+ return 0;
pr_debug("Enabling LCD controller\n");
+ if (fbi->reg_lcd) {
+ ret = regulator_enable(fbi->reg_lcd);
+ if (ret) {
+ dev_err(&fbi->pdev->dev,
+ "lcd regulator enable failed with error: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
writel(fbi->screen_dma, fbi->regs + LCDC_SSA);
/* panning offset 0 (0 pixel offset) */
@@ -593,12 +606,16 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
fbi->backlight_power(1);
if (fbi->lcd_power)
fbi->lcd_power(1);
+
+ return 0;
}
-static void imxfb_disable_controller(struct imxfb_info *fbi)
+static int imxfb_disable_controller(struct imxfb_info *fbi)
{
+ int ret;
+
if (!fbi->enabled)
- return;
+ return 0;
pr_debug("Disabling LCD controller\n");
@@ -613,6 +630,15 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
fbi->enabled = false;
writel(0, fbi->regs + LCDC_RMCR);
+
+ if (fbi->reg_lcd) {
+ ret = regulator_disable(fbi->reg_lcd);
+ if (ret)
+ dev_err(&fbi->pdev->dev,
+ "lcd regulator disable failed with error: %d\n",
+ ret);
+ return ret;
+ }
}
static int imxfb_blank(int blank, struct fb_info *info)
@@ -626,13 +652,12 @@ static int imxfb_blank(int blank, struct fb_info *info)
case FB_BLANK_VSYNC_SUSPEND:
case FB_BLANK_HSYNC_SUSPEND:
case FB_BLANK_NORMAL:
- imxfb_disable_controller(fbi);
- break;
+ return imxfb_disable_controller(fbi);
case FB_BLANK_UNBLANK:
- imxfb_enable_controller(fbi);
- break;
+ return imxfb_enable_controller(fbi);
}
+
return 0;
}
@@ -734,8 +759,7 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
pr_debug("%s\n", __func__);
- imxfb_disable_controller(fbi);
- return 0;
+ return imxfb_disable_controller(fbi);
}
static int imxfb_resume(struct platform_device *dev)
@@ -745,8 +769,7 @@ static int imxfb_resume(struct platform_device *dev)
pr_debug("%s\n", __func__);
- imxfb_enable_controller(fbi);
- return 0;
+ return imxfb_enable_controller(fbi);
}
#else
#define imxfb_suspend NULL
@@ -1020,6 +1043,12 @@ static int imxfb_probe(struct platform_device *pdev)
goto failed_register;
}
+ fbi->reg_lcd = devm_regulator_get(&pdev->dev, "lcd");
+ if (IS_ERR(fbi->reg_lcd)) {
+ dev_info(&pdev->dev, "No lcd regulator used.\n");
+ fbi->reg_lcd = NULL;
+ }
+
imxfb_enable_controller(fbi);
fbi->pdev = pdev;
#ifdef PWMR_BACKLIGHT_AVAILABLE
--
1.7.9.5
^ permalink raw reply related
* [PATCHv9][ 2/6] video: imxfb: Also add pwmr for the device tree.
From: Denis Carikli @ 2013-10-31 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1383210928-18906-1-git-send-email-denis@eukrea.com>
pwmr has to be set to get the imxfb backlight work,
though pwmr was only configurable trough the platform data.
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Acked-by: Grant Likely <grant.likely@linaro.org>
---
.../devicetree/bindings/video/fsl,imx-fb.txt | 3 +++
drivers/video/imxfb.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
index 46da08d..ac457ae 100644
--- a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
+++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
@@ -17,6 +17,9 @@ Required nodes:
Optional properties:
- fsl,dmacr: DMA Control Register value. This is optional. By default, the
register is not modified as recommended by the datasheet.
+- fsl,pwmr: LCDC PWM Contrast Control Register value. That property is
+ optional, but defining it is necessary to get the backlight working. If that
+ property is ommited, the register is zeroed.
- fsl,lscr1: LCDC Sharp Configuration Register value.
Example:
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 322b358..08e3c36 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -833,6 +833,8 @@ static int imxfb_init_fbinfo(struct platform_device *pdev)
of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr);
+ of_property_read_u32(np, "fsl,pwmr", &fbi->pwmr);
+
/* These two function pointers could be used by some specific
* platforms. */
fbi->lcd_power = NULL;
--
1.7.9.5
^ permalink raw reply related
* [PATCHv9][ 3/6] video: Kconfig: Allow more broad selection of the imxfb framebuffer driver.
From: Denis Carikli @ 2013-10-31 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1383210928-18906-1-git-send-email-denis@eukrea.com>
Without that patch, a user can't select the imxfb driver when the i.MX25 and/or
the i.MX27 device tree board are selected and that no boards that selects
IMX_HAVE_PLATFORM_IMX_FB are compiled in.
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Eric Bénard <eric@eukrea.com>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v8->v9:
- Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
---
drivers/video/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 4f2e1b3..22adaee 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -363,7 +363,7 @@ config FB_SA1100
config FB_IMX
tristate "Freescale i.MX1/21/25/27 LCD support"
- depends on FB && IMX_HAVE_PLATFORM_IMX_FB
+ depends on FB && ARCH_MXC
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v3] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Tomi Valkeinen @ 2013-10-31 10:32 UTC (permalink / raw)
To: Marek Belisko; +Cc: plagnioj, hns, linux-kernel, linux-omap, linux-fbdev
In-Reply-To: <1383085540-8063-1-git-send-email-marek@goldelico.com>
[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]
On 2013-10-30 00:25, Marek Belisko wrote:
> Signed-off-by: Marek Belisko <marek@goldelico.com>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> changes from v2:
> - move tx_buf from driver data to functions where it's used
> - update write functions names (to reflect how many bytes are transferred)
> - update delays from 1s to 1ms (probably typo)
> - remove unnecessary 90ms sleep (tested and works fine)
> - disable dpi output after disable panel
>
> changes from v1:
> - reworked to be spi driver instead platform with custom spi bitbang
> this configuration was tested with spi_gpio bitbang driver on gta04 board
> and works fine (thanks Tomi and Lars-Peter for comments)
> - address previous comments
>
> drivers/video/omap2/displays-new/Kconfig | 6 +
> drivers/video/omap2/displays-new/Makefile | 1 +
> .../omap2/displays-new/panel-tpo-td028ttec1.c | 480 +++++++++++++++++++++
> include/video/omap-panel-data.h | 13 +
> 4 files changed, 500 insertions(+)
> create mode 100644 drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c
Sparse gave these warnings:
drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c:67:5: warning:
symbol 'jbt_ret_write_0' was not declared. Should it be static?
drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c:81:5: warning:
symbol 'jbt_reg_write_1' was not declared. Should it be static?
drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c:97:5: warning:
symbol 'jbt_reg_write_2' was not declared. Should it be static?
I fixed them and queued the patch for 3.13.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* [PATCH 1/2] fb: reorder the lock sequence to fix potential dead lock
From: Gu Zheng @ 2013-10-31 10:33 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen
Cc: Linux Fbdev development list, linux-kernel
Following commits:
50e244cc79 fb: rework locking to fix lock ordering on takeover
e93a9a8687 fb: Yet another band-aid for fixing lockdep mess
054430e773 fbcon: fix locking harder
reworked locking to fix related lock ordering on takeover, and introduced console_lock
into fbmem, but it seems that the new lock sequence(fb_info->lock ---> console_lock)
is against with the one in console_callback(console_lock ---> fb_info->lock), and leads to
a potential dead lock as following:
[ 601.079000] ===========================
[ 601.079000] [ INFO: possible circular locking dependency detected ]
[ 601.079000] 3.11.0 #189 Not tainted
[ 601.079000] -------------------------------------------------------
[ 601.079000] kworker/0:3/619 is trying to acquire lock:
[ 601.079000] (&fb_info->lock){+.+.+.}, at: [<ffffffff81397566>] lock_fb_info+0x26/0x60
[ 601.079000]
but task is already holding lock:
[ 601.079000] (console_lock){+.+.+.}, at: [<ffffffff8141aae3>] console_callback+0x13/0x160
[ 601.079000]
which lock already depends on the new lock.
[ 601.079000]
the existing dependency chain (in reverse order) is:
[ 601.079000]
-> #1 (console_lock){+.+.+.}:
[ 601.079000] [<ffffffff810dc971>] lock_acquire+0xa1/0x140
[ 601.079000] [<ffffffff810c6267>] console_lock+0x77/0x80
[ 601.079000] [<ffffffff81399448>] register_framebuffer+0x1d8/0x320
[ 601.079000] [<ffffffff81cfb4c8>] efifb_probe+0x408/0x48f
[ 601.079000] [<ffffffff8144a963>] platform_drv_probe+0x43/0x80
[ 601.079000] [<ffffffff8144853b>] driver_probe_device+0x8b/0x390
[ 601.079000] [<ffffffff814488eb>] __driver_attach+0xab/0xb0
[ 601.079000] [<ffffffff814463bd>] bus_for_each_dev+0x5d/0xa0
[ 601.079000] [<ffffffff81447e6e>] driver_attach+0x1e/0x20
[ 601.079000] [<ffffffff81447a07>] bus_add_driver+0x117/0x290
[ 601.079000] [<ffffffff81448fea>] driver_register+0x7a/0x170
[ 601.079000] [<ffffffff8144a10a>] __platform_driver_register+0x4a/0x50
[ 601.079000] [<ffffffff8144a12d>] platform_driver_probe+0x1d/0xb0
[ 601.079000] [<ffffffff81cfb0a1>] efifb_init+0x273/0x292
[ 601.079000] [<ffffffff81002132>] do_one_initcall+0x102/0x1c0
[ 601.079000] [<ffffffff81cb80a6>] kernel_init_freeable+0x15d/0x1ef
[ 601.079000] [<ffffffff8166d2de>] kernel_init+0xe/0xf0
[ 601.079000] [<ffffffff816914ec>] ret_from_fork+0x7c/0xb0
[ 601.079000]
-> #0 (&fb_info->lock){+.+.+.}:
[ 601.079000] [<ffffffff810dc1d8>] __lock_acquire+0x1e18/0x1f10
[ 601.079000] [<ffffffff810dc971>] lock_acquire+0xa1/0x140
[ 601.079000] [<ffffffff816835ca>] mutex_lock_nested+0x7a/0x3b0
[ 601.079000] [<ffffffff81397566>] lock_fb_info+0x26/0x60
[ 601.079000] [<ffffffff813a4aeb>] fbcon_blank+0x29b/0x2e0
[ 601.079000] [<ffffffff81418658>] do_blank_screen+0x1d8/0x280
[ 601.079000] [<ffffffff8141ab34>] console_callback+0x64/0x160
[ 601.079000] [<ffffffff8108d855>] process_one_work+0x1f5/0x540
[ 601.079000] [<ffffffff8108e04c>] worker_thread+0x11c/0x370
[ 601.079000] [<ffffffff81095fbd>] kthread+0xed/0x100
[ 601.079000] [<ffffffff816914ec>] ret_from_fork+0x7c/0xb0
[ 601.079000]
other info that might help us debug this:
[ 601.079000] Possible unsafe locking scenario:
[ 601.079000] CPU0 CPU1
[ 601.079000] ---- ----
[ 601.079000] lock(console_lock);
[ 601.079000] lock(&fb_info->lock);
[ 601.079000] lock(console_lock);
[ 601.079000] lock(&fb_info->lock);
[ 601.079000]
*** DEADLOCK ***
so we reorder the lock sequence the same as it in console_callback() to
avoid this issue. And following Tomi's suggestion, fix these similar
issues all in fb subsystem.
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
drivers/video/fbmem.c | 50 ++++++++++++++++++++++++-------------
drivers/video/fbsysfs.c | 19 ++++++++++----
drivers/video/sh_mobile_lcdcfb.c | 10 ++++---
3 files changed, 51 insertions(+), 28 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index dacaf74..010d191 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1108,14 +1108,16 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
case FBIOPUT_VSCREENINFO:
if (copy_from_user(&var, argp, sizeof(var)))
return -EFAULT;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
info->flags |= FBINFO_MISC_USEREVENT;
ret = fb_set_var(info, &var);
info->flags &= ~FBINFO_MISC_USEREVENT;
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
if (!ret && copy_to_user(argp, &var, sizeof(var)))
ret = -EFAULT;
break;
@@ -1144,12 +1146,14 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
case FBIOPAN_DISPLAY:
if (copy_from_user(&var, argp, sizeof(var)))
return -EFAULT;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
ret = fb_pan_display(info, &var);
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
if (ret = 0 && copy_to_user(argp, &var, sizeof(var)))
return -EFAULT;
break;
@@ -1184,23 +1188,27 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
break;
}
event.data = &con2fb;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
event.info = info;
ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event);
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
break;
case FBIOBLANK:
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
info->flags |= FBINFO_MISC_USEREVENT;
ret = fb_blank(info, arg);
info->flags &= ~FBINFO_MISC_USEREVENT;
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
break;
default:
if (!lock_fb_info(info))
@@ -1660,12 +1668,15 @@ static int do_register_framebuffer(struct fb_info *fb_info)
registered_fb[i] = fb_info;
event.info = fb_info;
- if (!lock_fb_info(fb_info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(fb_info)) {
+ console_unlock();
+ return -ENODEV;
+ }
+
fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
- console_unlock();
unlock_fb_info(fb_info);
+ console_unlock();
return 0;
}
@@ -1678,13 +1689,16 @@ static int do_unregister_framebuffer(struct fb_info *fb_info)
if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
return -EINVAL;
- if (!lock_fb_info(fb_info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(fb_info)) {
+ console_unlock();
+ return -ENODEV;
+ }
+
event.info = fb_info;
ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
- console_unlock();
unlock_fb_info(fb_info);
+ console_unlock();
if (ret)
return -EINVAL;
diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
index ef476b0..53444ac 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c
@@ -177,9 +177,12 @@ static ssize_t store_modes(struct device *device,
if (i * sizeof(struct fb_videomode) != count)
return -EINVAL;
- if (!lock_fb_info(fb_info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(fb_info)) {
+ console_unlock();
+ return -ENODEV;
+ }
+
list_splice(&fb_info->modelist, &old_list);
fb_videomode_to_modelist((const struct fb_videomode *)buf, i,
&fb_info->modelist);
@@ -189,8 +192,8 @@ static ssize_t store_modes(struct device *device,
} else
fb_destroy_modelist(&old_list);
- console_unlock();
unlock_fb_info(fb_info);
+ console_unlock();
return 0;
}
@@ -404,12 +407,16 @@ static ssize_t store_fbstate(struct device *device,
state = simple_strtoul(buf, &last, 0);
- if (!lock_fb_info(fb_info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(fb_info)) {
+ console_unlock();
+ return -ENODEV;
+ }
+
fb_set_suspend(fb_info, (int)state);
- console_unlock();
+
unlock_fb_info(fb_info);
+ console_unlock();
return count;
}
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 0264704..45d0312 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -574,8 +574,9 @@ static int sh_mobile_lcdc_display_notify(struct sh_mobile_lcdc_chan *ch,
switch (event) {
case SH_MOBILE_LCDC_EVENT_DISPLAY_CONNECT:
/* HDMI plug in */
+ console_lock();
if (lock_fb_info(info)) {
- console_lock();
+
ch->display.width = monspec->max_x * 10;
ch->display.height = monspec->max_y * 10;
@@ -594,19 +595,20 @@ static int sh_mobile_lcdc_display_notify(struct sh_mobile_lcdc_chan *ch,
fb_set_suspend(info, 0);
}
- console_unlock();
+
unlock_fb_info(info);
}
+ console_unlock();
break;
case SH_MOBILE_LCDC_EVENT_DISPLAY_DISCONNECT:
/* HDMI disconnect */
+ console_lock();
if (lock_fb_info(info)) {
- console_lock();
fb_set_suspend(info, 1);
- console_unlock();
unlock_fb_info(info);
}
+ console_unlock();
break;
case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE:
--
1.7.7
^ permalink raw reply related
* Re: [PATCH v2] efifb: prevent null-deref when iterating dmi_list
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-31 10:45 UTC (permalink / raw)
To: David Herrmann
Cc: linux-fbdev, James Bates, linux-kernel, Tomi Valkeinen,
James Bates
In-Reply-To: <1380732056-5387-1-git-send-email-dh.herrmann@gmail.com>
On 18:40 Wed 02 Oct , David Herrmann wrote:
> The dmi_list array is initialized using gnu designated initializers, and
> therefore may contain fewer explicitly defined entries as there are
> elements in it. This is because the enum above with M_xyz constants
> contains more items than the designated initializer. Those elements not
> explicitly initialized are implicitly set to 0.
>
> Now efifb_setup() loops through all these array elements, and performs
> a strcmp on each item. For non explicitly initialized elements this will
> be a null pointer:
>
> This patch swaps the check order in the if statement, thus checks first
> whether dmi_list[i].base is null.
>
> Signed-off-by: James Bates <james.h.bates@yahoo.com>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
with the simpleDRM arriving next merge I'm wondering if we need to keep it?
Best Regaards,
J.
> ---
> Hi
>
> As James didn't respond to the last emails, I just rebased the patch and resent
> it. The efi M_xyz constants were moved to x86-sysfb so if anyone wants to remove
> unused bits, please send a separate patch to LKML and x86-ML. This patch just
> fixes the NULL-deref.
>
> Thanks
> David
>
> drivers/video/efifb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
> index 7f9ff75..fcb9500 100644
> --- a/drivers/video/efifb.c
> +++ b/drivers/video/efifb.c
> @@ -108,8 +108,8 @@ static int efifb_setup(char *options)
> if (!*this_opt) continue;
>
> for (i = 0; i < M_UNKNOWN; i++) {
> - if (!strcmp(this_opt, efifb_dmi_list[i].optname) &&
> - efifb_dmi_list[i].base != 0) {
> + if (efifb_dmi_list[i].base != 0 &&
> + !strcmp(this_opt, efifb_dmi_list[i].optname)) {
> screen_info.lfb_base = efifb_dmi_list[i].base;
> screen_info.lfb_linelength = efifb_dmi_list[i].stride;
> screen_info.lfb_width = efifb_dmi_list[i].width;
> --
> 1.8.4
>
^ permalink raw reply
* Re: [PATCH 12/12] fbdev: shmobile-lcdcfb: Convert to clk_prepare/unprepare
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-31 10:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1383000569-8916-13-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
On 23:49 Mon 28 Oct , Laurent Pinchart wrote:
> Turn clk_enable() and clk_disable() calls into clk_prepare_enable() and
> clk_disable_unprepare() to get ready for the migration to the common
> clock framework.
>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Best Regards,
J.
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> drivers/video/sh_mobile_lcdcfb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
> index 0264704..eaeae0f 100644
> --- a/drivers/video/sh_mobile_lcdcfb.c
> +++ b/drivers/video/sh_mobile_lcdcfb.c
> @@ -344,7 +344,7 @@ static void sh_mobile_lcdc_clk_on(struct sh_mobile_lcdc_priv *priv)
> {
> if (atomic_inc_and_test(&priv->hw_usecnt)) {
> if (priv->dot_clk)
> - clk_enable(priv->dot_clk);
> + clk_prepare_enable(priv->dot_clk);
> pm_runtime_get_sync(priv->dev);
> if (priv->meram_dev && priv->meram_dev->pdev)
> pm_runtime_get_sync(&priv->meram_dev->pdev->dev);
> @@ -358,7 +358,7 @@ static void sh_mobile_lcdc_clk_off(struct sh_mobile_lcdc_priv *priv)
> pm_runtime_put_sync(&priv->meram_dev->pdev->dev);
> pm_runtime_put(priv->dev);
> if (priv->dot_clk)
> - clk_disable(priv->dot_clk);
> + clk_disable_unprepare(priv->dot_clk);
> }
> }
>
> --
> 1.8.1.5
>
^ permalink raw reply
* Re: [PATCH 11/12] fbdev: shmobile-hdmi: Convert to clk_prepare/unprepare
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-31 10:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1383000569-8916-12-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
On 23:49 Mon 28 Oct , Laurent Pinchart wrote:
> Turn clk_enable() and clk_disable() calls into clk_prepare_enable() and
> clk_disable_unprepare() to get ready for the migration to the common
> clock framework.
>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Bset Regards,
J.
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> drivers/video/sh_mobile_hdmi.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> index bfe4728..190145e 100644
> --- a/drivers/video/sh_mobile_hdmi.c
> +++ b/drivers/video/sh_mobile_hdmi.c
> @@ -1326,7 +1326,7 @@ static int __init sh_hdmi_probe(struct platform_device *pdev)
> goto erate;
> }
>
> - ret = clk_enable(hdmi->hdmi_clk);
> + ret = clk_prepare_enable(hdmi->hdmi_clk);
> if (ret < 0) {
> dev_err(hdmi->dev, "Cannot enable clock: %d\n", ret);
> goto erate;
> @@ -1404,7 +1404,7 @@ emap_htop1:
> emap:
> release_mem_region(res->start, resource_size(res));
> ereqreg:
> - clk_disable(hdmi->hdmi_clk);
> + clk_disable_unprepare(hdmi->hdmi_clk);
> erate:
> clk_put(hdmi->hdmi_clk);
> egetclk:
> @@ -1427,7 +1427,7 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
> cancel_delayed_work_sync(&hdmi->edid_work);
> pm_runtime_put(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> - clk_disable(hdmi->hdmi_clk);
> + clk_disable_unprepare(hdmi->hdmi_clk);
> clk_put(hdmi->hdmi_clk);
> if (hdmi->htop1)
> iounmap(hdmi->htop1);
> --
> 1.8.1.5
>
^ permalink raw reply
* Re: [PATCH 19/51] DMA-API: media: dt3155v4l: replace dma_set_mask()+dma_set_coherent_mask() with new
From: Mauro Carvalho Chehab @ 2013-10-31 11:46 UTC (permalink / raw)
To: Hans Verkuil
Cc: Russell King, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Solarflare linux maintainers,
uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
Greg Kroah-Hartman, Russell King
In-Reply-To: <5249673B.5020705-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
Hi Russell,
Em Mon, 30 Sep 2013 13:57:47 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> On 09/19/2013 11:44 PM, Russell King wrote:
> > Replace the following sequence:
> >
> > dma_set_mask(dev, mask);
> > dma_set_coherent_mask(dev, mask);
> >
> > with a call to the new helper dma_set_mask_and_coherent().
> >
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
>
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Somehow, I lost your original post (I got unsubscribed on a few days
from all vger mailing lists at the end of september).
I suspect that you want to sent this via your tree, right?
If so:
Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
>
> Regards,
>
> Hans
>
> > ---
> > drivers/staging/media/dt3155v4l/dt3155v4l.c | 5 +----
> > 1 files changed, 1 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/media/dt3155v4l/dt3155v4l.c b/drivers/staging/media/dt3155v4l/dt3155v4l.c
> > index 90d6ac4..081407b 100644
> > --- a/drivers/staging/media/dt3155v4l/dt3155v4l.c
> > +++ b/drivers/staging/media/dt3155v4l/dt3155v4l.c
> > @@ -901,10 +901,7 @@ dt3155_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > int err;
> > struct dt3155_priv *pd;
> >
> > - err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
> > - if (err)
> > - return -ENODEV;
> > - err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> > + err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> > if (err)
> > return -ENODEV;
> > pd = kzalloc(sizeof(*pd), GFP_KERNEL);
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Cheers,
Mauro
^ 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