Message ID | 20190206151328.21629-1-rui.silva@linaro.org |
---|---|
Headers | show |
Series | media: staging/imx7: add i.MX7 media driver | expand |
On 2/6/19 4:13 PM, Rui Miguel Silva wrote: > Refactor and move media device initialization code to a new common > module, so it can be used by other devices, this will allow for example > a near to introduce imx7 CSI driver, to use this media device. > > Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org> > --- > drivers/staging/media/imx/Makefile | 1 + > .../staging/media/imx/imx-media-dev-common.c | 90 +++++++++++++++++++ > drivers/staging/media/imx/imx-media-dev.c | 86 ++++-------------- > drivers/staging/media/imx/imx-media-of.c | 6 +- > drivers/staging/media/imx/imx-media.h | 14 +++ > 5 files changed, 127 insertions(+), 70 deletions(-) > create mode 100644 drivers/staging/media/imx/imx-media-dev-common.c > > diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile > index 698a4210316e..a30b3033f9a3 100644 > --- a/drivers/staging/media/imx/Makefile > +++ b/drivers/staging/media/imx/Makefile > @@ -1,5 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > imx-media-objs := imx-media-dev.o imx-media-internal-sd.o imx-media-of.o > +imx-media-objs += imx-media-dev-common.o > imx-media-common-objs := imx-media-utils.o imx-media-fim.o > imx-media-ic-objs := imx-ic-common.o imx-ic-prp.o imx-ic-prpencvf.o > > diff --git a/drivers/staging/media/imx/imx-media-dev-common.c b/drivers/staging/media/imx/imx-media-dev-common.c > new file mode 100644 > index 000000000000..28bcb31cf6ca > --- /dev/null > +++ b/drivers/staging/media/imx/imx-media-dev-common.c > @@ -0,0 +1,90 @@ > +// SPDX-License-Identifier: GPL WARNING: 'SPDX-License-Identifier: GPL' is not supported in LICENSES/... #18: FILE: drivers/staging/media/imx/imx-media-dev-common.c:1: +// SPDX-License-Identifier: GPL You no doubt mean GPL-2.0. Just repost this patch with this change. Regards, Hans > +/* > + * V4L2 Media Controller Driver for Freescale common i.MX5/6/7 SOC > + * > + * Copyright (c) 2019 Linaro Ltd > + * Copyright (c) 2016 Mentor Graphics Inc. > + */ > + > +#include <linux/of_graph.h> > +#include <linux/of_platform.h> > +#include "imx-media.h" > + > +static const struct v4l2_async_notifier_operations imx_media_subdev_ops = { > + .bound = imx_media_subdev_bound, > + .complete = imx_media_probe_complete, > +}; > + > +static const struct media_device_ops imx_media_md_ops = { > + .link_notify = imx_media_link_notify, > +}; > + > +struct imx_media_dev *imx_media_dev_init(struct device *dev) > +{ > + struct imx_media_dev *imxmd; > + int ret; > + > + imxmd = devm_kzalloc(dev, sizeof(*imxmd), GFP_KERNEL); > + if (!imxmd) > + return ERR_PTR(-ENOMEM); > + > + dev_set_drvdata(dev, imxmd); > + > + strlcpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model)); > + imxmd->md.ops = &imx_media_md_ops; > + imxmd->md.dev = dev; > + > + mutex_init(&imxmd->mutex); > + > + imxmd->v4l2_dev.mdev = &imxmd->md; > + imxmd->v4l2_dev.notify = imx_media_notify; > + strlcpy(imxmd->v4l2_dev.name, "imx-media", > + sizeof(imxmd->v4l2_dev.name)); > + > + media_device_init(&imxmd->md); > + > + ret = v4l2_device_register(dev, &imxmd->v4l2_dev); > + if (ret < 0) { > + v4l2_err(&imxmd->v4l2_dev, > + "Failed to register v4l2_device: %d\n", ret); > + goto cleanup; > + } > + > + dev_set_drvdata(imxmd->v4l2_dev.dev, imxmd); > + > + INIT_LIST_HEAD(&imxmd->vdev_list); > + > + v4l2_async_notifier_init(&imxmd->notifier); > + > + return imxmd; > + > +cleanup: > + media_device_cleanup(&imxmd->md); > + > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL_GPL(imx_media_dev_init); > + > +int imx_media_dev_notifier_register(struct imx_media_dev *imxmd) > +{ > + int ret; > + > + /* no subdevs? just bail */ > + if (list_empty(&imxmd->notifier.asd_list)) { > + v4l2_err(&imxmd->v4l2_dev, "no subdevs\n"); > + return -ENODEV; > + } > + > + /* prepare the async subdev notifier and register it */ > + imxmd->notifier.ops = &imx_media_subdev_ops; > + ret = v4l2_async_notifier_register(&imxmd->v4l2_dev, > + &imxmd->notifier); > + if (ret) { > + v4l2_err(&imxmd->v4l2_dev, > + "v4l2_async_notifier_register failed with %d\n", ret); > + return ret; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(imx_media_dev_notifier_register); > diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c > index 25e916562c66..c42bddd78906 100644 > --- a/drivers/staging/media/imx/imx-media-dev.c > +++ b/drivers/staging/media/imx/imx-media-dev.c > @@ -116,9 +116,9 @@ static int imx_media_get_ipu(struct imx_media_dev *imxmd, > } > > /* async subdev bound notifier */ > -static int imx_media_subdev_bound(struct v4l2_async_notifier *notifier, > - struct v4l2_subdev *sd, > - struct v4l2_async_subdev *asd) > +int imx_media_subdev_bound(struct v4l2_async_notifier *notifier, > + struct v4l2_subdev *sd, > + struct v4l2_async_subdev *asd) > { > struct imx_media_dev *imxmd = notifier2dev(notifier); > int ret = 0; > @@ -302,7 +302,7 @@ static int imx_media_create_pad_vdev_lists(struct imx_media_dev *imxmd) > } > > /* async subdev complete notifier */ > -static int imx_media_probe_complete(struct v4l2_async_notifier *notifier) > +int imx_media_probe_complete(struct v4l2_async_notifier *notifier) > { > struct imx_media_dev *imxmd = notifier2dev(notifier); > int ret; > @@ -326,11 +326,6 @@ static int imx_media_probe_complete(struct v4l2_async_notifier *notifier) > return media_device_register(&imxmd->md); > } > > -static const struct v4l2_async_notifier_operations imx_media_subdev_ops = { > - .bound = imx_media_subdev_bound, > - .complete = imx_media_probe_complete, > -}; > - > /* > * adds controls to a video device from an entity subdevice. > * Continues upstream from the entity's sink pads. > @@ -374,8 +369,8 @@ static int imx_media_inherit_controls(struct imx_media_dev *imxmd, > return ret; > } > > -static int imx_media_link_notify(struct media_link *link, u32 flags, > - unsigned int notification) > +int imx_media_link_notify(struct media_link *link, u32 flags, > + unsigned int notification) > { > struct media_entity *source = link->source->entity; > struct imx_media_pad_vdev *pad_vdev; > @@ -438,13 +433,8 @@ static int imx_media_link_notify(struct media_link *link, u32 flags, > return ret; > } > > -static const struct media_device_ops imx_media_md_ops = { > - .link_notify = imx_media_link_notify, > -}; > - > -static void imx_media_notify(struct v4l2_subdev *sd, > - unsigned int notification, > - void *arg) > +void imx_media_notify(struct v4l2_subdev *sd, unsigned int notification, > + void *arg) > { > struct media_entity *entity = &sd->entity; > int i; > @@ -472,77 +462,37 @@ static int imx_media_probe(struct platform_device *pdev) > struct imx_media_dev *imxmd; > int ret; > > - imxmd = devm_kzalloc(dev, sizeof(*imxmd), GFP_KERNEL); > - if (!imxmd) > - return -ENOMEM; > - > - dev_set_drvdata(dev, imxmd); > - > - strscpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model)); > - imxmd->md.ops = &imx_media_md_ops; > - imxmd->md.dev = dev; > - > - mutex_init(&imxmd->mutex); > - > - imxmd->v4l2_dev.mdev = &imxmd->md; > - imxmd->v4l2_dev.notify = imx_media_notify; > - strscpy(imxmd->v4l2_dev.name, "imx-media", > - sizeof(imxmd->v4l2_dev.name)); > - > - media_device_init(&imxmd->md); > - > - ret = v4l2_device_register(dev, &imxmd->v4l2_dev); > - if (ret < 0) { > - v4l2_err(&imxmd->v4l2_dev, > - "Failed to register v4l2_device: %d\n", ret); > - goto cleanup; > - } > - > - dev_set_drvdata(imxmd->v4l2_dev.dev, imxmd); > - > - INIT_LIST_HEAD(&imxmd->vdev_list); > - > - v4l2_async_notifier_init(&imxmd->notifier); > + imxmd = imx_media_dev_init(dev); > + if (IS_ERR(imxmd)) > + return PTR_ERR(imxmd); > > ret = imx_media_add_of_subdevs(imxmd, node); > if (ret) { > v4l2_err(&imxmd->v4l2_dev, > "add_of_subdevs failed with %d\n", ret); > - goto notifier_cleanup; > + goto cleanup; > } > > ret = imx_media_add_internal_subdevs(imxmd); > if (ret) { > v4l2_err(&imxmd->v4l2_dev, > "add_internal_subdevs failed with %d\n", ret); > - goto notifier_cleanup; > - } > - > - /* no subdevs? just bail */ > - if (list_empty(&imxmd->notifier.asd_list)) { > - ret = -ENODEV; > - goto notifier_cleanup; > + goto cleanup; > } > > - /* prepare the async subdev notifier and register it */ > - imxmd->notifier.ops = &imx_media_subdev_ops; > - ret = v4l2_async_notifier_register(&imxmd->v4l2_dev, > - &imxmd->notifier); > - if (ret) { > - v4l2_err(&imxmd->v4l2_dev, > - "v4l2_async_notifier_register failed with %d\n", ret); > + ret = imx_media_dev_notifier_register(imxmd); > + if (ret) > goto del_int; > - } > > return 0; > > del_int: > imx_media_remove_internal_subdevs(imxmd); > -notifier_cleanup: > +cleanup: > v4l2_async_notifier_cleanup(&imxmd->notifier); > v4l2_device_unregister(&imxmd->v4l2_dev); > -cleanup: > media_device_cleanup(&imxmd->md); > + > return ret; > } > > @@ -556,8 +506,8 @@ static int imx_media_remove(struct platform_device *pdev) > v4l2_async_notifier_unregister(&imxmd->notifier); > imx_media_remove_internal_subdevs(imxmd); > v4l2_async_notifier_cleanup(&imxmd->notifier); > - v4l2_device_unregister(&imxmd->v4l2_dev); > media_device_unregister(&imxmd->md); > + v4l2_device_unregister(&imxmd->v4l2_dev); > media_device_cleanup(&imxmd->md); > > return 0; > diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c > index a01327f6e045..03446335ac03 100644 > --- a/drivers/staging/media/imx/imx-media-of.c > +++ b/drivers/staging/media/imx/imx-media-of.c > @@ -20,7 +20,8 @@ > #include <video/imx-ipu-v3.h> > #include "imx-media.h" > > -static int of_add_csi(struct imx_media_dev *imxmd, struct device_node *csi_np) > +int imx_media_of_add_csi(struct imx_media_dev *imxmd, > + struct device_node *csi_np) > { > int ret; > > @@ -45,6 +46,7 @@ static int of_add_csi(struct imx_media_dev *imxmd, struct device_node *csi_np) > > return 0; > } > +EXPORT_SYMBOL_GPL(imx_media_of_add_csi); > > int imx_media_add_of_subdevs(struct imx_media_dev *imxmd, > struct device_node *np) > @@ -57,7 +59,7 @@ int imx_media_add_of_subdevs(struct imx_media_dev *imxmd, > if (!csi_np) > break; > > - ret = of_add_csi(imxmd, csi_np); > + ret = imx_media_of_add_csi(imxmd, csi_np); > of_node_put(csi_np); > if (ret) > return ret; > diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h > index 7a0e658753f0..0d1c6bbf1109 100644 > --- a/drivers/staging/media/imx/imx-media.h > +++ b/drivers/staging/media/imx/imx-media.h > @@ -228,6 +228,18 @@ int imx_media_add_async_subdev(struct imx_media_dev *imxmd, > struct fwnode_handle *fwnode, > struct platform_device *pdev); > > +int imx_media_subdev_bound(struct v4l2_async_notifier *notifier, > + struct v4l2_subdev *sd, > + struct v4l2_async_subdev *asd); > +int imx_media_link_notify(struct media_link *link, u32 flags, > + unsigned int notification); > +void imx_media_notify(struct v4l2_subdev *sd, unsigned int notification, > + void *arg); > +int imx_media_probe_complete(struct v4l2_async_notifier *notifier); > + > +struct imx_media_dev *imx_media_dev_init(struct device *dev); > +int imx_media_dev_notifier_register(struct imx_media_dev *imxmd); > + > /* imx-media-fim.c */ > struct imx_media_fim; > void imx_media_fim_eof_monitor(struct imx_media_fim *fim, ktime_t timestamp); > @@ -251,6 +263,8 @@ int imx_media_create_of_links(struct imx_media_dev *imxmd, > struct v4l2_subdev *sd); > int imx_media_create_csi_of_links(struct imx_media_dev *imxmd, > struct v4l2_subdev *csi); > +int imx_media_of_add_csi(struct imx_media_dev *imxmd, > + struct device_node *csi_np); > > /* imx-media-capture.c */ > struct imx_media_video_dev * >
On 2/6/19 4:13 PM, Rui Miguel Silva wrote: > Adds MIPI CSI-2 subdev for i.MX7 to connect with sensors with a MIPI > CSI-2 interface. > > Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org> > --- > drivers/staging/media/imx/Makefile | 1 + > drivers/staging/media/imx/imx7-mipi-csis.c | 1186 ++++++++++++++++++++ > 2 files changed, 1187 insertions(+) > create mode 100644 drivers/staging/media/imx/imx7-mipi-csis.c > > diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile > index 074f016d3519..d2d909a36239 100644 > --- a/drivers/staging/media/imx/Makefile > +++ b/drivers/staging/media/imx/Makefile > @@ -14,3 +14,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o > obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o > > obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o > +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > new file mode 100644 > index 000000000000..516d308dc44b > --- /dev/null > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > @@ -0,0 +1,1186 @@ <snip> > +static int mipi_csi_registered(struct v4l2_subdev *mipi_sd) > +{ > + struct csi_state *state = mipi_sd_to_csis_state(mipi_sd); > + unsigned int i; > + int ret; > + > + for (i = 0; i < CSIS_PADS_NUM; i++) { > + state->pads[i].flags = (i == CSIS_PAD_SINK) ? > + MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE; > + } > + > + /* set a default mbus format */ > + ret = imx_media_init_mbus_fmt(&state->format_mbus, > + MIPI_CSIS_DEF_PIX_HEIGHT, > + MIPI_CSIS_DEF_PIX_WIDTH, 0, > + V4L2_FIELD_NONE, NULL); > + if (ret) > + return ret; > + > + return media_entity_pads_init(&mipi_sd->entity, CSIS_PADS_NUM, > + state->pads); > +} > + > +static const struct v4l2_subdev_core_ops mipi_csis_core_ops = { > + .log_status = mipi_csis_log_status, > +}; > + > +static const struct media_entity_operations mipi_csis_entity_ops = { > + .link_setup = mipi_csis_link_setup, > + .link_validate = v4l2_subdev_link_validate, > +}; > + > +static const struct v4l2_subdev_video_ops mipi_csis_video_ops = { > + .s_stream = mipi_csis_s_stream, > +}; > + > +static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = { > + .init_cfg = mipi_csis_init_cfg, > + .get_fmt = mipi_csis_get_fmt, > + .set_fmt = mipi_csis_set_fmt, > +}; > + > +static const struct v4l2_subdev_ops mipi_csis_subdev_ops = { > + .core = &mipi_csis_core_ops, > + .video = &mipi_csis_video_ops, > + .pad = &mipi_csis_pad_ops, > +}; > + > +static const struct v4l2_subdev_internal_ops mipi_csis_internal_ops = { > + .registered = mipi_csi_registered, > +}; This struct is not used, and therefor mipi_csi_registered() is never called either. Should it be called or can this be removed? Regards, Hans
Hi Hans, On Wed 20 Feb 2019 at 08:56, Hans Verkuil wrote: > On 2/6/19 4:13 PM, Rui Miguel Silva wrote: >> Adds MIPI CSI-2 subdev for i.MX7 to connect with sensors with a >> MIPI >> CSI-2 interface. >> >> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org> >> --- >> drivers/staging/media/imx/Makefile | 1 + >> drivers/staging/media/imx/imx7-mipi-csis.c | 1186 >> ++++++++++++++++++++ >> 2 files changed, 1187 insertions(+) >> create mode 100644 drivers/staging/media/imx/imx7-mipi-csis.c >> >> diff --git a/drivers/staging/media/imx/Makefile >> b/drivers/staging/media/imx/Makefile >> index 074f016d3519..d2d909a36239 100644 >> --- a/drivers/staging/media/imx/Makefile >> +++ b/drivers/staging/media/imx/Makefile >> @@ -14,3 +14,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += >> imx-media-csi.o >> obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o >> >> obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o >> +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o >> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c >> b/drivers/staging/media/imx/imx7-mipi-csis.c >> new file mode 100644 >> index 000000000000..516d308dc44b >> --- /dev/null >> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c >> @@ -0,0 +1,1186 @@ > > <snip> > >> +static int mipi_csi_registered(struct v4l2_subdev *mipi_sd) >> +{ >> + struct csi_state *state = mipi_sd_to_csis_state(mipi_sd); >> + unsigned int i; >> + int ret; >> + >> + for (i = 0; i < CSIS_PADS_NUM; i++) { >> + state->pads[i].flags = (i == CSIS_PAD_SINK) ? >> + MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE; >> + } >> + >> + /* set a default mbus format */ >> + ret = imx_media_init_mbus_fmt(&state->format_mbus, >> + MIPI_CSIS_DEF_PIX_HEIGHT, >> + MIPI_CSIS_DEF_PIX_WIDTH, 0, >> + V4L2_FIELD_NONE, NULL); >> + if (ret) >> + return ret; >> + >> + return media_entity_pads_init(&mipi_sd->entity, >> CSIS_PADS_NUM, >> + state->pads); >> +} >> + >> +static const struct v4l2_subdev_core_ops mipi_csis_core_ops = >> { >> + .log_status = mipi_csis_log_status, >> +}; >> + >> +static const struct media_entity_operations >> mipi_csis_entity_ops = { >> + .link_setup = mipi_csis_link_setup, >> + .link_validate = v4l2_subdev_link_validate, >> +}; >> + >> +static const struct v4l2_subdev_video_ops mipi_csis_video_ops >> = { >> + .s_stream = mipi_csis_s_stream, >> +}; >> + >> +static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = { >> + .init_cfg = mipi_csis_init_cfg, >> + .get_fmt = mipi_csis_get_fmt, >> + .set_fmt = mipi_csis_set_fmt, >> +}; >> + >> +static const struct v4l2_subdev_ops mipi_csis_subdev_ops = { >> + .core = &mipi_csis_core_ops, >> + .video = &mipi_csis_video_ops, >> + .pad = &mipi_csis_pad_ops, >> +}; >> + >> +static const struct v4l2_subdev_internal_ops >> mipi_csis_internal_ops = { >> + .registered = mipi_csi_registered, >> +}; > > This struct is not used, and therefor mipi_csi_registered() is > never called > either. Should it be called or can this be removed? Good question :), I will get back to you on this one later. --- Cheers, Rui