Message ID | 1699595289-25773-1-git-send-email-shengjiu.wang@nxp.com |
---|---|
Headers | show |
Series | Add audio support in v4l2 framework | expand |
On 10/11/2023 06:48, Shengjiu Wang wrote: > +static int asrc_m2m_probe(struct platform_device *pdev) > +{ > + struct fsl_asrc_m2m_pdata *data = pdev->dev.platform_data; > + struct device *dev = &pdev->dev; > + struct asrc_m2m *m2m; > + int ret; > + > + m2m = devm_kzalloc(dev, sizeof(struct asrc_m2m), GFP_KERNEL); sizeof(*) > + if (!m2m) > + return -ENOMEM; > + > + m2m->pdata = *data; > + m2m->pdev = pdev; > + > + ret = v4l2_device_register(dev, &m2m->v4l2_dev); > + if (ret) { > + dev_err(dev, "failed to register v4l2 device\n"); > + goto err_register; > + } > + > + m2m->m2m_dev = v4l2_m2m_init(&asrc_m2m_ops); > + if (IS_ERR(m2m->m2m_dev)) { > + dev_err(dev, "failed to register v4l2 device\n"); Why aren't you using dev_err_probe() at all? > + ret = PTR_ERR(m2m->m2m_dev); > + goto err_m2m; > + } > + > + m2m->dec_vdev = video_device_alloc(); > + if (!m2m->dec_vdev) { > + dev_err(dev, "failed to register v4l2 device\n"); Why do you print errors on ENOMEM? Did you run coccinelle? > + ret = -ENOMEM; > + goto err_vdev_alloc; > + } > + > + mutex_init(&m2m->mlock); > + > + m2m->dec_vdev->fops = &asrc_m2m_fops; > + m2m->dec_vdev->ioctl_ops = &asrc_m2m_ioctl_ops; > + m2m->dec_vdev->minor = -1; > + m2m->dec_vdev->release = video_device_release; > + m2m->dec_vdev->lock = &m2m->mlock; /* lock for ioctl serialization */ > + m2m->dec_vdev->v4l2_dev = &m2m->v4l2_dev; > + m2m->dec_vdev->vfl_dir = VFL_DIR_M2M; > + m2m->dec_vdev->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_AUDIO_M2M; > + > +#ifdef CONFIG_MEDIA_CONTROLLER > + m2m->mdev.dev = &pdev->dev; > + strscpy(m2m->mdev.model, M2M_DRV_NAME, sizeof(m2m->mdev.model)); > + snprintf(m2m->mdev.bus_info, sizeof(m2m->mdev.bus_info), > + "platform:%s", M2M_DRV_NAME); > + media_device_init(&m2m->mdev); > + m2m->mdev.ops = &asrc_m2m_media_ops; > + m2m->v4l2_dev.mdev = &m2m->mdev; > +#endif > + > + ret = video_register_device(m2m->dec_vdev, VFL_TYPE_AUDIO, -1); > + if (ret) { > + dev_err(dev, "failed to register video device\n"); > + goto err_vdev_register; > + } > + > +#ifdef CONFIG_MEDIA_CONTROLLER > + ret = v4l2_m2m_register_media_controller(m2m->m2m_dev, m2m->dec_vdev, > + MEDIA_ENT_F_PROC_AUDIO_RESAMPLER); > + if (ret) { > + dev_err(dev, "Failed to init mem2mem media controller\n"); > + goto error_v4l2; > + } > + > + ret = media_device_register(&m2m->mdev); > + if (ret) { > + dev_err(dev, "Failed to register mem2mem media device\n"); > + goto error_m2m_mc; > + } > +#endif > + > + video_set_drvdata(m2m->dec_vdev, m2m); > + platform_set_drvdata(pdev, m2m); > + pm_runtime_enable(&pdev->dev); > + > + return 0; > + > +#ifdef CONFIG_MEDIA_CONTROLLER > +error_m2m_mc: > + v4l2_m2m_unregister_media_controller(m2m->m2m_dev); > +#endif > +error_v4l2: > + video_unregister_device(m2m->dec_vdev); > +err_vdev_register: > + video_device_release(m2m->dec_vdev); > +err_vdev_alloc: > + v4l2_m2m_release(m2m->m2m_dev); > +err_m2m: > + v4l2_device_unregister(&m2m->v4l2_dev); > +err_register: > + return ret; > +} > + > +static void asrc_m2m_remove(struct platform_device *pdev) > +{ > + struct asrc_m2m *m2m = platform_get_drvdata(pdev); > + > + pm_runtime_disable(&pdev->dev); > +#ifdef CONFIG_MEDIA_CONTROLLER > + media_device_unregister(&m2m->mdev); > + v4l2_m2m_unregister_media_controller(m2m->m2m_dev); > +#endif > + video_unregister_device(m2m->dec_vdev); > + video_device_release(m2m->dec_vdev); > + v4l2_m2m_release(m2m->m2m_dev); > + v4l2_device_unregister(&m2m->v4l2_dev); > +} > + > +#ifdef CONFIG_PM_SLEEP > +/* suspend callback for m2m */ > +static int asrc_m2m_suspend(struct device *dev) > +{ > + struct asrc_m2m *m2m = dev_get_drvdata(dev); > + struct fsl_asrc *asrc = m2m->pdata.asrc; > + struct fsl_asrc_pair *pair; > + unsigned long lock_flags; > + int i; > + > + for (i = 0; i < PAIR_CTX_NUM; i++) { > + spin_lock_irqsave(&asrc->lock, lock_flags); > + pair = asrc->pair[i]; > + if (!pair || !pair->req_pair) { > + spin_unlock_irqrestore(&asrc->lock, lock_flags); > + continue; > + } > + if (!completion_done(&pair->complete[V4L_OUT])) { > + if (pair->dma_chan[V4L_OUT]) > + dmaengine_terminate_all(pair->dma_chan[V4L_OUT]); > + asrc_input_dma_callback((void *)pair); > + } > + if (!completion_done(&pair->complete[V4L_CAP])) { > + if (pair->dma_chan[V4L_CAP]) > + dmaengine_terminate_all(pair->dma_chan[V4L_CAP]); > + asrc_output_dma_callback((void *)pair); > + } > + > + if (asrc->m2m_pair_suspend) > + asrc->m2m_pair_suspend(pair); > + > + spin_unlock_irqrestore(&asrc->lock, lock_flags); > + } > + > + return 0; > +} > + > +static int asrc_m2m_resume(struct device *dev) > +{ > + struct asrc_m2m *m2m = dev_get_drvdata(dev); > + struct fsl_asrc *asrc = m2m->pdata.asrc; > + struct fsl_asrc_pair *pair; > + unsigned long lock_flags; > + int i; > + > + for (i = 0; i < PAIR_CTX_NUM; i++) { > + spin_lock_irqsave(&asrc->lock, lock_flags); > + pair = asrc->pair[i]; > + if (!pair || !pair->req_pair) { > + spin_unlock_irqrestore(&asrc->lock, lock_flags); > + continue; > + } > + if (asrc->m2m_pair_resume) > + asrc->m2m_pair_resume(pair); > + > + spin_unlock_irqrestore(&asrc->lock, lock_flags); > + } > + > + return 0; > +} > +#endif > + > +static const struct dev_pm_ops asrc_m2m_pm_ops = { > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(asrc_m2m_suspend, > + asrc_m2m_resume) > +}; > + > +static struct platform_driver asrc_m2m_driver = { > + .probe = asrc_m2m_probe, > + .remove_new = asrc_m2m_remove, > + .driver = { > + .name = M2M_DRV_NAME, > + .pm = &asrc_m2m_pm_ops, > + }, > +}; > +module_platform_driver(asrc_m2m_driver); > + > +MODULE_DESCRIPTION("Freescale ASRC M2M driver"); > +MODULE_ALIAS("platform:" M2M_DRV_NAME); You should not need MODULE_ALIAS() in normal cases. If you need it, usually it means your device ID table is wrong (e.g. misses either entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute for incomplete ID table. > +MODULE_LICENSE("GPL"); Best regards, Krzysztof
On 16/11/2023 09:32, Shengjiu Wang wrote: >>> +MODULE_DESCRIPTION("Freescale ASRC M2M driver"); >>> +MODULE_ALIAS("platform:" M2M_DRV_NAME); >> >> You should not need MODULE_ALIAS() in normal cases. If you need it, >> usually it means your device ID table is wrong (e.g. misses either >> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute >> for incomplete ID table. > > > This driver don't have MODULE_DEVICE_TABLE. it is only registered > by platform_device_register_data(). Which is the problem. I thought I made myself clear but if it is not the case: drop MODULE_ALIAS. Best regards, Krzysztof