Re: [PATCH/RFC] Booting Xilinx ML510 board using SystemACE
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
RE: [PATCH/RFC] Booting Xilinx ML510 board using SystemACE
Hi, On Monday, November 16, 2009, Stephen wrote: There are at least two other ways that you might be able to reset a board: 1) Internally through the ICAP device. 2) Through a GPIO connected externally to the reset logic. Unfortunately none of these is relevant for the specific board in question (Xilinx ML510 reference system)... Probably it would be best to have a mechanism in the device tree which references the reset mechanism? I am sorely lacking in expertise for this :(, and wouldn't even know where to begin... Is it possible at all to add custom information into the device tree? And even if yes--how will platform code bind to the reset mechanism? [...] In any event, you probably don't want a driver to eplicitly reference the plaform code. If you want to do it explicitly like this, it would better to have the plaform code reference the driver mechanism. I don't see how this can be done: if the driver is to publish some driver_reset_system function to the platform code, it needs _some_ mechanism for telling this fact to the system... And such a mechanism won't look all that different from my callback, IMO (except it may be slightly prettied up). 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 :) -az ** IMPORTANT: The contents of this email and any attachments are confidential. They are intended for the named recipient(s) only. If you have received this email in error, please notify the system manager or the sender immediately and do not disclose the contents to anyone or make copies thereof. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH/RFC] Booting Xilinx ML510 board using SystemACE
-Original Message- From: linuxppc-dev-bounces+stephen=neuendorffer.n...@lists.ozlabs.org [mailto:linuxppc-dev- bounces+stephen=neuendorffer.n...@lists.ozlabs.org] On Behalf Of Alon Ziv Sent: Thursday, November 19, 2009 4:57 AM To: Stephen Neuendorffer; linuxppc-dev Cc: John Linn Subject: RE: [PATCH/RFC] Booting Xilinx ML510 board using SystemACE Hi, On Monday, November 16, 2009, Stephen wrote: There are at least two other ways that you might be able to reset a board: 1) Internally through the ICAP device. 2) Through a GPIO connected externally to the reset logic. Unfortunately none of these is relevant for the specific board in question (Xilinx ML510 reference system)... Well, board != system. :) ML510 could easily include an ICAP device. Probably it would be best to have a mechanism in the device tree which references the reset mechanism? I am sorely lacking in expertise for this :(, and wouldn't even know where to begin... Is it possible at all to add custom information into the device tree? And even if yes--how will platform code bind to the reset mechanism? [...] In any event, you probably don't want a driver to eplicitly reference the plaform code. If you want to do it explicitly like this, it would better to have the plaform code reference the driver mechanism. I don't see how this can be done: if the driver is to publish some driver_reset_system function to the platform code, it needs _some_ mechanism for telling this fact to the system... Think of it this way: The driver is usable on many more platforms than just the one you've modified. Your addition of the hook into the platform code requires that that hook always be there. It would be much better to provide a configuration-based way of allowing the platform code to make use of the sysace reset, if it desires. And such a mechanism won't look all that different from my callback, IMO (except it may be slightly prettied up). The callback isn't the problem, it's how the callback gets registered with the platform code/device tree. 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? Steve This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH/RFC] Booting Xilinx ML510 board using SystemACE
Alon,
There are at least two other ways that you might be able to reset a
board:
1) Internally through the ICAP device.
2) Through a GPIO connected externally to the reset logic.
Part of this is board specific, part of is it design specific.
Probably it would be best to have a mechanism in the device tree which
references the
reset mechanism? In any event, you probably don't want a driver to
eplicitly reference the plaform code. If you want to do it explicitly
like this, it would better to have the plaform code reference the driver
mechanism. This would, to some extent, generalize to the device tree
case.
Steve
-Original Message-
From: linuxppc-dev-bounces+stephen=neuendorffer.n...@lists.ozlabs.org
[mailto:linuxppc-dev-
bounces+stephen=neuendorffer.n...@lists.ozlabs.org] On Behalf Of Alon
Ziv
Sent: Sunday, November 15, 2009 1:34 AM
To: linuxppc-dev
Subject: [PATCH/RFC] Booting Xilinx ML510 board using SystemACE
I am using locally the attached (hackish) patch; could someone propose
a
way to make it acceptable to mainline (or should I just keep this
quiet)?
diff --git a/arch/powerpc/platforms/44x/virtex.c
b/arch/powerpc/platforms/44x/virtex.c
index cf96cca..749a330 100644
--- a/arch/powerpc/platforms/44x/virtex.c
+++ b/arch/powerpc/platforms/44x/virtex.c
@@ -51,6 +51,16 @@ static int __init virtex_probe(void)
return 1;
}
+void (*board_reset_system)(char *);
+EXPORT_SYMBOL(board_reset_system);
+
+static void virtex_reset_system(char *cmd)
+{
+ if (board_reset_system)
+ board_reset_system(cmd);
+ ppc4xx_reset_system(cmd);
+}
+
define_machine(virtex) {
.name = Xilinx Virtex440,
.probe = virtex_probe,
@@ -58,5 +68,5 @@ define_machine(virtex) {
.init_IRQ = xilinx_intc_init_tree,
.get_irq= xilinx_intc_get_irq,
.calibrate_decr = generic_calibrate_decr,
- .restart= ppc4xx_reset_system,
+ .restart= virtex_reset_system,
};
diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index b20abe1..f3b4ab9 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -950,6 +950,19 @@ static struct block_device_operations ace_fops =
{
.getgeo = ace_getgeo,
};
+extern void (*board_reset_system)(char *);
+static struct ace_device *the_ace_device;
+
+/*
+ * Board reset using ACE (forced FPGA reconfigure)
+ */
+static void ace_reset_system(char *cmd)
+{
+ printk(KERN_EMERG ace_reset_system: attempting suicide\n);
+ ace_out(the_ace_device, ACE_CTRL, ACE_CTRL_FORCECFGMODE |
+ ACE_CTRL_CFGMODE | ACE_CTRL_CFGSTART | ACE_CTRL_CFGSEL);
+}
+
/*
* SystemACE device setup/teardown code
*/
@@ -1040,6 +1053,9 @@ static int __devinit ace_setup(struct ace_device
*ace)
val |= ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ;
ace_out(ace, ACE_CTRL, val);
+ board_reset_system = ace_reset_system;
+ the_ace_device = ace;
+
/* Print the identification */
dev_info(ace-dev, Xilinx SystemACE revision %i.%i.%i\n,
(version 12) 0xf, (version 8) 0x0f, version
0xff);
**
IMPORTANT: The contents of this email and any attachments are
confidential. They are intended for the
named recipient(s) only.
If you have received this email in error, please notify the system
manager or the sender immediately
and do
not disclose the contents to anyone or make copies thereof.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
This email and any attachments are intended for the sole use of the named
recipient(s) and contain(s) confidential information that may be proprietary,
privileged or copyrighted under applicable law. If you are not the intended
recipient, do not read, copy, or forward this email message or any attachments.
Delete this email message and any attachments immediately.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
