anju Mary wrote: > On Sun, Mar 21, 2010 at 12:17 AM, Jan Kiszka <jan.kis...@web.de> wrote: > >> anju Mary wrote: >>> Hello everybody, >>> I got reply for some of my queries.I was told to >> post >>> the changes in form of a patch against rtnet.git.But i dont know how to >> post >>> in the specified format. >> Unified diff (diff -up orig.file modified.file). Or have at look at git, >> clone rtnet's repository and add your changes as commits. Those can be >> exported in the required format as well. >> >>> Now i am sending my entire driver program with this >>> mail can anyone check it out and give me feedbacks??????? >>> thanks in advance.... >> Uhh, this looks nasty. >> >> There is no ts109 in the upstream kernel, and the tsi108 there looks >> much nicer as it was cleaned up before being merged. The usage of the >> various Linux spinlock in ts109 make me doubt that the original driver >> was ever reviewed by the kernel community. Where did you get your >> version from? Do you have some link to that code? >> >> > >> Some more remarks on your changes: >> - The copyright header only indirectly states that this code is under >> GPL and it fails to mention the version. >> - Make sure to use proper locking. rtdm_irq_disable/enable should not >> be used to disable IRQs for a critical section, rtdm_lock_* comes >> with proper IRQ-blocking services. >> - Do no convert a Linux spinlock to an rtdm_lock if it is not used in >> some critical path. E.g. the stat functions do not belong into this >> category, the RTnet core call them in normal Linux context. >> - rtdm_lock_get is not equivalent to spin_lock, the latter implies >> preemption disabling, the former not. >> >> I already commented on the other topics like (non-existent) skb >> fragmentation. >> >> Jan >> >> Hai, > > I got some feed-backs of my ethernet driver.Thank you very much.But i > need some more clarifications on certain functions and part of code > mentioned below. > > > There is no ts109 in the upstream kernel, and the tsi108 there looks > much nicer as it was cleaned up before being merged. > Where did you get your version from? Do you have some link to that code? > The usage of the various Linux spinlock in ts109 make me doubt that the > original driver was ever reviewed by the kernel community. > > >>> I got this driver[tsi108] from Tundra web site on request. In fact it > was > tsi108 driver, i just renamed variables and functions. The data sheet > tells > 108 and 109 have no significant difference.
[It's hard to read your reply, cited text and your own additions cannot easily be to told apart. Please don't post HTML mails, use standard citation.] OK. So does the latest upstream driver already work for your device under Linux? If so, it looks like the better foundation for a RTnet port than what Tundra is spreading. E.g. that strange re-enabling of IRQs in tsi109_check_for_completed_tx won't map easily on RTDM anyway as here IRQ handlers are called with IRQs disabled. > [I am also sure that this driver may not have reviewed by kernel > community. > There are too many spin locks compared to any other drivers i have > seen.] > > > Make sure to use proper locking. rtdm_irq_disable/enable should not be used > to > disable IRQs for a critical section, rtdm_lock_* comes with proper > IRQ-blocking services. > > > >> I blindly followed the README.drvporting section 17.1 > README.drvporting does not say anything about spin_lock_irq. > How can i convert the function spin_lock_irq to the one which suitable > for Rtnet format? > I see. I'm going to clarify this part in the docs. Note that there was already a comment explaining that the IRQ-line-disabling pattern should only be used if the critical path is too long. From my reading of the tsi, this should not be a problem here. So use the first patter. > > Do no convert a Linux spinlock to an rtdm_lock if it is not used in some > critical path.[stat] > > >> Does this means i can just drop any locking in stat function??? > Do not drop it, but keep it as is, ie. use Linux spinlocks when they are only acquired over Linux contexts. Only if the lock may also be taken by the RT interrupt handler or the xmit function, then a RTDM lock has to be used. > > > rtdm_lock_get is not equivalent to spin_lock, the latter implies preemption > disabling, the former not. > > >> Is README.drvporting section 19 applicable only within interrupt > Handler??? > That's what it should already express. Jan
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev
_______________________________________________ RTnet-users mailing list RTnet-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/rtnet-users
