Re: [PATCH 1/4] ASoC: add a WM8978 codec driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2010-01-20 at 21:01 +0100, Guennadi Liakhovetski wrote:
> On Tue, 19 Jan 2010, Liam Girdwood wrote:
> > 
> > It would be nice to have the relevant bits defined here for set_fmt()
> > etc instead of just the magic numbers used in the above codec driver. 
> 
> As I explained privately, I agree, that using names instead of bits helps 
> - but (mostly) only where those bits are reused multiple times in the 
> code. If you only have to initialise a register once with some bitmask, I 
> think, code like
> 
> 	/* Enable input X, output Y, set default W polarity to Z */
> 	__raw_writel(0x123, reg);
> 
> looks better than
> 
> 	__raw_writel(CHIP_INPUT_X_ENABLE | CHIP_OUTPUT_Y_ENABLE | 
> 			CHIP_SIGNAL_W_POLARITY_Z, reg);
> 
> so, unless there strong preferences in ALSA world, I'll try to combine 
> both. Let me know if this contradicts the common ALSA style.

I disagree, it's far better to use the bottom example as I can see
explicitly which bits you intend to write.  This makes it easier for
others to extend and debug your code.

Furthermore, WM codecs also have software generated register bits and
register macros (available upon request) that further reduce any effort
and any potential register value bugs.

Liam

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux