Re: [PATCH] ASoC: cs42l43: Move shutter IRQ handling into a worker thread

[Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>]

On Wed, Jul 31, 2024 at 08:30:15AM +0200, Takashi Iwai wrote:
> On Wed, 31 Jul 2024 01:20:40 +0200,
> Mark Brown wrote:
> > 
> > On Tue, Jul 30, 2024 at 11:47:48PM +0200, Jaroslav Kysela wrote:
> > > On 30. 07. 24 16:44, Mark Brown wrote:
> > 
> > > > With some of these slower buses it's not immediately obvious that it's
> > > > worth the bother of caching - the overhead of doing the lookup is
> > > > negligable in the overall context of handling the interrupt.
> > 
> > > I don't buy that argument. We should always try to write an optimal code.
> > > Every CPU tick counts.
> > 
> > So do the bytes of memory you'd use caching the pointers!
> 
> Adding two more works would be even more wastes, that's the beginning
> of the discussion :)
> 
> The problem is the order of ASoC component initialization, though;
> AFAIUC, the kcontrols aren't instantiated at the point of component
> probe, hence you can't get kcontrol objects there.
> 

I think there probably should be some way to cache the kcontrols,
I will have a look at implementing that. In general I chose the
delayed work as caching didn't obviously seem worth it as Mark notes,
and also the sdw_dev_lock tends to cause a lot of these inversions
and the delayed work is a very safe solution, whereas caching makes
me slightly nervous we will find another inversion at some point.

In general I think that is the actual problem here, the sdw_dev_lock
wraps too much stuff and frequently causes these inversions. Although
fixing that may not be possible and certainly requires a lot of
thinking.

Thanks,
Charles