From: Alan Cox <alan@lxorguk.ukuu.org.uk> To: "Subhasish" <subhasish@mistralsolutions.com> Cc: <davinci-linux-open-source@linux.davincidsp.com>, <linux-arm-kernel@lists.infradead.org>, <m-watkins@ti.com>, <nsekhar@ti.com>, <sachi@mistralsolutions.com>, "Greg Kroah-Hartman" <gregkh@suse.de>, "open list" <linux-kernel@vger.kernel.org>, "Stalin Srinivasan" <stalin.s@mistralsolutions.com> Subject: Re: [PATCH v2 13/13] tty: pruss SUART driver Date: Tue, 22 Feb 2011 11:11:03 +0000 [thread overview] Message-ID: <20110222111103.5d0dd0a7@lxorguk.ukuu.org.uk> (raw) In-Reply-To: <A6D1CF0E941748448610BE8874FC586F@subhasishg> > we used separate files and hence we decided to keep the code in a separate > directory so that the related files can be identified easily. Fair enough but I would have thought you could drop the two files in the serial directory if they have obviously related names- trivial item/ > > > > > > > > >> +#ifdef __SUART_DEBUG > >> +#define __suart_debug(fmt, args...) \ > >> + printk(KERN_DEBUG "suart_debug: " fmt, ## args) > >> +#else > >> +#define __suart_debug(fmt, args...) > >> +#endif > >> + > >> +#define __suart_err(fmt, args...) printk(KERN_ERR "suart_err: " fmt, ## > >> args) > > > > Use dev_dbg/dev_err/pr_debug/pr_err > > SG - did you mean replace the printks above with dev_dgb/err or the > suart_dbg/err. Ideally all the messages shopuld use dev_dbg/dev_err etc. That allows you to configure debug levels and the like nicely as well as producing clearer printk info. In some cases with tty code you may not know the device so have to use pr_err/pr_debug etc. Ok > > Which is never checked. Far better to use WARN_ON and the like for such > > cases - or if like this one they don't appear to be possible to simply > > delete them > > SG -- OK, does this look ok ? > ================================= > if (h_uart == NULL) { > +WARN_ON(1); > - return PRU_SUART_ERR_HANDLE_INVALID; > +return -EINVAL; > } Yep - the user will now get a backtrace, and in addition kerneloops.org can capture it if that is set up in the distro in use. Alan
WARNING: multiple messages have this Message-ID (diff)
From: alan@lxorguk.ukuu.org.uk (Alan Cox) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 13/13] tty: pruss SUART driver Date: Tue, 22 Feb 2011 11:11:03 +0000 [thread overview] Message-ID: <20110222111103.5d0dd0a7@lxorguk.ukuu.org.uk> (raw) In-Reply-To: <A6D1CF0E941748448610BE8874FC586F@subhasishg> > we used separate files and hence we decided to keep the code in a separate > directory so that the related files can be identified easily. Fair enough but I would have thought you could drop the two files in the serial directory if they have obviously related names- trivial item/ > > > > > > > > >> +#ifdef __SUART_DEBUG > >> +#define __suart_debug(fmt, args...) \ > >> + printk(KERN_DEBUG "suart_debug: " fmt, ## args) > >> +#else > >> +#define __suart_debug(fmt, args...) > >> +#endif > >> + > >> +#define __suart_err(fmt, args...) printk(KERN_ERR "suart_err: " fmt, ## > >> args) > > > > Use dev_dbg/dev_err/pr_debug/pr_err > > SG - did you mean replace the printks above with dev_dgb/err or the > suart_dbg/err. Ideally all the messages shopuld use dev_dbg/dev_err etc. That allows you to configure debug levels and the like nicely as well as producing clearer printk info. In some cases with tty code you may not know the device so have to use pr_err/pr_debug etc. Ok > > Which is never checked. Far better to use WARN_ON and the like for such > > cases - or if like this one they don't appear to be possible to simply > > delete them > > SG -- OK, does this look ok ? > ================================= > if (h_uart == NULL) { > +WARN_ON(1); > - return PRU_SUART_ERR_HANDLE_INVALID; > +return -EINVAL; > } Yep - the user will now get a backtrace, and in addition kerneloops.org can capture it if that is set up in the distro in use. Alan
next prev parent reply other threads:[~2011-02-22 11:09 UTC|newest] Thread overview: 161+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-02-11 14:51 [PATCH v2 00/13] pruss mfd drivers Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 01/13] mfd: pruss mfd driver Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-21 16:30 ` Samuel Ortiz 2011-02-21 16:30 ` Samuel Ortiz 2011-02-22 5:43 ` Subhasish Ghosh 2011-02-22 5:43 ` Subhasish Ghosh 2011-02-22 10:31 ` Samuel Ortiz 2011-02-22 10:31 ` Samuel Ortiz 2011-02-22 10:48 ` Wolfgang Grandegger 2011-02-22 10:48 ` Wolfgang Grandegger 2011-02-22 11:33 ` Samuel Ortiz 2011-02-22 11:33 ` Samuel Ortiz 2011-02-22 12:49 ` Subhasish Ghosh 2011-02-22 12:49 ` Subhasish Ghosh 2011-02-22 16:27 ` Wolfgang Grandegger 2011-02-22 16:27 ` Wolfgang Grandegger 2011-02-23 12:25 ` Subhasish Ghosh 2011-02-23 12:25 ` Subhasish Ghosh 2011-02-23 13:09 ` Russell King - ARM Linux 2011-02-23 13:09 ` Russell King - ARM Linux 2011-02-11 14:51 ` [PATCH v2 02/13] da850: pruss platform specific additions Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 18:41 ` Sergei Shtylyov 2011-02-11 18:41 ` Sergei Shtylyov 2011-02-18 7:18 ` Subhasish Ghosh 2011-02-18 7:18 ` Subhasish Ghosh 2011-02-28 13:04 ` TK, Pratheesh Gangadhar 2011-02-28 13:04 ` TK, Pratheesh Gangadhar 2011-03-01 6:59 ` Subhasish Ghosh 2011-03-01 6:59 ` Subhasish Ghosh 2011-03-03 11:12 ` TK, Pratheesh Gangadhar 2011-03-03 11:12 ` TK, Pratheesh Gangadhar 2011-02-11 14:51 ` [PATCH v2 03/13] da850: pruss board " Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 18:43 ` Sergei Shtylyov 2011-02-11 18:43 ` Sergei Shtylyov 2011-02-18 7:18 ` Subhasish Ghosh 2011-02-18 7:18 ` Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 04/13] mfd: pruss CAN private data Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 05/13] da850: pruss CAN platform specific additions Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 06/13] da850: pruss CAN board " Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 18:45 ` Sergei Shtylyov 2011-02-11 18:45 ` Sergei Shtylyov 2011-02-18 7:19 ` Subhasish Ghosh 2011-02-18 7:19 ` Subhasish Ghosh 2011-02-18 7:19 ` Subhasish Ghosh 2011-02-18 7:19 ` Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 07/13] da850: pruss CAN platform specific changes for gpios Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 18:47 ` Sergei Shtylyov 2011-02-11 18:47 ` Sergei Shtylyov 2011-02-18 7:20 ` Subhasish Ghosh 2011-02-18 7:20 ` Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 08/13] da850: pruss CAN board " Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 09/13] can: pruss CAN driver Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 15:06 ` Kurt Van Dijck 2011-02-11 15:06 ` Kurt Van Dijck 2011-02-11 15:06 ` Kurt Van Dijck 2011-02-14 4:54 ` Subhasish Ghosh 2011-02-14 4:54 ` Subhasish Ghosh 2011-02-14 7:23 ` Wolfgang Grandegger [not found] ` <4D58D854.5090503-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org> 2011-02-14 7:42 ` Kurt Van Dijck 2011-02-14 7:42 ` Kurt Van Dijck 2011-02-14 8:45 ` Subhasish Ghosh 2011-02-14 8:45 ` Subhasish Ghosh 2011-02-14 9:28 ` Wolfgang Grandegger 2011-02-14 9:35 ` Marc Kleine-Budde 2011-02-14 13:15 ` Subhasish Ghosh 2011-02-14 13:15 ` Subhasish Ghosh 2011-02-14 13:15 ` Subhasish Ghosh 2011-02-14 13:33 ` Marc Kleine-Budde 2011-02-14 13:42 ` Wolfgang Grandegger 2011-02-11 15:20 ` Kurt Van Dijck 2011-02-11 15:20 ` Kurt Van Dijck 2011-02-11 15:20 ` Kurt Van Dijck 2011-02-18 7:07 ` Subhasish Ghosh 2011-02-18 7:07 ` Subhasish Ghosh 2011-02-18 7:07 ` Subhasish Ghosh 2011-02-18 7:53 ` Wolfgang Grandegger 2011-02-18 8:15 ` Subhasish Ghosh 2011-02-18 8:15 ` Subhasish Ghosh 2011-02-18 8:15 ` Subhasish Ghosh 2011-02-18 8:36 ` Marc Kleine-Budde 2011-02-18 8:36 ` Marc Kleine-Budde 2011-02-18 8:36 ` Marc Kleine-Budde 2011-02-18 9:09 ` Subhasish Ghosh 2011-02-18 9:09 ` Subhasish Ghosh 2011-02-18 9:09 ` Subhasish Ghosh [not found] ` <1297435892-28278-10-git-send-email-subhasish-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/@public.gmane.org> 2011-02-11 20:33 ` Wolfgang Grandegger 2011-02-11 21:33 ` Marc Kleine-Budde 2011-02-18 15:07 ` Arnd Bergmann 2011-02-18 15:07 ` Arnd Bergmann 2011-03-22 7:30 ` Subhasish Ghosh 2011-03-22 7:30 ` Subhasish Ghosh 2011-03-22 7:30 ` Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 10/13] mfd: pruss SUART private data Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 11/13] da850: pruss SUART board specific additions Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 15:26 ` Michael Williamson 2011-02-11 15:26 ` Michael Williamson 2011-02-18 7:13 ` Subhasish Ghosh 2011-02-18 7:13 ` Subhasish Ghosh 2011-02-11 18:50 ` Sergei Shtylyov 2011-02-11 18:50 ` Sergei Shtylyov 2011-02-22 6:22 ` Subhasish Ghosh 2011-02-22 6:22 ` Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 12/13] da850: pruss SUART platform " Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 18:55 ` Sergei Shtylyov 2011-02-11 18:55 ` Sergei Shtylyov 2011-02-22 9:18 ` Subhasish Ghosh 2011-02-22 9:18 ` Subhasish Ghosh 2011-02-22 11:20 ` Sergei Shtylyov 2011-02-22 11:20 ` Sergei Shtylyov 2011-02-22 13:24 ` Subhasish Ghosh 2011-02-22 13:24 ` Subhasish Ghosh 2011-02-11 14:51 ` [PATCH v2 13/13] tty: pruss SUART driver Subhasish Ghosh 2011-02-11 14:51 ` Subhasish Ghosh 2011-02-11 16:28 ` Alan Cox 2011-02-11 16:28 ` Alan Cox 2011-02-18 13:47 ` Subhasish Ghosh 2011-02-18 13:47 ` Subhasish Ghosh 2011-02-18 14:35 ` Alan Cox 2011-02-18 14:35 ` Alan Cox 2011-02-18 18:23 ` Thomas Gleixner 2011-02-18 18:23 ` Thomas Gleixner 2011-02-18 18:51 ` Arnd Bergmann 2011-02-18 18:51 ` Arnd Bergmann 2011-02-22 8:42 ` Subhasish Ghosh 2011-02-22 8:42 ` Subhasish Ghosh 2011-02-22 14:37 ` Greg KH 2011-02-22 14:37 ` Greg KH 2011-02-23 5:30 ` Subhasish Ghosh 2011-02-23 5:30 ` Subhasish Ghosh 2011-02-23 18:20 ` Greg KH 2011-02-23 18:20 ` Greg KH 2011-02-22 8:43 ` Subhasish Ghosh 2011-02-22 8:43 ` Subhasish Ghosh 2011-02-22 16:34 ` Arnd Bergmann 2011-02-22 16:34 ` Arnd Bergmann 2011-02-24 10:31 ` Subhasish Ghosh 2011-02-24 10:31 ` Subhasish Ghosh 2011-02-22 10:26 ` Subhasish 2011-02-22 10:26 ` Subhasish 2011-02-22 11:11 ` Alan Cox [this message] 2011-02-22 11:11 ` Alan Cox 2011-03-01 13:37 ` Subhasish Ghosh 2011-03-01 13:37 ` Subhasish Ghosh 2011-03-01 14:07 ` Alan Cox 2011-03-01 14:07 ` Alan Cox 2011-02-23 13:35 Subhasish Ghosh 2011-02-23 13:34 ` Subhasish Ghosh 2011-02-23 18:21 ` Greg KH 2011-02-23 18:21 ` Greg KH
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20110222111103.5d0dd0a7@lxorguk.ukuu.org.uk \ --to=alan@lxorguk.ukuu.org.uk \ --cc=davinci-linux-open-source@linux.davincidsp.com \ --cc=gregkh@suse.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=m-watkins@ti.com \ --cc=nsekhar@ti.com \ --cc=sachi@mistralsolutions.com \ --cc=stalin.s@mistralsolutions.com \ --cc=subhasish@mistralsolutions.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.