Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
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 signature.asc Description: OpenPGP digital signature
Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
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 signature.asc Description: OpenPGP digital signature
Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
On 10.01.2014 12:56, Tomi Valkeinen wrote: On 2014-01-05 15:13, Ivaylo Dimitrov wrote: From: Ivaylo Dimitrov 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
On 2014-01-05 15:13, Ivaylo Dimitrov wrote:
> From: Ivaylo Dimitrov
>
> 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;
signature.asc
Description: OpenPGP digital signature
[PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
From: Ivaylo Dimitrov
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
---
.../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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
