On 05.04.2014 20:24, Russell King - ARM Linux wrote:
On Sat, Apr 05, 2014 at 07:32:50PM +0200, Tomasz Figa wrote:
Not exactly. The approach we found does mostly the same as componentized
subsystem framework but without _any_ extra data in Device Tree. Just
based on the list of subsystem sub-drivers that is already available to
the master driver.
The existing approach is fundamentally broken. Yes, your solution may
work for the probing case, but have you tried unbinding any of your
sub-drivers?
From what I can see, that causes a kernel oops for one very simple reason -
you destroy stuff while it's still in use. Let's look at an example:
struct platform_driver ipp_driver = {
.probe = ipp_probe,
.remove = ipp_remove,
.driver = {
.name = "exynos-drm-ipp",
.owner = THIS_MODULE,
.pm = &ipp_pm_ops,
},
};
static int ipp_remove(struct platform_device *pdev)
{
struct ipp_context *ctx = platform_get_drvdata(pdev);
/* unregister sub driver */
exynos_drm_subdrv_unregister(&ctx->subdrv);
/* remove,destroy ipp idr */
idr_destroy(&ctx->ipp_idr);
idr_destroy(&ctx->prop_idr);
mutex_destroy(&ctx->ipp_lock);
mutex_destroy(&ctx->prop_lock);
/* destroy command, event work queue */
destroy_workqueue(ctx->cmd_workq);
destroy_workqueue(ctx->event_workq);
return 0;
}
int exynos_drm_subdrv_unregister(struct exynos_drm_subdrv *subdrv)
{
if (!subdrv)
return -EINVAL;
list_del(&subdrv->list);
return 0;
}
Oh dear, that destroys a whole pile of resources which could already
be in use without telling anything that it's about to do that.
I'm sure if I continue looking at the exynos stuff, it'll show similar
crap all over the place.
What you have now in mainline is not a solution. It's a crappy bodge.
Undoubtedly. Nobody here is trying to state the opposite.
Maybe my words have been misinterpreted, but all I'm suggesting here is
that there is no need to add any new data to DT to solve the same issue
to the same extent as componentized subsystem framework, at least in
Exynos case.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
