Summary:	DAHLIN-00003: [patch] external timing source
Reporter:	crich (crich)	Labels:
Date Opened:	2007-01-23 05:25:49.000-0600	Date Closed:	2008-11-20 10:53:12.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	NewFeature
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) ztdummy.patch ( 1) ztdummy-trunk.patch
Description:	This patch adds support for getting a 1khz clock from external modules. The first implementation which provides this external clock is done in mISDN (hfcmulti). The hfc multi chip has a 1khz timing interrupt which is very accurate and can be driven by the pstn, if the system has no zaptel card in it it makes sense to use the interrupt from the hfc chip. ztdummy uses RTC if available in the first place, when the hfcmulti driver is loaded it registers at ztdummy and changes the timing source. ztdummy.patch ****** ADDITIONAL INFORMATION ****** We've btw. indented the ztdummy file to use 8 char tabs, that's why the patch is that big.
Comments:	By: Caio Begotti (caio1982) 2007-01-23 07:06:21.000-0600 It's difficult to read a patch that's not in a unified form. Could you please upload it again but this time running "diff -ruN" instead? :-) By the way, why don't you port it to zaptel-1.4? I'd love to test a secondary source of timing, since the RTC is a blocker issue for me. We deploy virtual machines running Asterisk under Xen, whitch has some limitations with the USB timing and non-working RTC for VMs. By: crich (crich) 2007-01-23 07:28:44.000-0600 new patch is in unified form now. i think ztdummy hasn't changed a lot between 1.2 and 1.4 (not sure though), so the patch *should* apply there as well. By: Jason Parker (jparker) 2007-01-23 10:40:46.000-0600 It would be best to split out the whitespace patch By: Paul Cadach (pcadach) 2007-01-24 01:46:33.000-0600 1.4 and up uses PLL techniques to be able to provide 1000 Hz clock from any "external" synchronization source (RTC, USB, kernel timer) and tested on RTC with interrupt frequencies from 2 up to 8192 Hz. As I can see, your code still rely on outdated HZ==1000 expectation, so it will not be able to work with 1.4 without slight modifications (probably if you had not touch indentation it could be work well). Could you please take a look at ztdummy a bit more deeply - I think it is not so hard to register mISDN with zaptel as regular zaptel driver (like ztdummy it does)? I have stupid plans to slightly re-work zaptel to have prioritized synchronization sources, which should take zaptel to be able to switch to most accurate source when it available, and use less accurate when more accurate is not available. ztdummy will be one sort of synchronization source with minimal priority. WBR, Paul. By: Tilghman Lesher (tilghman) 2007-01-24 02:21:14.000-0600 Another problem is that this patch is against 1.2.  As a new feature, it needs to be against trunk. By: crich (crich) 2007-01-24 03:28:32.000-0600 qwell: sorry, the next patch is without Whitespace fixes. PCadach: The code we've added does get it's timing from an mISDN compatible interface card which has an accurate 1khz timing source, which is quite independt on HZ==1000 or HZ==250 or whatever, it's just 1khz anytime, and it's additionally driven by the PSTN. PCadach: We would have loced to develop an external module which directly registers at zaptel as a span, unfortunately this is not quite manageable, because if an external (not in the zaptel source tree) kernel module likes to use zt_register and so forth it will need to include "zaptel.h". If it includes the system wide installed zaptel.h (/usr/include/linux/zaptel.h) and defines __KERNEL__ because it is a Kernel Module parts of zaptel.h include other headers which can not be found, like zconfig.h and the echocancel headers and so forth. Unfortunately these headerfiles are not installed system wide, so it is not really possible to write a zaptel driver which is not included in the zaptel source package. Corydon:  This patch fixes the bug that you cannot send faxes between a tdm4xxP and a b410P or any other mISDN hfc-multi-chip based card. Additionally it fixes the bug that if you have only a mISDN card in your system, the Meetme application (or whatever application requires timing) works correctly (without this patch and with RTC for example after about 1 hour you get an audio delay of 2 seconds per conference partner). By: Tzafrir Cohen (tzafrir) 2007-01-24 04:04:02.000-0600 Interesting. One small thing that may be helpful: look at the module parameter prefmaster for xpp (xpp/xpp_zap.c): allows the user to decide whether or not this module will be try to be the sync master when registering to zaptel. By: Tzafrir Cohen (tzafrir) 2007-01-24 04:07:09.000-0600 Another comment: should it be a patch? Or a separate module that may have dependencies on both Zaptel and mISDN? By: Paul Cadach (pcadach) 2007-01-24 14:29:45.000-0600 I agree with tzafrir about suggestion to have additional driver which provides interface between zaptel and mISDN. crich, I also agree that all timing-driven circuits should be able to generate timing for zaptel for synchronization purposes. Currently zaptel's synchronization methods are ugly, and doesn't allows to have prioritized timing sources except for E1/T1 cards. In my mind I would like to have something like next: 1) All timing-generation circuits (T1/E1 cards, FXS/FXO cards, mISDN cards, including TDMoE) are registered in zaptel.conf with its timing source priority, globally unique for a system; 2) Each device have its "clock accuracy" constant (higher means better accuracy), zaptel have "clock accuracy" parameter too which is lowest possible value to take to it minimal priority; 3) When one or another circuit losts or founds external timing, it notifies zaptel about timing changes happened on this circuit; 4) Combination of 1)...3) should allow to zaptel to keep its internal timing from most accurate circuit available in the system. Also, it would be nice to have mechanism to play with "slave" circuits timing (for example, T1/E1 port which provides timing to connected PBX, while another port/card receives timing from PSTN network). In this scheme, I think it is would be better to have additional pseudo-driver (like ztdummy) which takes timing from mISDN cards and passes it to zaptel. In case of further timing enhancements, it would be more simple to assign some reasonable "accuracy" value to such driver instead of complex modifications of ztdummy. Also, sometime ztdummy could also be integrated into zaptel as default internal timing circuit with some sort of PLL techniques to synhronize ztdummy's clock to external source to keep internal timing as much accurate as possible when other external clock sources get lost. By: crich (crich) 2007-01-24 16:50:04.000-0600 Well i don't see why we need different prios for timing sources. I would say all buffers should be read/written by the same clock, if that clock fails a backup clock could be used. but anyway, ztdummy could easily have a module parameter which modifies its timing prio .. i've attached a new patch now which is quite smaller and does not contain the whitespace fixes, so you can see this patch doesn't hurt existing installations, the user needs to turn it on explicitly. I would rather like to implement a zaptel span from within mISDN, but unfortunately as explained above the current zaptel.h header includes other headers in __KERNEL__ - mode which are not installed in /usr/include/linux, there is only zaptel.h, but zconfig.h ec.h and so on are all missing, that's why it is not possible to write zaptel spans from outside of the zaptel source tree. so we thought ztdummy would be the most apporpriate place where we could add this litle piece of code which fixes a lot of trouble. By: Paul Cadach (pcadach) 2007-01-24 23:16:16.000-0600 What will happen if I would like to unload mISDN after your patch will be applied? As I can see, there is no "unregister" routine. Also, there is no error-detection code. For me, I would more prefer to have something like next: typedef void (timing_callback)(void); timing_callback *ztdummy_register_source(void); int ztdummy_unregister_source(void); When you register mISDN with ztdummy, you will receive an address of procedure to call at each 1 ms interval. When you unregister (before unloading), ztdummy will restore interrupt handling from internal source. To simplify ztdummy internals, I also would like to not play with interrupts/etc (because it could be RTC, USB or kernel timing) but disable calls to ztreceive()/zttransmit() within such handler. Also, when registration succeed, you should increment usage counter of ztdummy driver to prevent its unload (and decrement one at de-registration). By: Paul Cadach (pcadach) 2007-01-24 23:25:03.000-0600 Different timing priority could be nice to differentiate type of ports and clock accuracy they provide. For example, T3/E3 will have highest priority, T2/E2 slightly lower, T1/E1 lower, ISDN even lower than T1/E1, TDMoE lower than ISDN and ztdummy will have lowest priority. You will still have to choose different timing precedence per individual port/span, but ANY higher ierarchical port should have more priority than lower one (due to higher frequiencies and higher accuracy of timing at higher hierarchical levels). By: crich (crich) 2007-01-25 07:54:24.000-0600 new patch with register/unregister functions. i think the kernel interface timer_add/timer_del functions and so forth are meant to be used. I agree regarding timing prio. Indeed a PRI should have higher prio then BRI then ztdummy. By: Paul Cadach (pcadach) 2007-01-26 03:27:29.000-0600 Ok, you still not play with ztdummy usage counter to prevent its unloading while used as interface to zaptel. Are your changes will work correctly while ztdummy runs in USB mode? By: crich (crich) 2007-01-28 15:00:39.000-0600 this time with usage counter handling. USB Mode means Kernel 2.4 which means that mISDN will not run.. By: Paul Cadach (pcadach) 2007-01-29 03:09:41.000-0600 crich, you modifying ztdummy which is not depends on mISDN and should be happily run on 2.4 kernels... Also, decrementing of module reference counter should be IMHO performed AFTER restoring original functionality, not BEFORE. By: Serge Vecher (serge-v) 2007-02-01 08:54:50.000-0600 crich: since this is a new feature, can you please redo the patch against trunk? By: crich (crich) 2007-02-26 15:15:12.000-0600 yes i'll do that. By: kwenzel (kwenzel) 2007-04-03 02:20:39 New Patch against trunk. By: Jason Parker (jparker) 2007-04-06 13:51:22 kwenzel: Do you have a disclaimer on file with Digium?  We're going to need one if we use your updated patch. By: crich (crich) 2007-04-07 06:07:51 kwenzel is working for beronet, he is doing this for beroNet and beronet has a disclaimer on file with digium, so you can use it. By: Tzafrir Cohen (tzafrir) 2007-09-22 15:54:43 The patch probably does not apply to ztdummy after the changes to add hires timers. In addition it should support the hires times case, not only the RTC case. It should also probably set the desc(ription) of the span to note the different clock source. By: Terry Wilson (twilson) 2008-11-19 16:59:05.000-0600 Any updates on this, or should we close this out?  mmichelson recently committed: http://svn.digium.com/view/asterisk?view=rev&rev=157820 which adds a non-zaptel timing source for kernel >= 2.6.25 By: crich (crich) 2008-11-20 03:05:46.000-0600 I think we can close this. By: Terry Wilson (twilson) 2008-11-20 10:53:10.000-0600 Closing at the request of the reporter