[PATCH] ASoC: davinci-mcasp: Choose PCM driver based on configured DMA controller

[PATCH] ASoC: davinci-mcasp: Choose PCM driver based on configured DMA controller

[Jyri Sarha]

Find the configured DMA controller by asking for a DMA channel in the
probe phase and releasing it right after. The controller device can be
found via the dma_chan struct and the controller is recognized from
the compatible property of its device node. The patch assumes EDMA if
there is no device node.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 61 ++++++++++++++++++++++++++++++++-------
 1 file changed, 51 insertions(+), 10 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index d793494..5bc2712 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -1565,6 +1565,46 @@ static int davinci_mcasp_init_ch_constraints(struct davinci_mcasp *mcasp)
 	return ret;
 }
 
+enum {
+	MCASP_EDMA = 1,
+	MCASP_SDMA,
+};
+static const char *sdma_prefix = "ti,omap";
+
+static int davinci_mcasp_get_dma_type(struct davinci_mcasp *mcasp)
+{
+	struct dma_chan *chan;
+	const char *tmp;
+	int ret = MCASP_EDMA;
+
+	tmp = mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK].filter_data;
+	chan = dma_request_slave_channel_reason(mcasp->dev, tmp);
+	if (IS_ERR(chan)) {
+		if (PTR_ERR(chan) != -EPROBE_DEFER)
+			dev_err(mcasp->dev,
+				"Can't verify DMA configuration (%ld)\n",
+				PTR_ERR(chan));
+		return PTR_ERR(chan);
+	}
+	BUG_ON(!chan->device || !chan->device->dev);
+
+	if (chan->device->dev->of_node)
+		ret = of_property_read_string(chan->device->dev->of_node,
+					      "compatible", &tmp);
+	else
+		dev_dbg(mcasp->dev, "DMA controller has no of-node\n");
+
+	dma_release_channel(chan);
+	if (ret)
+		return ret;
+
+	dev_dbg(mcasp->dev, "DMA controller compatible = \"%s\"\n", tmp);
+	if (!strncmp(tmp, sdma_prefix, strlen(sdma_prefix)))
+		return MCASP_SDMA;
+
+	return MCASP_EDMA;
+}
+
 static int davinci_mcasp_probe(struct platform_device *pdev)
 {
 	struct snd_dmaengine_dai_dma_data *dma_data;
@@ -1763,27 +1803,28 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 	if (ret != 0)
 		goto err;
 
-	switch (mcasp->version) {
+	ret = -EINVAL;
+	switch (davinci_mcasp_get_dma_type(mcasp)) {
+	case MCASP_EDMA:
 #if IS_BUILTIN(CONFIG_SND_EDMA_SOC) || \
 	(IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
 	 IS_MODULE(CONFIG_SND_EDMA_SOC))
-	case MCASP_VERSION_1:
-	case MCASP_VERSION_2:
-	case MCASP_VERSION_3:
 		ret = edma_pcm_platform_register(&pdev->dev);
-		break;
+#else
+		dev_err(&pdev->dev, "Missing SND_EDMA_SOC\n");
 #endif
+		break;
+	case MCASP_SDMA:
 #if IS_BUILTIN(CONFIG_SND_OMAP_SOC) || \
 	(IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
 	 IS_MODULE(CONFIG_SND_OMAP_SOC))
-	case MCASP_VERSION_4:
 		ret = omap_pcm_platform_register(&pdev->dev);
-		break;
+#else
+		dev_err(&pdev->dev, "Missing SND_SDMA_SOC\n");
 #endif
+		break;
 	default:
-		dev_err(&pdev->dev, "Invalid McASP version: %d\n",
-			mcasp->version);
-		ret = -EINVAL;
+		dev_err(&pdev->dev, "No DMA controller found\n");
 		break;
 	}
 
-- 
1.9.1

Re: [PATCH] ASoC: davinci-mcasp: Choose PCM driver based on configured DMA controller

[Peter Ujfalusi]

On 06/02/2015 11:58 AM, Jyri Sarha wrote:
> Find the configured DMA controller by asking for a DMA channel in the
> probe phase and releasing it right after. The controller device can be
> found via the dma_chan struct and the controller is recognized from
> the compatible property of its device node. The patch assumes EDMA if
> there is no device node.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  sound/soc/davinci/davinci-mcasp.c | 61 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 51 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
> index d793494..5bc2712 100644
> --- a/sound/soc/davinci/davinci-mcasp.c
> +++ b/sound/soc/davinci/davinci-mcasp.c
> @@ -1565,6 +1565,46 @@ static int davinci_mcasp_init_ch_constraints(struct davinci_mcasp *mcasp)
>  	return ret;
>  }
>  
> +enum {
> +	MCASP_EDMA = 1,

Why start from 1?
I'm not sure about the name. It somehow implies that McASP has something to do
with the dma engine.
MCASP_SERVICED_BY_E/SDMA
PLATFORM_USES_E/SDMA

I can not come up with a decent name :(


> +	MCASP_SDMA,
> +};
> +static const char *sdma_prefix = "ti,omap";
> +
> +static int davinci_mcasp_get_dma_type(struct davinci_mcasp *mcasp)
> +{
> +	struct dma_chan *chan;
> +	const char *tmp;
> +	int ret = MCASP_EDMA;
> +
> +	tmp = mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK].filter_data;
> +	chan = dma_request_slave_channel_reason(mcasp->dev, tmp);

this will return with error in case of !dev->of_node.
The first check should be:
if (!mcasp->dev->of_node)
	return MCASP_EDMA;

since we can only boot in legacy mode with daVinci devices and they are using
eDMA.

> +	if (IS_ERR(chan)) {
> +		if (PTR_ERR(chan) != -EPROBE_DEFER)
> +			dev_err(mcasp->dev,
> +				"Can't verify DMA configuration (%ld)\n",
> +				PTR_ERR(chan));
> +		return PTR_ERR(chan);
> +	}
> +	BUG_ON(!chan->device || !chan->device->dev);
> +
> +	if (chan->device->dev->of_node)
> +		ret = of_property_read_string(chan->device->dev->of_node,
> +					      "compatible", &tmp);
> +	else
> +		dev_dbg(mcasp->dev, "DMA controller has no of-node\n");

Now this is not likely to happen...

> +
> +	dma_release_channel(chan);
> +	if (ret)
> +		return ret;
> +
> +	dev_dbg(mcasp->dev, "DMA controller compatible = \"%s\"\n", tmp);
> +	if (!strncmp(tmp, sdma_prefix, strlen(sdma_prefix)))
> +		return MCASP_SDMA;
> +
> +	return MCASP_EDMA;
> +}
> +
>  static int davinci_mcasp_probe(struct platform_device *pdev)
>  {
>  	struct snd_dmaengine_dai_dma_data *dma_data;
> @@ -1763,27 +1803,28 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
>  	if (ret != 0)
>  		goto err;
>  
> -	switch (mcasp->version) {
> +	ret = -EINVAL;

Why? What was the problem with setting the ret in the default case?

> +	switch (davinci_mcasp_get_dma_type(mcasp)) {
> +	case MCASP_EDMA:
>  #if IS_BUILTIN(CONFIG_SND_EDMA_SOC) || \
>  	(IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
>  	 IS_MODULE(CONFIG_SND_EDMA_SOC))
> -	case MCASP_VERSION_1:
> -	case MCASP_VERSION_2:
> -	case MCASP_VERSION_3:
>  		ret = edma_pcm_platform_register(&pdev->dev);
> -		break;
> +#else
> +		dev_err(&pdev->dev, "Missing SND_EDMA_SOC\n");
>  #endif
> +		break;
> +	case MCASP_SDMA:
>  #if IS_BUILTIN(CONFIG_SND_OMAP_SOC) || \
>  	(IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
>  	 IS_MODULE(CONFIG_SND_OMAP_SOC))
> -	case MCASP_VERSION_4:
>  		ret = omap_pcm_platform_register(&pdev->dev);
> -		break;
> +#else
> +		dev_err(&pdev->dev, "Missing SND_SDMA_SOC\n");
>  #endif
> +		break;
>  	default:
> -		dev_err(&pdev->dev, "Invalid McASP version: %d\n",
> -			mcasp->version);
> -		ret = -EINVAL;
> +		dev_err(&pdev->dev, "No DMA controller found\n");
>  		break;
>  	}
>  
> 


-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ASoC: davinci-mcasp: Choose PCM driver based on configured DMA controller

[Jyri Sarha]

On 06/02/15 13:31, Peter Ujfalusi wrote:
> On 06/02/2015 11:58 AM, Jyri Sarha wrote:
>> Find the configured DMA controller by asking for a DMA channel in the
>> probe phase and releasing it right after. The controller device can be
>> found via the dma_chan struct and the controller is recognized from
>> the compatible property of its device node. The patch assumes EDMA if
>> there is no device node.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>   sound/soc/davinci/davinci-mcasp.c | 61 ++++++++++++++++++++++++++++++++-------
>>   1 file changed, 51 insertions(+), 10 deletions(-)
>>
>> diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
>> index d793494..5bc2712 100644
>> --- a/sound/soc/davinci/davinci-mcasp.c
>> +++ b/sound/soc/davinci/davinci-mcasp.c
>> @@ -1565,6 +1565,46 @@ static int davinci_mcasp_init_ch_constraints(struct davinci_mcasp *mcasp)
>>   	return ret;
>>   }
>>
>> +enum {
>> +	MCASP_EDMA = 1,
>
> Why start from 1?

This is just an implementation detail, for "if" statement below. Maybe I 
should restucture for better readability.

> I'm not sure about the name. It somehow implies that McASP has something to do
> with the dma engine.
> MCASP_SERVICED_BY_E/SDMA
> PLATFORM_USES_E/SDMA
>
> I can not come up with a decent name :(
>

What about PCM_EDMA/PCM_SDMA? This is a local definition after all.

>
>> +	MCASP_SDMA,
>> +};
>> +static const char *sdma_prefix = "ti,omap";
>> +
>> +static int davinci_mcasp_get_dma_type(struct davinci_mcasp *mcasp)
>> +{
>> +	struct dma_chan *chan;
>> +	const char *tmp;
>> +	int ret = MCASP_EDMA;
>> +
>> +	tmp = mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK].filter_data;
>> +	chan = dma_request_slave_channel_reason(mcasp->dev, tmp);
>
> this will return with error in case of !dev->of_node.
> The first check should be:
> if (!mcasp->dev->of_node)
> 	return MCASP_EDMA;
>
> since we can only boot in legacy mode with daVinci devices and they are using
> eDMA.
>
>> +	if (IS_ERR(chan)) {
>> +		if (PTR_ERR(chan) != -EPROBE_DEFER)
>> +			dev_err(mcasp->dev,
>> +				"Can't verify DMA configuration (%ld)\n",
>> +				PTR_ERR(chan));
>> +		return PTR_ERR(chan);
>> +	}
>> +	BUG_ON(!chan->device || !chan->device->dev);
>> +
>> +	if (chan->device->dev->of_node)
>> +		ret = of_property_read_string(chan->device->dev->of_node,
>> +					      "compatible", &tmp);
>> +	else
>> +		dev_dbg(mcasp->dev, "DMA controller has no of-node\n");
>
> Now this is not likely to happen...
>

Not having of_node happens on am335x, but not having compatible property 
is unlikely...

>> +
>> +	dma_release_channel(chan);
>> +	if (ret)
>> +		return ret;
>> +
>> +	dev_dbg(mcasp->dev, "DMA controller compatible = \"%s\"\n", tmp);
>> +	if (!strncmp(tmp, sdma_prefix, strlen(sdma_prefix)))
>> +		return MCASP_SDMA;
>> +
>> +	return MCASP_EDMA;
>> +}
>> +
>>   static int davinci_mcasp_probe(struct platform_device *pdev)
>>   {
>>   	struct snd_dmaengine_dai_dma_data *dma_data;
>> @@ -1763,27 +1803,28 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
>>   	if (ret != 0)
>>   		goto err;
>>
>> -	switch (mcasp->version) {
>> +	ret = -EINVAL;
>
> Why? What was the problem with setting the ret in the default case?
>
>> +	switch (davinci_mcasp_get_dma_type(mcasp)) {
>> +	case MCASP_EDMA:
>>   #if IS_BUILTIN(CONFIG_SND_EDMA_SOC) || \
>>   	(IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
>>   	 IS_MODULE(CONFIG_SND_EDMA_SOC))
>> -	case MCASP_VERSION_1:
>> -	case MCASP_VERSION_2:
>> -	case MCASP_VERSION_3:
>>   		ret = edma_pcm_platform_register(&pdev->dev);
>> -		break;
>> +#else
>> +		dev_err(&pdev->dev, "Missing SND_EDMA_SOC\n");
>>   #endif
>> +		break;
>> +	case MCASP_SDMA:
>>   #if IS_BUILTIN(CONFIG_SND_OMAP_SOC) || \
>>   	(IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
>>   	 IS_MODULE(CONFIG_SND_OMAP_SOC))
>> -	case MCASP_VERSION_4:
>>   		ret = omap_pcm_platform_register(&pdev->dev);
>> -		break;
>> +#else
>> +		dev_err(&pdev->dev, "Missing SND_SDMA_SOC\n");
>>   #endif
>> +		break;
>>   	default:
>> -		dev_err(&pdev->dev, "Invalid McASP version: %d\n",
>> -			mcasp->version);
>> -		ret = -EINVAL;
>> +		dev_err(&pdev->dev, "No DMA controller found\n");
>>   		break;
>>   	}
>>
>>
>
>

Re: [PATCH] ASoC: davinci-mcasp: Choose PCM driver based on configured DMA controller

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 309 bytes --]

On Tue, Jun 02, 2015 at 01:31:28PM +0300, Peter Ujfalusi wrote:
> On 06/02/2015 11:58 AM, Jyri Sarha wrote:

> > +enum {
> > +	MCASP_EDMA = 1,

> Why start from 1?

It's a fairly common style to ensure that zero initialised structures
don't have a default value.  Don't know if that's the intent here or
not.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] ASoC: davinci-mcasp: Choose PCM driver based on configured DMA controller

[Peter Ujfalusi]

On 06/02/2015 01:40 PM, Jyri Sarha wrote:
>>> +enum {
>>> +    MCASP_EDMA = 1,
>>
>> Why start from 1?
> 
> This is just an implementation detail, for "if" statement below. Maybe I
> should restucture for better readability.

No need to change if it makes the code flow better. I was just asking.

>> I'm not sure about the name. It somehow implies that McASP has something to do
>> with the dma engine.
>> MCASP_SERVICED_BY_E/SDMA
>> PLATFORM_USES_E/SDMA
>>
>> I can not come up with a decent name :(
>>
> 
> What about PCM_EDMA/PCM_SDMA? This is a local definition after all.

Sounds much better!

>>> +    MCASP_SDMA,
>>> +};
>>> +static const char *sdma_prefix = "ti,omap";
>>> +
>>> +static int davinci_mcasp_get_dma_type(struct davinci_mcasp *mcasp)
>>> +{
>>> +    struct dma_chan *chan;
>>> +    const char *tmp;
>>> +    int ret = MCASP_EDMA;
>>> +
>>> +    tmp = mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK].filter_data;
>>> +    chan = dma_request_slave_channel_reason(mcasp->dev, tmp);
>>
>> this will return with error in case of !dev->of_node.
>> The first check should be:
>> if (!mcasp->dev->of_node)
>>     return MCASP_EDMA;
>>
>> since we can only boot in legacy mode with daVinci devices and they are using
>> eDMA.
>>
>>> +    if (IS_ERR(chan)) {
>>> +        if (PTR_ERR(chan) != -EPROBE_DEFER)
>>> +            dev_err(mcasp->dev,
>>> +                "Can't verify DMA configuration (%ld)\n",
>>> +                PTR_ERR(chan));
>>> +        return PTR_ERR(chan);
>>> +    }
>>> +    BUG_ON(!chan->device || !chan->device->dev);
>>> +
>>> +    if (chan->device->dev->of_node)
>>> +        ret = of_property_read_string(chan->device->dev->of_node,
>>> +                          "compatible", &tmp);
>>> +    else
>>> +        dev_dbg(mcasp->dev, "DMA controller has no of-node\n");
>>
>> Now this is not likely to happen...
>>
> 
> Not having of_node happens on am335x, but not having compatible property is
> unlikely...

Yes, you are right. The edma dmaengine driver is not probed via DT, but the
arch/arm/common/edma.c will create a platform device for it to load, so it
does not have of_node.
On the other hand omap-dma is probed via DT.

> 
>>> +
>>> +    dma_release_channel(chan);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    dev_dbg(mcasp->dev, "DMA controller compatible = \"%s\"\n", tmp);
>>> +    if (!strncmp(tmp, sdma_prefix, strlen(sdma_prefix)))
>>> +        return MCASP_SDMA;
>>> +
>>> +    return MCASP_EDMA;
>>> +}
>>> +
>>>   static int davinci_mcasp_probe(struct platform_device *pdev)
>>>   {
>>>       struct snd_dmaengine_dai_dma_data *dma_data;
>>> @@ -1763,27 +1803,28 @@ static int davinci_mcasp_probe(struct
>>> platform_device *pdev)
>>>       if (ret != 0)
>>>           goto err;
>>>
>>> -    switch (mcasp->version) {
>>> +    ret = -EINVAL;
>>
>> Why? What was the problem with setting the ret in the default case?
>>
>>> +    switch (davinci_mcasp_get_dma_type(mcasp)) {
>>> +    case MCASP_EDMA:
>>>   #if IS_BUILTIN(CONFIG_SND_EDMA_SOC) || \
>>>       (IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
>>>        IS_MODULE(CONFIG_SND_EDMA_SOC))
>>> -    case MCASP_VERSION_1:
>>> -    case MCASP_VERSION_2:
>>> -    case MCASP_VERSION_3:
>>>           ret = edma_pcm_platform_register(&pdev->dev);
>>> -        break;
>>> +#else
>>> +        dev_err(&pdev->dev, "Missing SND_EDMA_SOC\n");
>>>   #endif
>>> +        break;
>>> +    case MCASP_SDMA:
>>>   #if IS_BUILTIN(CONFIG_SND_OMAP_SOC) || \
>>>       (IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
>>>        IS_MODULE(CONFIG_SND_OMAP_SOC))
>>> -    case MCASP_VERSION_4:
>>>           ret = omap_pcm_platform_register(&pdev->dev);
>>> -        break;
>>> +#else
>>> +        dev_err(&pdev->dev, "Missing SND_SDMA_SOC\n");
>>>   #endif
>>> +        break;
>>>       default:
>>> -        dev_err(&pdev->dev, "Invalid McASP version: %d\n",
>>> -            mcasp->version);
>>> -        ret = -EINVAL;
>>> +        dev_err(&pdev->dev, "No DMA controller found\n");
>>>           break;
>>>       }
>>>
>>>
>>
>>
> 


-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ASoC: davinci-mcasp: Choose PCM driver based on configured DMA controller

[Jyri Sarha]

I missed two comments first time around...

On 06/02/15 13:31, Peter Ujfalusi wrote:
> On 06/02/2015 11:58 AM, Jyri Sarha wrote:
>> Find the configured DMA controller by asking for a DMA channel in the
>> probe phase and releasing it right after. The controller device can be
>> found via the dma_chan struct and the controller is recognized from
>> the compatible property of its device node. The patch assumes EDMA if
>> there is no device node.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>   sound/soc/davinci/davinci-mcasp.c | 61 ++++++++++++++++++++++++++++++++-------
>>   1 file changed, 51 insertions(+), 10 deletions(-)
>>
>> diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
>> index d793494..5bc2712 100644
>> --- a/sound/soc/davinci/davinci-mcasp.c
>> +++ b/sound/soc/davinci/davinci-mcasp.c
>> @@ -1565,6 +1565,46 @@ static int davinci_mcasp_init_ch_constraints(struct davinci_mcasp *mcasp)
>>   	return ret;
>>   }
>>
>> +enum {
>> +	MCASP_EDMA = 1,
>
> Why start from 1?
> I'm not sure about the name. It somehow implies that McASP has something to do
> with the dma engine.
> MCASP_SERVICED_BY_E/SDMA
> PLATFORM_USES_E/SDMA
>
> I can not come up with a decent name :(
>
>
>> +	MCASP_SDMA,
>> +};
>> +static const char *sdma_prefix = "ti,omap";
>> +
>> +static int davinci_mcasp_get_dma_type(struct davinci_mcasp *mcasp)
>> +{
>> +	struct dma_chan *chan;
>> +	const char *tmp;
>> +	int ret = MCASP_EDMA;
>> +
>> +	tmp = mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK].filter_data;
>> +	chan = dma_request_slave_channel_reason(mcasp->dev, tmp);
>
> this will return with error in case of !dev->of_node.
> The first check should be:
> if (!mcasp->dev->of_node)
> 	return MCASP_EDMA;
>
> since we can only boot in legacy mode with daVinci devices and they are using
> eDMA.
>

Yep, I'll add that back. I tought that I would not need it, but am not 
so familiar with the generic dmaengine pcm...

>> +	if (IS_ERR(chan)) {
>> +		if (PTR_ERR(chan) != -EPROBE_DEFER)
>> +			dev_err(mcasp->dev,
>> +				"Can't verify DMA configuration (%ld)\n",
>> +				PTR_ERR(chan));
>> +		return PTR_ERR(chan);
>> +	}
>> +	BUG_ON(!chan->device || !chan->device->dev);
>> +
>> +	if (chan->device->dev->of_node)
>> +		ret = of_property_read_string(chan->device->dev->of_node,
>> +					      "compatible", &tmp);
>> +	else
>> +		dev_dbg(mcasp->dev, "DMA controller has no of-node\n");
>
> Now this is not likely to happen...
>
>> +
>> +	dma_release_channel(chan);
>> +	if (ret)
>> +		return ret;
>> +
>> +	dev_dbg(mcasp->dev, "DMA controller compatible = \"%s\"\n", tmp);
>> +	if (!strncmp(tmp, sdma_prefix, strlen(sdma_prefix)))
>> +		return MCASP_SDMA;
>> +
>> +	return MCASP_EDMA;
>> +}
>> +
>>   static int davinci_mcasp_probe(struct platform_device *pdev)
>>   {
>>   	struct snd_dmaengine_dai_dma_data *dma_data;
>> @@ -1763,27 +1803,28 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
>>   	if (ret != 0)
>>   		goto err;
>>
>> -	switch (mcasp->version) {
>> +	ret = -EINVAL;
>
> Why? What was the problem with setting the ret in the default case?
>

I would need to have that I three places now for the #else cases, when 
devicetree tries to configure pcm driver that has not been compiled.

>> +	switch (davinci_mcasp_get_dma_type(mcasp)) {
>> +	case MCASP_EDMA:
>>   #if IS_BUILTIN(CONFIG_SND_EDMA_SOC) || \
>>   	(IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
>>   	 IS_MODULE(CONFIG_SND_EDMA_SOC))
>> -	case MCASP_VERSION_1:
>> -	case MCASP_VERSION_2:
>> -	case MCASP_VERSION_3:
>>   		ret = edma_pcm_platform_register(&pdev->dev);
>> -		break;
>> +#else
>> +		dev_err(&pdev->dev, "Missing SND_EDMA_SOC\n");
>>   #endif
>> +		break;
>> +	case MCASP_SDMA:
>>   #if IS_BUILTIN(CONFIG_SND_OMAP_SOC) || \
>>   	(IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
>>   	 IS_MODULE(CONFIG_SND_OMAP_SOC))
>> -	case MCASP_VERSION_4:
>>   		ret = omap_pcm_platform_register(&pdev->dev);
>> -		break;
>> +#else
>> +		dev_err(&pdev->dev, "Missing SND_SDMA_SOC\n");
>>   #endif
>> +		break;
>>   	default:
>> -		dev_err(&pdev->dev, "Invalid McASP version: %d\n",
>> -			mcasp->version);
>> -		ret = -EINVAL;
>> +		dev_err(&pdev->dev, "No DMA controller found\n");
>>   		break;
>>   	}
>>
>>
>
>

Re: [PATCH] ASoC: davinci-mcasp: Choose PCM driver based on configured DMA controller

[Peter Ujfalusi]

On 06/02/2015 02:11 PM, Jyri Sarha wrote:
>>> -    switch (mcasp->version) {
>>> +    ret = -EINVAL;
>>
>> Why? What was the problem with setting the ret in the default case?
>>
> 
> I would need to have that I three places now for the #else cases, when
> devicetree tries to configure pcm driver that has not been compiled.

I see now, thanks for pointing out ;)

>>> +    switch (davinci_mcasp_get_dma_type(mcasp)) {
>>> +    case MCASP_EDMA:
>>>   #if IS_BUILTIN(CONFIG_SND_EDMA_SOC) || \
>>>       (IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
>>>        IS_MODULE(CONFIG_SND_EDMA_SOC))
>>> -    case MCASP_VERSION_1:
>>> -    case MCASP_VERSION_2:
>>> -    case MCASP_VERSION_3:
>>>           ret = edma_pcm_platform_register(&pdev->dev);
>>> -        break;
>>> +#else
>>> +        dev_err(&pdev->dev, "Missing SND_EDMA_SOC\n");
>>>   #endif
>>> +        break;
>>> +    case MCASP_SDMA:
>>>   #if IS_BUILTIN(CONFIG_SND_OMAP_SOC) || \
>>>       (IS_MODULE(CONFIG_SND_DAVINCI_SOC_MCASP) && \
>>>        IS_MODULE(CONFIG_SND_OMAP_SOC))
>>> -    case MCASP_VERSION_4:
>>>           ret = omap_pcm_platform_register(&pdev->dev);
>>> -        break;
>>> +#else
>>> +        dev_err(&pdev->dev, "Missing SND_SDMA_SOC\n");
>>>   #endif
>>> +        break;
>>>       default:
>>> -        dev_err(&pdev->dev, "Invalid McASP version: %d\n",
>>> -            mcasp->version);
>>> -        ret = -EINVAL;
>>> +        dev_err(&pdev->dev, "No DMA controller found\n");
>>>           break;
>>>       }

-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html