On Fri, May 04, 2012 at 01:08:36PM -0500, H Hartley Sweeten wrote: > 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. That's ok. > 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. Sounds reasonable. > 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. I think they are proably all just "noise". A driver shouldn't say anything unless an error happens. Little "I am attached, here is my version number" messages should all be removed as they are useless and just slow boot times down. thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
