On 10/27/2014 08:07 PM, Max Filippov wrote:
A few minor things.
[...]
+static irqreturn_t xtfpga_i2s_interrupt(int irq, void *dev_id)
+{
+ struct xtfpga_i2s *i2s = dev_id;
+ struct snd_pcm_substream *tx_substream;
+ int tx_active;
+ unsigned int_status;
+
+ regmap_read(i2s->regmap, XTFPGA_I2S_INT_STATUS, &int_status);
+ if (!(int_status & XTFPGA_I2S_INT_VALID))
+ return IRQ_NONE;
+
+ if (int_status & XTFPGA_I2S_INT_UNDERRUN) {
+ i2s->tx_fifo_sz = 0;
+ regmap_update_bits(i2s->regmap, XTFPGA_I2S_CONFIG,
+ XTFPGA_I2S_CONFIG_INT_ENABLE |
+ XTFPGA_I2S_CONFIG_TX_ENABLE, 0);
+ } else {
+ i2s->tx_fifo_sz = i2s->tx_fifo_low;
+ regmap_update_bits(i2s->regmap, XTFPGA_I2S_CONFIG,
+ XTFPGA_I2S_CONFIG_INT_ENABLE, 0);
+ }
+
+ rcu_read_lock();
+ tx_substream = rcu_dereference(i2s->tx_substream);
+ tx_active = tx_substream && snd_pcm_running(tx_substream);
+ if (tx_active) {
+ snd_pcm_period_elapsed(tx_substream);
+ if (int_status & XTFPGA_I2S_INT_UNDERRUN)
+ dev_dbg_ratelimited(i2s->dev, "%s: underrun\n",
+ __func__);
+ tx_substream = rcu_dereference(i2s->tx_substream);
+ tx_active = tx_substream && snd_pcm_running(tx_substream);
+ }
+ rcu_read_unlock();
+
+ if (tx_active) {
+ if (i2s->tx_fifo_high < 256)
+ xtfpga_i2s_refill_fifo(i2s);
+ else
+ tasklet_hi_schedule(&i2s->refill_fifo);
Maybe use threaded IRQs instead of IRQ + tasklet.
+ } else {
+ xtfpga_i2s_irq_update_config(i2s, int_status);
+ }
+
+ return IRQ_HANDLED;
+}
[...]
+
+static int xtfpga_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+ int ret;
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:
+ case SNDRV_PCM_TRIGGER_RESUME:
+ case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+ case SNDRV_PCM_TRIGGER_STOP:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ ret = 0;
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
If you don't do anything you don't need the handler. The core will handle
things if it is NULL.
+static struct snd_pcm_ops xtfpga_pcm_ops = {
const
+ .open = xtfpga_pcm_open,
+ .close = xtfpga_pcm_close,
+ .ioctl = snd_pcm_lib_ioctl,
+ .hw_params = xtfpga_pcm_hw_params,
+ .trigger = xtfpga_pcm_trigger,
+ .pointer = xtfpga_pcm_pointer,
+};
[...]
+static int xtfpga_i2s_probe(struct platform_device *pdev)
+{
+ struct xtfpga_i2s *i2s;
+ struct resource *mem, *irq;
+ int err;
+
+ i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL);
+ if (!i2s) {
+ err = -ENOMEM;
+ goto err;
+ }
+ dev_set_drvdata(&pdev->dev, i2s);
platform_set_drvdata(...)
+ i2s->dev = &pdev->dev;
+ dev_dbg(&pdev->dev, "dev: %p, i2s: %p\n", &pdev->dev, i2s);
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!mem) {
+ dev_err(&pdev->dev, "No memory resource\n");
+ err = -ENODEV;
+ goto err;
+ }
devm_ioremap_resource will check if mem is NULL and print a error message,
so the check above can be removed.
+ i2s->regs = devm_ioremap_resource(&pdev->dev, mem);
+ if (IS_ERR(i2s->regs)) {
+ err = PTR_ERR(i2s->regs);
+ goto err;
+ }
+
+ i2s->regmap = devm_regmap_init_mmio(&pdev->dev, i2s->regs,
+ &xtfpga_i2s_regmap_config);
+ if (IS_ERR(i2s->regmap)) {
+ dev_err(&pdev->dev, "regmap init failed\n");
+ err = PTR_ERR(i2s->regmap);
+ goto err;
+ }
+
+ i2s->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(i2s->clk)) {
+ dev_err(&pdev->dev, "couldn't get clock\n");
+ err = PTR_ERR(i2s->clk);
+ goto err;
+ }
+
+ irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
platform_get_irq(...)
+ if (!irq) {
+ dev_err(&pdev->dev, "No IRQ resource\n");
+ err = -ENODEV;
+ goto err;
+ }
+ err = devm_request_irq(&pdev->dev, irq->start, xtfpga_i2s_interrupt,
+ IRQF_SHARED, pdev->name, i2s);
+ if (err < 0) {
+ dev_err(&pdev->dev, "request_irq failed\n");
+ goto err;
+ }
+ tasklet_init(&i2s->refill_fifo, xtfpga_i2s_refill_fifo_tasklet,
+ (unsigned long)i2s);
+
Since the tasklet is is used in the interrupt handler it should initialized
before the IRQ is requested.
+ regmap_write(i2s->regmap, XTFPGA_I2S_CONFIG,
+ (0x1 << XTFPGA_I2S_CONFIG_CHANNEL_BASE));
+ regmap_write(i2s->regmap, XTFPGA_I2S_INT_STATUS, XTFPGA_I2S_INT_VALID);
+ regmap_write(i2s->regmap, XTFPGA_I2S_INT_MASK, XTFPGA_I2S_INT_UNDERRUN);
+
+ err = snd_soc_register_platform(&pdev->dev, &xtfpga_soc_platform);
+ if (err < 0) {
+ dev_err(&pdev->dev, "couldn't register platform\n");
+ goto err;
+ }
+ err = devm_snd_soc_register_component(&pdev->dev,
+ &xtfpga_i2s_component,
+ xtfpga_i2s_dai,
+ ARRAY_SIZE(xtfpga_i2s_dai));
+ if (err < 0)
+ dev_err(&pdev->dev, "couldn't register component\n");
+err:
+ if (err)
+ dev_err(&pdev->dev, "%s: err = %d\n", __func__, err);
+ return err;
+}
+
+static int xtfpga_i2s_remove(struct platform_device *pdev)
+{
+ struct xtfpga_i2s *i2s = dev_get_drvdata(&pdev->dev);
+
+ if (i2s) {
This will always be non NULL.
+ tasklet_kill(&i2s->refill_fifo);
+ if (i2s->regmap && !IS_ERR(i2s->regmap)) {
+ regmap_write(i2s->regmap, XTFPGA_I2S_CONFIG, 0);
+ regmap_write(i2s->regmap, XTFPGA_I2S_INT_MASK, 0);
+ regmap_write(i2s->regmap, XTFPGA_I2S_INT_STATUS,
+ XTFPGA_I2S_INT_VALID);
+ }
+ if (i2s->clk_enabled)
+ clk_disable_unprepare(i2s->clk);
+ }
+
+ return 0;
+}
[...]
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
