[alsa-devel] [PATCH 1/2] ASoC: wm8904: rework FLL handling

[Charles Keepax]

On Tue, Jul 23, 2019 at 01:54:15PM +0200, Michał Mirosław wrote:
> On Tue, Jul 23, 2019 at 11:52:48AM +0100, Mark Brown wrote:
> > On Tue, Jul 23, 2019 at 09:27:16AM +0100, Charles Keepax wrote:
> > > On Mon, Jul 22, 2019 at 07:57:21PM +0200, Michał Mirosław wrote:
> > > > Extract and rework FLL handling. This makes it possible to reuse
> > > > the code for other Wolfson codecs and makes codec adapt SYSCLK to
> > > > exactly match frequency required for used sampling rate.
> > 
> > > Do you have thoughts on which CODECs you would be including in
> > > this? These older parts often have small differences between the
> > > configuration that might make this challenging so if you have
> > > plans here would be good to have a look from this end.
> > 
> > Right, it's not like it's the same IP being dropped into multiple chips
> > in an identical fashion.  There's a lot of high level similarities in
> > the register interfaces but also many small per device tweaks, and it's
> > not clear what benefit we get from refactoring at this point.
> 
> This would be mainly code separation, so it's easier to understand and
> has a potential for direct reuse. I can see that clock selection needs
> to be changed, but the idea is to have it configurable via device-tree.
> 
> I picked at random WM9081. It's FLL implementation looks very similar -
> major diffferences being in FLL_OUTDIV selection (direct divider vs 2^N)
> and register block offset.
> 
> Another random pick - WM8900. The general FLL idea seems the same, but
> this one has a bit more complicated register layout, so I wouldn't
> consider it at first.
> 
> WM8994 - it has two FLL's but otherwise has identical register layout
> for them as WM8904. The only difference is in clock source selection.
> 

The register layouts do look similar but the code controlling the
FLLs in these cases is quite different. I am somewhat nervous
there are subtle factors at play here which are going to cause
problems and its very hard to seperate divergence and actually
required sequencing here.

At the very least if you are really sure you want to proceed in
this direction I think we should look at splitting the patch into
two parts one that factors out the functionality and a separate
patch that adds any new functionality. It makes things much
easier to review that way.

Thanks,
Charles