dev_printk macro

Hi LKML,

I'm glad to see this patch available, as Greg was. Now I have some questions about it.

a. Is it only for drivers? If so, why?
Filesystems and other subsystems that are not drivers could use something like this also.

b. Is it only for drivers that have a device?
What does a driver use for dev_printk() if it doesn't have a <dev>? However, these do cover the large majority of cases, so that's good.

c. Maybe I'm too cautious (I don't like panics or oopsen :), but I would
have checked <dev> for NULL in those macros.

d. and I know that <sev> is severity, but spelling it as severity
or level would have been better. :)

IOW, this is a good start, no doubt. However, it appears to be limited in scope to drivers. That's still a good start, but there's more to do.

~Randy

PS:  Yes, I know Greg's 2 favorite words:  Patches accepted.
Maybe early next year.  ;)

On Sun, 22 Dec 2002, Linux Kernel Mailing List wrote:

| ChangeSet 1.865.28.18, 2002/12/21 23:54:35-08:00, [email protected]
|
| 	[PATCH] dev_printk macro
|
| #	include/linux/device.h	1.69    -> 1.70
|
| diff -Nru a/include/linux/device.h b/include/linux/device.h
| --- a/include/linux/device.h	Sun Dec 22 13:04:59 2002
| +++ b/include/linux/device.h	Sun Dec 22 13:04:59 2002
| @@ -396,22 +396,21 @@
|  extern void firmware_unregister(struct subsystem *);
|
|  /* debugging and troubleshooting/diagnostic helpers. */
| +#define dev_printk(sev, dev, format, arg...)	\
| +	printk(sev "%s %s: " format , (dev).driver->name , (dev).bus_id , ## arg)
| +
|  #ifdef DEBUG
|  #define dev_dbg(dev, format, arg...)		\
| -	printk (KERN_DEBUG "%s %s: " format ,	\
| -		(dev).driver->name , (dev).bus_id , ## arg)
| +	dev_printk(KERN_DEBUG , (dev) , format , ## arg)
|  #else
|  #define dev_dbg(dev, format, arg...) do {} while (0)
|  #endif
|
|  #define dev_err(dev, format, arg...)		\
| -	printk (KERN_ERR "%s %s: " format ,	\
| -		(dev).driver->name , (dev).bus_id , ## arg)
| +	dev_printk(KERN_ERR , (dev) , format , ## arg)
|  #define dev_info(dev, format, arg...)		\
| -	printk (KERN_INFO "%s %s: " format ,	\
| -		(dev).driver->name , (dev).bus_id , ## arg)
| +	dev_printk(KERN_INFO , (dev) , format , ## arg)
|  #define dev_warn(dev, format, arg...)		\
| -	printk (KERN_WARNING "%s %s: " format ,	\
| -		(dev).driver->name , (dev).bus_id , ## arg)
| +	dev_printk(KERN_WARNING , (dev) , format , ## arg)
|
|  #endif /* _DEVICE_H_ */

Definitely - fs_printk, inode_printk etc may all make sense

This case isnt covered. All devices should eventually have a dev right
8)

On 25 Dec 2002, Alan Cox wrote:

| On Mon, 2002-12-23 at 00:46, Randy.Dunlap wrote:
| >
| > I'm glad to see this patch available, as Greg was.
| > Now I have some questions about it.
| >
| > a.  Is it only for drivers?  If so, why?
| >     Filesystems and other subsystems that are not drivers could use
| >     something like this also.
|
| Definitely - fs_printk, inode_printk etc may all make sense
| >
| > b.  Is it only for drivers that have a device?
| >     What does a driver use for dev_printk() if it doesn't have a <dev>?
| >     However, these do cover the large majority of cases, so that's good.
|
| This case isnt covered. All devices should eventually have a dev right
| 8)

Eventually, but not always. Anyway, after another day or 2 to reflect on it, I'll withdraw the comment I made about checking <dev> for NULL in those macros. They can/should be context-sensitive.

~Randy