Re: [PATCH v1 07/11] media: Add registration helpers for V4L2 flash sub-devices

[Sakari Ailus <sakari.ailus@iki.fi>]

Hi Jacek,

One more comment on this one:

On Fri, Mar 20, 2015 at 04:03:27PM +0100, Jacek Anaszewski wrote:
...
> +struct v4l2_flash *v4l2_flash_init(struct led_classdev_flash *fled_cdev,
> +				   const struct v4l2_flash_ops *ops,
> +				   struct v4l2_flash_ctrl_config *config)
> +{
> +	struct v4l2_flash *v4l2_flash;
> +	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
> +	struct v4l2_subdev *sd;
> +	int ret;
> +
> +	if (!fled_cdev || !ops || !config)
> +		return ERR_PTR(-EINVAL);
> +
> +	v4l2_flash = devm_kzalloc(led_cdev->dev, sizeof(*v4l2_flash),
> +					GFP_KERNEL);
> +	if (!v4l2_flash)
> +		return ERR_PTR(-ENOMEM);
> +
> +	sd = &v4l2_flash->sd;
> +	v4l2_flash->fled_cdev = fled_cdev;
> +	v4l2_flash->ops = ops;
> +	sd->dev = led_cdev->dev;
> +	v4l2_subdev_init(sd, &v4l2_flash_subdev_ops);
> +	sd->internal_ops = &v4l2_flash_subdev_internal_ops;
> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	strlcpy(sd->name, config->dev_name, sizeof(sd->name));
> +
> +	ret = media_entity_init(&sd->entity, 0, NULL, 0);
> +	if (ret < 0)
> +		return ERR_PTR(ret);
> +
> +	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> +
> +	ret = v4l2_flash_init_controls(v4l2_flash, config);
> +	if (ret < 0)
> +		return ERR_PTR(ret);
> +
> +	of_node_get(led_cdev->dev->of_node);
> +
> +	ret = v4l2_async_register_subdev(sd);
> +	if (ret < 0)
> +		goto err_async_register_sd;
> +
> +	return v4l2_flash;
> +
> +err_async_register_sd:
> +	of_node_put(led_cdev->dev->of_node);
> +	v4l2_ctrl_handler_free(sd->ctrl_handler);
> +	media_entity_cleanup(&sd->entity);
> +
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_flash_init);
> +
> +void v4l2_flash_release(struct v4l2_flash *v4l2_flash)
> +{
> +	struct v4l2_subdev *sd = &v4l2_flash->sd;
> +	struct led_classdev *led_cdev = &v4l2_flash->fled_cdev->led_cdev;
> +
> +	v4l2_async_unregister_subdev(sd);
> +	of_node_put(led_cdev->dev->of_node);
> +	v4l2_ctrl_handler_free(sd->ctrl_handler);
> +	media_entity_cleanup(&sd->entity);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_flash_release);

It'd be very nice if v4l2_flash_release() could graciously behave with NULL
or negative error code as an argument, such as those produced by
v4l2_flash_init(). This makes error handling a lot easier in drivers.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk