* [PATCH] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
@ 2013-12-30 16:17 Ivaylo Dimitrov
2014-01-04 12:51 ` Pavel Machek
0 siblings, 1 reply; 10+ messages in thread
From: Ivaylo Dimitrov @ 2013-12-30 16:17 UTC (permalink / raw)
To: tomi.valkeinen
Cc: plagnioj, pali.rohar, pavel, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov
From: Ivaylo Dimitrov <freemangordon@abv.bg>
Commit c37dd677988ca50bc8bc60ab5ab053720583c168 fixes the unbalanced
unlock in acx565akm_enable but introduces another problem - if
acx565akm_panel_power_on exits early, the mutex is not unlocked. Fix
that by unlocking the mutex on early return. Also add mutex protection in
acx565akm_panel_power_off and remove an unused variable
Signed-off-by: Ivaylo Dimitrov <freemangordon@abv.bg>
---
.../omap2/displays-new/panel-sony-acx565akm.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
index d94f35d..a093d3a 100644
--- a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
+++ b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
@@ -535,6 +535,7 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
r = in->ops.sdi->enable(in);
if (r) {
+ mutex_unlock(&ddata->mutex);
pr_err("%s sdi enable failed\n", __func__);
return r;
}
@@ -546,6 +547,7 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
gpio_set_value(ddata->reset_gpio, 1);
if (ddata->enabled) {
+ mutex_unlock(&ddata->mutex);
dev_dbg(&ddata->spi->dev, "panel already enabled\n");
return 0;
}
@@ -578,10 +580,14 @@ static void acx565akm_panel_power_off(struct omap_dss_device *dssdev)
struct panel_drv_data *ddata = to_panel_data(dssdev);
struct omap_dss_device *in = ddata->in;
+ mutex_lock(&ddata->mutex);
+
dev_dbg(dssdev->dev, "%s\n", __func__);
- if (!ddata->enabled)
+ if (!ddata->enabled) {
+ mutex_unlock(&ddata->mutex);
return;
+ }
set_display_state(ddata, 0);
set_sleep_mode(ddata, 1);
@@ -601,11 +607,13 @@ static void acx565akm_panel_power_off(struct omap_dss_device *dssdev)
msleep(100);
in->ops.sdi->disable(in);
+
+ mutex_unlock(&ddata->mutex);
+
}
static int acx565akm_enable(struct omap_dss_device *dssdev)
{
- struct panel_drv_data *ddata = to_panel_data(dssdev);
int r;
dev_dbg(dssdev->dev, "%s\n", __func__);
--
1.5.6.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
2013-12-30 16:17 [PATCH] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks Ivaylo Dimitrov
@ 2014-01-04 12:51 ` Pavel Machek
2014-01-05 12:58 ` Ivaylo Dimitrov
0 siblings, 1 reply; 10+ messages in thread
From: Pavel Machek @ 2014-01-04 12:51 UTC (permalink / raw)
To: Ivaylo Dimitrov
Cc: tomi.valkeinen, plagnioj, pali.rohar, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov
On Mon 2013-12-30 18:17:52, Ivaylo Dimitrov wrote:
> From: Ivaylo Dimitrov <freemangordon@abv.bg>
>
> Commit c37dd677988ca50bc8bc60ab5ab053720583c168 fixes the unbalanced
> unlock in acx565akm_enable but introduces another problem - if
> acx565akm_panel_power_on exits early, the mutex is not unlocked. Fix
> that by unlocking the mutex on early return. Also add mutex protection in
> acx565akm_panel_power_off and remove an unused variable
>
> Signed-off-by: Ivaylo Dimitrov <freemangordon@abv.bg>
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
2014-01-04 12:51 ` Pavel Machek
@ 2014-01-05 12:58 ` Ivaylo Dimitrov
2014-01-05 13:13 ` [PATCH v2] " Ivaylo Dimitrov
0 siblings, 1 reply; 10+ messages in thread
From: Ivaylo Dimitrov @ 2014-01-05 12:58 UTC (permalink / raw)
To: Pavel Machek, Ivaylo Dimitrov
Cc: tomi.valkeinen, plagnioj, pali.rohar, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov
On 04.01.2014 14:51, Pavel Machek wrote:
> On Mon 2013-12-30 18:17:52, Ivaylo Dimitrov wrote:
>> From: Ivaylo Dimitrov <freemangordon@abv.bg>
>>
>> Commit c37dd677988ca50bc8bc60ab5ab053720583c168 fixes the unbalanced
>> unlock in acx565akm_enable but introduces another problem - if
>> acx565akm_panel_power_on exits early, the mutex is not unlocked. Fix
>> that by unlocking the mutex on early return. Also add mutex protection in
>> acx565akm_panel_power_off and remove an unused variable
>>
>> Signed-off-by: Ivaylo Dimitrov <freemangordon@abv.bg>
> Reviewed-by: Pavel Machek <pavel@ucw.cz>
Hmm, I introduced a bug with that patch (recursive lock), will send a
new version that fixes it
Regards,
Ivo
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
2014-01-05 12:58 ` Ivaylo Dimitrov
@ 2014-01-05 13:13 ` Ivaylo Dimitrov
2014-01-10 10:56 ` Tomi Valkeinen
0 siblings, 1 reply; 10+ messages in thread
From: Ivaylo Dimitrov @ 2014-01-05 13:13 UTC (permalink / raw)
To: tomi.valkeinen
Cc: plagnioj, pali.rohar, pavel, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov
From: Ivaylo Dimitrov <freemangordon@abv.bg>
Commit c37dd677988ca50bc8bc60ab5ab053720583c168 fixes the unbalanced
unlock in acx565akm_enable but introduces another problem - if
acx565akm_panel_power_on exits early, the mutex is not unlocked. Fix
that by unlocking the mutex on early return. Also add mutex protection in
acx565akm_panel_power_off and remove an unused variable
Signed-off-by: Ivaylo Dimitrov <freemangordon@abv.bg>
---
.../omap2/displays-new/panel-sony-acx565akm.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
index d94f35d..9aef7fa 100644
--- a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
+++ b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
@@ -535,6 +535,7 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
r = in->ops.sdi->enable(in);
if (r) {
+ mutex_unlock(&ddata->mutex);
pr_err("%s sdi enable failed\n", __func__);
return r;
}
@@ -546,6 +547,7 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
gpio_set_value(ddata->reset_gpio, 1);
if (ddata->enabled) {
+ mutex_unlock(&ddata->mutex);
dev_dbg(&ddata->spi->dev, "panel already enabled\n");
return 0;
}
@@ -578,10 +580,14 @@ static void acx565akm_panel_power_off(struct omap_dss_device *dssdev)
struct panel_drv_data *ddata = to_panel_data(dssdev);
struct omap_dss_device *in = ddata->in;
+ mutex_lock(&ddata->mutex);
+
dev_dbg(dssdev->dev, "%s\n", __func__);
- if (!ddata->enabled)
+ if (!ddata->enabled) {
+ mutex_unlock(&ddata->mutex);
return;
+ }
set_display_state(ddata, 0);
set_sleep_mode(ddata, 1);
@@ -601,11 +607,13 @@ static void acx565akm_panel_power_off(struct omap_dss_device *dssdev)
msleep(100);
in->ops.sdi->disable(in);
+
+ mutex_unlock(&ddata->mutex);
+
}
static int acx565akm_enable(struct omap_dss_device *dssdev)
{
- struct panel_drv_data *ddata = to_panel_data(dssdev);
int r;
dev_dbg(dssdev->dev, "%s\n", __func__);
@@ -627,16 +635,12 @@ static int acx565akm_enable(struct omap_dss_device *dssdev)
static void acx565akm_disable(struct omap_dss_device *dssdev)
{
- struct panel_drv_data *ddata = to_panel_data(dssdev);
-
dev_dbg(dssdev->dev, "%s\n", __func__);
if (!omapdss_device_is_enabled(dssdev))
return;
- mutex_lock(&ddata->mutex);
acx565akm_panel_power_off(dssdev);
- mutex_unlock(&ddata->mutex);
dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
}
--
1.5.6.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
2014-01-05 13:13 ` [PATCH v2] " Ivaylo Dimitrov
@ 2014-01-10 10:56 ` Tomi Valkeinen
2014-01-11 9:39 ` Ivaylo Dimitrov
0 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2014-01-10 10:56 UTC (permalink / raw)
To: Ivaylo Dimitrov
Cc: plagnioj, pali.rohar, pavel, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov, Aaro Koskinen
[-- Attachment #1: Type: text/plain, Size: 3767 bytes --]
On 2014-01-05 15:13, Ivaylo Dimitrov wrote:
> From: Ivaylo Dimitrov <freemangordon@abv.bg>
>
> Commit c37dd677988ca50bc8bc60ab5ab053720583c168 fixes the unbalanced
> unlock in acx565akm_enable but introduces another problem - if
> acx565akm_panel_power_on exits early, the mutex is not unlocked. Fix
> that by unlocking the mutex on early return. Also add mutex protection in
> acx565akm_panel_power_off and remove an unused variable
>
I think this is just getting more messy. How about we more or less revert the c37dd677988ca50bc8bc60ab5ab053720583c168 and fix it like this:
diff --git a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
index 714ee92dfb9f..8e97d06921ff 100644
--- a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
+++ b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
@@ -346,28 +346,22 @@ static int acx565akm_get_actual_brightness(struct panel_drv_data *ddata)
static int acx565akm_bl_update_status(struct backlight_device *dev)
{
struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev);
- int r;
int level;
dev_dbg(&ddata->spi->dev, "%s\n", __func__);
- mutex_lock(&ddata->mutex);
-
if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
dev->props.power == FB_BLANK_UNBLANK)
level = dev->props.brightness;
else
level = 0;
- r = 0;
if (ddata->has_bc)
acx565akm_set_brightness(ddata, level);
else
- r = -ENODEV;
-
- mutex_unlock(&ddata->mutex);
+ return -ENODEV;
- return r;
+ return 0;
}
static int acx565akm_bl_get_intensity(struct backlight_device *dev)
@@ -390,9 +384,33 @@ static int acx565akm_bl_get_intensity(struct backlight_device *dev)
return 0;
}
+static int acx565akm_bl_update_status_locked(struct backlight_device *dev)
+{
+ struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev);
+ int r;
+
+ mutex_lock(&ddata->mutex);
+ r = acx565akm_bl_update_status(dev);
+ mutex_unlock(&ddata->mutex);
+
+ return r;
+}
+
+static int acx565akm_bl_get_intensity_locked(struct backlight_device *dev)
+{
+ struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev);
+ int r;
+
+ mutex_lock(&ddata->mutex);
+ r = acx565akm_bl_get_intensity(dev);
+ mutex_unlock(&ddata->mutex);
+
+ return r;
+}
+
static const struct backlight_ops acx565akm_bl_ops = {
- .get_brightness = acx565akm_bl_get_intensity,
- .update_status = acx565akm_bl_update_status,
+ .get_brightness = acx565akm_bl_get_intensity_locked,
+ .update_status = acx565akm_bl_update_status_locked,
};
/*--------------------Auto Brightness control via Sysfs---------------------*/
@@ -526,8 +544,6 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
struct omap_dss_device *in = ddata->in;
int r;
- mutex_lock(&ddata->mutex);
-
dev_dbg(&ddata->spi->dev, "%s\n", __func__);
in->ops.sdi->set_timings(in, &ddata->videomode);
@@ -568,8 +584,6 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
set_display_state(ddata, 1);
set_cabc_mode(ddata, ddata->cabc_mode);
- mutex_unlock(&ddata->mutex);
-
return acx565akm_bl_update_status(ddata->bl_dev);
}
@@ -605,6 +619,7 @@ static void acx565akm_panel_power_off(struct omap_dss_device *dssdev)
static int acx565akm_enable(struct omap_dss_device *dssdev)
{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
int r;
dev_dbg(dssdev->dev, "%s\n", __func__);
@@ -615,7 +630,9 @@ static int acx565akm_enable(struct omap_dss_device *dssdev)
if (omapdss_device_is_enabled(dssdev))
return 0;
+ mutex_lock(&ddata->mutex);
r = acx565akm_panel_power_on(dssdev);
+ mutex_unlock(&ddata->mutex);
if (r)
return r;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
2014-01-10 10:56 ` Tomi Valkeinen
@ 2014-01-11 9:39 ` Ivaylo Dimitrov
2014-01-13 10:20 ` Tomi Valkeinen
0 siblings, 1 reply; 10+ messages in thread
From: Ivaylo Dimitrov @ 2014-01-11 9:39 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: plagnioj, pali.rohar, pavel, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov, Aaro Koskinen
On 10.01.2014 12:56, Tomi Valkeinen wrote:
> On 2014-01-05 15:13, Ivaylo Dimitrov wrote:
>> From: Ivaylo Dimitrov <freemangordon@abv.bg>
>>
>> Commit c37dd677988ca50bc8bc60ab5ab053720583c168 fixes the unbalanced
>> unlock in acx565akm_enable but introduces another problem - if
>> acx565akm_panel_power_on exits early, the mutex is not unlocked. Fix
>> that by unlocking the mutex on early return. Also add mutex protection in
>> acx565akm_panel_power_off and remove an unused variable
>>
>
> I think this is just getting more messy. How about we more or less revert the c37dd677988ca50bc8bc60ab5ab053720583c168 and fix it like this:
>
I am fine with whatever patch you may come with, as long as it fixes the
issue.
>
> diff --git a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
> index 714ee92dfb9f..8e97d06921ff 100644
> --- a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
> +++ b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
The patch does not apply cleanly on top of rc7, however I applied it by
hand. So far it seems it fixes the issue brought by
c37dd677988ca50bc8bc60ab5ab053720583c168, though I didn't test if
mutex_lock/mutex_unlock are complementary in every code path (at least
not explicitly, I guess maemo is doing it for us anyway :) ).
So, shall I send a patch incorporating your code changes, or you will do it?
Regards,
Ivo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
2014-01-11 9:39 ` Ivaylo Dimitrov
@ 2014-01-13 10:20 ` Tomi Valkeinen
2015-12-25 13:29 ` Ivaylo Dimitrov
0 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2014-01-13 10:20 UTC (permalink / raw)
To: Ivaylo Dimitrov
Cc: plagnioj, pali.rohar, pavel, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov, Aaro Koskinen
[-- Attachment #1: Type: text/plain, Size: 518 bytes --]
On 2014-01-11 11:39, Ivaylo Dimitrov wrote:
> The patch does not apply cleanly on top of rc7, however I applied it by
> hand. So far it seems it fixes the issue brought by
> c37dd677988ca50bc8bc60ab5ab053720583c168, though I didn't test if
> mutex_lock/mutex_unlock are complementary in every code path (at least
> not explicitly, I guess maemo is doing it for us anyway :) ).
Ok, thanks.
> So, shall I send a patch incorporating your code changes, or you will do
> it?
I can handle it.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
2014-01-13 10:20 ` Tomi Valkeinen
@ 2015-12-25 13:29 ` Ivaylo Dimitrov
2015-12-29 7:46 ` Tomi Valkeinen
0 siblings, 1 reply; 10+ messages in thread
From: Ivaylo Dimitrov @ 2015-12-25 13:29 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: plagnioj, pali.rohar, pavel, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov, Aaro Koskinen
Hi Tomi,
On 13.01.2014 12:20, Tomi Valkeinen wrote:
> On 2014-01-11 11:39, Ivaylo Dimitrov wrote:
>
>> The patch does not apply cleanly on top of rc7, however I applied it by
>> hand. So far it seems it fixes the issue brought by
>> c37dd677988ca50bc8bc60ab5ab053720583c168, though I didn't test if
>> mutex_lock/mutex_unlock are complementary in every code path (at least
>> not explicitly, I guess maemo is doing it for us anyway :) ).
>
> Ok, thanks.
>
>> So, shall I send a patch incorporating your code changes, or you will do
>> it?
>
> I can handle it.
>
> Tomi
>
>
I still don't see those fixes in mainline, shall I send a patch?
Ivo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
2015-12-25 13:29 ` Ivaylo Dimitrov
@ 2015-12-29 7:46 ` Tomi Valkeinen
2016-01-01 12:25 ` Ivaylo Dimitrov
0 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2015-12-29 7:46 UTC (permalink / raw)
To: Ivaylo Dimitrov
Cc: plagnioj, pali.rohar, pavel, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov, Aaro Koskinen
[-- Attachment #1: Type: text/plain, Size: 833 bytes --]
On 25/12/15 15:29, Ivaylo Dimitrov wrote:
>
> Hi Tomi,
>
> On 13.01.2014 12:20, Tomi Valkeinen wrote:
>> On 2014-01-11 11:39, Ivaylo Dimitrov wrote:
>>
>>> The patch does not apply cleanly on top of rc7, however I applied it by
>>> hand. So far it seems it fixes the issue brought by
>>> c37dd677988ca50bc8bc60ab5ab053720583c168, though I didn't test if
>>> mutex_lock/mutex_unlock are complementary in every code path (at least
>>> not explicitly, I guess maemo is doing it for us anyway :) ).
>>
>> Ok, thanks.
>>
>>> So, shall I send a patch incorporating your code changes, or you will do
>>> it?
>>
>> I can handle it.
>>
>> Tomi
>>
>>
>
> I still don't see those fixes in mainline, shall I send a patch?
Oh, I'm sorry, I must have forgotten about that. Please, send a new patch.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
2015-12-29 7:46 ` Tomi Valkeinen
@ 2016-01-01 12:25 ` Ivaylo Dimitrov
0 siblings, 0 replies; 10+ messages in thread
From: Ivaylo Dimitrov @ 2016-01-01 12:25 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: plagnioj, pali.rohar, pavel, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov, Aaro Koskinen
On 29.12.2015 09:46, Tomi Valkeinen wrote:
>
> Oh, I'm sorry, I must have forgotten about that. Please, send a new patch.
>
> Tomi
>
Actually it is me to be sorry for making noise, I've missed
0eb0dafb674cd6bfac2e3204b2f8b907e26b1138 with all those patches moving
files around.
Ivo
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-01-01 12:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-30 16:17 [PATCH] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks Ivaylo Dimitrov
2014-01-04 12:51 ` Pavel Machek
2014-01-05 12:58 ` Ivaylo Dimitrov
2014-01-05 13:13 ` [PATCH v2] " Ivaylo Dimitrov
2014-01-10 10:56 ` Tomi Valkeinen
2014-01-11 9:39 ` Ivaylo Dimitrov
2014-01-13 10:20 ` Tomi Valkeinen
2015-12-25 13:29 ` Ivaylo Dimitrov
2015-12-29 7:46 ` Tomi Valkeinen
2016-01-01 12:25 ` Ivaylo Dimitrov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).