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@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > --- > [...] > > 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
Attachment:
pgpOtiRXUH8mA.pgp
Description: PGP signature
