* [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() @ 2013-10-21 9:28 Guennadi Liakhovetski 2013-10-21 9:28 ` [PATCH/RFC 2/2] V4L2: em28xx: tell the ov2640 driver to balance clock enabling internally Guennadi Liakhovetski 2013-10-23 16:57 ` [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() Frank Schäfer 0 siblings, 2 replies; 5+ messages in thread From: Guennadi Liakhovetski @ 2013-10-21 9:28 UTC (permalink / raw) To: Linux Media Mailing List Cc: Frank Schäfer, Laurent Pinchart, Sylwester Nawrocki Some non soc-camera drivers, e.g. em28xx, use subdevice drivers, originally written for soc-camera, which use soc_camera_power_on() and soc_camera_power_off() helpers to implement their .s_power() methods. Those helpers in turn can enable and disable a clock, if it is supplied to them as a parameter. This works well when camera host drivers balance their calls to subdevices' .s_power() methods. However, some such drivers fail to do that, which leads to unbalanced calls to v4l2_clk_enable() / v4l2_clk_disable(), which then in turn produce kernel warnings. Such behaviour is wrong and should be fixed, however, sometimes it is difficult, because some of those drivers are rather old and use lots of subdevices, which all should be tested after such a fix. To support such drivers this patch adds a work-around, allowing host drivers or platforms to set a flag, in which case soc-camera helpers will only enable the clock, if it is disabled, and disable it only once on the first call to .s_power(0). Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> --- As promised yesterday, this is an alternative approach to fixing the em28xx problem, compile tested only. drivers/media/platform/soc_camera/soc_camera.c | 22 ++++++++++++++++------ include/media/soc_camera.h | 14 ++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c index 387a232..21136a2 100644 --- a/drivers/media/platform/soc_camera/soc_camera.c +++ b/drivers/media/platform/soc_camera/soc_camera.c @@ -71,11 +71,21 @@ static int video_dev_create(struct soc_camera_device *icd); int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc *ssdd, struct v4l2_clk *clk) { - int ret = clk ? v4l2_clk_enable(clk) : 0; - if (ret < 0) { - dev_err(dev, "Cannot enable clock: %d\n", ret); - return ret; + int ret; + bool clock_toggle; + + if (clk && (!ssdd->unbalanced_power || + !test_and_set_bit(0, &ssdd->clock_state))) { + ret = v4l2_clk_enable(clk); + if (ret < 0) { + dev_err(dev, "Cannot enable clock: %d\n", ret); + return ret; + } + clock_toggle = true; + } else { + clock_toggle = false; } + ret = regulator_bulk_enable(ssdd->num_regulators, ssdd->regulators); if (ret < 0) { @@ -98,7 +108,7 @@ epwron: regulator_bulk_disable(ssdd->num_regulators, ssdd->regulators); eregenable: - if (clk) + if (clock_toggle) v4l2_clk_disable(clk); return ret; @@ -127,7 +137,7 @@ int soc_camera_power_off(struct device *dev, struct soc_camera_subdev_desc *ssdd ret = ret ? : err; } - if (clk) + if (clk && (!ssdd->unbalanced_power || test_and_clear_bit(0, &ssdd->clock_state))) v4l2_clk_disable(clk); return ret; diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h index 34d2414..5678a39 100644 --- a/include/media/soc_camera.h +++ b/include/media/soc_camera.h @@ -150,6 +150,15 @@ struct soc_camera_subdev_desc { struct regulator_bulk_data *regulators; int num_regulators; + /* + * Set unbalanced_power to true to deal with legacy drivers, failing to + * balance their calls to subdevice's .s_power() method. clock_state is + * then used internally by helper functions, it shouldn't be touched by + * drivers or the platform code. + */ + bool unbalanced_power; + unsigned long clock_state; + /* Optional callbacks to power on or off and reset the sensor */ int (*power)(struct device *, int); int (*reset)(struct device *); @@ -206,6 +215,11 @@ struct soc_camera_link { struct regulator_bulk_data *regulators; int num_regulators; + /* Set by platforms to handle misbehaving drivers */ + bool unbalanced_power; + /* Used by soc-camera helper functions */ + unsigned long clock_state; + /* Optional callbacks to power on or off and reset the sensor */ int (*power)(struct device *, int); int (*reset)(struct device *); -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH/RFC 2/2] V4L2: em28xx: tell the ov2640 driver to balance clock enabling internally 2013-10-21 9:28 [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() Guennadi Liakhovetski @ 2013-10-21 9:28 ` Guennadi Liakhovetski 2013-10-23 16:57 ` [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() Frank Schäfer 1 sibling, 0 replies; 5+ messages in thread From: Guennadi Liakhovetski @ 2013-10-21 9:28 UTC (permalink / raw) To: Linux Media Mailing List Cc: Frank Schäfer, Laurent Pinchart, Sylwester Nawrocki The em28xx driver only calls subdevices' .s_power() method to power them down, relying on the hardware to wake up automatically, which is usually the case with tuners. This was acceptable with the old .standby() method, but is wrong with .s_power(). Fixing the driver would be difficult due to a broad supported hardware base. Instead this patch makes use of the unbalanced_power soc-camera subdevice flag to tell the ov2640 driver to balance calls to v4l2_clk_enable() and v4l2_clk_disable() internally. Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> --- drivers/media/usb/em28xx/em28xx-camera.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c index 73cc50a..f30043e 100644 --- a/drivers/media/usb/em28xx/em28xx-camera.c +++ b/drivers/media/usb/em28xx/em28xx-camera.c @@ -47,6 +47,7 @@ static struct soc_camera_link camlink = { .bus_id = 0, .flags = 0, .module_name = "em28xx", + .unbalanced_power = true, }; -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() 2013-10-21 9:28 [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() Guennadi Liakhovetski 2013-10-21 9:28 ` [PATCH/RFC 2/2] V4L2: em28xx: tell the ov2640 driver to balance clock enabling internally Guennadi Liakhovetski @ 2013-10-23 16:57 ` Frank Schäfer 2013-10-23 17:26 ` Guennadi Liakhovetski 1 sibling, 1 reply; 5+ messages in thread From: Frank Schäfer @ 2013-10-23 16:57 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Linux Media Mailing List, Laurent Pinchart, Sylwester Nawrocki Hi Guennadi, thanks for the patches and sorry for the delay. I've tested them a few minutes ago and they are working fine. 2 minor things/questions: 1.) Why not always balance v4l2_clk_enable/disable() calls ? 2.) For someone who reads the code it likely looks a bit odd that if the flag "unbalanced_power" is set, soc_camera balances v4l2_clk_enable/disable but not power on/off calls (ssdd->power() calls). So maybe "balance_clk" or "clk_balancing_needed" would be a better name for the flag ? Ok, let's summarize what we need: - the 3 fake clock patches patches - em28xx patch "make sure that all subdevices are powered on when needed" - these 2 patches All patches need to marked for stable (3.11). Can you pick up my me28xx patch and send Mauro a pull request ? Regards, Frank Am 21.10.2013 11:28, schrieb Guennadi Liakhovetski: > Some non soc-camera drivers, e.g. em28xx, use subdevice drivers, originally > written for soc-camera, which use soc_camera_power_on() and > soc_camera_power_off() helpers to implement their .s_power() methods. Those > helpers in turn can enable and disable a clock, if it is supplied to them > as a parameter. This works well when camera host drivers balance their > calls to subdevices' .s_power() methods. However, some such drivers fail to > do that, which leads to unbalanced calls to v4l2_clk_enable() / > v4l2_clk_disable(), which then in turn produce kernel warnings. Such > behaviour is wrong and should be fixed, however, sometimes it is difficult, > because some of those drivers are rather old and use lots of subdevices, > which all should be tested after such a fix. To support such drivers this > patch adds a work-around, allowing host drivers or platforms to set a flag, > in which case soc-camera helpers will only enable the clock, if it is > disabled, and disable it only once on the first call to .s_power(0). > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > --- > > As promised yesterday, this is an alternative approach to fixing the > em28xx problem, compile tested only. > > drivers/media/platform/soc_camera/soc_camera.c | 22 ++++++++++++++++------ > include/media/soc_camera.h | 14 ++++++++++++++ > 2 files changed, 30 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c > index 387a232..21136a2 100644 > --- a/drivers/media/platform/soc_camera/soc_camera.c > +++ b/drivers/media/platform/soc_camera/soc_camera.c > @@ -71,11 +71,21 @@ static int video_dev_create(struct soc_camera_device *icd); > int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc *ssdd, > struct v4l2_clk *clk) > { > - int ret = clk ? v4l2_clk_enable(clk) : 0; > - if (ret < 0) { > - dev_err(dev, "Cannot enable clock: %d\n", ret); > - return ret; > + int ret; > + bool clock_toggle; > + > + if (clk && (!ssdd->unbalanced_power || > + !test_and_set_bit(0, &ssdd->clock_state))) { > + ret = v4l2_clk_enable(clk); > + if (ret < 0) { > + dev_err(dev, "Cannot enable clock: %d\n", ret); > + return ret; > + } > + clock_toggle = true; > + } else { > + clock_toggle = false; > } > + > ret = regulator_bulk_enable(ssdd->num_regulators, > ssdd->regulators); > if (ret < 0) { > @@ -98,7 +108,7 @@ epwron: > regulator_bulk_disable(ssdd->num_regulators, > ssdd->regulators); > eregenable: > - if (clk) > + if (clock_toggle) > v4l2_clk_disable(clk); > > return ret; > @@ -127,7 +137,7 @@ int soc_camera_power_off(struct device *dev, struct soc_camera_subdev_desc *ssdd > ret = ret ? : err; > } > > - if (clk) > + if (clk && (!ssdd->unbalanced_power || test_and_clear_bit(0, &ssdd->clock_state))) > v4l2_clk_disable(clk); > > return ret; > diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h > index 34d2414..5678a39 100644 > --- a/include/media/soc_camera.h > +++ b/include/media/soc_camera.h > @@ -150,6 +150,15 @@ struct soc_camera_subdev_desc { > struct regulator_bulk_data *regulators; > int num_regulators; > > + /* > + * Set unbalanced_power to true to deal with legacy drivers, failing to > + * balance their calls to subdevice's .s_power() method. clock_state is > + * then used internally by helper functions, it shouldn't be touched by > + * drivers or the platform code. > + */ > + bool unbalanced_power; > + unsigned long clock_state; > + > /* Optional callbacks to power on or off and reset the sensor */ > int (*power)(struct device *, int); > int (*reset)(struct device *); > @@ -206,6 +215,11 @@ struct soc_camera_link { > struct regulator_bulk_data *regulators; > int num_regulators; > > + /* Set by platforms to handle misbehaving drivers */ > + bool unbalanced_power; > + /* Used by soc-camera helper functions */ > + unsigned long clock_state; > + > /* Optional callbacks to power on or off and reset the sensor */ > int (*power)(struct device *, int); > int (*reset)(struct device *); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() 2013-10-23 16:57 ` [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() Frank Schäfer @ 2013-10-23 17:26 ` Guennadi Liakhovetski 2013-10-23 18:13 ` Frank Schäfer 0 siblings, 1 reply; 5+ messages in thread From: Guennadi Liakhovetski @ 2013-10-23 17:26 UTC (permalink / raw) To: Frank Schäfer Cc: Linux Media Mailing List, Laurent Pinchart, Sylwester Nawrocki Hi Frank On Wed, 23 Oct 2013, Frank Schäfer wrote: > Hi Guennadi, > > thanks for the patches and sorry for the delay. > I've tested them a few minutes ago and they are working fine. > > 2 minor things/questions: > > 1.) Why not always balance v4l2_clk_enable/disable() calls ? Because I consider this a work-around for legacy / buggy drivers, that we cannot fix properly. New drivers should balance power-on / off calls. > 2.) For someone who reads the code it likely looks a bit odd that if the > flag "unbalanced_power" is set, soc_camera balances > v4l2_clk_enable/disable but not power on/off calls (ssdd->power() calls). > So maybe "balance_clk" or "clk_balancing_needed" would be a better name > for the flag ? Well, the name makes sense to me - the user doesn't balnce calls to power enable / disable methods, so, we have to balance clock enable / disable ourselves. The name might not look very logical in soc-camera, but it's a flag for the user, and the user doesn't know about clocks. It just tells the subdevice - sorry, I won't be balancing power on / off calls. > Ok, let's summarize what we need: > - the 3 fake clock patches patches > - em28xx patch "make sure that all subdevices are powered on when needed" > - these 2 patches In fact I don't even think the second entry - your em28xx patch - is needed strictly speaking. It might be correct and good, , but this specific issue should be fixed also without it. Could you confirm? > All patches need to marked for stable (3.11). > Can you pick up my me28xx patch and send Mauro a pull request ? I'd first like to get a confirmation from Mauro (at least), that this approach is acceptable for him. Thanks Guennadi > > Regards, > Frank > > Am 21.10.2013 11:28, schrieb Guennadi Liakhovetski: > > Some non soc-camera drivers, e.g. em28xx, use subdevice drivers, originally > > written for soc-camera, which use soc_camera_power_on() and > > soc_camera_power_off() helpers to implement their .s_power() methods. Those > > helpers in turn can enable and disable a clock, if it is supplied to them > > as a parameter. This works well when camera host drivers balance their > > calls to subdevices' .s_power() methods. However, some such drivers fail to > > do that, which leads to unbalanced calls to v4l2_clk_enable() / > > v4l2_clk_disable(), which then in turn produce kernel warnings. Such > > behaviour is wrong and should be fixed, however, sometimes it is difficult, > > because some of those drivers are rather old and use lots of subdevices, > > which all should be tested after such a fix. To support such drivers this > > patch adds a work-around, allowing host drivers or platforms to set a flag, > > in which case soc-camera helpers will only enable the clock, if it is > > disabled, and disable it only once on the first call to .s_power(0). > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > --- > > > > As promised yesterday, this is an alternative approach to fixing the > > em28xx problem, compile tested only. > > > > drivers/media/platform/soc_camera/soc_camera.c | 22 ++++++++++++++++------ > > include/media/soc_camera.h | 14 ++++++++++++++ > > 2 files changed, 30 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c > > index 387a232..21136a2 100644 > > --- a/drivers/media/platform/soc_camera/soc_camera.c > > +++ b/drivers/media/platform/soc_camera/soc_camera.c > > @@ -71,11 +71,21 @@ static int video_dev_create(struct soc_camera_device *icd); > > int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc *ssdd, > > struct v4l2_clk *clk) > > { > > - int ret = clk ? v4l2_clk_enable(clk) : 0; > > - if (ret < 0) { > > - dev_err(dev, "Cannot enable clock: %d\n", ret); > > - return ret; > > + int ret; > > + bool clock_toggle; > > + > > + if (clk && (!ssdd->unbalanced_power || > > + !test_and_set_bit(0, &ssdd->clock_state))) { > > + ret = v4l2_clk_enable(clk); > > + if (ret < 0) { > > + dev_err(dev, "Cannot enable clock: %d\n", ret); > > + return ret; > > + } > > + clock_toggle = true; > > + } else { > > + clock_toggle = false; > > } > > + > > ret = regulator_bulk_enable(ssdd->num_regulators, > > ssdd->regulators); > > if (ret < 0) { > > @@ -98,7 +108,7 @@ epwron: > > regulator_bulk_disable(ssdd->num_regulators, > > ssdd->regulators); > > eregenable: > > - if (clk) > > + if (clock_toggle) > > v4l2_clk_disable(clk); > > > > return ret; > > @@ -127,7 +137,7 @@ int soc_camera_power_off(struct device *dev, struct soc_camera_subdev_desc *ssdd > > ret = ret ? : err; > > } > > > > - if (clk) > > + if (clk && (!ssdd->unbalanced_power || test_and_clear_bit(0, &ssdd->clock_state))) > > v4l2_clk_disable(clk); > > > > return ret; > > diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h > > index 34d2414..5678a39 100644 > > --- a/include/media/soc_camera.h > > +++ b/include/media/soc_camera.h > > @@ -150,6 +150,15 @@ struct soc_camera_subdev_desc { > > struct regulator_bulk_data *regulators; > > int num_regulators; > > > > + /* > > + * Set unbalanced_power to true to deal with legacy drivers, failing to > > + * balance their calls to subdevice's .s_power() method. clock_state is > > + * then used internally by helper functions, it shouldn't be touched by > > + * drivers or the platform code. > > + */ > > + bool unbalanced_power; > > + unsigned long clock_state; > > + > > /* Optional callbacks to power on or off and reset the sensor */ > > int (*power)(struct device *, int); > > int (*reset)(struct device *); > > @@ -206,6 +215,11 @@ struct soc_camera_link { > > struct regulator_bulk_data *regulators; > > int num_regulators; > > > > + /* Set by platforms to handle misbehaving drivers */ > > + bool unbalanced_power; > > + /* Used by soc-camera helper functions */ > > + unsigned long clock_state; > > + > > /* Optional callbacks to power on or off and reset the sensor */ > > int (*power)(struct device *, int); > > int (*reset)(struct device *); > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() 2013-10-23 17:26 ` Guennadi Liakhovetski @ 2013-10-23 18:13 ` Frank Schäfer 0 siblings, 0 replies; 5+ messages in thread From: Frank Schäfer @ 2013-10-23 18:13 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Linux Media Mailing List, Laurent Pinchart, Sylwester Nawrocki Am 23.10.2013 19:26, schrieb Guennadi Liakhovetski: > Hi Frank > > On Wed, 23 Oct 2013, Frank Schäfer wrote: > >> Hi Guennadi, >> >> thanks for the patches and sorry for the delay. >> I've tested them a few minutes ago and they are working fine. >> >> 2 minor things/questions: >> >> 1.) Why not always balance v4l2_clk_enable/disable() calls ? > Because I consider this a work-around for legacy / buggy drivers, that we > cannot fix properly. New drivers should balance power-on / off calls. > >> 2.) For someone who reads the code it likely looks a bit odd that if the >> flag "unbalanced_power" is set, soc_camera balances >> v4l2_clk_enable/disable but not power on/off calls (ssdd->power() calls). >> So maybe "balance_clk" or "clk_balancing_needed" would be a better name >> for the flag ? > Well, the name makes sense to me - the user doesn't balnce calls to power > enable / disable methods, so, we have to balance clock enable / disable > ourselves. The name might not look very logical in soc-camera, but it's a > flag for the user, and the user doesn't know about clocks. It just tells > the subdevice - sorry, I won't be balancing power on / off calls. > >> Ok, let's summarize what we need: >> - the 3 fake clock patches patches >> - em28xx patch "make sure that all subdevices are powered on when needed" >> - these 2 patches > In fact I don't even think the second entry - your em28xx patch - is > needed strictly speaking. It might be correct and good, , but this > specific issue should be fixed also without it. Could you confirm? Ah yes, right, your patch also avoids warning if the device is powered off before beeing powered on, so we don't need it for this specific issue. And yes, it should be applied to make sure the subdevices are powered on when needed (separate issue). >> All patches need to marked for stable (3.11). >> Can you pick up my me28xx patch and send Mauro a pull request ? > I'd first like to get a confirmation from Mauro (at least), that this > approach is acceptable for him. Ok. Regards, Frank > > Thanks > Guennadi > >> Regards, >> Frank >> >> Am 21.10.2013 11:28, schrieb Guennadi Liakhovetski: >>> Some non soc-camera drivers, e.g. em28xx, use subdevice drivers, originally >>> written for soc-camera, which use soc_camera_power_on() and >>> soc_camera_power_off() helpers to implement their .s_power() methods. Those >>> helpers in turn can enable and disable a clock, if it is supplied to them >>> as a parameter. This works well when camera host drivers balance their >>> calls to subdevices' .s_power() methods. However, some such drivers fail to >>> do that, which leads to unbalanced calls to v4l2_clk_enable() / >>> v4l2_clk_disable(), which then in turn produce kernel warnings. Such >>> behaviour is wrong and should be fixed, however, sometimes it is difficult, >>> because some of those drivers are rather old and use lots of subdevices, >>> which all should be tested after such a fix. To support such drivers this >>> patch adds a work-around, allowing host drivers or platforms to set a flag, >>> in which case soc-camera helpers will only enable the clock, if it is >>> disabled, and disable it only once on the first call to .s_power(0). >>> >>> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> >>> --- >>> >>> As promised yesterday, this is an alternative approach to fixing the >>> em28xx problem, compile tested only. >>> >>> drivers/media/platform/soc_camera/soc_camera.c | 22 ++++++++++++++++------ >>> include/media/soc_camera.h | 14 ++++++++++++++ >>> 2 files changed, 30 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c >>> index 387a232..21136a2 100644 >>> --- a/drivers/media/platform/soc_camera/soc_camera.c >>> +++ b/drivers/media/platform/soc_camera/soc_camera.c >>> @@ -71,11 +71,21 @@ static int video_dev_create(struct soc_camera_device *icd); >>> int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc *ssdd, >>> struct v4l2_clk *clk) >>> { >>> - int ret = clk ? v4l2_clk_enable(clk) : 0; >>> - if (ret < 0) { >>> - dev_err(dev, "Cannot enable clock: %d\n", ret); >>> - return ret; >>> + int ret; >>> + bool clock_toggle; >>> + >>> + if (clk && (!ssdd->unbalanced_power || >>> + !test_and_set_bit(0, &ssdd->clock_state))) { >>> + ret = v4l2_clk_enable(clk); >>> + if (ret < 0) { >>> + dev_err(dev, "Cannot enable clock: %d\n", ret); >>> + return ret; >>> + } >>> + clock_toggle = true; >>> + } else { >>> + clock_toggle = false; >>> } >>> + >>> ret = regulator_bulk_enable(ssdd->num_regulators, >>> ssdd->regulators); >>> if (ret < 0) { >>> @@ -98,7 +108,7 @@ epwron: >>> regulator_bulk_disable(ssdd->num_regulators, >>> ssdd->regulators); >>> eregenable: >>> - if (clk) >>> + if (clock_toggle) >>> v4l2_clk_disable(clk); >>> >>> return ret; >>> @@ -127,7 +137,7 @@ int soc_camera_power_off(struct device *dev, struct soc_camera_subdev_desc *ssdd >>> ret = ret ? : err; >>> } >>> >>> - if (clk) >>> + if (clk && (!ssdd->unbalanced_power || test_and_clear_bit(0, &ssdd->clock_state))) >>> v4l2_clk_disable(clk); >>> >>> return ret; >>> diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h >>> index 34d2414..5678a39 100644 >>> --- a/include/media/soc_camera.h >>> +++ b/include/media/soc_camera.h >>> @@ -150,6 +150,15 @@ struct soc_camera_subdev_desc { >>> struct regulator_bulk_data *regulators; >>> int num_regulators; >>> >>> + /* >>> + * Set unbalanced_power to true to deal with legacy drivers, failing to >>> + * balance their calls to subdevice's .s_power() method. clock_state is >>> + * then used internally by helper functions, it shouldn't be touched by >>> + * drivers or the platform code. >>> + */ >>> + bool unbalanced_power; >>> + unsigned long clock_state; >>> + >>> /* Optional callbacks to power on or off and reset the sensor */ >>> int (*power)(struct device *, int); >>> int (*reset)(struct device *); >>> @@ -206,6 +215,11 @@ struct soc_camera_link { >>> struct regulator_bulk_data *regulators; >>> int num_regulators; >>> >>> + /* Set by platforms to handle misbehaving drivers */ >>> + bool unbalanced_power; >>> + /* Used by soc-camera helper functions */ >>> + unsigned long clock_state; >>> + >>> /* Optional callbacks to power on or off and reset the sensor */ >>> int (*power)(struct device *, int); >>> int (*reset)(struct device *); > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-23 18:13 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-21 9:28 [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() Guennadi Liakhovetski 2013-10-21 9:28 ` [PATCH/RFC 2/2] V4L2: em28xx: tell the ov2640 driver to balance clock enabling internally Guennadi Liakhovetski 2013-10-23 16:57 ` [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() Frank Schäfer 2013-10-23 17:26 ` Guennadi Liakhovetski 2013-10-23 18:13 ` Frank Schäfer
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).