Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Mark Brown <broonie@kernel.org>
Cc: linux-kernel@lists.codethink.co.uk, alsa-devel@alsa-project.org,
	Lars-Peter Clausen <lars@metafoo.de>,
	kuninori.morimoto.gx@renesas.com, lgirdwood@gmail.com,
	Ben Dooks <ben.dooks@codethink.co.uk>
Subject: Re: [PATCH] ak4642: show error if register write fails
Date: Tue, 11 Mar 2014 16:15:01 +0100	[thread overview]
Message-ID: <s5heh2822dm.wl%tiwai@suse.de> (raw)
In-Reply-To: <20140311142138.GU28112@sirena.org.uk>

At Tue, 11 Mar 2014 14:21:38 +0000,
Mark Brown wrote:
> 
> On Tue, Mar 11, 2014 at 02:59:55PM +0100, Lars-Peter Clausen wrote:
> > On 03/11/2014 12:21 PM, Mark Brown wrote:
> 
> > >>If you think that changing the two snd_soc calls to print errors
> > >>when anything bad happens then that would also be a good idea then
> > >>I can send a patch for that.
> 
> > >That would be better, yes.
> 
> > In my opinion it's better to pass the error on to the upper levels.
> > E.g. if userspace opens the PCM device and there is an IO error in
> > the startup callback then that error should be passed on to the
> > userspace application rather than doing a out of band error
> > reporting and adding a entry to the kernel log.
> 
> It would, overall, be much better to be passing errors back.  However
> what's actually happening now is I/O errors are routinely ignored and
> our error handling is somewhat shaky (and realistically it's hard to
> know what to do a lot of the time when I/O with the device is failing).
> Given that improving the diagnostics seems like it's going in the right
> direction, we can always remove this later.

This should be decided for each case, and I agree with the usefulness
of error logging for this particular case.  The I/O errors are
basically exceptions, and shouldn't happen in normal operations.

That said, if the driver behavior depends on such lowlevel errors,
its design is wrong, must be fixed in anyway.

Still an open question is whether it should be dev_err() or
dev_dbg().  Ideally speaking, dev_dbg() should be used, while
dev_err() would be more practical.


Takashi

  reply	other threads:[~2014-03-11 15:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-10 18:15 [PATCH] ak4642: show error if register write fails Ben Dooks
2014-03-10 23:40 ` Mark Brown
2014-03-11 11:18   ` Ben Dooks
2014-03-11 11:21     ` Mark Brown
2014-03-11 13:59       ` Lars-Peter Clausen
2014-03-11 14:17         ` Ben Dooks
2014-03-11 14:20           ` Lars-Peter Clausen
2014-03-11 14:21         ` Mark Brown
2014-03-11 15:15           ` Takashi Iwai [this message]
2014-03-11 18:44             ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5heh2822dm.wl%tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=ben.dooks@codethink.co.uk \
    --cc=broonie@kernel.org \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=lars@metafoo.de \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@lists.codethink.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox