Question: staging: comedi: printk to dev_* version changes

H Hartley Sweeten hartleys at visionengravers.com
Fri May 4 18:08:36 UTC 2012


On Thursday, May 03, 2012 5:02 PM, Greg KH wrote:
> On Thu, May 03, 2012 at 03:59:43PM -0700, H Hartley Sweeten wrote:
>> In comedidev.h the hw_dev variable in struct comedi_device is
>> defined as:
>> 
>> 	/* hw_dev is passed to dma_alloc_coherent when allocating async buffers
>> 	 * for subdevices that have async_dma_dir set to something other than
>> 	 * DMA_NONE */
>> 	struct device *hw_dev;
>> 
>> This pointer is only setup by a couple comedi pci drivers. For
>> most comedi drivers it's going to be NULL.
>> 
>> It appears the "correct" struct device * to use would be the
>> class_dev pointer in struct comedi_device. As a bonus, this
>> pointer is created in comedi_fops.c:comedi_alloc_board_minor()
>> like this:
>> 
>> 	info->device->minor = i;
>> 	csdev = device_create(comedi_class, hardware_device,
>> 			      MKDEV(COMEDI_MAJOR, i), NULL, "comedi%i", i);
>> 	if (!IS_ERR(csdev))
>> 		info->device->class_dev = csdev;
>> 
>> So, if I get this right, most of the printk's and current
>> dev_* variants of this form:
>> 
>> 	printk(KERN_INFO "comedi%d: 8255:", dev->minor);
>> 
>> could be changed to simply:
>> 
>> 	dev_info(dev->class_dev, "8255:");
>> 
>> And the "comedi" prefix is automatically added. Do I have this right?
>
> Yes.  You can drop the 8255: as well if you want, but, you probably
> don't want to, see the thread on linux-kernel today from me and Kay and
> Dmitry with the subject: "proper struct device selection for
> dev_printk()" for more details.
>
> Thanks for finding this, and fixing it up.

Greg,

I looked at the thread, thanks for the pointer.

>From what I can tell the best option for the dev_printk's in comedi
is the class_dev in struct comedi_device. Unfortunately I can only
compile test the change. I don't have any hardware to actually test
what the output message would actually be.

For now should I just fix up the dev->hw_dev issue? After that I can
work on changing the remaining printk/pr_printk uses to the equivalent
dev_printk.

A lot of the messages appear to be noise and should probably be
removed eventually. Some of the common ones in the attach/detach
routines might make more sense if they were moved up a layer.

For instance, almost all the drivers have a message similar to this
in their detach function:

	printk(KERN_INFO "comedi%d: 8255: remove\n", dev->minor);

Changing these to dev_printk's should result in something like this:

	dev_info(dev->class_dev, "8255: remove");

And should generate a message like this (the '0' would be the dev->minor
used when the device was created):

comedi0: 8255: remove

The driver name might actually get output also but I can't verify
that. If these messages are "important" maybe is should be moved up
to the __comedi_device_detach routine instead of being in every
driver. If they are just noise they should all be removed.

Anyway, just wanted you opinion before proceeding.

Regards,
Hartley



More information about the devel mailing list