From patchwork Wed Aug 29 13:06:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 963439 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-i2c-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 420m9H4F7gz9ryn for ; Wed, 29 Aug 2018 23:06:43 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727595AbeH2RDd (ORCPT ); Wed, 29 Aug 2018 13:03:33 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:56974 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727204AbeH2RDd (ORCPT ); Wed, 29 Aug 2018 13:03:33 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8B87040241CB; Wed, 29 Aug 2018 13:06:40 +0000 (UTC) Received: from shalem.localdomain.com (ovpn-117-15.ams2.redhat.com [10.36.117.15]) by smtp.corp.redhat.com (Postfix) with ESMTP id D674D10031E3; Wed, 29 Aug 2018 13:06:38 +0000 (UTC) From: Hans de Goede To: "Rafael J . Wysocki" , Len Brown , Andy Shevchenko , Jarkko Nikula , Wolfram Sang , Mika Westerberg Cc: Hans de Goede , linux-acpi@vger.kernel.org, linux-i2c@vger.kernel.org, "4 . 15+" Subject: [PATCH 1/2] i2c: designware: Re-init controllers with pm_disabled set on resume Date: Wed, 29 Aug 2018 15:06:31 +0200 Message-Id: <20180829130632.31111-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Wed, 29 Aug 2018 13:06:40 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Wed, 29 Aug 2018 13:06:40 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'hdegoede@redhat.com' RCPT:'' Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org On Bay Trail and Cherry Trail devices we set the pm_disabled flag for I2C busses which the OS shares with the PUNIT as these need special handling. Until now we called dev_pm_syscore_device(dev, true) for I2C controllers with this flag set to keep these I2C controllers always on. After commit 12864ff8545f ("ACPI / LPSS: Avoid PM quirks on suspend and resume from hibernation"), this no longer works. This commit modifies lpss_iosf_exit_d3_state() to only run if lpss_iosf_enter_d3_state() has ran before it, so that it does not run on a resume from hibernate (or from S3). On these systems the conditions for lpss_iosf_enter_d3_state() to run never become true, so lpss_iosf_exit_d3_state() never gets called and the 2 LPSS DMA controllers never get forced into D0 mode, instead they are left in their default automatic power-on when needed mode. The not forcing of D0 mode for the DMA controllers enables these systems to properly enter S0ix modes, which is a good thing. But after entering S0ix modes the I2C controller connected to the PMIC no longer works, leading to e.g. broken battery monitoring. The _PS3 method for this I2C controller looks like this: Method (_PS3, 0, NotSerialized) // _PS3: Power State 3 { If ((((PMID == 0x04) || (PMID == 0x05)) || (PMID == 0x06))) { Return (Zero) } PSAT |= 0x03 Local0 = PSAT /* \_SB_.I2C5.PSAT */ } Where PMID = 0x05, so we enter the Return (Zero) path on these systems. So even if we were to not call dev_pm_syscore_device(dev, true) the I2C controller will be left in D0 rather then be switched to D3. Yet on other Bay and Cherry Trail devices S0ix is not entered unless *all* I2C controllers are in D3 mode. This combined with the I2C controller no longer working now that we reach S0ix states on these systems leads to me believing that the PUNIT itself puts the I2C controller in D3 when all other conditions for entering S0ix states are true. Since now the I2C controller is put in D3 over a suspend/resume we must re-initialize it afterwards and that does indeed fix it no longer working. This commit implements this fix by: 1) Making the suspend_late callback a no-op if pm_disabled is set and making the resume_early callback skip the clock re-enable (since it now was not disabled) while still doing the necessary I2C controller re-init. 2) Removing the dev_pm_syscore_device(dev, true) call, so that the suspend and resume callbacks are actually called. Normally this would cause the ACPI pm code to call _PS3 putting the I2C controller in D3, wreaking havoc since it is shared with the PUNIT, but in this special case the _PS3 method is a no-op so we can safely allow a "fake" suspend / resume. Fixes: 12864ff8545f ("ACPI / LPSS: Avoid PM quirks on suspend and resume ...") Link: https://bugzilla.kernel.org/show_bug.cgi?id=200861 Cc: 4.15+ # 4.15+ Signed-off-by: Hans de Goede Reviewed-by: Andy Shevchenko Acked-by: Jarkko Nikula --- drivers/i2c/busses/i2c-designware-master.c | 1 - drivers/i2c/busses/i2c-designware-platdrv.c | 7 ++++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c index 27436a937492..54b2a3a86677 100644 --- a/drivers/i2c/busses/i2c-designware-master.c +++ b/drivers/i2c/busses/i2c-designware-master.c @@ -693,7 +693,6 @@ int i2c_dw_probe(struct dw_i2c_dev *dev) i2c_set_adapdata(adap, dev); if (dev->pm_disabled) { - dev_pm_syscore_device(dev->dev, true); irq_flags = IRQF_NO_SUSPEND; } else { irq_flags = IRQF_SHARED | IRQF_COND_SUSPEND; diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index 5660daf6c92e..d281d21cdd8e 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -448,6 +448,9 @@ static int dw_i2c_plat_suspend(struct device *dev) { struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); + if (i_dev->pm_disabled) + return 0; + i_dev->disable(i_dev); i2c_dw_prepare_clk(i_dev, false); @@ -458,7 +461,9 @@ static int dw_i2c_plat_resume(struct device *dev) { struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); - i2c_dw_prepare_clk(i_dev, true); + if (!i_dev->pm_disabled) + i2c_dw_prepare_clk(i_dev, true); + i_dev->init(i_dev); return 0; From patchwork Wed Aug 29 13:06:32 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 963440 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-i2c-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 420m9J3f9Hz9s3C for ; Wed, 29 Aug 2018 23:06:44 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727204AbeH2RDe (ORCPT ); Wed, 29 Aug 2018 13:03:34 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:37290 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727537AbeH2RDe (ORCPT ); Wed, 29 Aug 2018 13:03:34 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0806B7788D; Wed, 29 Aug 2018 13:06:42 +0000 (UTC) Received: from shalem.localdomain.com (ovpn-117-15.ams2.redhat.com [10.36.117.15]) by smtp.corp.redhat.com (Postfix) with ESMTP id BE513112D194; Wed, 29 Aug 2018 13:06:40 +0000 (UTC) From: Hans de Goede To: "Rafael J . Wysocki" , Len Brown , Andy Shevchenko , Jarkko Nikula , Wolfram Sang , Mika Westerberg Cc: Hans de Goede , linux-acpi@vger.kernel.org, linux-i2c@vger.kernel.org Subject: [PATCH 2/2] i2c: designware: Remove Cherry Trail PMIC I2C bus pm_disabled workaround Date: Wed, 29 Aug 2018 15:06:32 +0200 Message-Id: <20180829130632.31111-2-hdegoede@redhat.com> In-Reply-To: <20180829130632.31111-1-hdegoede@redhat.com> References: <20180829130632.31111-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Wed, 29 Aug 2018 13:06:42 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Wed, 29 Aug 2018 13:06:42 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'hdegoede@redhat.com' RCPT:'' Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org Commit a3d411fb38c0 ("i2c: designware: Disable pm for PMIC i2c-bus even if there is no _SEM method"), always set the pm_disabled flag on the I2C7 controller, even if its bus was not shared with the PUNIT. This was a workaround for various suspend/resume issues, after the following 2 commits this workaround is no longer necessary: Commit 541527728341 ("PM: i2c-designware-platdrv: Suspend/resume at the late/early stages") Commit e6ce0ce34f65 ("ACPI / LPSS: Add device link for CHT SD card dependency on I2C") Therefor this commit removes this workaround. After this commit the pm_disabled flag is only used to indicate that the bus is shared with the PUNIT and after other recent changes we no longer call dev_pm_syscore_device(dev, true), so we are no longer actually disabling (non-runtime) pm, so this commit also renames the flag to shared_with_punit to better reflect what it is for. Since we now no longer keep the I2C controller alive during suspend, we also no longer need to set IRQF_NO_SUSPEND. Signed-off-by: Hans de Goede --- drivers/i2c/busses/i2c-designware-baytrail.c | 2 +- drivers/i2c/busses/i2c-designware-core.h | 4 ++-- drivers/i2c/busses/i2c-designware-master.c | 10 ++------- drivers/i2c/busses/i2c-designware-platdrv.c | 23 ++++---------------- 4 files changed, 9 insertions(+), 30 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c index dbda8c9c8a1c..812438cce341 100644 --- a/drivers/i2c/busses/i2c-designware-baytrail.c +++ b/drivers/i2c/busses/i2c-designware-baytrail.c @@ -172,7 +172,7 @@ int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev) dev_info(dev->dev, "I2C bus managed by PUNIT\n"); dev->acquire_lock = baytrail_i2c_acquire; dev->release_lock = baytrail_i2c_release; - dev->pm_disabled = true; + dev->shared_with_punit = true; pm_qos_add_request(&dev->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE); diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h index d690e648bc01..f90de032e8ec 100644 --- a/drivers/i2c/busses/i2c-designware-core.h +++ b/drivers/i2c/busses/i2c-designware-core.h @@ -224,7 +224,7 @@ * @pm_qos: pm_qos_request used while holding a hardware lock on the bus * @acquire_lock: function to acquire a hardware lock on the bus * @release_lock: function to release a hardware lock on the bus - * @pm_disabled: true if power-management should be disabled for this i2c-bus + * @shared_with_punit: true if this bus is shared with the SoCs PUNIT * @disable: function to disable the controller * @disable_int: function to disable all interrupts * @init: function to initialize the I2C hardware @@ -279,7 +279,7 @@ struct dw_i2c_dev { struct pm_qos_request pm_qos; int (*acquire_lock)(struct dw_i2c_dev *dev); void (*release_lock)(struct dw_i2c_dev *dev); - bool pm_disabled; + bool shared_with_punit; void (*disable)(struct dw_i2c_dev *dev); void (*disable_int)(struct dw_i2c_dev *dev); int (*init)(struct dw_i2c_dev *dev); diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c index 54b2a3a86677..d563a2c2ec97 100644 --- a/drivers/i2c/busses/i2c-designware-master.c +++ b/drivers/i2c/busses/i2c-designware-master.c @@ -672,7 +672,6 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev) int i2c_dw_probe(struct dw_i2c_dev *dev) { struct i2c_adapter *adap = &dev->adapter; - unsigned long irq_flags; int ret; init_completion(&dev->cmd_complete); @@ -692,14 +691,9 @@ int i2c_dw_probe(struct dw_i2c_dev *dev) adap->dev.parent = dev->dev; i2c_set_adapdata(adap, dev); - if (dev->pm_disabled) { - irq_flags = IRQF_NO_SUSPEND; - } else { - irq_flags = IRQF_SHARED | IRQF_COND_SUSPEND; - } - i2c_dw_disable_int(dev); - ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, irq_flags, + ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, + IRQF_SHARED | IRQF_COND_SUSPEND, dev_name(dev->dev), dev); if (ret) { dev_err(dev->dev, "failure requesting irq %i: %d\n", diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index d281d21cdd8e..fcb07be8f684 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -97,10 +97,7 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) { struct dw_i2c_dev *dev = platform_get_drvdata(pdev); u32 ss_ht = 0, fp_ht = 0, hs_ht = 0, fs_ht = 0; - acpi_handle handle = ACPI_HANDLE(&pdev->dev); const struct acpi_device_id *id; - struct acpi_device *adev; - const char *uid; dev->adapter.nr = -1; dev->tx_fifo_depth = 32; @@ -135,18 +132,6 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) if (id && id->driver_data) dev->flags |= (u32)id->driver_data; - if (acpi_bus_get_device(handle, &adev)) - return -ENODEV; - - /* - * Cherrytrail I2C7 gets used for the PMIC which gets accessed - * through ACPI opregions during late suspend / early resume - * disable pm for it. - */ - uid = adev->pnp.unique_id; - if ((dev->flags & MODEL_CHERRYTRAIL) && !strcmp(uid, "7")) - dev->pm_disabled = true; - return 0; } @@ -231,7 +216,7 @@ static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev) { pm_runtime_disable(dev->dev); - if (dev->pm_disabled) + if (dev->shared_with_punit) pm_runtime_put_noidle(dev->dev); } @@ -362,7 +347,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_set_active(&pdev->dev); - if (dev->pm_disabled) + if (dev->shared_with_punit) pm_runtime_get_noresume(&pdev->dev); pm_runtime_enable(&pdev->dev); @@ -448,7 +433,7 @@ static int dw_i2c_plat_suspend(struct device *dev) { struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); - if (i_dev->pm_disabled) + if (i_dev->shared_with_punit) return 0; i_dev->disable(i_dev); @@ -461,7 +446,7 @@ static int dw_i2c_plat_resume(struct device *dev) { struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); - if (!i_dev->pm_disabled) + if (!i_dev->shared_with_punit) i2c_dw_prepare_clk(i_dev, true); i_dev->init(i_dev);