Re: [GIT PULL] sound updates for 5.9

From: Takashi Iwai <tiwai@suse.de>
To: Mark Brown <broonie@kernel.org>
Cc: John Stultz <john.stultz@linaro.org>,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Srini Kandagatla <srinivas.kandagatla@linaro.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Amit Pundir <amit.pundir@linaro.org>,
	Vinod Koul <vkoul@kernel.org>
Subject: Re: [GIT PULL] sound updates for 5.9
Date: Mon, 10 Aug 2020 15:52:38 +0200	[thread overview]
Message-ID: <s5hmu324nh5.wl-tiwai@suse.de> (raw)
In-Reply-To: <20200810122254.GA6438@sirena.org.uk>

On Mon, 10 Aug 2020 14:22:54 +0200,
Mark Brown wrote:
> 
> On Sat, Aug 08, 2020 at 10:07:36AM +0200, Takashi Iwai wrote:
> > Takashi Iwai wrote:
> 
> > > Does the patch below fix the bug?  If so, it's rather a bug in the
> > > commit cf6e26c71bfd ("ASoC: soc-component: merge
> > > snd_soc_component_read() and snd_soc_component_read32()").
> 
> > That said, the commit cf6e26c71bfd dropped the capability of returning
> > an error code from snd_soc_component_read() completely, while many
> > code still expect an error gets returned.  The assumption mentioned in
> > the patch (the error can be ignored) looks too naive.
> 
> I did an audit of the users when the series was posted and wasn't able
> to turn up any code doing anything constructive with the return values,
> but then once you're past probe error handling often makes things worse
> if you try.  This is the first one which actually seems to have had an
> impact.
> 
> > Morimoto-san, Mark, could you address it?  IMO, we may still need two
> > variants in the end again: the former snd_soc_component_read32() that
> > returns the value directly and snd_soc_component_read() that returns 0
> > or an error.  Only once after we deal with the error handling in each
> > caller side, we can unify the read functions.
> 
> I'm not sure if that specifically is what we need but yeah we should do
> something, if it fixes things your change is certainly good for the
> immediate problem so could you send it with a signoff please?  

OK, will do soon later.

> With the new code we do now have the core code printing an error message
> if the I/O fails, before they were just being ignored more often than
> not.  This did turn up a couple of cases where drivers were relying on
> being able to do things like silently read from registers that just
> don't exist or aren't currently accessible without any diagnostics which
> is it's own problem :/ (especially the volatile cases).

Yeah, we may need some raw access helper for such a case...

thanks,

Takashi