On Mon, 2017-01-02 at 10:55 +0100, Hans de Goede wrote: > Hi, > > On 02-01-17 10:36, Andy Shevchenko wrote: > > On Sun, 2017-01-01 at 21:15 +0100, Hans de Goede wrote: > > > Use iosf_mbi_modify instead of iosf_mbi_read + iosf_mbi_write so > > > that > > > we keep the iosf_mbi_lock locked during the read-modify-write done > > > to > > > reset the semaphore. > > > > > > > While patch itself looks good to me, I think it reduces a > > probability to > > race and doesn't eliminate an issue completely. > > All accesses through this register are done through the > iosf_mbi_ helpers, and the modify helper holds the > iosf_mbi_lock during the entire read-modify-wrtie cycle, > so I don't see how there can still be any issue left. Ah, seems you are right. Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > > > Can you check i915 code > > how it's done there? > > AFAIK the i915 code never takes the pmic i2c bus semaphore. > > Regards, > > Hans > > p.s. > > Since you're interested in this set, you may also be interested > in this RFC patch-set: > > https://www.spinics.net/lists/intel-gfx/msg115833.html > > > > > > > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > > > --- > > > Changes in v5: > > > -New patch in v5 of this patch-set > > > --- > > > drivers/i2c/busses/i2c-designware-baytrail.c | 11 ++--------- > > > 1 file changed, 2 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c > > > b/drivers/i2c/busses/i2c-designware-baytrail.c > > > index 650a700..8df529c 100644 > > > --- a/drivers/i2c/busses/i2c-designware-baytrail.c > > > +++ b/drivers/i2c/busses/i2c-designware-baytrail.c > > > @@ -47,15 +47,8 @@ static int get_sem(struct dw_i2c_dev *dev, u32 > > > *sem) > > > > > > static void reset_semaphore(struct dw_i2c_dev *dev) > > > { > > > - u32 data; > > > - > > > - if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, > > > PUNIT_SEMAPHORE, &data)) { > > > - dev_err(dev->dev, "iosf failed to reset punit > > > semaphore during read\n"); > > > - return; > > > - } > > > - > > > - data &= ~PUNIT_SEMAPHORE_BIT; > > > - if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, > > > PUNIT_SEMAPHORE, data)) > > > + if (iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, > > > PUNIT_SEMAPHORE, > > > + 0, PUNIT_SEMAPHORE_BIT)) > > > dev_err(dev->dev, "iosf failed to reset punit > > > semaphore during write\n"); > > > > > > pm_qos_update_request(&dev->pm_qos, > > > PM_QOS_DEFAULT_VALUE); -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
