| Message ID | 52CFD1ED.7010302@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;