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@xxxxxxxxxxxxxxx> > 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@xxxxxxxxxx> > Tested-by: Takashi Iwai <tiwai@xxxxxxx> > --- > 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@xxxxxxxxxxxxxxx> Intel Finland Oy _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
