On Tue, Dec 23, 2014 at 03:30:20PM +0800, Mark Zhang wrote:
On 12/19/2014 11:24 PM, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
Previously the struct bus_type exported by the host1x infrastructure was only a very basic skeleton. Turn that implementation into a more full- fledged bus to support proper probe ordering and power management.
Note that the bus infrastructure needs to be available before any of the drivers can be registered, so the bus needs to be registered before the host1x module. Otherwise drivers could be registered before the module is loaded and trigger a BUG_ON() in driver_register(). To ensure the bus infrastructure is always there, always build the code into the kernel when enabled and register it with a postcore initcall.
So this means there is no chance that host1x can be built as a kernel module, right? I'm fine with that, just asking.
No, it means that not all of host1x can be built as a module. The host1x bus infrastructure will always be built-in when TEGRA_HOST1X is enabled.
Signed-off-by: Thierry Reding treding@nvidia.com
[...]
diff --git a/drivers/gpu/host1x/Makefile b/drivers/gpu/host1x/Makefile index c1189f004441..a3e667a1b6f5 100644 --- a/drivers/gpu/host1x/Makefile +++ b/drivers/gpu/host1x/Makefile @@ -1,5 +1,4 @@ host1x-y = \
- bus.o \ syncpt.o \ dev.o \ intr.o \
@@ -13,3 +12,5 @@ host1x-y = \ hw/host1x04.o
obj-$(CONFIG_TEGRA_HOST1X) += host1x.o
+obj-y += bus.o
I didn't get it, why we need to do this?
If CONFIG_TEGRA_HOST1X=m, then obj-$(CONFIG_TEGRA_HOST1X) builds the bus.o into a module. But we want it to always be built-in. The build system will descend into the drivers/gpu/host1x directory only if the TEGRA_HOST1X symbol is selected (either =y or =m), therefore obj-y here will result in bus.o being built-in whether the rest of host1x is built as a module or built-in.
diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c index 0b52f0ea8871..28630a5e9397 100644 --- a/drivers/gpu/host1x/bus.c +++ b/drivers/gpu/host1x/bus.c @@ -72,13 +72,14 @@ static void host1x_subdev_del(struct host1x_subdev *subdev)
[...]
static inline struct host1x_device *to_host1x_device(struct device *dev)
The change looks good to me. Just one thing I feel not comfortable: "struct host1x_device" is not a real device, it represents the drm device actually. The real tegra host1x device is represented by "struct host1x". But the name "host1x_device" makes people confusing, I mean, it will make people thinking it's the real "tegra host1x" device then bring the reading difficulty.
The reason behind that name is that host1x provides a bus (real to some degree, but also virtual). host1x is the device that provides the bus whereas a host1x_device is a "device" on the "host1x" bus. That's just like an i2c_client is a "client" on the "I2C" bus. Or an spi_device is a "device" on the "SPI" bus.
Why don't we change this to something like "drm_device" or "tegra_drm_device"?
Other devices can be host1x devices. Some time ago work was being done on a driver for the CSI/VI hardware (for camera or video input). The idea was that it would also be instantiated as a host1x_device in some other subsystem (V4L2 at the time).
The functionality here is generic and in no way DRM specific.
Thierry