From patchwork Mon Oct 21 09:28:02 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guennadi Liakhovetski X-Patchwork-Id: 3076391 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 8E902BF924 for ; Mon, 21 Oct 2013 09:28:23 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D512E20318 for ; Mon, 21 Oct 2013 09:28:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E3BAD20360 for ; Mon, 21 Oct 2013 09:28:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754409Ab3JUJ2G (ORCPT ); Mon, 21 Oct 2013 05:28:06 -0400 Received: from moutng.kundenserver.de ([212.227.126.171]:60442 "EHLO moutng.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753434Ab3JUJ2E (ORCPT ); Mon, 21 Oct 2013 05:28:04 -0400 Received: from axis700.grange (dslb-088-076-026-183.pools.arcor-ip.net [88.76.26.183]) by mrelayeu.kundenserver.de (node=mreu3) with ESMTP (Nemesis) id 0LZYSm-1VzWm63DIq-00lUPh; Mon, 21 Oct 2013 11:28:03 +0200 Received: by axis700.grange (Postfix, from userid 1000) id 681C540BB4; Mon, 21 Oct 2013 11:28:02 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by axis700.grange (Postfix) with ESMTP id 655F140BB3; Mon, 21 Oct 2013 11:28:02 +0200 (CEST) Date: Mon, 21 Oct 2013 11:28:02 +0200 (CEST) From: Guennadi Liakhovetski X-X-Sender: lyakh@axis700.grange To: Linux Media Mailing List cc: =?ISO-8859-15?Q?Frank_Sch=E4fer?= , Laurent Pinchart , Sylwester Nawrocki Subject: [PATCH/RFC 1/2] V4L2: soc-camera: work around unbalanced calls to .s_power() Message-ID: MIME-Version: 1.0 X-Provags-ID: V02:K0:Fha1HxiM/AV/djfbGpeAjKKfN3OSedDx0wp+Qf2OKRJ vkjPLWq2fe0pPfbI7QFJAy/hzTKEVjAl9uUi25ipnJUd3loEKT 0U0E2QGVGkbGL6tG92mMoPp9E2Hj9dsLJM+XUYqdGJYPIC4V1t 1kKFS+Z8XatugUvA8Zb77ITC4GFCdU3VvNBrp+5Bb3dxvAhs/D dAs5spTfjt1tCnOjJdwG1Z3sEFOXTyqqbClF6/4PdF+7nvPg1D dF9QDn13Ysn1lstZJoY0FfTHUkJ/1Q4Ocpqq0ZFhZDeqUqJqWf kI3uijikoBcgxXAbb8mHnTfB2s2eb6Ar3AytAqmlFDuE8w8PI4 ZJ89Xvd2LoXH0gh8GLMHfc5va5xyd5UpYtP/Rffg6qkyAXbQVf MP7YHVgl8d/jA== Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 --- 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 *);