On Wed 2011-02-16 16:20:37, Timur Tabi wrote:
On Tue, Feb 15, 2011 at 4:56 PM, zhaoming.zeng@freescale.com wrote:
From: Zeng Zhaoming zhaoming.zeng@freescale.com
Add Freescale SGTL5000 codec support
Please provide a description of this part. What is it, what features does it have, etc.
@@ -176,6 +177,9 @@ config SND_SOC_MAX98088 config SND_SOC_PCM3008 tristate
+config SND_SOC_SGTL5000
- tristate
Please provide some kind of descriptive text here.
- l = l < 0x3c ? 0x3c : l;
- l = l > 0xfc ? 0xfc : l;
- r = r < 0x3c ? 0x3c : r;
- r = r > 0xfc ? 0xfc : r;
Use the clamp() macro instead.
thanks for the hint.
- /* revert it */
- l = 0xfc - l;
- r = 0xfc - r;
I think you mean "invert it". "revert" means to undo a change.
yes, invert it. ;)
- if (mute)
- snd_soc_update_bits(codec, SGTL5000_CHIP_ADCDAC_CTRL,
- adcdac_ctrl, adcdac_ctrl);
- else
- snd_soc_update_bits(codec, SGTL5000_CHIP_ADCDAC_CTRL,
- adcdac_ctrl, 0);
How about:
snd_soc_update_bits(codec, SGTL5000_CHIP_ADCDAC_CTRL, adcdac_ctrl, mute ? adcdac_ctrl : 0);
Yes, this one is better. I will change to this style in the whole file.
+static int sgtl5000_suspend(struct snd_soc_codec *codec, pm_message_t state) +{
- sgtl5000_set_bias_level(codec, SND_SOC_BIAS_OFF);
- return 0;
+}
#ifdef CONFIG_PM ?
- /*
- * copy DAP default values to default value array.
- * sgtl5000 register space has a big hole, merge it
- * at init phase makes life easy.
- * FIXME: should we drop 'const' of sgtl5000_regs?
- */
- memcpy((void *)(&sgtl5000_regs[0] + (SGTL5000_DAP_REG_OFFSET >> 1)),
- sgtl5000_dap_regs,
- 0x3a);
I don't think you need the "(void *)"
And what is 0x3a?
+MODULE_DESCRIPTION("ASoC sgtl5000 driver"); +MODULE_AUTHOR("Freescale Semiconductor, Inc.");
Please consider putting your name here, so that people know who the real author is.
diff --git a/sound/soc/codecs/sgtl5000.h b/sound/soc/codecs/sgtl5000.h new file mode 100644 index 0000000..7f24655 --- /dev/null +++ b/sound/soc/codecs/sgtl5000.h @@ -0,0 +1,403 @@ +/*
- sgtl5000.h - SGTL5000 audio codec interface
This whole file should probably be merged into sgtl5000.c, since that file is the only file that #includes sgtl5000.h. No point in having a header file that is only included by one other file.
This is not common on ASoC codec drivers, many drivers have their own header file. I think it make the code more clear.
- Copyright 2008-2009 Freescale Semiconductor, Inc.
So the header file is copyright 2009, but the source file is copyright 2011?
- This program is free software; you can redistribute it and/or modify it
- under the terms of the GNU General Public License as published by the
- Free Software Foundation; either version 2 of the License, or (at your
- option) any later version.
Also, the header file is "GPL v2 or later", but the source file is "GPL v2 only". Pick one or the other, but don't use both. I'm surprised your patch was approved by the PPP legal review.
Ok, I will change it to GPL v2 only, thanks to point it out.
-- Timur Tabi Linux kernel developer at Freescale _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel