On Thu, Nov 19, 2009 at 10:32 AM, Stephen Neuendorffer <stephen.neuendorf...@xilinx.com> wrote: >> Of course, one obvious thing that must be done is move this code out > of >> arch/powerpc/platforms/44x/virtex.c and into (e.g.) >> arch/powerpc/kernel/setup-common.c, and add some >> "set_machine_restart_function" wrapper to access it more cleanly (also >> defining this function as a null function when inapplicable). If this >> satisfies your standards, I can easily post an updated patch :) > > The driver isn't even powerpc specific, it could also be used on the > microblaze, > and I think you'll find alot of resistance to adding that kind of hook > to an architecture > that has just spent a bunch of time getting rid of alot of direct > binding between > platform code and drivers. Grant, do you have a comment here?
Sorry for the delay, I was at a conference and got behind on email. Yes, I've got comments. First, the practical matter is that this is only applicable as a reset mechanism for Xilinx PowerPC (440 or 405) and Microblaze platforms, but the sysace device also appears on other boards, and is used to reconfigure FPGA's that don't reset the whole system. One of the AMCC boards for example. So, you want the ability to have the driver reset the system, but you cannot break other platforms. You also cannot call out to the driver from platform code because the xsysace driver may get loaded as a module. The cleanest solution would be to call out to the xsysace driver from platform code; but as already stated that isn't possible because the xsysace driver may be loaded as a module. The only way I can think of to do that cleanly would be to register a bus notifier from platform code to wait for an xsysace device to get probed. But still not exactly pretty. For the time being, I don't think there is a good way to avoid architecture specific hooks into the driver itself. I recommend adding a new pair of functions to the driver (protected by #if defined(CONFIG_PPC)) that looks for a 'xlnx,can-reset-system' property. It the property is present, then simply replace the ppc_md.restart hook in the machine definition. Make sure to also restore it again when the driver is removed. Similar code would be needed for Microblaze too, but someone else can write that. It is important to trigger the behaviour on whether or not a property like 'xlnx,can-restart-system' is present so that it can be chosen at runtime whether or not the sysace should actually be used for restart so that it doesn't break things for multiplatform. Also, you need to add documentation about the new property to Documentation/powerpc/dts-bindings/ before I will merge the change. The documentation change can appear in the same patch. It may not be the prettiest thing in the world, but in the absence of a cross-architecture machine restart infrastructure I think it is the best option. At the very least it contains the changes to the driver itself. Cheers, g. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev