Re: [PATCH 4/8] sound:asoc:spdif_in: Add spdif IN support

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1739 bytes --]

On Tue, Mar 20, 2012 at 05:03:48PM +0530, Rajeev Kumar wrote:

This looks good, a few minor things but almost good to go.

> This patch implements the spdif IN driver for ST peripheral

S/PDIF.

> +	if (irq_status & SPDIF_IRQ_FIFOWRITE)
> +		pr_err("spdif in: fifo write error\n");
> +	if (irq_status & SPDIF_IRQ_EMPTYFIFOREAD)
> +		pr_err("spdif in: empty fifo read error\n");
> +	if (irq_status & SPDIF_IRQ_FIFOFULL)
> +		pr_err("spdif in: fifo full error\n");
> +	if (irq_status & SPDIF_IRQ_OUTOFRANGE)
> +		pr_err("spdif in: out of range error\n");

dev_err().

> +	if (!devm_request_mem_region(&pdev->dev, res->start,
> +				resource_size(res), pdev->name)) {
> +		dev_warn(&pdev->dev, "Failed to get memory resourse\n");
> +		return -ENOENT;
> +	}
> +
> +	host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
> +	if (!host) {
> +		dev_warn(&pdev->dev, "kzalloc fail\n");
> +		return -ENOMEM;
> +	}

Good to see this - this is the sort of stuff I was looking for in the
I2S driver.

> +	host->clk = clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(host->clk))
> +		return PTR_ERR(host->clk);

> +	pdata = dev_get_platdata(&pdev->dev);

Should really be error checking in case you didn't get your platform
data.

> +	ret = devm_request_irq(&pdev->dev, host->irq, spdif_in_irq, 0,
> +			"spdif-in", host);
> +	if (ret) {

I'm really not enthused about the idea of using devm_request_irq() here
- what steps are you taking to make sure that the IRQ can't possibly
fire after you've started tearing down the device.  In general it's
relatively hard to use devm_request_irq() safely.

> +#define SPDIF_IN_DEV_PM_OPS NULL

Just remove this if it's unconditionally empty.

> +static int __init spdif_in_init(void)

module_platform_driver().

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]