пт, 6 лист. 2026 р. о 15:14 Thierry Reding <[email protected]> пише:
>
> On Thu, Nov 05, 2026 at 04:05:01PM +0200, Svyatoslav Ryhel wrote:
> [...]
> > diff --git a/drivers/gpu/drm/drm_mipi_dbi_bus.c
> > b/drivers/gpu/drm/drm_mipi_dbi_bus.c
> [...]
> > +/**
> > + * devm_mipi_dbi_device_register() - create a managed MIPI DBI device
> > + * @dev: device to tie the MIPI-DBI device lifetime to
> > + * @host: DBI host to which this device is connected
> > + * @np: device tree node associated with the panel MIPI DBI device
> > + *
> > + * This function is called by DBI host driver to create panel device.
> > + * It automatically calls mipi_dbi_device_unregister() when @dev is
> > + * unbound. It does not automatically free passed device node pointer,
> > + * it must be put manually.
> > + *
> > + * Returns:
> > + * A pointer to the newly created MIPI DBI device, or, a pointer encoded
> > + * with an error.
> > + */
> > +struct mipi_dbi_device *
> > +devm_mipi_dbi_device_register(struct device *dev, struct mipi_dbi_host
> > *host,
> > + struct device_node *np)
> > +{
> > + struct mipi_dbi_device *dbidev;
> > + int ret;
> > +
> > + if (!host || !host->dev || !host->dbi || !host->dbi->command ||
> > + !host->ops || !host->ops->attach || !host->ops->detach || !np)
> > + return ERR_PTR(-EINVAL);
>
> Do we want dev->of_node to serve as a fallback for !np?
>
No we don't dev->of_node is hosts node, and np is panel node.
> > +
> > + if (!of_device_is_available(np))
> > + return ERR_PTR(-ENODEV);
> > +
> > + dbidev = kzalloc_obj(*dbidev);
> > + if (!dbidev)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + if (of_node_test_and_set_flag(np, OF_POPULATED)) {
> > + kfree(dbidev);
> > + return ERR_PTR(-EBUSY);
> > + }
> > +
> > + dbidev->host = host;
> > + dbidev->dbi = host->dbi;
> > + device_initialize(&dbidev->dev);
> > + dbidev->dev.parent = host->dev;
> > + dbidev->dev.bus = &mipi_dbi_bus_type;
> > + dbidev->dev.release = mipi_dbi_device_release;
> > + device_set_node(&dbidev->dev, of_fwnode_handle(of_node_get(np)));
> > +
> > + ret = dev_set_name(&dbidev->dev, "%s:%s", dev_name(host->dev),
> > np->full_name);
> > + if (ret)
> > + goto put_device;
> > +
> > + ret = device_add(&dbidev->dev);
> > + if (ret)
> > + goto put_device;
> > +
> > + ret = devm_add_action_or_reset(dev, mipi_dbi_device_unregister,
> > + &dbidev->dev);
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + return dbidev;
> > +
> > +put_device:
> > + of_node_clear_flag(np, OF_POPULATED);
> > + put_device(&dbidev->dev);
> > + return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL_GPL(devm_mipi_dbi_device_register);
>
> Don't we need to of_node_put() here on failure? Otherwise we're leaking
> the reference that we obtained inside the device_set_node() argument.
>
This is fully repeats device_register logic of drm_mipi_dsi.c so
unless DSI framework has leak, this should be correct
> > +static int mipi_dbi_drv_remove(struct device *dev)
> > +{
> > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(dev->driver);
> > + struct mipi_dbi_device *dbidev = to_mipi_dbi_device(dev);
> > +
> > + drv->remove(dbidev);
> > +
> > + return 0;
> > +}
>
> Might as well make drv->remove() return an error and propagate that
> here.
>
Have you even checked the code? remove() callback is and should be void.
> Thierry
