[alsa-devel] [PATCH V2] ASoC: add RT286 CODEC driver

[Leon Romanovsky]

On Tue, Dec 10, 2013 at 4:46 AM, Bard Liao <bardliao at realtek.com> wrote:
>> -----Original Message-----
>> From: Leon Romanovsky [mailto:leon at leon.nu]
>> Sent: Wednesday, December 04, 2013 10:37 PM
>> To: Bard Liao
>> Cc: Mark Brown; Liam Girdwood; Oder Chiou; alsa-devel at alsa-project.org;
>> Flove
>> Subject: Re: [alsa-devel] [PATCH V2] ASoC: add RT286 CODEC driver
>>
>> On Wed, Dec 4, 2013 at 3:41 PM,  <bardliao at realtek.com> wrote:
>> > From: Bard Liao <bardliao at realtek.com>
>> >
>> > This patch adds the ALC286 codec driver.
>> >
>> > Signed-off-by: Bard Liao <bardliao at realtek.com>
>> >
>>
>> Please add change log while you are sending verX patch.
>
> Please see the change log as below.
> * Add platform data to determine if combo jack and/or IRQ is using or not.
> * Implement rt286_hw_read/write in regmap.
> * Configure codec in runtime instead of in probe.
> * Other minor changes (coding style, remove version control, remove empty functions, share code ...)
>
> Should I send another patch with these change log?
I don't think so, just remember it for future submissions.

>>
>>
>> > +
>> > +static unsigned int rt286_reg_cache[TOTAL_NODE_ID + 1] = {
>> You don't need TOTAL_NODE_ID + 1 in array declaration.
>
> Without "TOTAL_NODE_ID + 1", I should always set [NODE_ID_MIXER_IN2] = 0x0000, right?
> Ie. Assign the latest array item a value.
>
>>
>> > +       [NODE_ID_DAC_OUT1] = 0x7f7f,
>> > +       [NODE_ID_DAC_OUT2] = 0x7f7f,
>> > +       [NODE_ID_SPDIF] = 0x0000,
>> > +       [NODE_ID_ADC_IN1] = 0x4343,
>> > +       [NODE_ID_ADC_IN2] = 0x4343,
>> > +       [NODE_ID_MIC1] = 0x0000,
>> > +       [NODE_ID_MIXER_IN] = 0x000b,
>> > +       [NODE_ID_MIXER_OUT1] = 0x0002,
>> > +       [NODE_ID_MIXER_OUT2] = 0x0000,
>> > +       [NODE_ID_SPK_OUT] = 0x0000,
>> > +       [NODE_ID_HP_OUT] = 0x0000,
>> > +       [NODE_ID_MIXER_IN1] = 0x0000,
>> > +       [NODE_ID_MIXER_IN2] = 0x0000,
>> > +};
1. static variable is always initialized to 0x0 by C standard.
2. It is better to initialize all cache registers in one place
(rt286_reg_cache), including zero values. In such case TOTAL_NODE_ID +
1 is redundant.


>
> Thanks.
>



-- 
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon at leon.nu