On Mon, Jul 07, 2014 at 03:37:01PM +0800, Oder Chiou wrote:
+int rt5677_spi_write(u8 *txbuf, size_t len) +{
- static DEFINE_MUTEX(lock);
- int status;
- mutex_lock(&lock);
- status = spi_write(g_spi, txbuf, len);
- mutex_unlock(&lock);
Since this lock is local to the function it doesn't do anything, the SPI framework has its own locking which makes it reentrant.
+int rt5677_spi_burst_write(u32 addr, const struct firmware *fw) +{
- u8 spi_cmd = 0x05;
- u8 *write_buf;
- unsigned int i, end, offset = 0;
- write_buf = kmalloc(8 * 30 + 6, GFP_KERNEL);
Magic numbers! You also need to be aware that memory allocated by kmalloc() may not be contigous and while things using the SPI core DMA handling ought to be able to cope not everything will be.
+static int rt5677_spi_probe(struct spi_device *spi) +{
- g_spi = spi;
- return 0;
+}
+static int rt5677_spi_remove(struct spi_device *spi) +{
- return 0;
+}
Delete empty functions.
+static int __init rt5677_spi_init(void) +{
- return spi_register_driver(&rt5677_spi_driver);
+} +module_init(rt5677_spi_init);
module_spi_driver()
+static int rt5677_dsp_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
- struct rt5677_priv *rt5677 = snd_soc_codec_get_drvdata(codec);
- if (ucontrol->value.integer.value[0] != 0 && rt5677->dsp_en == false) {
rt5677->dsp_en = true;regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG1,RT5677_LDO1_SEL_MASK, 0x0);regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG2,RT5677_PWR_LDO1, RT5677_PWR_LDO1);regmap_write(rt5677->regmap, RT5677_GLB_CLK2, 0x0080);regmap_write(rt5677->regmap, RT5677_PWR_DSP2, 0x07ff);regmap_write(rt5677->regmap, RT5677_PWR_DSP1, 0x07ff);
This doesn't seem joined up with the rest of the CODEC power management which suggests there's going to be problems - for example LDO1_SEL_MASK is also being managed by the existing set_bias_level(). What other devices do is manage the power for the DSP using a DAPM widget, enabling it when there's audio flowing through it.
- mutex_init(&rt5677->dsp_cmd_lock);
- request_firmware_nowait(THIS_MODULE, FW_ACTION_HOTPLUG,
RT5677_FIRMWARE1, codec->dev, GFP_KERNEL, codec,rt5677_fw1_loaded);- request_firmware_nowait(THIS_MODULE, FW_ACTION_HOTPLUG,
RT5677_FIRMWARE2, codec->dev, GFP_KERNEL, codec,rt5677_fw2_loaded);
It would be nicer to be able to only request firmware when needed rather than loading it once a boot time, that way we use a little less memory and we'll always have the latest firmware rather than what happened to be there when the system is booting.