LKML: Robert Walsh: Re: [PATCH 01/13] [RFC] ipath basic headers

<?xml version="1.0" encoding="UTF-8" standalone="yes"?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"><html xmlns="http://www.w3.org/1999/xhtml"><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /><title>LKML: Robert Walsh: Re: [PATCH 01/13] [RFC] ipath basic headers</title><link href="/css/message.css" rel="stylesheet" type="text/css" /><link href="/css/wrap.css" rel="alternate stylesheet" type="text/css" title="wrap" /><link href="/css/nowrap.css" rel="stylesheet" type="text/css" title="nowrap" /><link href="/favicon.ico" rel="shortcut icon" /><script src="/js/simple-calendar.js" type="text/javascript"></script><script src="/js/styleswitcher.js" type="text/javascript"></script><link rel="alternate" type="application/rss+xml" title="lkml.org : last 100 messages" href="/rss.php" /><link rel="alternate" type="application/rss+xml" title="lkml.org : last messages by Robert Walsh" href="/groupie.php?aid=5237" /><script> var _paq = window._paq = window._paq || []; /* tracker methods like "setCustomDimension" should be called before "trackPageView" */ _paq.push(["setDoNotTrack", true]); _paq.push(["disableCookies"]); _paq.push(['trackPageView']); _paq.push(['enableLinkTracking']); (function() { var u="//m.lkml.org/"; _paq.push(['setTrackerUrl', u+'matomo.php']); _paq.push(['setSiteId', '1']); var d=document, g=d.createElement('script'), s=d.getElementsByTagName('script')[0]; g.async=true; g.src=u+'matomo.js'; s.parentNode.insertBefore(g,s); })(); </script></head><body onload="es.jasper.simpleCalendar.init();" itemscope="itemscope" itemtype="http://schema.org/BlogPosting"><table border="0" cellpadding="0" cellspacing="0"><tr><td width="180" align="center"><a href="/"><img style="border:0;width:135px;height:32px" src="/images/toprowlk.gif" alt="lkml.org" /></a></td><td width="32">聽</td><td class="nb"><div><a class="nb" href="/lkml"> [lkml]</a> 聽 <a class="nb" href="/lkml/2005"> [2005]</a> 聽 <a class="nb" href="/lkml/2005/12"> [Dec]</a> 聽 <a class="nb" href="/lkml/2005/12/17"> [17]</a> 聽 <a class="nb" href="/lkml/last100"> [last100]</a> 聽 <a href="/rss.php"><img src="/images/rss-or.gif" border="0" alt="RSS Feed" /></a></div><div>Views: <a href="#" class="nowrap" onclick="setActiveStyleSheet('wrap');return false;">[wrap]</a><a href="#" class="wrap" onclick="setActiveStyleSheet('nowrap');return false;">[no wrap]</a> 聽 <a class="nb" href="/lkml/mheaders/2005/12/17/102" onclick="this.href='/lkml/headers'+'/2005/12/17/102';">[headers]</a>聽 <a href="/lkml/bounce/2005/12/17/102">[forward]</a>聽 </div></td><td width="32">聽</td></tr><tr><td valign="top"><div class="es-jasper-simpleCalendar" baseurl="/lkml/"></div><div class="threadlist">Messages in this thread</div><ul class="threadlist"><li class="root"><a href="/lkml/2005/12/16/290">First message in thread</a></li><li><a href="/lkml/2005/12/16/293">Roland Dreier</a><ul><li><a href="/lkml/2005/12/16/289">Roland Dreier</a><ul><li><a href="/lkml/2005/12/16/291">Roland Dreier</a><ul><li><a href="/lkml/2005/12/16/303">Roland Dreier</a><ul><li><a href="/lkml/2005/12/16/301">Roland Dreier</a></li><li><a href="/lkml/2005/12/17/72">Andrew Morton</a></li></ul></li><li><a href="/lkml/2005/12/17/25">Pekka Enberg</a><ul><li><a href="/lkml/2005/12/17/92">Robert Walsh</a></li></ul></li><li><a href="/lkml/2005/12/17/31">Christoph Hellwig</a></li><li><a href="/lkml/2005/12/17/78">Andrew Morton</a><ul><li><a href="/lkml/2005/12/17/103">Robert Walsh</a></li></ul></li></ul></li></ul></li><li><a href="/lkml/2005/12/17/23">Pekka Enberg</a><ul><li><a href="/lkml/2005/12/17/97">Robert Walsh</a></li></ul></li><li><a href="/lkml/2005/12/17/29">Christoph Hellwig</a><ul><li><a href="/lkml/2005/12/17/96">(Eric W. Biederman)</a><ul><li><a href="/lkml/2005/12/17/129">Andi Kleen</a><ul><li><a href="/lkml/2005/12/18/34">(Eric W. Biederman)</a></li></ul></li></ul></li><li><a href="/lkml/2005/12/17/98">Robert Walsh</a><ul><li><a href="/lkml/2005/12/17/100">Arjan van de Ven</a><ul><li><a href="/lkml/2005/12/17/104">Robert Walsh</a></li></ul></li></ul></li></ul></li><li><a href="/lkml/2005/12/17/77">Andrew Morton</a><ul><li class="origin"><a href="/lkml/2005/12/17/126">Robert Walsh</a><ul><li><a href="/lkml/2005/12/17/126">Andrew Morton</a></li></ul></li><li><a href="/lkml/2005/12/19/207">Robert Walsh</a></li></ul></li></ul></li></ul></td><td width="32" rowspan="2" class="c" valign="top"><img src="/images/icornerl.gif" width="32" height="32" alt="/" /></td><td class="c" rowspan="2" valign="top" style="padding-top: 1em"><table><tr><td><table><tr><td class="lp">Subject</td><td class="rp" itemprop="name">Re: [PATCH 01/13] [RFC] ipath basic headers</td></tr><tr><td class="lp">From</td><td class="rp" itemprop="author">Robert Walsh <></td></tr><tr><td class="lp">Date</td><td class="rp" itemprop="datePublished">Sat, 17 Dec 2005 14:39:18 -0800</td></tr></table></td><td></td></tr></table><pre itemprop="articleBody">> > +#define yield() sched_yield() > > Some might get upset about what I assume is userspace test harness code or > what _is_ this doing?) in a driver. But if the maintainers find it useful > we can live with it, That is cosimulator code. It's easy enough to remove. I'll look into it. > > +#ifndef _BITS_PER_BYTE > > +#define _BITS_PER_BYTE 8 > > +#endif > > I'd be inclined to stick BITS_PER_BYTE into include/linux/types.h. Really? I was just going to suggest removing it, but if sticking it in types.h works for you, then fine. > > +static __inline__ void ipath_shortcopy(void *dest, void *src, uint32_t cnt) > > + __attribute__ ((always_inline)); > > s/__inline__/inline/ throughout. OK. > > +#define round_up(v,sz) (((v) + (sz)-1) & ~((sz)-1)) > > We have ALIGN() Yup. > > +struct ipath_int_vec { > > + int long long addr; > > long long OK. > > +#define IPATH_USERINIT _IOW('s', 16, struct ipath_user_info) > > +/* init; kernel/chip params to user */ > > +#define IPATH_BASEINFO _IOR('s', 17, struct ipath_base_info) > > +/* send a packet */ > > +#define IPATH_SENDPKT _IOW('s', 18, struct ipath_sendpkt) > > uh-oh. ioctls. Do we have compat conversions for them all, if needed? For those that are needed, I believe we covered them all. Some have suggested removing ioctls. I'm willing to look into alternatives, but if you think they're OK, I'd rather leave them. > > +/* > > + * A segment is a linear region of low physical memory. > > + * XXX Maybe we should use phys addr here and kmap()/kunmap() > > + * Used by the verbs layer. > > + */ > > +struct ipath_seg { > > + void *vaddr; > > + u64 length; > > +}; > > Suggest `long' for the length. We don't need 64 bits on 32-bit machines. OK. > > +struct ipath_mregion { > > + u64 user_base; /* User's address for this region */ > > void *. > > > + u64 iova; /* IB start address of this region */ > > Maybe here too. OK. > > +int ipath_mlock(unsigned long, size_t, struct page **); > > Sometimes it does `int foo()' and sometimes `extern int foo()'. I tend to > think the `extern' is a waste of space. Yup. > > +#define ipath_func_krecord(a) > > +#define ipath_func_urecord(a, b) > > +#define ipath_func_mrecord(a, b) > > +#define ipath_func_rkrecord(a) > > +#define ipath_func_rurecord(a, b) > > +#define ipath_func_rmrecord(a, b) > > +#define ipath_func_rsrecord(a) > > +#define ipath_func_rcrecord(a) > > What are all these doing? Might need do{}while(0) for safety. I'll look at cleaning them out. Probably left-overs from some earlier experiment. > > +#ifdef IPATH_COSIM > > +extern __u32 sim_readl(const volatile void __iomem * addr); > > +extern __u64 sim_readq(const volatile void __iomem * addr); > > The driver has a strange mixture of int32_t, s32 and __s32. s32 is > preferred. Yea - I'll clean that up. > > + */ > > +static __inline__ uint32_t ipath_kget_ureg32(const ipath_type stype, > > + ipath_ureg regno, int port) > > +{ > > + uint64_t *ubase; > > + > > + ubase = (uint64_t *) (devdata[stype].ipath_uregbase > > + + (char *)devdata[stype].ipath_kregbase > > + + devdata[stype].ipath_palign * port); > > + return ubase ? ipath_readl(ubase + regno) : 0; > > +} > > Are all these u64's needed on 32-bit? Don't know - I'll ask around. We don't support the hardware in 32-bit anyway, so... > > +static __inline__ uint64_t ipath_kget_kreg64(const ipath_type stype, > > + ipath_kreg regno) > > +{ > > + if (!devdata[stype].ipath_kregbase) > > + return ~0ULL; > > We don't know that the architecture implements u64 as unsigned long long. > Some use unsigned long. Best way of implmenting the all-ones pattern is > just `-1'. OK. > Gee. Big driver. Tell me about it :-) Basically, we're doing infiniband in software: no offload. Regards, Robert. -- Robert Walsh Email: rjwalsh@pathscale.com PathScale, Inc. Phone: +1 650 934 8117 2071 Stierlin Court, Suite 200 Fax: +1 650 428 1969 Mountain View, CA 94043. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at <a href="http://vger.kernel.org/majordomo-info.html">http://vger.kernel.org/majordomo-info.html</a> Please read the FAQ at <a href="http://www.tux.org/lkml/">http://www.tux.org/lkml/</a> </pre></td><td width="32" rowspan="2" class="c" valign="top"><img src="/images/icornerr.gif" width="32" height="32" alt="\" /></td></tr><tr><td align="right" valign="bottom"> 聽 </td></tr><tr><td align="right" valign="bottom">聽</td><td class="c" valign="bottom" style="padding-bottom: 0px"><img src="/images/bcornerl.gif" width="32" height="32" alt="\" /></td><td class="c">聽</td><td class="c" valign="bottom" style="padding-bottom: 0px"><img src="/images/bcornerr.gif" width="32" height="32" alt="/" /></td></tr><tr><td align="right" valign="top" colspan="2"> 聽 </td><td class="lm">Last update: 2005-12-17 23:42 聽聽 [from the cache] 漏2003-2020 <a href="http://blog.jasper.es/">Jasper Spaans</a>|hosted at <a href="https://www.digitalocean.com/?refcode=9a8e99d24cf9">Digital Ocean</a> and my Meterkast|<a href="http://blog.jasper.es/categories.html#lkml-ref">Read the blog</a></td><td>聽</td></tr></table><script language="javascript" src="/js/styleswitcher.js" type="text/javascript"></script></body></html>

CINXE.COM

LKML: Robert Walsh: Re: [PATCH 01/13] [RFC] ipath basic headers