From patchwork Tue Apr 12 07:57:35 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 609280 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3qkfRt2f4Wz9s2Q for ; Tue, 12 Apr 2016 17:57:46 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b=Nw7gTxNZ; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932222AbcDLH5o (ORCPT ); Tue, 12 Apr 2016 03:57:44 -0400 Received: from mail-lf0-f42.google.com ([209.85.215.42]:33891 "EHLO mail-lf0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932217AbcDLH5n (ORCPT ); Tue, 12 Apr 2016 03:57:43 -0400 Received: by mail-lf0-f42.google.com with SMTP id j11so13719869lfb.1 for ; Tue, 12 Apr 2016 00:57:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=SkthwS9a+5rNLnVeKee5nJDmBxMrK3t1VCUwZetImhU=; b=Nw7gTxNZl4OtPTmIZnU8pMAx0DmqI6Wf4WPVALO/xreauR9x6FYrnQ9FL3i4WOZD+a N1tnq8EJLDeMjHLw+pQKUJQJ2kEMk7mRN7PcVnlUpdcIvaBJTQIfvzVwEeqLMEBBqOQ2 js9dmewXoSXMj+cwME9zv7am9T86WVAVzqN6k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=SkthwS9a+5rNLnVeKee5nJDmBxMrK3t1VCUwZetImhU=; b=BzaIEexuH9d9nrBiAYGZfnAWdZX9NvCbXsvydw7av5AmCtjZ68MiYvDLU++6ZZDrd5 /KZpmoy7Qz389YDavaFxKCf33n/dnL1QsLXdV3VDTtrTbzmg0YOZ1Ctf03fbMLPAxY15 MhZaVZXAeRP9uNEZ3nntIMOuW6tD0//B7Unww4I4qNL9kx32MEF7N86lo8vkF6zn+DW9 zjvaovbN1uvusmEQLayIyv8qSSWL1VC+38ma++oOuw2fuqFPMf/WzfvyfcA/uA8B7X4g Q8AAcDRA6rvo3Ttr5ZSFH+pGMOdd1b3SaPhpYLlwPsM8Te3Cz4wHEyuT8nh9+BBn2vQu ATuQ== X-Gm-Message-State: AOPr4FWALvTbJp+bz1qaHEiPW9S6HAGMKPUx+e6gvb2P8G5L/CYiIFa2AnyI3O7lf+R3OoDS X-Received: by 10.25.19.99 with SMTP id j96mr835740lfi.114.1460447861815; Tue, 12 Apr 2016 00:57:41 -0700 (PDT) Received: from localhost.localdomain ([85.235.10.227]) by smtp.gmail.com with ESMTPSA id wy8sm5122189lbb.23.2016.04.12.00.57.40 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 Apr 2016 00:57:40 -0700 (PDT) From: Linus Walleij To: Wolfram Sang , linux-i2c@vger.kernel.org Cc: Linus Walleij , Mark Brown , Ulf Hansson , "Rafael J. Wysocki" , linux-pm@vger.kernel.org Subject: [PATCH v3] i2c: let I2C masters ignore their children for PM Date: Tue, 12 Apr 2016 09:57:35 +0200 Message-Id: <1460447855-18050-1-git-send-email-linus.walleij@linaro.org> X-Mailer: git-send-email 2.4.3 Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org When using a certain I2C device with runtime PM enabled on a certain I2C bus adaper the following happens: struct amba_device *foo \ struct i2c_adapter *bar \ struct i2c_client *baz The AMBA device foo has its device PM struct set to ignore children with pm_suspend_ignore_children(&foo->dev, true). This makes runtime PM work just fine locally in the driver: the fact that devices on the bus are suspended or resumed individually does not affect its operation, and the hardware does not power up unless transferring messages. However this child ignorance property is not inherited into the struct i2c_adapter *bar. On system suspend things will work fine. On system resume the following annoying phenomenon occurs: - In the pm_runtime_force_resume() path of struct i2c_client *baz, pm_runtime_set_active(&baz->dev); is eventually called. - This becomes __pm_runtime_set_status(&baz->dev, RPM_ACTIVE); - __pm_runtime_set_status() detects that RPM state is changed, and checks whether the parent is: not active (RPM_ACTIVE) and not ignoring its children If this happens it concludes something is wrong, because a parent that is not ignoring its children must be active before any children activate. - Since the struct i2c_adapter *bar does not ignore its children, the PM core thinks that it must indeed go online before its children, the check bails out with -EBUSY, i.e. the i2c_client *baz thinks it can't work because it's parent is not online, and it respects its parent. - In the driver the .resume() callback returns -EBUSY from the runtime_force_resume() call as per above. This leaves the device in a suspended state, leading to bad behaviour later when the device is used. The following debug print is made with an extra printg patch but illustrates the problem: [ 17.040832] bh1780 2-0029: parent (i2c-2) is not active parent->power.ignore_children = 0 [ 17.040832] bh1780 2-0029: pm_runtime_force_resume: pm_runtime_set_active() failed (-16) [ 17.040863] dpm_run_callback(): pm_runtime_force_resume+0x0/0x88 returns -16 [ 17.040863] PM: Device 2-0029 failed to resume: error -16 Fix this by letting all struct i2c_adapter:s ignore their children: i2c children have no business doing keeping their parents awake: they are completely autonomous devices that just use their parent to talk, a usecase which must be power managed in the host on a per-message basis. Cc: Wolfram Sang Cc: Mark Brown Cc: Ulf Hansson Cc: Rafael J. Wysocki Cc: linux-pm@vger.kernel.org Signed-off-by: Linus Walleij Reviewed-by: Ulf Hansson --- ChangeLog v2->v3: - Move the pm_suspend_ignore_children() call before the pm_runtime_enable() call. ChangeLog v1->v2: - Change subject from "let I2C masters inherit suspend child ignorance" to "let I2C masters ignore their children for PM" - Use the big hammer and do the sensible thing: mark all i2c adapters as ignoring their children when it comes to power management. --- drivers/i2c/i2c-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 0f2f8484e8ec..7083785a232c 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -1565,6 +1565,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap) dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name); pm_runtime_no_callbacks(&adap->dev); + pm_suspend_ignore_children(&adap->dev, true); pm_runtime_enable(&adap->dev); #ifdef CONFIG_I2C_COMPAT