On Mon, Jun 04, 2012 at 04:03:19PM -0400, Scott Jiang wrote:
This driver includes i2s dai, dma and sport driver. It enables i2s mode support on blackfin bf60x platform.
This should be a patch series, for example the DAI, DMA and SPORT drivers would normally all be split out.
Overall this is mostly good, the main thing that's jumping out at me is that the SPORT API looks like it doesn't really have too many device specifics in it so it's a bit surprising that we need completely separate I2S and DMA drivers for the two chip generations.
+config SND_BF6XX_I2S
- tristate "SoC I2S Audio for the ADI BF6xx chip"
- depends on BLACKFIN && BF60x
Shouldn't BF60x already depend on BLACKFIN?
config SND_BF5XX_SOC_SSM2602 tristate "SoC SSM2602 Audio support for BF52x ezkit"
This bit also needs updating...
config SND_BF5XX_SPORT_NUM int "Set a SPORT for Sound chip"
- depends on (SND_BF5XX_I2S || SND_BF5XX_AC97 || SND_BF5XX_TDM)
- depends on (BLACKFIN && SND_SOC)
Shouldn't have a dependency on SND_SOC - this menu can't be entered without ASoC being enabled.
+static int __init snd_bfin_i2s_pcm_init(void) +{
- return platform_driver_register(&bfin_i2s_pcm_driver);
+} +module_init(snd_bfin_i2s_pcm_init);
module_platform_driver().
+#ifdef CONFIG_PM +static int bfin_i2s_suspend(struct snd_soc_dai *dai) +{
- struct sport_device *sport = snd_soc_dai_get_drvdata(dai);
- if (dai->capture_active)
sport_rx_stop(sport);- if (dai->playback_active)
sport_tx_stop(sport);- return 0;
+}
The framework should do this for you, by the time we get to suspending there shouldn't be any active streams that aren't supposed to stay active over the time the device is suspended.
+static int __init bfin_i2s_init(void) +{
- return platform_driver_register(&bfin_i2s_driver);
+}
+static void __exit bfin_i2s_exit(void) +{
- platform_driver_unregister(&bfin_i2s_driver);
+}
module_platform_driver().
+void sport_tx_start(struct sport_device *sport) +{
Is it not possible to make the SPORT API the same between the 5xx and the 6xx such that we can share all the drivers between the two chip generations with just the SPORT code varying?
- dev_info(dev, "SPORT create success\n");
Make this dev_dbg at most.