Re: [RFC v2 2/5] video: panel: Add DPI panel support

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

Hi,

On 2012-11-22 23:45, Laurent Pinchart wrote:

> +static void panel_dpi_release(struct display_entity *entity)
> +{
> +	struct panel_dpi *panel = to_panel_dpi(entity);
> +
> +	kfree(panel);
> +}
> +
> +static int panel_dpi_remove(struct platform_device *pdev)
> +{
> +	struct panel_dpi *panel = platform_get_drvdata(pdev);
> +
> +	platform_set_drvdata(pdev, NULL);
> +	display_entity_unregister(&panel->entity);
> +
> +	return 0;
> +}
> +
> +static int __devinit panel_dpi_probe(struct platform_device *pdev)
> +{
> +	const struct panel_dpi_platform_data *pdata = pdev->dev.platform_data;
> +	struct panel_dpi *panel;
> +	int ret;
> +
> +	if (pdata == NULL)
> +		return -ENODEV;
> +
> +	panel = kzalloc(sizeof(*panel), GFP_KERNEL);
> +	if (panel == NULL)
> +		return -ENOMEM;
> +
> +	panel->pdata = pdata;
> +	panel->entity.dev = &pdev->dev;
> +	panel->entity.release = panel_dpi_release;
> +	panel->entity.ops.ctrl = &panel_dpi_control_ops;
> +
> +	ret = display_entity_register(&panel->entity);
> +	if (ret < 0) {
> +		kfree(panel);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, panel);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops panel_dpi_dev_pm_ops = {
> +};
> +
> +static struct platform_driver panel_dpi_driver = {
> +	.probe = panel_dpi_probe,
> +	.remove = panel_dpi_remove,
> +	.driver = {
> +		.name = "panel_dpi",
> +		.owner = THIS_MODULE,
> +		.pm = &panel_dpi_dev_pm_ops,
> +	},
> +};

I'm not sure of how the free/release works. The release func is called
when the ref count drops to zero. But... The object in question, the
panel_dpi struct which contains the display entity, is not only about
data, it's also about code located in this module.

So I don't see anything preventing from unloading this module, while
some other component is holding a ref for the display entity. While its
holding the ref, it's valid to call ops in the display entity, but the
code for the ops in this module is already unloaded.

I don't really know how the kref can be used properly in this use case...

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]