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 <jsa...@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;
        }





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

Reply via email to