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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
