From patchwork Mon Sep 7 08:13:35 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Wolfram Sang X-Patchwork-Id: 33067 X-Patchwork-Delegate: grant.likely@secretlab.ca Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id DDB27B7BA6 for ; Mon, 7 Sep 2009 18:14:12 +1000 (EST) Received: by ozlabs.org (Postfix) id D3D6DDDD0B; Mon, 7 Sep 2009 18:14:12 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from bilbo.ozlabs.org (bilbo.ozlabs.org [203.10.76.25]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "bilbo.ozlabs.org", Issuer "CAcert Class 3 Root" (verified OK)) by ozlabs.org (Postfix) with ESMTPS id CE407DDD01 for ; Mon, 7 Sep 2009 18:14:12 +1000 (EST) Received: from bilbo.ozlabs.org (localhost [127.0.0.1]) by bilbo.ozlabs.org (Postfix) with ESMTP id 8F4EAB7E2F for ; Mon, 7 Sep 2009 18:13:55 +1000 (EST) Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id 1CD05B7B96 for ; Mon, 7 Sep 2009 18:13:48 +1000 (EST) Received: by ozlabs.org (Postfix) id 0CA99DDD0B; Mon, 7 Sep 2009 18:13:48 +1000 (EST) Delivered-To: linuxppc-dev@ozlabs.org Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [92.198.50.35]) by ozlabs.org (Postfix) with ESMTP id 5F389DDD04 for ; Mon, 7 Sep 2009 18:13:45 +1000 (EST) Received: from [2001:6f8:1178:2:221:70ff:fe71:1890] (helo=pengutronix.de) by metis.ext.pengutronix.de with esmtp (Exim 4.63) (envelope-from ) id 1MkZM0-0008Ar-LP; Mon, 07 Sep 2009 10:13:36 +0200 Date: Mon, 7 Sep 2009 10:13:35 +0200 From: Wolfram Sang To: Damien Dusha Subject: Re: [PATCH] mpc512x: Reset module Message-ID: <20090907081335.GA3190@pengutronix.de> References: MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) X-SA-Exim-Connect-IP: 2001:6f8:1178:2:221:70ff:fe71:1890 X-SA-Exim-Mail-From: w.sang@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linuxppc-dev@ozlabs.org Cc: linuxppc-dev@ozlabs.org X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Hi, On Mon, Sep 07, 2009 at 10:06:01AM +1000, Damien Dusha wrote: > Dear all, > > I have written a small patch to get the software reset working on the > mpc5121.  It is similar to the reset driver found in > arch/powerpc/platforms/83xx/misc.c except it has been modified to look > for the reset node in the device tree before writing to the reset > module. Heh, my patch looks quite similar :) From: Wolfram Sang Subject: mpc5121: add restart capability Hack? Not intended for upstream yet. Signed-off-by: Wolfram Sang --- arch/powerpc/platforms/512x/mpc5121_generic.c | 32 ++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) .init_IRQ = mpc512x_init_IRQ, .get_irq = ipic_get_irq, .calibrate_decr = generic_calibrate_decr, + .restart = mpc5121_restart, }; There is one reason which I wanted to check before submitting this (alas, no time :( ). From mpc52xx_common.c: ... /* * This variable is mapped in mpc52xx_map_wdt() and used in mpc52xx_restart(). * Permanent mapping is required because mpc52xx_restart() can be called * from interrupt context while node mapping (which calls ioremap()) * cannot be used at such point. */ static DEFINE_SPINLOCK(mpc52xx_lock); static struct mpc52xx_gpt __iomem *mpc52xx_wdt; ... I think this may be needed here also... > > Comments are welcome. It's my first patch to the mailing list, so > I'll apologise in advance for any problems with the submission > procedure. > > Damien. > diff --git a/arch/powerpc/platforms/512x/mpc5121_ads.c b/arch/powerpc/platforms/512x/mpc5121_ads.c > index a8976b4..16ca250 100644 > --- a/arch/powerpc/platforms/512x/mpc5121_ads.c > +++ b/arch/powerpc/platforms/512x/mpc5121_ads.c > @@ -70,4 +70,5 @@ define_machine(mpc5121_ads) { > .init_IRQ = mpc5121_ads_init_IRQ, > .get_irq = ipic_get_irq, > .calibrate_decr = generic_calibrate_decr, > + .restart = mpc512x_restart, Please use tabs and not spaces (See CodingStyle). > }; > diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h > index f4db8a7..d77f0ab 100644 > --- a/arch/powerpc/platforms/512x/mpc512x.h > +++ b/arch/powerpc/platforms/512x/mpc512x.h > @@ -14,5 +14,6 @@ > extern unsigned long mpc512x_find_ips_freq(struct device_node *node); > extern void __init mpc512x_init_IRQ(void); > extern void __init mpc512x_init_i2c(void); > +extern void mpc512x_restart(char *cmd); Um, do we need that? > void __init mpc512x_declare_of_platform_devices(void); > #endif /* __MPC512X_H__ */ > diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c > index 135fd6b..deddafc 100644 > --- a/arch/powerpc/platforms/512x/mpc512x_shared.c > +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c > @@ -22,6 +22,7 @@ > #include > #include > > +#include > #include "mpc512x.h" > > unsigned long > @@ -89,6 +90,33 @@ void __init mpc512x_init_i2c(void) > } > } > > +void mpc512x_restart(char *cmd) > +{ > + struct device_node *np; Again spaces instead of a tab. > + struct mpc512x_reset_module *rm; > + > + local_irq_disable(); > + > + np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-reset"); > + > + if (np) { > + > + rm = of_iomap(np, 0); > + if (rm) { Here you used tabs :) > + > + /* Enable software reset "RSTE" (in hex) */ > + out_be32( &(rm->rpr) , 0x52535445); > + > + /* Set the software hard reset */ > + out_be32( &(rm->rcr), 0x2); > + } > + } While one probably won't see that because the reset comes immediately, the flow of code may lead to the false assumption that the following error message gets printed if somebody doesn't have knowledge about this controller. > + > + printk (KERN_EMERG "Error: Unable to map reset module, spinning forever!\n"); KERN_ERR should be enough, I think. > + for(;;); > +} > + > + > /* > * Nodes to do bus probe on, soc and localbus > */ Regards, Wolfram Index: arch/powerpc/platforms/512x/mpc5121_generic.c =================================================================== --- arch/powerpc/platforms/512x/mpc5121_generic.c.orig +++ arch/powerpc/platforms/512x/mpc5121_generic.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "mpc512x.h" @@ -49,6 +50,36 @@ static int __init mpc5121_generic_probe( return board[i] != NULL; } +static void mpc5121_restart(char *cmd) { + + struct mpc512x_reset_module *rm; + struct device_node *rmnode; + + /* HACK: Proably not good from IRQ context */ + + rmnode = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-reset"); + if (!rmnode) { + printk(KERN_ERR "Missing 'fsl,mpc5121-reset' " + "node in device tree!\n"); + goto out; + } + + rm = of_iomap(rmnode, 0); + if (!rm) { + printk(KERN_ERR "Error mapping reset module node!\n"); + goto out; + } + + local_irq_disable(); + + /* Unlock register first, then reset */ + out_be32(&rm->rpr, 0x52535445); + out_be32(&rm->rcr, 0x00000002); +out: + while (1) ; +} + define_machine(mpc5121_generic) { .name = "MPC5121 generic", .probe = mpc5121_generic_probe, @@ -56,4 +89,5 @@ define_machine(mpc5121_generic) {