[questions] dmesg: Non-NULL drvdata on register
Hello, Not so many time ago i noticed such line in dmesg: radio-mr800 2-1:1.0: Non-NULL drvdata on register Quick review showed that it appears in usb_amradio_probe fucntions. Then i found such code in v4l2_device_register() function (v4l2-device.c file): /* Set name to driver name + device name if it is empty. */ if (!v4l2_dev-name[0]) snprintf(v4l2_dev-name, sizeof(v4l2_dev-name), %s % s, dev-driver-name, dev_name(dev)); if (dev_get_drvdata(dev)) v4l2_warn(v4l2_dev, Non-NULL drvdata on register\n); dev_set_drvdata(dev, v4l2_dev); return 0; The questions is - should i deal with this warning in dmesg? Probably the order of callbacks in radio-mr800 probe function is incorrect. The second questions - should i make atomic_t users counter instead of int users counter? Then i can use atomic_inc(), atomic_dec(), atomic_set(). It helps me to remove lock/unlock_kernel() functions. -- Best regards, Klimov Alexey -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [questions] dmesg: Non-NULL drvdata on register
On Mon, May 4, 2009 at 5:03 PM, Hans Verkuil hverk...@xs4all.nl wrote: Hello, Not so many time ago i noticed such line in dmesg: radio-mr800 2-1:1.0: Non-NULL drvdata on register Quick review showed that it appears in usb_amradio_probe fucntions. Then i found such code in v4l2_device_register() function (v4l2-device.c file): /* Set name to driver name + device name if it is empty. */ if (!v4l2_dev-name[0]) snprintf(v4l2_dev-name, sizeof(v4l2_dev-name), %s % s, dev-driver-name, dev_name(dev)); if (dev_get_drvdata(dev)) v4l2_warn(v4l2_dev, Non-NULL drvdata on register\n); dev_set_drvdata(dev, v4l2_dev); return 0; The questions is - should i deal with this warning in dmesg? Probably the order of callbacks in radio-mr800 probe function is incorrect. I (or you :-) should look into this: I think the usb subsystem is calling dev_set_drvdata as well, so we could have a clash here. I can if i'll find free time for this :) The second questions - should i make atomic_t users counter instead of int users counter? Then i can use atomic_inc(), atomic_dec(), atomic_set(). It helps me to remove lock/unlock_kernel() functions. 'users' can go away completely: if you grep for it, then you'll see that it is only set, but never used. I think I've commented on the kernel lock before: I think it is bogus here. And that the amradio_set_mute handling is wrong as well: you open the radio device twice, then close one file descriptor, and suddenly the audio will be muted, even though there still is a file descriptor open. Regards, Hans Well, looks like it's possible to null usb_amradio_open, right? The only reason why users counter here is suspend/resume handling. The idea was to stop radio in suspend procedure if there users0 and start radio on resume if users0. If there are no users we can do nothing on suspend/resume. The decision was based on radio-users counter, and i see that i can do this using radio-muted. This idea was not implemented yet. May i ask your comments here, Hans? -- Best regards, Klimov Alexey -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [questions] dmesg: Non-NULL drvdata on register
On Mon, May 04, 2009 at 03:03:40PM +0200, Hans Verkuil wrote: Not so many time ago i noticed such line in dmesg: radio-mr800 2-1:1.0: Non-NULL drvdata on register Quick review showed that it appears in usb_amradio_probe fucntions. Then i found such code in v4l2_device_register() function (v4l2-device.c file): /* Set name to driver name + device name if it is empty. */ if (!v4l2_dev-name[0]) snprintf(v4l2_dev-name, sizeof(v4l2_dev-name), %s % s, dev-driver-name, dev_name(dev)); if (dev_get_drvdata(dev)) v4l2_warn(v4l2_dev, Non-NULL drvdata on register\n); dev_set_drvdata(dev, v4l2_dev); return 0; The questions is - should i deal with this warning in dmesg? Probably the order of callbacks in radio-mr800 probe function is incorrect. I (or you :-) should look into this: I think the usb subsystem is calling dev_set_drvdata as well, so we could have a clash here. I don't think so. But probably all USB drivers call usb_set_intfdata (just a wrapper around dev_set_drvdata). Most (media) drivers use it to get driver struct back on a disconnect. My change to use usb interface's struct device for v4l2_device_register caused an oops on disconnect for em28xx based devices. Other drivers (hdpvr, au0828) fortunately call usb_set_intfdata after calling v4l2_device_register. I'm not sure how to resolve this. USB drivers need for the disconnect a way of of getting their private driver struct. We could add a data field to v4l2_device. A couple of drivers seems to be already based on the assumption that dev_get_drvdata(parent.dev) returns a v4l2_device. Janne -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [questions] dmesg: Non-NULL drvdata on register
Hello, Not so many time ago i noticed such line in dmesg: radio-mr800 2-1:1.0: Non-NULL drvdata on register Quick review showed that it appears in usb_amradio_probe fucntions. Then i found such code in v4l2_device_register() function (v4l2-device.c file): /* Set name to driver name + device name if it is empty. */ if (!v4l2_dev-name[0]) snprintf(v4l2_dev-name, sizeof(v4l2_dev-name), %s % s, dev-driver-name, dev_name(dev)); if (dev_get_drvdata(dev)) v4l2_warn(v4l2_dev, Non-NULL drvdata on register\n); dev_set_drvdata(dev, v4l2_dev); return 0; The questions is - should i deal with this warning in dmesg? Probably the order of callbacks in radio-mr800 probe function is incorrect. I (or you :-) should look into this: I think the usb subsystem is calling dev_set_drvdata as well, so we could have a clash here. The second questions - should i make atomic_t users counter instead of int users counter? Then i can use atomic_inc(), atomic_dec(), atomic_set(). It helps me to remove lock/unlock_kernel() functions. 'users' can go away completely: if you grep for it, then you'll see that it is only set, but never used. I think I've commented on the kernel lock before: I think it is bogus here. And that the amradio_set_mute handling is wrong as well: you open the radio device twice, then close one file descriptor, and suddenly the audio will be muted, even though there still is a file descriptor open. Regards, Hans -- Best regards, Klimov Alexey -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [questions] dmesg: Non-NULL drvdata on register
Hello, Not so many time ago i noticed such line in dmesg: radio-mr800 2-1:1.0: Non-NULL drvdata on register Quick review showed that it appears in usb_amradio_probe fucntions. Then i found such code in v4l2_device_register() function (v4l2-device.c file): /* Set name to driver name + device name if it is empty. */ if (!v4l2_dev-name[0]) snprintf(v4l2_dev-name, sizeof(v4l2_dev-name), %s % s, dev-driver-name, dev_name(dev)); if (dev_get_drvdata(dev)) v4l2_warn(v4l2_dev, Non-NULL drvdata on register\n); dev_set_drvdata(dev, v4l2_dev); return 0; The questions is - should i deal with this warning in dmesg? Probably the order of callbacks in radio-mr800 probe function is incorrect. I (or you :-) should look into this: I think the usb subsystem is calling dev_set_drvdata as well, so we could have a clash here. The second questions - should i make atomic_t users counter instead of int users counter? Then i can use atomic_inc(), atomic_dec(), atomic_set(). It helps me to remove lock/unlock_kernel() functions. 'users' can go away completely: if you grep for it, then you'll see that it is only set, but never used. I think I've commented on the kernel lock before: I think it is bogus here. And that the amradio_set_mute handling is wrong as well: you open the radio device twice, then close one file descriptor, and suddenly the audio will be muted, even though there still is a file descriptor open. Regards, Hans -- Best regards, Klimov Alexey -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
