On 04/18/16 12:27, Petr Kulhavy wrote:
static int davinci_i2s_probe(struct platform_device *pdev) {
- struct snd_dmaengine_dai_dma_data *dma_data;
- struct snd_platform_data *pdata; struct davinci_mcbsp_dev *dev; struct resource *mem, *res; void __iomem *io_base; int *dma; int ret;
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pdata = davinci_i2s_set_pdata_from_of(pdev);
- if (IS_ERR(pdata)) {
dev_err(&pdev->dev, "Error populating platform data, err %ld\n",PTR_ERR(pdata));return PTR_ERR(pdata);- }
The current driver does not use the pdata as far as I can see. Kind of strange. So now in DT boot you will allocate memory for the pdata, which is not used by the driver at all. I think we should implement the pdata handling in the driver first for the legacy mode, then probably having pdata allocated for DT case might make sense.
Indeed the current driver does not use pdata. Neither the legacy board drivers in arch/mach-davinci do. Well, except for the dm365_evm which sets the asp_chan_q which is later not used:
static struct snd_platform_data dm365_evm_snd_data __maybe_unused = { .asp_chan_q = EVENTQ_3, };
If the pdata is not used at all I would completely drop it.
the asp_chan_q was used with the old davinci_pcm which has been removed a while ago.
Or what do you think the pdata should carry?
Good question. Since the driver itself does not care about pdata I would remove it. On the other hand if you implement the FIFO handling you will need pdata for legacy mode to select the FIFO threshold. Currently there is one pdata structure for legacy mode used by the McASP, McBSP and the Voice Codec. It contains mixed options applicable for all and some which applicable only for one of the drivers... I think if we want to make things clean and neat we would aim to have separate pdata for each of the drivers.
I would probably drop the current pdata for McBSP and when I see what data I need for the driver later I would create one for the McBSP only.