[alsa-devel] [PATCH 2/5] ASoC : dwc : support dw i2s in AMD platform

[Mark Brown]

On Fri, Sep 25, 2015 at 05:48:23PM -0400, Alex Deucher wrote:
> From: Maruthi Srinivas Bayyavarapu <Maruthi.Bayyavarapu at amd.com>
> 
> Vendor specific quirk was added to:
> 1. Support AMD platform which has two dwc controllers with different
>    base address for playback and capture. Also, I2S_COMP_PARAM_*
>    registers offsets differed.

> 2. Resume audio which was active before system suspend.
>    After 'resume', dwc need to be reconfigured with params
>    configured during 'hw_params' callback. With this, audio usecase
>    continues from where it got stopped because of 'suspend'.

This is hard to review since there are several different changes in
here, one change per commit is muuch more helpful.  As is I'm not 100%
sure exactly what each bit of the change is intended to do.  One example
here is that a separate patch splitting pbase and cbase would be really
helpful.

> --- a/include/sound/designware_i2s.h
> +++ b/include/sound/designware_i2s.h
> @@ -44,6 +44,9 @@ struct i2s_platform_data {
>  	int channel;
>  	u32 snd_fmts;
>  	u32 snd_rates;
> +	#define DW_I2S_VENDOR_AMD (1 << 0)
> +	unsigned int quirks;

I would expect to see a set of quirks with the AMD devices selecting
those that apply to them rather than just a per vendor define - this
is more generic and more maintainable long term.  AMD may require
different sets of quirks with some future device and systems from other
vendors may require some but not all of the quirks that AMD requires.

> +	if (dev->quirks & DW_I2S_VENDOR_AMD) {
> +		comp1 = i2s_read_reg(dev->i2s_pbase, 0x124);
> +		comp2 = i2s_read_reg(dev->i2s_pbase, 0x120);

Please add defines for these registers.  Rather than having the quirk
code throughout the driver it might be clearer to have the comp1 and
comp2 register offsets stored in the driver data so you can just do the
actual quirk once at probe time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20151005/00250f24/attachment.sig>