Re: [GIT PULL] sound updates for 5.9

From: Takashi Iwai
Date: Mon Aug 10 2020 - 09:52:41 EST


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