On Wed, Aug 27, 2008 at 6:54 PM, Mark Brown <broonie@xxxxxxxxxxxxx> wrote:
> On Wed, Aug 27, 2008 at 05:39:26PM +0800, Bryan Wu wrote:
>
> This looks basically good, thanks - I've picked up a few things below
> but they are mostly either minor or reflect the fact that the patch
> looks like it's been developed against current release kernels but
> there's been some churn recently in the ASoC APIs which need updates.
>
>> #define I2C_DRIVERID_CS5345 96 /* cs5345 audio processor */
>> +#define I2C_DRIVERID_SSM2602 97 /* BF52xC built in audio codec */
>
> It should be possible to just remove this - it shouldn't be needed.
>
OK, I'll take care of this i2c stuff here.
>> --- a/sound/soc/codecs/Kconfig
>> +++ b/sound/soc/codecs/Kconfig
>
> Current kernels have a Kconfig option SND_SOC_ALL_CODECS which should
> have your codec added - this allows codec drivers to be built without
> boards for test purposes.
>
Is this option in alsa git tree or in the mainline? I fail to find it
in upstream mainline.
And do you mean I don't need to add SND_SOC_SSM2602 at all?
>> +#define SSM2602_DEBUG 0
>> +
>> +#ifdef SSM2602_DEBUG
>> +#define dbg(format, arg...) \
>> + printk(KERN_DEBUG AUDIO_NAME ": " format "\n" , ## arg)
>> +#else
>> +#define dbg(format, arg...) do {} while (0)
>> +#endif
>> +#define err(format, arg...) \
>> + printk(KERN_ERR AUDIO_NAME ": " format "\n" , ## arg)
>> +#define info(format, arg...) \
>> + printk(KERN_INFO AUDIO_NAME ": " format "\n" , ## arg)
>> +#define warn(format, arg...) \
>> + printk(KERN_WARNING AUDIO_NAME ": " format "\n" , ## arg)
>
> Please convert these to use the standard pr_ macros (or ideally the dev_
> ones where possible) for debug prints.
>
Right, I killed this local definition and replaced them to pr_xxxx.
>> +
>> +#define ssm2602_reset(c) ssm2602_write(c, SSM2602_RESET, 0)
>> +/*Appending several "None"s just for OSS mixer use*/
>> +static const char *ssm2602_input_select[] = {"Line", "Mic", "None", "None", "None",
>> + "None", "None", "None"};
>> +static const char *ssm2602_deemph[] = {"None", "32Khz", "44.1Khz", "48Khz"};
>
> Please keep the lines under 80 characters where reasonable. A little
> more while space (blank lines and at the start and end of comments)
> would be nice too.
>
Yes, I will fix this issue by running checkpatch.pl. And for other API
conflicts and I2C interface upgrading stuffs,
I will leave them to Cliff.
Thanks
-Bryan
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
