On Mon, Oct 5, 2015 at 9:11 PM, Mark Brown <broonie@xxxxxxxxxx> wrote:
> On Fri, Sep 25, 2015 at 05:48:23PM -0400, Alex Deucher wrote:
>> From: Maruthi Srinivas Bayyavarapu <Maruthi.Bayyavarapu@xxxxxxx>
>>
>> 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.
Ok, I will split into multiple patches and send again.
>
>> --- 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.
Agreed. I will send a revised patch.
>
>> + 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.
Agreed. I will send a revised patch.
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@xxxxxxxxxxxxxxxx
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
