On Mon, 18 Feb 2013 07:20:56 -0800 Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > On Mon, Feb 18, 2013 at 03:34:07PM +0100, Stefan Tauner wrote: > > This allows user space to retrieve the current frame number on a USB > > as returned by usb_get_current_frame_number(...) via sysfs. > > > > Signed-off-by: Stefan Tauner <stefan.tauner@xxxxxxxxxxxxxxxxxxxx> > > --- > > That's sadly not necessarily the raw value seen on the bus because > > the individual driver functions called by usb_get_current_frame_number(...) > > seem to limit the possible range to the "schedule horizon" of isochronous > > packets. IMHO the function name is a misnomer and I would like to hear your > > opinion on this, a possible new name for it and a better way to retrieve the > > real/raw value. > > > > The rationale for this patch can be found in the thread with the subject > > "Correlating SOF with host system time" from last december > > (201212042020.qB4KKt0N008541@xxxxxxxxxxxxxxxxxxxxxxxxxx). > > My whole use case/idea was kinda frowned upon then, but there was little > > discussion about how it should/could be implemented in the kernel code in > > case one really wants to try it. > > For those of us with bad memories, care to expand on why you want this > in the changelog body of the patch so that the world will remember it as > well? Well, I am not entirely sure if even I want this in (as is) yet, because it is just a part of a complete solution for my problem and I can't imagine a reason why user space would want to know this odd value as is. So I can not give a committable rationale yet, but I can sum up what I am trying to do eventually: I try to map SOF counters to CLOCK_REALTIME timestamps in user space. Later I want to use that to establish a global time base on microcontrollers connected via USB without the hassle of running NTP or PTP over USB. In user space I am waiting for changes in the frame_counter sysfs file by reading it and comparing its contents to values previously read in a busy loop, which is of course not very elegant but it works - apart from the fact that the returned values are not the raw values (and as Alan noted, might also be off a bit)... :) Therefore I am currently playing around with a usb_hcd_get_real_frame_number() which uses a ehci_get_real_frame() function hacked into ehci-hcd.c which returns the actual SOF counter without the artificial horizon limit, but this is of course not committable. I wonder though why the limitation is there in the first place. Most users of usb_get_current_frame_number() seem rather unhappy with the limitation (e.g. drivers/media/usb/uvc/uvc_video.c) or don't care because they cap the returned value to the (locally defined(!)) minimum of the schedule horizon (0xFF everywhere AFAICS). Only very few use it directly to determine a possible scheduling (e.g. drivers/isdn/hisax/st5481_d.c which looks rather dubious to my naive eyes BTW). So from the user's perspective I don't see a reason why the artificially limited reply should be returned instead of the complete value. I have not checked if the raw value is available on all HCDs, but I assume it is. Wouldn't it make sense to change usb_hcd_get_frame_number() to return the raw value and add another one that returns the actual limit of the schedule horizon e.g. periodic_size in the case of EHCI(?) for those users that want to scheduler packets? If not, I would really like to see the documentation of usb_get_current_frame_number() be changed to make it more clear that it is not the raw SOF value and *why*. I have not figured out what *exactly* it is yet/how the iso scheduling works... grasping the kernel - even a subsystem - is quite an effort :) If/when I do I will send a documentation patch. If you think refactoring get_frame_number should be done I would be glad to work on it. A few pointers to what the result should look like and any obvious pitfalls would be appreciated in that case. > Whenever you add/modify/remove a sysfs attribute, you also need to > document it in Documentation/ABI/. Care to do that and resend this > patch? Sure, if you think it makes sense to add the code as is, I can do that any time. Thanks for the amazingly fast reply BTW. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
