Re: Reparenting a platform device

[Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2979 bytes --]

* Stephen Warren wrote:
> On 04/05/2012 02:42 AM, Thierry Reding wrote:
> > Hi,
> > 
> > I have a device tree where I have a GART device and a DRM device which uses
> > the GART. The GART is implemented by an IOMMU driver (tegra-gart) and
> > requires the user device to be a child of the GART device (it explicitly
> > checks for this when the user device is attached).
> 
> Isn't this wrong?
> 
> I would expect the device parent/child relationship to reflect the
> CPU-initiated register access bus topology.
> 
> A device's interaction with an IOMMU is an aspect of a device's
> initiating accesses itself, not CPU-initiated register accesses.

Actually I have no idea why this was made a requirement. Maybe Hiroshi can
comment on this. The driver really only needs this to basically obtain a
pointer to itself. The MSM I/O MMU implementation does something similar,
though, and goes on to register actual child devices (they are instantiated
in arch/arm/mach-msm/devices-iommu.c). Each of those devices is then assigned
a specific memory area it seems.

> > I've tried two alternatives to achieve this: create the GART device in the
> > user driver's .probe() function and explicitly set the DRM device's parent
> > to the resulting platform device like so:
> > 
> > 	gart = platform_device_alloc(...);
> > 	...
> > 	pdev->dev.parent = &gart->dev;
> 
> I guess that won't work when there's more than one device affected by
> the IOMMU?

I don't think having more than one device using the IOMMU will work properly
anyway in the context of the Tegra GART driver because there is not means to
allocate specific regions of the GART aperture to individual devices. So
really the one and only client actually needs to manage the allocations from
the GART aperture.

I'm also not sure if it makes much sense to use the GART from anything other
than the DRM driver.

> > The alternative is to use the device tree to look up the GART device node and
> > resolve it to the corresponding struct device:
> > 
> > 	gart_node = of_parse_phandle(drm->dev->of_node, "gart-parent", 0);
> 
> That seems more logical to me.
> 
> > 	gart = bus_find_device(drm->dev->bus, NULL, gart_node, match_of_node);
> 
> That part should probably be encapsulated into the IOMMU subsystem? In
> fact, even the of_parse_phandle should perhaps be hidden inside some
> IOMMU iommu_get() call, that can use DT as a data source, or some other
> data structure set up by board files.

Both of these methods don't work too well because they actually require a
reordering of the device hierarchy at runtime. For some reason this leads to
the mutex becoming confused and the warning that I posted. I came up with a
third alternative that does the reparenting in board-dt-tegra20.c after the
call to of_platform_populate() which works similar to the second version and
gets rid of the warning.

I'm still not at all happy with it, though.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 190 bytes --]

_______________________________________________
iommu mailing list
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu