On Thu, Jan 03, 2008 at 09:54:45AM -0500, Jon Smirl wrote:
On 1/2/08, David Gibson david@gibson.dropbear.id.au wrote:
[snip]
Instantiating the fabric driver off any node is wrong, precisely because it is an abstraction. The fabric driver should be instantiated by the platform code.
Instantiating it from the platform code forces me to put it either the of_platform_bus or the platform_bus since there aren't any other buses around when the platform code runs. Platform bus doesn't implement dynamic module loading. So that means it has to go onto the of_platform_bus. That implies that is it a pseudo-device without a pseudo-device entry in the device tree which is fine with me. I'll need to poke around in the of_bus code and see if the driver will load without a device tree entry.
You're letting implementation warts influence basic design decisions. This is not sensible. Step back and think for a moment, work out a sane organization *then* think how you might need to fix or workaround limitations of existing infrastructure.
A simple fix to this would be to let me instantiate the driver off from the root node of the tree. That's the conceptually correct place for instantiating a driver that extends the platform code. Should I try adjusting the of probing code to pass the node in, or are there major objections?
The current probing system can't instantiate a device for the root node in any sane way, since it takes a list of suitable busses.
The constructor based approach we're looking at implementing could do it. It should, in any case, be constructing a platform_device - so the platform bus code would still need to be extended to handle the module loading. Creating it as an of_platform device bound to the root node might be a workable interim solution though.
of_platform_device simply does not *ever* make sense conceptually: the type of struct device wrapper in use depends on the bus the device is attached to, not on how we figured out the device was there. OF can potentially give information about any sort of device be it simple-bus, i2c, PCI or whatever connected.
Also, as others have pointed out, this driver is not an abstraction. It represents the mess of wires hooking the codec up to the jacks on the back panel and possibly GPIO pins that control the wiring. You need this because the pins on HD audio codecs are completely reconfigurable and the same chip can be wired in a thousand different ways. It lets you have a generic codec driver and the move the platform specific code out of the driver.
Well, "abstraction" is maybe not the right word. But the point is that the fabric driver doesn't represent a neatly isolated device with well defined bus connections. Instead it represents the tangle of essentially every link between audio devices in the platform. About the clearest possible example of a true "platform device" (as opposed to a device on some bus that just doesn't have any bus-specific logic).