diff mbox

[v4,4/5] i2c: designware-baytrail: Disallow the CPU to enter C6 or C7 while holding the punit semaphore

Message ID 20161212215612.9262-4-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hans de Goede Dec. 12, 2016, 9:56 p.m. UTC
On my cherrytrail tablet with axp288 pmic, just doing a bunch of repeated
reads from the pmic, e.g. "i2cdump -y 14 0x34" would lookup the tablet in
1 - 3 runs guaranteed.

This seems to be causes by the cpu trying to enter C6 or C7 while we hold
the punit bus semaphore, at which point everything just hangs.

Avoid this by disallowing the CPU to enter C6 or C7 before acquiring the
punit bus semaphore.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Takashi Iwai <tiwai@suse.de>
---
Changes in v2:
-New patch in v2 of this set
Changes in v3:
-Change commit message and comment in the code from "force the CPU to C1"
 to "Disallow the CPU to enter C6 or C7", as the CPU may still be in either
 C0 or C1 with the request pm_qos
Changes in v4:
-Rename i2c_dw_eval_lock_support to i2c_dw_probe_lock_support so that we can
 add a matching i2c_dw_remove_lock_support cleanup function
-Move qm_pos removal to new i2c_dw_remove_lock_support function
-Move pm_qos_add_request to the end of i2c_dw_probe_lock_support
---
 drivers/i2c/busses/i2c-designware-baytrail.c | 21 ++++++++++++++++++++-
 drivers/i2c/busses/i2c-designware-core.h     |  9 +++++++--
 drivers/i2c/busses/i2c-designware-platdrv.c  |  4 +++-
 3 files changed, 30 insertions(+), 4 deletions(-)

Comments

Andy Shevchenko Dec. 13, 2016, 9:56 a.m. UTC | #1
On Mon, 2016-12-12 at 22:56 +0100, Hans de Goede wrote:
> On my cherrytrail tablet with axp288 pmic, just doing a bunch of
> repeated
> reads from the pmic, e.g. "i2cdump -y 14 0x34" would lookup the tablet
> in
> 1 - 3 runs guaranteed.
> 
> This seems to be causes by the cpu trying to enter C6 or C7 while we
> hold
> the punit bus semaphore, at which point everything just hangs.
> 
> Avoid this by disallowing the CPU to enter C6 or C7 before acquiring
> the
> punit bus semaphore.
> 

Just a nitpick for abbreviations: pmic -> PMIC, punit -> P-Unit, but I'm
okay with the contents which is more important.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=109051

What would be good is to have comments / tags from Len and Ville.

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Tested-by: Takashi Iwai <tiwai@suse.de>
> ---
> Changes in v2:
> -New patch in v2 of this set
> Changes in v3:
> -Change commit message and comment in the code from "force the CPU to
> C1"
>  to "Disallow the CPU to enter C6 or C7", as the CPU may still be in
> either
>  C0 or C1 with the request pm_qos
> Changes in v4:
> -Rename i2c_dw_eval_lock_support to i2c_dw_probe_lock_support so that
> we can
>  add a matching i2c_dw_remove_lock_support cleanup function
> -Move qm_pos removal to new i2c_dw_remove_lock_support function
> -Move pm_qos_add_request to the end of i2c_dw_probe_lock_support
> ---
>  drivers/i2c/busses/i2c-designware-baytrail.c | 21
> ++++++++++++++++++++-
>  drivers/i2c/busses/i2c-designware-core.h     |  9 +++++++--
>  drivers/i2c/busses/i2c-designware-platdrv.c  |  4 +++-
>  3 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c
> b/drivers/i2c/busses/i2c-designware-baytrail.c
> index cf02222..95d7013 100644
> --- a/drivers/i2c/busses/i2c-designware-baytrail.c
> +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
> @@ -16,6 +16,7 @@
>  #include <linux/acpi.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> +#include <linux/pm_qos.h>
>  
>  #include <asm/iosf_mbi.h>
>  
> @@ -33,6 +34,13 @@ static int get_sem(struct dw_i2c_dev *dev, u32
> *sem)
>  	u32 data;
>  	int ret;
>  
> +	/*
> +	 * Disallow the CPU to enter C6 or C7 state, entering these
> states
> +	 * requires the punit to talk to the pmic and if this happens
> while
> +	 * we're holding the semaphore, the SoC hangs.
> +	 */
> +	pm_qos_update_request(&dev->pm_qos, 0);
> +
>  	ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
> PUNIT_SEMAPHORE, &data);
>  	if (ret) {
>  		dev_err(dev->dev, "iosf failed to read punit
> semaphore\n");
> @@ -56,6 +64,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev)
>  	data &= ~PUNIT_SEMAPHORE_BIT;
>  	if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
> PUNIT_SEMAPHORE, data))
>  		dev_err(dev->dev, "iosf failed to reset punit
> semaphore during write\n");
> +
> +	pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
>  }
>  
>  static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
> @@ -121,7 +131,7 @@ static void baytrail_i2c_release(struct dw_i2c_dev
> *dev)
>  		jiffies_to_msecs(jiffies - acquired));
>  }
>  
> -int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev)
> +int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev)
>  {
>  	acpi_status status;
>  	unsigned long long shared_host = 0;
> @@ -149,5 +159,14 @@ int i2c_dw_eval_lock_support(struct dw_i2c_dev
> *dev)
>  	dev->release_lock = baytrail_i2c_release;
>  	dev->pm_runtime_disabled = true;
>  
> +	pm_qos_add_request(&dev->pm_qos, PM_QOS_CPU_DMA_LATENCY,
> +			   PM_QOS_DEFAULT_VALUE);
> +
>  	return 0;
>  }
> +
> +void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
> +{
> +	if (dev->acquire_lock)
> +		pm_qos_remove_request(&dev->pm_qos);
> +}
> diff --git a/drivers/i2c/busses/i2c-designware-core.h
> b/drivers/i2c/busses/i2c-designware-core.h
> index fb143f5..871970e 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -22,6 +22,7 @@
>   *
>   */
>  
> +#include <linux/pm_qos.h>
>  
>  #define DW_IC_CON_MASTER		0x1
>  #define DW_IC_CON_SPEED_STD		0x2
> @@ -67,6 +68,7 @@
>   * @fp_lcnt: fast plus LCNT value
>   * @hs_hcnt: high speed HCNT value
>   * @hs_lcnt: high speed LCNT value
> + * @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_runtime_disabled: true if pm runtime is disabled
> @@ -114,6 +116,7 @@ struct dw_i2c_dev {
>  	u16			fp_lcnt;
>  	u16			hs_hcnt;
>  	u16			hs_lcnt;
> +	struct pm_qos_request	pm_qos;
>  	int			(*acquire_lock)(struct dw_i2c_dev
> *dev);
>  	void			(*release_lock)(struct dw_i2c_dev
> *dev);
>  	bool			pm_runtime_disabled;
> @@ -131,7 +134,9 @@ extern u32 i2c_dw_read_comp_param(struct
> dw_i2c_dev *dev);
>  extern int i2c_dw_probe(struct dw_i2c_dev *dev);
>  
>  #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_BAYTRAIL)
> -extern int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev);
> +extern int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev);
> +extern void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev);
>  #else
> -static inline int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev) {
> return 0; }
> +static inline int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev) {
> return 0; }
> +static inline void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
> {}
>  #endif
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 97a2ca1..b0a64a2 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -210,7 +210,7 @@ static int dw_i2c_plat_probe(struct
> platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	r = i2c_dw_eval_lock_support(dev);
> +	r = i2c_dw_probe_lock_support(dev);
>  	if (r)
>  		return r;
>  
> @@ -291,6 +291,8 @@ static int dw_i2c_plat_remove(struct
> platform_device *pdev)
>  	if (!dev->pm_runtime_disabled)
>  		pm_runtime_disable(&pdev->dev);
>  
> +	i2c_dw_remove_lock_support(dev);
> +
>  	return 0;
>  }
>
Hans de Goede Dec. 13, 2016, 12:21 p.m. UTC | #2
Hi,

On 13-12-16 10:56, Andy Shevchenko wrote:
> On Mon, 2016-12-12 at 22:56 +0100, Hans de Goede wrote:
>> On my cherrytrail tablet with axp288 pmic, just doing a bunch of
>> repeated
>> reads from the pmic, e.g. "i2cdump -y 14 0x34" would lookup the tablet
>> in
>> 1 - 3 runs guaranteed.
>>
>> This seems to be causes by the cpu trying to enter C6 or C7 while we
>> hold
>> the punit bus semaphore, at which point everything just hangs.
>>
>> Avoid this by disallowing the CPU to enter C6 or C7 before acquiring
>> the
>> punit bus semaphore.
>>
>
> Just a nitpick for abbreviations: pmic -> PMIC, punit -> P-Unit, but I'm
> okay with the contents which is more important.

Erm, the rest of the code, including dev_info and dev_err messages
also uses punit without the - in there, anyways not planning to
send a v5 for now.

> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thank you.

>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
>
> What would be good is to have comments / tags from Len and Ville.

About this patch vs bug bko109051, yesterday I've spend time reading
that entire bug. It seems it is a combination of at least 3 bugs
combined, 2 i915 related with commits which seem to trigger
the problem (2 different groups of users with a different problem
it seems) which causes a hang every few hours. And one other
bug where the system freezes in minutes, that one sounds like
what I was seeing without this patch (but may well be yet
another issue).

As for the 2 i915 bugs, there have been git bisects for both of
them, it would be good if someone could take a look at these, just
search for bisect in that huge bug.

Regards,

Hans




>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Tested-by: Takashi Iwai <tiwai@suse.de>
>> ---
>> Changes in v2:
>> -New patch in v2 of this set
>> Changes in v3:
>> -Change commit message and comment in the code from "force the CPU to
>> C1"
>>  to "Disallow the CPU to enter C6 or C7", as the CPU may still be in
>> either
>>  C0 or C1 with the request pm_qos
>> Changes in v4:
>> -Rename i2c_dw_eval_lock_support to i2c_dw_probe_lock_support so that
>> we can
>>  add a matching i2c_dw_remove_lock_support cleanup function
>> -Move qm_pos removal to new i2c_dw_remove_lock_support function
>> -Move pm_qos_add_request to the end of i2c_dw_probe_lock_support
>> ---
>>  drivers/i2c/busses/i2c-designware-baytrail.c | 21
>> ++++++++++++++++++++-
>>  drivers/i2c/busses/i2c-designware-core.h     |  9 +++++++--
>>  drivers/i2c/busses/i2c-designware-platdrv.c  |  4 +++-
>>  3 files changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c
>> b/drivers/i2c/busses/i2c-designware-baytrail.c
>> index cf02222..95d7013 100644
>> --- a/drivers/i2c/busses/i2c-designware-baytrail.c
>> +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
>> @@ -16,6 +16,7 @@
>>  #include <linux/acpi.h>
>>  #include <linux/i2c.h>
>>  #include <linux/interrupt.h>
>> +#include <linux/pm_qos.h>
>>
>>  #include <asm/iosf_mbi.h>
>>
>> @@ -33,6 +34,13 @@ static int get_sem(struct dw_i2c_dev *dev, u32
>> *sem)
>>  	u32 data;
>>  	int ret;
>>
>> +	/*
>> +	 * Disallow the CPU to enter C6 or C7 state, entering these
>> states
>> +	 * requires the punit to talk to the pmic and if this happens
>> while
>> +	 * we're holding the semaphore, the SoC hangs.
>> +	 */
>> +	pm_qos_update_request(&dev->pm_qos, 0);
>> +
>>  	ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
>> PUNIT_SEMAPHORE, &data);
>>  	if (ret) {
>>  		dev_err(dev->dev, "iosf failed to read punit
>> semaphore\n");
>> @@ -56,6 +64,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev)
>>  	data &= ~PUNIT_SEMAPHORE_BIT;
>>  	if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
>> PUNIT_SEMAPHORE, data))
>>  		dev_err(dev->dev, "iosf failed to reset punit
>> semaphore during write\n");
>> +
>> +	pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
>>  }
>>
>>  static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
>> @@ -121,7 +131,7 @@ static void baytrail_i2c_release(struct dw_i2c_dev
>> *dev)
>>  		jiffies_to_msecs(jiffies - acquired));
>>  }
>>
>> -int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev)
>> +int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev)
>>  {
>>  	acpi_status status;
>>  	unsigned long long shared_host = 0;
>> @@ -149,5 +159,14 @@ int i2c_dw_eval_lock_support(struct dw_i2c_dev
>> *dev)
>>  	dev->release_lock = baytrail_i2c_release;
>>  	dev->pm_runtime_disabled = true;
>>
>> +	pm_qos_add_request(&dev->pm_qos, PM_QOS_CPU_DMA_LATENCY,
>> +			   PM_QOS_DEFAULT_VALUE);
>> +
>>  	return 0;
>>  }
>> +
>> +void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
>> +{
>> +	if (dev->acquire_lock)
>> +		pm_qos_remove_request(&dev->pm_qos);
>> +}
>> diff --git a/drivers/i2c/busses/i2c-designware-core.h
>> b/drivers/i2c/busses/i2c-designware-core.h
>> index fb143f5..871970e 100644
>> --- a/drivers/i2c/busses/i2c-designware-core.h
>> +++ b/drivers/i2c/busses/i2c-designware-core.h
>> @@ -22,6 +22,7 @@
>>   *
>>   */
>>
>> +#include <linux/pm_qos.h>
>>
>>  #define DW_IC_CON_MASTER		0x1
>>  #define DW_IC_CON_SPEED_STD		0x2
>> @@ -67,6 +68,7 @@
>>   * @fp_lcnt: fast plus LCNT value
>>   * @hs_hcnt: high speed HCNT value
>>   * @hs_lcnt: high speed LCNT value
>> + * @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_runtime_disabled: true if pm runtime is disabled
>> @@ -114,6 +116,7 @@ struct dw_i2c_dev {
>>  	u16			fp_lcnt;
>>  	u16			hs_hcnt;
>>  	u16			hs_lcnt;
>> +	struct pm_qos_request	pm_qos;
>>  	int			(*acquire_lock)(struct dw_i2c_dev
>> *dev);
>>  	void			(*release_lock)(struct dw_i2c_dev
>> *dev);
>>  	bool			pm_runtime_disabled;
>> @@ -131,7 +134,9 @@ extern u32 i2c_dw_read_comp_param(struct
>> dw_i2c_dev *dev);
>>  extern int i2c_dw_probe(struct dw_i2c_dev *dev);
>>
>>  #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_BAYTRAIL)
>> -extern int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev);
>> +extern int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev);
>> +extern void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev);
>>  #else
>> -static inline int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev) {
>> return 0; }
>> +static inline int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev) {
>> return 0; }
>> +static inline void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
>> {}
>>  #endif
>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
>> b/drivers/i2c/busses/i2c-designware-platdrv.c
>> index 97a2ca1..b0a64a2 100644
>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>> @@ -210,7 +210,7 @@ static int dw_i2c_plat_probe(struct
>> platform_device *pdev)
>>  		return -EINVAL;
>>  	}
>>
>> -	r = i2c_dw_eval_lock_support(dev);
>> +	r = i2c_dw_probe_lock_support(dev);
>>  	if (r)
>>  		return r;
>>
>> @@ -291,6 +291,8 @@ static int dw_i2c_plat_remove(struct
>> platform_device *pdev)
>>  	if (!dev->pm_runtime_disabled)
>>  		pm_runtime_disable(&pdev->dev);
>>
>> +	i2c_dw_remove_lock_support(dev);
>> +
>>  	return 0;
>>  }
>>
>
Andy Shevchenko Dec. 13, 2016, 12:34 p.m. UTC | #3
On Tue, 2016-12-13 at 13:21 +0100, Hans de Goede wrote:
> Hi,
> 
> On 13-12-16 10:56, Andy Shevchenko wrote:
> > On Mon, 2016-12-12 at 22:56 +0100, Hans de Goede wrote:
> > > On my cherrytrail tablet with axp288 pmic, just doing a bunch of
> > > repeated
> > > reads from the pmic, e.g. "i2cdump -y 14 0x34" would lookup the
> > > tablet
> > > in
> > > 1 - 3 runs guaranteed.
> > > 
> > > This seems to be causes by the cpu trying to enter C6 or C7 while
> > > we
> > > hold
> > > the punit bus semaphore, at which point everything just hangs.
> > > 
> > > Avoid this by disallowing the CPU to enter C6 or C7 before
> > > acquiring
> > > the
> > > punit bus semaphore.
> > > 
> > 
> > Just a nitpick for abbreviations: pmic -> PMIC, punit -> P-Unit, but
> > I'm
> > okay with the contents which is more important.
> 
> Erm, the rest of the code, including dev_info and dev_err messages
> also uses punit without the - in there, anyways not planning to
> send a v5 for now.

Yes, no need in v5 until something more serious comes up.

> 
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Thank you.
> 
> > > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
> > 
> > What would be good is to have comments / tags from Len and Ville.
> 
> About this patch vs bug bko109051, yesterday I've spend time reading
> that entire bug. It seems it is a combination of at least 3 bugs
> combined, 2 i915 related with commits which seem to trigger
> the problem (2 different groups of users with a different problem
> it seems) which causes a hang every few hours. And one other
> bug where the system freezes in minutes, that one sounds like
> what I was seeing without this patch (but may well be yet
> another issue).

There also a dw_dmac bug which I fixed, but people are still referring
to it in that bug report.

> 
> As for the 2 i915 bugs, there have been git bisects for both of
> them, it would be good if someone could take a look at these, just
> search for bisect in that huge bug.

Agreed.
Jarkko Nikula Dec. 13, 2016, 1:42 p.m. UTC | #4
On 12/13/2016 11:56 AM, Andy Shevchenko wrote:
> On Mon, 2016-12-12 at 22:56 +0100, Hans de Goede wrote:
>> On my cherrytrail tablet with axp288 pmic, just doing a bunch of
>> repeated
>> reads from the pmic, e.g. "i2cdump -y 14 0x34" would lookup the tablet
>> in
>> 1 - 3 runs guaranteed.
>>
>> This seems to be causes by the cpu trying to enter C6 or C7 while we
>> hold
>> the punit bus semaphore, at which point everything just hangs.
>>
>> Avoid this by disallowing the CPU to enter C6 or C7 before acquiring
>> the
>> punit bus semaphore.
>>
>
> Just a nitpick for abbreviations: pmic -> PMIC, punit -> P-Unit, but I'm
> okay with the contents which is more important.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
No need for v5 from my side either (Andy agreed it later in the thread)

Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
>
> What would be good is to have comments / tags from Len and Ville.
>
We can have also follow up patch if some other PM QoS acrobatics are 
required than what's implemented here. Getting real bug fixed is quite 
big benefit.
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index cf02222..95d7013 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -16,6 +16,7 @@ 
 #include <linux/acpi.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/pm_qos.h>
 
 #include <asm/iosf_mbi.h>
 
@@ -33,6 +34,13 @@  static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
 	u32 data;
 	int ret;
 
+	/*
+	 * Disallow the CPU to enter C6 or C7 state, entering these states
+	 * requires the punit to talk to the pmic and if this happens while
+	 * we're holding the semaphore, the SoC hangs.
+	 */
+	pm_qos_update_request(&dev->pm_qos, 0);
+
 	ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &data);
 	if (ret) {
 		dev_err(dev->dev, "iosf failed to read punit semaphore\n");
@@ -56,6 +64,8 @@  static void reset_semaphore(struct dw_i2c_dev *dev)
 	data &= ~PUNIT_SEMAPHORE_BIT;
 	if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, PUNIT_SEMAPHORE, data))
 		dev_err(dev->dev, "iosf failed to reset punit semaphore during write\n");
+
+	pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
 }
 
 static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
@@ -121,7 +131,7 @@  static void baytrail_i2c_release(struct dw_i2c_dev *dev)
 		jiffies_to_msecs(jiffies - acquired));
 }
 
-int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev)
+int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev)
 {
 	acpi_status status;
 	unsigned long long shared_host = 0;
@@ -149,5 +159,14 @@  int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev)
 	dev->release_lock = baytrail_i2c_release;
 	dev->pm_runtime_disabled = true;
 
+	pm_qos_add_request(&dev->pm_qos, PM_QOS_CPU_DMA_LATENCY,
+			   PM_QOS_DEFAULT_VALUE);
+
 	return 0;
 }
+
+void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
+{
+	if (dev->acquire_lock)
+		pm_qos_remove_request(&dev->pm_qos);
+}
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index fb143f5..871970e 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -22,6 +22,7 @@ 
  *
  */
 
+#include <linux/pm_qos.h>
 
 #define DW_IC_CON_MASTER		0x1
 #define DW_IC_CON_SPEED_STD		0x2
@@ -67,6 +68,7 @@ 
  * @fp_lcnt: fast plus LCNT value
  * @hs_hcnt: high speed HCNT value
  * @hs_lcnt: high speed LCNT value
+ * @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_runtime_disabled: true if pm runtime is disabled
@@ -114,6 +116,7 @@  struct dw_i2c_dev {
 	u16			fp_lcnt;
 	u16			hs_hcnt;
 	u16			hs_lcnt;
+	struct pm_qos_request	pm_qos;
 	int			(*acquire_lock)(struct dw_i2c_dev *dev);
 	void			(*release_lock)(struct dw_i2c_dev *dev);
 	bool			pm_runtime_disabled;
@@ -131,7 +134,9 @@  extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
 extern int i2c_dw_probe(struct dw_i2c_dev *dev);
 
 #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_BAYTRAIL)
-extern int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev);
+extern int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev);
+extern void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev);
 #else
-static inline int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev) { return 0; }
+static inline int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev) { return 0; }
+static inline void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev) {}
 #endif
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 97a2ca1..b0a64a2 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -210,7 +210,7 @@  static int dw_i2c_plat_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	r = i2c_dw_eval_lock_support(dev);
+	r = i2c_dw_probe_lock_support(dev);
 	if (r)
 		return r;
 
@@ -291,6 +291,8 @@  static int dw_i2c_plat_remove(struct platform_device *pdev)
 	if (!dev->pm_runtime_disabled)
 		pm_runtime_disable(&pdev->dev);
 
+	i2c_dw_remove_lock_support(dev);
+
 	return 0;
 }