Re: [PATCH v4 17/18] iommu: exynos: init from dt-specific callback instead of initcall

[Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>]

On Mon, Jan 19, 2015 at 01:11:07AM +0000, Laurent Pinchart wrote:
> On Friday 16 January 2015 10:13:11 Marek Szyprowski wrote:
> > This patch introduces IOMMU_OF_DECLARE-based initialization to the
> > driver, which replaces subsys_initcall-based procedure.
> > exynos_iommu_of_setup ensures that each sysmmu controller is probed
> > before its master device.

[...]

> > +static int __init exynos_iommu_of_setup(struct device_node *np)
> > +{
> > +	struct platform_device *pdev;
> > +
> > +	if (!init_done)
> > +		exynos_iommu_init();
> > +
> > +	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> > +	if (IS_ERR(pdev))
> > +		return PTR_ERR(pdev);
> 
> This feels like a hack to me. What happens here is that you're using the 
> IOMMU_OF_DECLARE mechanism to make sure that the iommu platform device will be 
> created and registered before the normal OF bus populate mechanism kicks in, 
> thus ensuring that the iommu gets probed before other devices. In practice 
> this is pretty similar to using different init levels, which is what Will's 
> patch set was trying to avoid in the first place. Creating a new kind of init 
> levels mechanism doesn't sound very good to me.
> 
> The existing exynos-iommu driver is based on classic instantiation of a 
> platform device from DT, using the normal device probing mechanism. As such it 
> relies on the availability of a struct device for various helper functions. I 
> thus understand why you want a struct device being registered for the iommu, 
> instead of initializing the device right from the exynos_iommu_of_setup() 
> function without a corresponding struct device being registered.
> 
> This leads me to question whether we should really introduce IOMMU_OF_DECLARE. 
> Using regular deferred probing seems more and more like a better solution to 
> me.

We seem to be going round and round on this argument. I said before that
I'm not against changing this [1], but somebody would need to propose
patches, which hasn't happened in recent history.

Arnd also makes some good arguments against using probing [2], which would
need further discussion.

Basically, it looks like there are two sides to this argument and I don't
see anything changing without patch proposals. The only thing that the
current discussions seem to be achieving is blocking people like Marek,
who are trying to make use of what we have in mainline today!

Will

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310783.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310992.html