| Message ID | 1423587970-19136-2-git-send-email-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Feb 10, 2015 at 07:06:07PM +0200, Andy Shevchenko wrote: > It seems we have same message for different return values in get_sem() and > baytrail_i2c_acquire(). I suspect this is just a typo, so this patch fixes it. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: David E. Box <david.e.box@linux.intel.com> Thanks for spotting this. Timeouts are infrequent and I didn't notice the error here. Dave > --- > drivers/i2c/busses/i2c-designware-baytrail.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c > index e9cb355..9b67655 100644 > --- a/drivers/i2c/busses/i2c-designware-baytrail.c > +++ b/drivers/i2c/busses/i2c-designware-baytrail.c > @@ -99,8 +99,8 @@ int baytrail_i2c_acquire(struct dw_i2c_dev *dev) > reset_semaphore(dev->dev); > > ret = iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ, > - PUNIT_SEMAPHORE, &sem); > - if (!ret) > + PUNIT_SEMAPHORE, &sem); > + if (ret) > dev_err(dev->dev, "iosf failed to read punit semaphore\n"); > else > dev_err(dev->dev, "PUNIT SEM: %d\n", sem); > -- > 2.1.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> + PUNIT_SEMAPHORE, &sem); > + if (ret) > dev_err(dev->dev, "iosf failed to read punit semaphore\n"); > else > dev_err(dev->dev, "PUNIT SEM: %d\n", sem); Shouldn't the latter be a dev_dbg?
On Wed, 2015-02-11 at 17:46 +0100, Wolfram Sang wrote: > > + PUNIT_SEMAPHORE, &sem); > > + if (ret) > > dev_err(dev->dev, "iosf failed to read punit semaphore\n"); > > else > > dev_err(dev->dev, "PUNIT SEM: %d\n", sem); > > Shouldn't the latter be a dev_dbg? For me it seems not. Here is error patch and we have already in error recovery, so, intention to see if the semaphore becomes alive after reset. Am I right, David?
On Wed, Feb 11, 2015 at 06:59:51PM +0200, Andy Shevchenko wrote: > On Wed, 2015-02-11 at 17:46 +0100, Wolfram Sang wrote: > > > + PUNIT_SEMAPHORE, &sem); > > > + if (ret) > > > dev_err(dev->dev, "iosf failed to read punit semaphore\n"); > > > else > > > dev_err(dev->dev, "PUNIT SEM: %d\n", sem); > > > > Shouldn't the latter be a dev_dbg? > > For me it seems not. Here is error patch and we have already in error > recovery, so, intention to see if the semaphore becomes alive after > reset. Am I right, David? Yes. We've timed out by this section of code so we want to verify the semaphore was reset. Failure to read the semaphore though is a separate error as well. Dave > > > -- > Andy Shevchenko <andriy.shevchenko@intel.com> > Intel Finland Oy > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2015-02-11 at 17:46 +0100, Wolfram Sang wrote: > > + PUNIT_SEMAPHORE, &sem); > > + if (ret) > > dev_err(dev->dev, "iosf failed to read punit semaphore\n"); > > else > > dev_err(dev->dev, "PUNIT SEM: %d\n", sem); > > Shouldn't the latter be a dev_dbg? Wolfram, do we agree on this as states in the patch? Maybe you have more comments, questions? Otherwise, should I rebase this on top of 4.0-rc1 or you will be okay with current version?
> > Shouldn't the latter be a dev_dbg? > > Wolfram, do we agree on this as states in the patch? Maybe you have more > comments, questions? Yes, it is fine this way. > Otherwise, should I rebase this on top of 4.0-rc1 or you will be okay > with current version? Well, if it needs rebasing, please do. If not, please say so, too :)
On Mon, 2015-02-23 at 19:42 +0100, Wolfram Sang wrote: > > > Shouldn't the latter be a dev_dbg? > > > > Wolfram, do we agree on this as states in the patch? Maybe you have more > > comments, questions? > > Yes, it is fine this way. > > > Otherwise, should I rebase this on top of 4.0-rc1 or you will be okay > > with current version? > > Well, if it needs rebasing, please do. If not, please say so, too :) It applies clearly on top of current linux-next without any changes from my side. So, no rebase is needed.
diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c index e9cb355..9b67655 100644 --- a/drivers/i2c/busses/i2c-designware-baytrail.c +++ b/drivers/i2c/busses/i2c-designware-baytrail.c @@ -99,8 +99,8 @@ int baytrail_i2c_acquire(struct dw_i2c_dev *dev) reset_semaphore(dev->dev); ret = iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ, - PUNIT_SEMAPHORE, &sem); - if (!ret) + PUNIT_SEMAPHORE, &sem); + if (ret) dev_err(dev->dev, "iosf failed to read punit semaphore\n"); else dev_err(dev->dev, "PUNIT SEM: %d\n", sem);
It seems we have same message for different return values in get_sem() and baytrail_i2c_acquire(). I suspect this is just a typo, so this patch fixes it. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/i2c/busses/i2c-designware-baytrail.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)