Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks

[Tomi Valkeinen]

On 2014-01-05 15:13, Ivaylo Dimitrov wrote:
> From: Ivaylo Dimitrov <freemangor...@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;
 

signature.asc
Description: OpenPGP digital signature