Re: [PATCH] ASoC: rl6347a: Use dev_err_once for I2C communication error prints

Re: [PATCH] ASoC: rl6347a: Use dev_err_once for I2C communication error prints

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 905 bytes --]

On Wed, Oct 05, 2016 at 05:05:36PM +0300, Jarkko Nikula wrote:

> It's difficult to guess from bunch of "ret=-121" errors what driver and
> device actually caused them. Since struct i2c_client has the dev pointer
> use that for dev_err_once() in order to see the device and not spam the
> log needlessly.

dev_err() is obviously an improvement (as would a better message be) but
dev_err_once() seems like a big jump that we don't normally do for our
I/O errors - transient errors do happen from time to time (especially
when people get their power up sequences wrong) and it can be useful to
know if something is just dead, failed gradually over time or just had a
single transient error.  It's also helpful since a lot of the errors
that do occur result in long timeouts so it's not always clear that
we're grinding through lots of I/Os if we don't say anything.  Am I
missing something about this case?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: rl6347a: Use dev_err_once for I2C communication error prints

[Jarkko Nikula]

On 10/06/2016 07:26 PM, Mark Brown wrote:
> On Wed, Oct 05, 2016 at 05:05:36PM +0300, Jarkko Nikula wrote:
>
>> It's difficult to guess from bunch of "ret=-121" errors what driver and
>> device actually caused them. Since struct i2c_client has the dev pointer
>> use that for dev_err_once() in order to see the device and not spam the
>> log needlessly.
>
> dev_err() is obviously an improvement (as would a better message be) but
> dev_err_once() seems like a big jump that we don't normally do for our
> I/O errors - transient errors do happen from time to time (especially
> when people get their power up sequences wrong) and it can be useful to
> know if something is just dead, failed gradually over time or just had a
> single transient error.  It's also helpful since a lot of the errors
> that do occur result in long timeouts so it's not always clear that
> we're grinding through lots of I/Os if we don't say anything.  Am I
> missing something about this case?
>
You are right. This looks like it is either due HW issue or pincontrol 
misconfiguration after resume and device starts making interrupt storm 
and acknowledging them fails before I2C is resumed. I switch it to 
dev_err().

-- 
Jarkko