[Intel-gfx] [PATCH v4 4/5] i2c: designware-baytrail: Disallow the CPU to enter C6 or C7 while holding the punit semaphore
Andy Shevchenko
andriy.shevchenko at linux.intel.com
Tue Dec 13 09:56:16 UTC 2016
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 at 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 at redhat.com>
> Tested-by: Takashi Iwai <tiwai at 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 <andriy.shevchenko at linux.intel.com>
Intel Finland Oy
More information about the Intel-gfx
mailing list