lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220223163213.rypgz2tzfq73mbtx@uno.localdomain> Date: Wed, 23 Feb 2022 17:32:13 +0100 From: Jacopo Mondi <jacopo@...ndi.org> To: Eugen Hristev <eugen.hristev@...rochip.com> Cc: linux-media@...r.kernel.org, hverkuil-cisco@...all.nl, nicolas.ferre@...rochip.com, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, claudiu.beznea@...rochip.com Subject: Re: [PATCH v5 04/13] media: atmel: atmel-isc: implement media controller Hi Eugen On Thu, Feb 17, 2022 at 03:56:36PM +0200, Eugen Hristev wrote: > Implement the support for media-controller. > This means that the capabilities of the driver have changed and now > it also advertises the IO_MC . > The driver will register it's media device, and add the video entity to this s/it's/its > media device. The subdevices are registered to the same media device. > The ISC will have a base entity which is auto-detected as atmel_isc_base. > It will also register a subdevice that allows cropping of the incoming frame > to the maximum frame size supported by the ISC. > The ISC will create a link between the subdevice that is asynchronously > registered and the atmel_isc_scaler entity. > Then, the atmel_isc_scaler and atmel_isc_base are connected through another > link. > > Signed-off-by: Eugen Hristev <eugen.hristev@...rochip.com> > --- > Changes in v5: > - reworked s_fmt to pass the same format from sink to source > - simplified enum_mbus_code > - separated tgt and bounds to report correctly in g_sel > > Changes in v4: > As suggested by Jacopo: > - renamed atmel_isc_mc to atmel_isc_scaler.c > - moved init_mc/clean_mc to isc_base file > > Changes in v2: > - implement try formats > > drivers/media/platform/atmel/Makefile | 2 +- > drivers/media/platform/atmel/atmel-isc-base.c | 73 ++++- > .../media/platform/atmel/atmel-isc-scaler.c | 261 ++++++++++++++++++ > drivers/media/platform/atmel/atmel-isc.h | 40 +++ > .../media/platform/atmel/atmel-sama5d2-isc.c | 14 +- > .../media/platform/atmel/atmel-sama7g5-isc.c | 12 +- > 6 files changed, 394 insertions(+), 8 deletions(-) > create mode 100644 drivers/media/platform/atmel/atmel-isc-scaler.c > > diff --git a/drivers/media/platform/atmel/Makefile b/drivers/media/platform/atmel/Makefile > index 794e8f739287..f02d03df89d6 100644 > --- a/drivers/media/platform/atmel/Makefile > +++ b/drivers/media/platform/atmel/Makefile > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0-only > atmel-isc-objs = atmel-sama5d2-isc.o > atmel-xisc-objs = atmel-sama7g5-isc.o > -atmel-isc-common-objs = atmel-isc-base.o atmel-isc-clk.o > +atmel-isc-common-objs = atmel-isc-base.o atmel-isc-clk.o atmel-isc-scaler.o > > obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel-isi.o > obj-$(CONFIG_VIDEO_ATMEL_ISC_BASE) += atmel-isc-common.o > diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c > index 67b4a2323fed..74f575544e09 100644 > --- a/drivers/media/platform/atmel/atmel-isc-base.c > +++ b/drivers/media/platform/atmel/atmel-isc-base.c > @@ -1712,6 +1712,7 @@ static int isc_async_bound(struct v4l2_async_notifier *notifier, > struct isc_device, v4l2_dev); > struct isc_subdev_entity *subdev_entity = > container_of(notifier, struct isc_subdev_entity, notifier); > + int pad; > > if (video_is_registered(&isc->video_dev)) { > v4l2_err(&isc->v4l2_dev, "only supports one sub-device.\n"); > @@ -1720,6 +1721,16 @@ static int isc_async_bound(struct v4l2_async_notifier *notifier, > > subdev_entity->sd = subdev; > > + pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode, > + MEDIA_PAD_FL_SOURCE); > + if (pad < 0) { > + v4l2_err(&isc->v4l2_dev, "failed to find pad for %s\n", > + subdev->name); > + return pad; > + } > + > + isc->remote_pad = pad; > + > return 0; > } > > @@ -1734,8 +1745,8 @@ static void isc_async_unbind(struct v4l2_async_notifier *notifier, > v4l2_ctrl_handler_free(&isc->ctrls.handler); > } > > -static struct isc_format *find_format_by_code(struct isc_device *isc, > - unsigned int code, int *index) > +struct isc_format *isc_find_format_by_code(struct isc_device *isc, > + unsigned int code, int *index) > { > struct isc_format *fmt = &isc->formats_list[0]; > unsigned int i; > @@ -1751,6 +1762,7 @@ static struct isc_format *find_format_by_code(struct isc_device *isc, > > return NULL; > } > +EXPORT_SYMBOL_GPL(isc_find_format_by_code); > > static int isc_formats_init(struct isc_device *isc) > { > @@ -1767,7 +1779,7 @@ static int isc_formats_init(struct isc_device *isc) > NULL, &mbus_code)) { > mbus_code.index++; > > - fmt = find_format_by_code(isc, mbus_code.code, &i); > + fmt = isc_find_format_by_code(isc, mbus_code.code, &i); > if (!fmt) { > v4l2_warn(&isc->v4l2_dev, "Mbus code %x not supported\n", > mbus_code.code); > @@ -1893,7 +1905,8 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier) > vdev->queue = q; > vdev->lock = &isc->lock; > vdev->ctrl_handler = &isc->ctrls.handler; > - vdev->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE; > + vdev->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE | > + V4L2_CAP_IO_MC; > video_set_drvdata(vdev, isc); > > ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1); > @@ -1903,8 +1916,18 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier) > goto isc_async_complete_err; > } > > + ret = isc_scaler_link(isc); > + if (ret < 0) > + goto isc_async_complete_unregister_device; > + > + ret = media_device_register(&isc->mdev); > + if (ret < 0) > + goto isc_async_complete_unregister_device; Empty line ? Just one clarification, the media device is initialize by mc_init() which is called by the soc-specific code. Do all you platforms call it ? > return 0; > > +isc_async_complete_unregister_device: > + video_unregister_device(vdev); > + > isc_async_complete_err: > mutex_destroy(&isc->lock); > return ret; > @@ -1971,6 +1994,48 @@ int isc_pipeline_init(struct isc_device *isc) > } > EXPORT_SYMBOL_GPL(isc_pipeline_init); > > +int isc_mc_init(struct isc_device *isc, u32 ver) > +{ > + const struct of_device_id *match; > + int ret; > + > + isc->video_dev.entity.function = MEDIA_ENT_F_IO_V4L; It's weird very few drivers use this. It seems correct to me.. > + isc->video_dev.entity.flags = MEDIA_ENT_FL_DEFAULT; > + isc->pads[ISC_PAD_SINK].flags = MEDIA_PAD_FL_SINK; > + > + ret = media_entity_pads_init(&isc->video_dev.entity, ISC_PADS_NUM, > + isc->pads); > + if (ret < 0) { > + dev_err(isc->dev, "media entity init failed\n"); > + return ret; > + } > + > + isc->mdev.dev = isc->dev; > + > + match = of_match_node(isc->dev->driver->of_match_table, > + isc->dev->of_node); > + > + strscpy(isc->mdev.driver_name, KBUILD_MODNAME, > + sizeof(isc->mdev.driver_name)); > + strscpy(isc->mdev.model, match->compatible, sizeof(isc->mdev.model)); > + snprintf(isc->mdev.bus_info, sizeof(isc->mdev.bus_info), "platform:%s", > + isc->v4l2_dev.name); > + isc->mdev.hw_revision = ver; > + > + media_device_init(&isc->mdev); > + > + isc->v4l2_dev.mdev = &isc->mdev; > + > + return isc_scaler_init(isc); > +} > +EXPORT_SYMBOL_GPL(isc_mc_init); > + > +void isc_mc_cleanup(struct isc_device *isc) > +{ > + media_entity_cleanup(&isc->video_dev.entity); do you need to media_device_cleanup() ? > +} > +EXPORT_SYMBOL_GPL(isc_mc_cleanup); > + > /* regmap configuration */ > #define ATMEL_ISC_REG_MAX 0xd5c > const struct regmap_config isc_regmap_config = { > diff --git a/drivers/media/platform/atmel/atmel-isc-scaler.c b/drivers/media/platform/atmel/atmel-isc-scaler.c > new file mode 100644 > index 000000000000..0c1f49db47b4 > --- /dev/null > +++ b/drivers/media/platform/atmel/atmel-isc-scaler.c > @@ -0,0 +1,261 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Microchip Image Sensor Controller (ISC) Scaler entity support > + * > + * Copyright (C) 2022 Microchip Technology, Inc. > + * > + * Author: Eugen Hristev <eugen.hristev@...rochip.com> > + * > + */ > + > +#include <media/media-device.h> > +#include <media/media-entity.h> > +#include <media/v4l2-device.h> > +#include <media/v4l2-subdev.h> > + > +#include "atmel-isc-regs.h" > +#include "atmel-isc.h" > + > +static int isc_scaler_get_fmt(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_format *format) > +{ > + struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd); > + struct v4l2_mbus_framefmt *v4l2_try_fmt; > + > + if (format->which == V4L2_SUBDEV_FORMAT_TRY) { > + v4l2_try_fmt = v4l2_subdev_get_try_format(sd, sd_state, > + format->pad); > + format->format = *v4l2_try_fmt; > + > + return 0; > + } > + > + if (format->pad == ISC_SCALER_PAD_SOURCE) > + format->format = isc->scaler_format_source; > + else > + format->format = isc->scaler_format_sink; Having an [] might make this nicer.. > + > + return 0; > +} > + > +static int isc_scaler_set_fmt(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_format *req_fmt) > +{ > + struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd); > + struct v4l2_mbus_framefmt *v4l2_try_fmt; > + struct isc_format *fmt; > + unsigned int i; > + > + /* Source format is fixed, we cannot change it */ > + if (req_fmt->pad == ISC_SCALER_PAD_SOURCE) { > + req_fmt->format = isc->scaler_format_source; > + return 0; > + } > + > + /* There is no limit on the frame size on the sink pad */ > + v4l_bound_align_image > + (&req_fmt->format.width, 16, UINT_MAX, 0, Fits on the previous line > + &req_fmt->format.height, 16, UINT_MAX, 0, 0); > + > + req_fmt->format.colorspace = V4L2_COLORSPACE_SRGB; > + req_fmt->format.field = V4L2_FIELD_NONE; > + req_fmt->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + req_fmt->format.quantization = V4L2_QUANTIZATION_DEFAULT; > + req_fmt->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; > + > + fmt = isc_find_format_by_code(isc, req_fmt->format.code, &i); > + > + if (!fmt) > + fmt = &isc->formats_list[0]; The fmt = isc_find_format_by_code(); if (!fmt) fmt = &isc->formats_list[0]; pattern is repeated in a few places. Maybe isc_find_format_by_code() could default back to format_list[0] instead of returning NULL ? > + > + req_fmt->format.code = fmt->mbus_code; > + > + if (req_fmt->which == V4L2_SUBDEV_FORMAT_TRY) { > + v4l2_try_fmt = v4l2_subdev_get_try_format(sd, sd_state, > + req_fmt->pad); > + *v4l2_try_fmt = req_fmt->format; > + /* Trying on the sink pad makes the source pad change too */ > + v4l2_try_fmt = > + v4l2_subdev_get_try_format(sd, sd_state, Fits on previous line ? > + ISC_SCALER_PAD_SOURCE); > + *v4l2_try_fmt = req_fmt->format; > + > + v4l_bound_align_image(&v4l2_try_fmt->width, > + 16, isc->max_width, 0, > + &v4l2_try_fmt->height, > + 16, isc->max_height, 0, 0); > + /* if we are just trying, we are done */ > + return 0; > + } > + > + isc->scaler_format_sink = req_fmt->format; > + > + /* The source pad is the same as the sink, but we have to crop it */ > + isc->scaler_format_source = isc->scaler_format_sink; > + v4l_bound_align_image > + (&isc->scaler_format_source.width, 16, isc->max_width, 0, > + &isc->scaler_format_source.height, 16, isc->max_height, 0, 0); > + > + return 0; > +} > + > +static int isc_scaler_enum_mbus_code(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_mbus_code_enum *code) > +{ > + struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd); > + > + /* > + * All formats supported by the ISC are supported by the scaler. > + * Advertise the formats which the ISC can take as input, as the scaler > + * entity cropping is part of the PFE module (parallel front end) > + */ > + if (code->index < isc->formats_list_size) { > + code->code = isc->formats_list[code->index].mbus_code; > + return 0; > + } > + > + return -EINVAL; > +} > + > +static int isc_scaler_g_sel(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_selection *sel) > +{ > + struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd); > + > + if (sel->pad == ISC_SCALER_PAD_SOURCE) > + return -EINVAL; > + > + if (sel->target == V4L2_SEL_TGT_CROP_BOUNDS) { > + /* bounds are the input format received */ > + sel->r.height = isc->scaler_format_sink.height; > + sel->r.width = isc->scaler_format_sink.width; > + sel->r.left = 0; > + sel->r.top = 0; > + } else if (sel->target == V4L2_SEL_TGT_CROP) { > + /* > + * crop is done to the output format, > + * limited by ISC maximum size > + */ > + sel->r.height = isc->scaler_format_source.height; > + sel->r.width = isc->scaler_format_source.width; > + sel->r.left = 0; > + sel->r.top = 0; > + } else { > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int isc_scaler_init_cfg(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state) > +{ > + struct v4l2_mbus_framefmt *v4l2_try_fmt = > + v4l2_subdev_get_try_format(sd, sd_state, 0); > + struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd); > + > + *v4l2_try_fmt = isc->scaler_format_source; Try crop rectangles should be initialized too ? > + > + return 0; > +} > + > +static const struct v4l2_subdev_pad_ops isc_scaler_pad_ops = { > + .enum_mbus_code = isc_scaler_enum_mbus_code, > + .set_fmt = isc_scaler_set_fmt, > + .get_fmt = isc_scaler_get_fmt, > + .get_selection = isc_scaler_g_sel, > + .init_cfg = isc_scaler_init_cfg, > +}; > + > +static const struct v4l2_subdev_ops xisc_scaler_subdev_ops = { > + .pad = &isc_scaler_pad_ops, > +}; > + > +int isc_scaler_init(struct isc_device *isc) > +{ > + int ret; > + > + v4l2_subdev_init(&isc->scaler_sd, &xisc_scaler_subdev_ops); > + > + isc->scaler_sd.owner = THIS_MODULE; > + isc->scaler_sd.dev = isc->dev; > + snprintf(isc->scaler_sd.name, sizeof(isc->scaler_sd.name), > + "atmel_isc_scaler"); > + > + isc->scaler_sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > + isc->scaler_sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER; > + isc->scaler_pads[ISC_SCALER_PAD_SINK].flags = MEDIA_PAD_FL_SINK; > + isc->scaler_pads[ISC_SCALER_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE; > + > + isc->scaler_format_source.height = isc->max_height; > + isc->scaler_format_source.width = isc->max_width; > + isc->scaler_format_source.code = isc->formats_list[0].mbus_code; > + isc->scaler_format_source.colorspace = V4L2_COLORSPACE_SRGB; > + isc->scaler_format_source.field = V4L2_FIELD_NONE; > + isc->scaler_format_source.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + isc->scaler_format_source.quantization = V4L2_QUANTIZATION_DEFAULT; > + isc->scaler_format_source.xfer_func = V4L2_XFER_FUNC_DEFAULT; Minor detail: you assign the field from colorspace on at s_ftm time too. Maybe if you define a static const default_fmt you can re-use it instead of assigning fields one by one ? > + > + isc->scaler_format_sink = isc->scaler_format_source; > + > + ret = media_entity_pads_init(&isc->scaler_sd.entity, > + ISC_SCALER_PADS_NUM, > + isc->scaler_pads); > + if (ret < 0) { > + dev_err(isc->dev, "scaler sd media entity init failed\n"); > + return ret; > + } empty line ? > + ret = v4l2_device_register_subdev(&isc->v4l2_dev, &isc->scaler_sd); > + if (ret < 0) { > + dev_err(isc->dev, "scaler sd failed to register subdev\n"); > + return ret; > + } > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(isc_scaler_init); > + > +int isc_scaler_link(struct isc_device *isc) > +{ > + int ret; > + > + ret = media_create_pad_link(&isc->current_subdev->sd->entity, > + isc->remote_pad, &isc->scaler_sd.entity, > + ISC_SCALER_PAD_SINK, > + MEDIA_LNK_FL_ENABLED | > + MEDIA_LNK_FL_IMMUTABLE); > + > + if (ret < 0) { > + dev_err(isc->dev, "Failed to create pad link: %s to %s\n", > + isc->current_subdev->sd->entity.name, > + isc->scaler_sd.entity.name); > + return ret; > + } > + > + dev_dbg(isc->dev, "link with %s pad: %d\n", > + isc->current_subdev->sd->name, isc->remote_pad); > + > + ret = media_create_pad_link(&isc->scaler_sd.entity, > + ISC_SCALER_PAD_SOURCE, > + &isc->video_dev.entity, ISC_PAD_SINK, > + MEDIA_LNK_FL_ENABLED | > + MEDIA_LNK_FL_IMMUTABLE); > + > + if (ret < 0) { > + dev_err(isc->dev, "Failed to create pad link: %s to %s\n", > + isc->scaler_sd.entity.name, > + isc->video_dev.entity.name); > + return ret; > + } > + > + dev_dbg(isc->dev, "link with %s pad: %d\n", isc->scaler_sd.name, > + ISC_SCALER_PAD_SOURCE); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(isc_scaler_link); > + > diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h > index f9ad7ec6bd13..c1ca3916700e 100644 > --- a/drivers/media/platform/atmel/atmel-isc.h > +++ b/drivers/media/platform/atmel/atmel-isc.h > @@ -183,6 +183,17 @@ struct isc_reg_offsets { > u32 his_entry; > }; > > +enum isc_mc_pads { > + ISC_PAD_SINK = 0, > + ISC_PADS_NUM = 1, > +}; > + > +enum isc_scaler_pads { > + ISC_SCALER_PAD_SINK = 0, > + ISC_SCALER_PAD_SOURCE = 1, > + ISC_SCALER_PADS_NUM = 2, > +}; > + > /* > * struct isc_device - ISC device driver data/config struct > * @regmap: Register map > @@ -258,6 +269,14 @@ struct isc_reg_offsets { > * be used as an input to the controller > * @controller_formats_size: size of controller_formats array > * @formats_list_size: size of formats_list array > + * @pads: media controller pads for isc video entity > + * @mdev: media device that is registered by the isc > + * @remote_pad: remote pad on the connected subdevice > + * @scaler_sd: subdevice for the scaler that isc registers > + * @scaler_pads: media controller pads for the scaler subdevice > + * @scaler_format_sink: current format for the scaler subdevice on the sink pad > + * @scaler_format_source: current format for the scaler subdevice on the I'm still not super happy with the scaler configuration being part of the larger ISC base structure. But I understand this is fine with you. Please consider having an array for the scaler pads formats, but it's a minor detail. > + * source pad > */ > struct isc_device { > struct regmap *regmap; > @@ -346,6 +365,20 @@ struct isc_device { > struct isc_format *formats_list; > u32 controller_formats_size; > u32 formats_list_size; > + > + struct { > + struct media_pad pads[ISC_PADS_NUM]; > + struct media_device mdev; > + > + u32 remote_pad; > + }; > + > + struct { > + struct v4l2_subdev scaler_sd; > + struct media_pad scaler_pads[ISC_SCALER_PADS_NUM]; > + struct v4l2_mbus_framefmt scaler_format_sink; > + struct v4l2_mbus_framefmt scaler_format_source; > + }; > }; > > extern const struct regmap_config isc_regmap_config; > @@ -357,4 +390,11 @@ int isc_clk_init(struct isc_device *isc); > void isc_subdev_cleanup(struct isc_device *isc); > void isc_clk_cleanup(struct isc_device *isc); > > +int isc_scaler_link(struct isc_device *isc); > +int isc_scaler_init(struct isc_device *isc); > +int isc_mc_init(struct isc_device *isc, u32 ver); > +void isc_mc_cleanup(struct isc_device *isc); > + > +struct isc_format *isc_find_format_by_code(struct isc_device *isc, > + unsigned int code, int *index); > #endif > diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c > index c5b9563e36cb..c244682ea22f 100644 > --- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c > +++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c > @@ -553,6 +553,12 @@ static int atmel_isc_probe(struct platform_device *pdev) > break; > } > > + regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver); > + > + ret = isc_mc_init(isc, ver); > + if (ret < 0) > + goto isc_probe_mc_init_err; > + > pm_runtime_set_active(dev); > pm_runtime_enable(dev); > pm_request_idle(dev); > @@ -562,7 +568,7 @@ static int atmel_isc_probe(struct platform_device *pdev) > ret = clk_prepare_enable(isc->ispck); > if (ret) { > dev_err(dev, "failed to enable ispck: %d\n", ret); > - goto cleanup_subdev; > + goto isc_probe_mc_init_err; > } > > /* ispck should be greater or equal to hclock */ > @@ -572,7 +578,6 @@ static int atmel_isc_probe(struct platform_device *pdev) > goto unprepare_clk; > } > > - regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver); > dev_info(dev, "Microchip ISC version %x\n", ver); > > return 0; > @@ -580,6 +585,9 @@ static int atmel_isc_probe(struct platform_device *pdev) > unprepare_clk: > clk_disable_unprepare(isc->ispck); > > +isc_probe_mc_init_err: > + isc_mc_cleanup(isc); > + > cleanup_subdev: > isc_subdev_cleanup(isc); > > @@ -600,6 +608,8 @@ static int atmel_isc_remove(struct platform_device *pdev) > > pm_runtime_disable(&pdev->dev); > > + isc_mc_cleanup(isc); > + > isc_subdev_cleanup(isc); > > v4l2_device_unregister(&isc->v4l2_dev); > diff --git a/drivers/media/platform/atmel/atmel-sama7g5-isc.c b/drivers/media/platform/atmel/atmel-sama7g5-isc.c > index 07a80b08bc54..9dc75eed0098 100644 > --- a/drivers/media/platform/atmel/atmel-sama7g5-isc.c > +++ b/drivers/media/platform/atmel/atmel-sama7g5-isc.c > @@ -547,15 +547,23 @@ static int microchip_xisc_probe(struct platform_device *pdev) > break; > } > > + regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver); > + > + ret = isc_mc_init(isc, ver); > + if (ret < 0) > + goto isc_probe_mc_init_err; > + > pm_runtime_set_active(dev); > pm_runtime_enable(dev); > pm_request_idle(dev); > > - regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver); > dev_info(dev, "Microchip XISC version %x\n", ver); > > return 0; > > +isc_probe_mc_init_err: > + isc_mc_cleanup(isc); > + > cleanup_subdev: > isc_subdev_cleanup(isc); > > @@ -576,6 +584,8 @@ static int microchip_xisc_remove(struct platform_device *pdev) > > pm_runtime_disable(&pdev->dev); > > + isc_mc_cleanup(isc); > + > isc_subdev_cleanup(isc); > > v4l2_device_unregister(&isc->v4l2_dev); > -- > 2.25.1 >
Powered by blists - more mailing lists