Hi Lars-Peter,
On Tue, Oct 28, 2014 at 6:58 PM, Lars-Peter Clausen lars@metafoo.de wrote:
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)
if (tx_active) {if (i2s->tx_fifo_high < 256)xtfpga_i2s_refill_fifo(i2s);elsetasklet_hi_schedule(&i2s->refill_fifo);Maybe use threaded IRQs instead of IRQ + tasklet.
Ok, I'll look at it.
+static int xtfpga_pcm_trigger(struct snd_pcm_substream *substream, int
If you don't do anything you don't need the handler. The core will handle things if it is NULL.
Ok.
+static struct snd_pcm_ops xtfpga_pcm_ops = {
const
Ok.
+static int xtfpga_i2s_probe(struct platform_device *pdev)
dev_set_drvdata(&pdev->dev, i2s);platform_set_drvdata(...)
Ok.
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.
Ok.
irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);platform_get_irq(...)
Ok.
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.
Ok.
+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.
Ok.