#qi-hardware IRC log for Tuesday, 2010-11-09

tiduxdoes anyone know if/when the next round of SIEs is coming out?00:01
wolfspraultidux: from my side: as soon as someone orders :-)00:07
wolfspraulthe plan was this: the last run was V2, and (that's a good thing) as part of the V2 run we realized several bugs/shortcomings00:07
wolfspraulso work started on a V3 to address those errata00:07
wolfspraulI'm able and willing to manufacture more, and I have a lot of parts in stock already00:07
wolfspraulI can either manufacture V2 or V300:08
wolfspraulbut, now to the problems..00:08
wolfspraulI do not recommend making more V2, see the bugs we found00:08
wolfspraulthe yield was also not good, if I would do another V2 run I would factor the yield into the price in advance00:08
wolfspraulI need an order of at least 200 or so00:08
wolfspraulbetter would be to go to V3, but I am not sure whether it is 100% finished and reviewed yet00:08
wolfspraulalso for V3, I need a minimum order of maybe 200 or so00:09
wolfspraultidux: the guy who is working on this is tuxbrain_away00:09
tiduxsorry, I was afk for a bit00:09
wolfspraultidux: are you asking about 1 unit for yourself, or a bigger order?00:09
wolfspraulif you just want 1 unit, you need to talk to tuxbrain_away, maybe he is having some sort of preorder/registration list00:09
wolfsprauldon't know00:09
wolfspraulI'm ready to make more00:10
wolfspraulbut I cannot make 100:10
wolfspraulunless you pay a few thousand USD for it00:10
tiduxI understand00:10
wolfsprauland I suggest we apply what we learnt in V2 first, and make a nice step to a better V300:10
tiduxno, that's totally cool00:10
tiduxalas, gentlemen, I must retire for the evening00:12
wpwrakyawn :_05:50
drizztbig yawn in my case05:50
wpwrakbusy night ? :)05:51
drizztgot to bed at 3am05:56
drizztI just read your answer05:56
drizztthere is in fact reference designs05:57
drizztfor example : http://focus.ti.com/docs/toolsw/folders/print/tmdxevm8168ddr2.html05:58
drizztthis one is for the AM389x processor05:58
drizztfor the case/mechanical parts, I already have someone :)05:59
drizztmy brother in law in fact05:59
drizzthe's just started a business with his brother : http://www.lan-gear.com/langear-home06:01
wpwrak(evb) hmm, no documentation. and no mention of design files.06:03
wpwrak(mech) excellent ! that helps :)06:03
wpwrak(evb) it's actually even a bit more expensive that i had thought. but okay, EVBs prices are more or less arbitrary.06:04
drizztevb documentation is listed under Feature -> software06:05
drizztAM389x and C6A816x Software Development kits provided via included 8GB SD Card06:05
drizzt8168 EVM Quick Start Guide06:05
drizzt8168 EVM Hardware Users Guide (SD card)06:05
wpwrakthat's just a user's manual06:06
wpwraknot schematics, layout, etc.06:06
drizzthum.. I do not know how they got them, but we are actually working on a Davinci processor (TI) with a evb from spectrum digital, and i have all this stuff06:07
drizzt(and no direct NDA with TI or spectrum digital)06:08
wpwrakokay :)06:08
wpwrakschematics are usually included with evbs. that's why i was surprised they didn't mention it. layouts usually not, though.06:09
drizztThe evb I had (I now have the prototypes) : http://focus.ti.com/docs/toolsw/folders/print/tmdsevm6446.html06:12
wpwrakbut what do you actually expect to need all this processing power for ? most of the time, this device should do pretty much nothing. maybe chat with the sensors once a minute. process an event or two. the big moment would be then the user presses a soft-button. then you'd have to animate it. a sinclair zx81 could probably do it ;-)06:13
drizztit's even more expensive than the new one !06:13
wpwrakwhoopie ! must be very low volume :)06:13
drizztwpwrak: that for a use limited to domotics control06:14
drizzt(wich could be done by anything, in fact)06:14
wpwrakfor watching tv, you have a big screen elsewhere :)06:14
drizztbut the goal is to provide a tablet as well06:15
drizztfor anything the community would think of06:15
wpwrakhmm, how would that work ? i thought this device was the domotics controller ? so it may have cables connecting to power line/comms network. not very convenient for a tablet.06:15
drizztwe already have a few other aplications for it in mind06:15
drizztyep, depends on the intended use06:16
drizztthere are many many use cases06:16
wpwrakbut a the others would be completely different classes of devices06:17
drizztsome allowing for wired protocols, some not06:17
wpwrakif you want to compete with the iPad, that's a different story :)06:17
drizztnot compete with, but yes, that's part of the idea06:17
wpwraksigh. whatever apple does, one year later, everyone wants to do it too.06:21
wpwrakin a way, that's nice, though. so all the competition crowds in that year's fashionable corner, fights their patente wars, etc., and leaves plenty of niches for the rest of us :)06:22
wpwraki like wolfgnag's approach for the ben. take things that are simple. fully commodized. large parts of the design proven and r&d already written off. that way, you can focus your resources on what makes you different, not waste them in the struggle to stay at the bleeding edge.06:27
wpwrakalso leaves more room to play with the really outlandish ideas, like milkymist ;-)06:29
wpwrakanyway, let's see what he thinks about that tablet06:30
wpwrakrafa: btw, will you also upload the respective source packages ?07:26
wpwrakrafa: (makes things much easier for gpl compliance and such)07:26
rafawpwrak: just for extra packages.. (few ones < 10) The rest is the repo built from OE without modifications. If some user wants to get the source he could read the metadata of the package which has a "Source : http://..."  field.07:30
rafaget the source=get the source of a specific package07:31
wpwrakah, i see. i guess that's alright.07:32
rafawpwrak: but if we need to host every source code I can upload them :) .. The OE building thing used 100GB+ and a bit part of that were the sources07:33
wpwrakdisks are cheap ;-))07:33
rafawpwrak: for example.. if you check the metadata of bash package downloaded from jlime or soon from qi :07:36
rafa wget http://jlime.com/downloads/repository/muffinman/ipk/mipsel/bash_3.2-r8_mipsel.ipk07:36
rafa dpkg -I bash_3.2-r8_mipsel.ipk | grep Source:07:37
rafa Source: ftp://ftp.gnu.org/gnu/bash/bash-3.2.tar.gz file://builtins.patch;patch=1 http://ftp.gnu.org/gnu/bash/bash-3.2-patches/bash32-001;patch=1;pnum=0 http://ftp.gnu.org/gnu/bash/bash-3.2-patches/bash32-002;patch=1;pnum=0 http://ftp.gnu.org/gnu/bash/bash-3.2-patches/bash32-003;patch=1;pnum.....continues..07:37
rafawpwrak: but I am okey with both things.. if we need to uploaded the sources then I will07:41
rafawpwrak: about disks are cheap : http://www.happypenguin.org/ (it is the web site of the company making/selling propietary linux games)07:42
rafacompany=linux game publishing company07:43
rafa"The Western Digital Caviar Green 2TB drive that failed has chemical degredation on the surface making the data recovery much slower and harder"07:43
wpwrakrafa: nice :) and despite having a backup system07:45
rafawpwrak: I was thinking to talk with them to ask if they need people/developers (they use a lot sdl and opengl for everything).. No now it seems :P07:48
wpwrakright now they need data restoration experts :)07:50
wolfspraulrafa: I added your pubkey to the www-data and root accounts on turandot.qi-hardware.com08:02
wolfspraulif you are not sure what you are doing, I suggest to use the www-data account first08:02
wolfspraulyou can definitely copy files into the downloads.qi-hardware.com root folder with it08:02
wolfspraul/srv/www/downloads.qi-hardware.com in the local filesystem08:02
rafawolfspraul: which will be the final link?.. for example.. if, for example, I make a directory named "jlime" (/srv/www/downloads.qi-hardware.com/jlime/). Will it be http://downloads.qi-hardware.com/jlime/ for web browsers ?08:04
rafawolfspraul: BTW, thanks for the info and pubkey work08:04
wolfspraulyes I think so08:05
wolfspraulthe problem is that we have a similar downloads.qi-hardware.com/openwrt url, but that is for sources only08:05
wolfspraulI have no problem with that, so if it doesn't bother you then just use a /jlime folder08:06
wolfspraulthen /openwrt will be sources, but /jlime will be binaries08:06
rafawolfspraul: ah.. great.. Let me work and I will tell you if I feel confident that all is okey (rights, copy, etc).08:06
wolfspraulrafa: ah wait. wrong!08:07
wolfspraulthe openwrt source location is just called /sources08:07
rafawolfspraul: for me is okey.. If you have a better idea for names please tell me :)08:07
wolfspraulwell, just make a /jlime and put whatever you like in there08:07
wolfspraulno I don't08:07
rafawolfspraul: okey, I will08:07
wolfspraul /jlime is good08:07
wolfsprauljust please keep all mp3 and h.264/mpeg4 stuff out08:07
wolfspraulthat's my only concern :-)08:08
rafayes.. /jlime is okey for me as well. so we can have /jlime/images/ and /jlime/repository/ for example.08:08
rafawolfspraul: Yes, I am doing my best about shit packages08:08
rafawpwrak: wolfspraul : tuxbrain_away : In order to have a bit of documentation about the "removed" items: http://fz.hobby-site.org/hp660lx/nn/README_problematic_packages_removed.txt08:11
rafawpwrak: wolfspraul : tuxbrain_away : I am going to check the files that every package contains.. trying to find if some package has mp3 or mp4 inside, or similar. I think that because it is a repo built from OE there is not packages with those file formats08:12
rafabut I want to be sure08:12
wpwrakrafa: excellent, thanks !08:15
wpwrakbtw, what is "db" ?08:15
wpwraksounds like the berkeley database, but that wouldn't be a problem package08:16
rafawpwrak: yes, that is berkely database.. and I did not remove that finally ;) I checked every package, why it is there in the README file yet :)08:17
rafawpwrak: my grep found that because this is a Oracle product08:17
rafawhy=I do not why08:18
wpwrakaah :) well, oracle per se isn't evil ... even if they lately try hard to look that way :)08:18
rafawpwrak: the idea was to find if there is a "Oracle's java"08:19
wpwrakaah, i see08:19
rafawpwrak: I did two lists: one list to remove the mpeg* technologies. And another to find forbidden items in Fedora.08:22
wolfsprauldrizzt: are you there?09:36
wolfspraulyou said you have a meeting today, but I would like to postpone my answer until tomorrow, if possible09:36
wolfspraulalso I have a question for you: what kind of products did you outsource/build in this style before? starting with a reference design, and then doing everything else your own/based on your directives?09:37
wolfspraulwere your partners based in France or other countries?09:37
kyakxiangfu_: are you there?09:45
xiangfu_kyak: yes09:45
kyakxiangfu_: abook in openwrt-packages is marked as broken because it needs at least 70x20 terminal.. What do you think if i set it dependent on setfont2 and start abook via wrapper that would first set the 4x6 terminal font which provides even more than 70x20?09:46
xiangfu_kyak: hmm... how about when abook exit. can it set the font back?09:50
kyakhm i think i can do it09:51
kyaksetfont .. && abook && setfont ..09:52
kyakthe last setfont won't start until abook exists09:53
kyak(in fact, we can even patch the abook to setfont on start and unset it on exit :)09:56
kyakgot to go now09:56
rafawpwrak: fffff.. a package which the source is from maemo has a mp3 inside.. pff11:05
wpwrakrafa: oh, lovely ;-)11:22
wpwrakrafa: what is it that package called ?11:40
qi-bot[commit] Xiangfu Liu: [linux] support increase/decrease in screen brightness http://qi-hw.com/p/openwrt-xburst/edfd9f411:50
kyakxiangfu_: awesome, how one can control brightness?12:07
xiangfu_kyak: /sys/devices/platform/spi_gpio.1/spi1.0/backlight/gpm940b0-bl/brightness ,  0 ~ 25512:08
xiangfu_kyak: about abook, I will try abook first. then let you know what i am thinking :)12:12
xiangfu_compiling ...12:12
kyakxiangfu_: sure.. i noticed that it uses "q" letter instead of frame borders (-). We lack some glyphs in setfont2 fonts12:13
kyakmeanwhile, i'll try this brightness thing :)12:14
rafawpwrak:  Package: osso-sounds12:24
rafawpwrak:  Source: http://repository.maemo.org/pool/maemo/ossw/source/o/osso-sounds/osso-sounds_0.3-1.tar.gz12:25
wpwrakhmm, egrep sound|audio ? :-(12:26
kyakxiangfu_: does work! great job!12:27
rafawpwrak: you mean to look for that into every package?12:28
xiangfu_kyak: the abook compile fine and it can start without change font to 4x6 right? (only cann't edit)12:29
wpwrakrafa: well, if they're that good at hiding the thing :) but it's probably enough if you scan the files names for MP3 or mp3.12:29
wpwrakrafa: getting rid of this junk really is harder than it seems :-(12:30
rafawpwrak: I did :) I have a text file with the list of files of every package on the repo.. so I can look for file names ;)12:30
wpwrakah, cool :)12:30
rafawpwrak: I did egrep -i "..mp3$|avi$.. etc.." just that package looks no nice12:31
rafawpwrak: but well, there could be some hidden stuff which is not easy to find as you said :(12:32
kyakxiangfu_: yeah, it doesn't seem to give any warning messages.. but it also doesn't fit to the screen (compare with what you see via ssh or at your PC)12:33
rafawpwrak: I tested the www-data user under /srv/www/downloads.qi-hardware.com/ as wolfgang suggested and all is okey. So we have all we need to upload stuff ;)12:34
xiangfu_kyak: I found if we use "setfont2 ...4x6" in ssh terminal. the nanonote's console will changed.12:44
xiangfu_kyak: I am testing this for if someone run the "setfont .. && abook && setfont .." in ssh12:45
xiangfu_kyak: so I think let's just wrap the abook. and add it to config.full_system :)12:46
kyakxiangfu_: good. let's do it :) give me some time12:46
xiangfu_kyak: sure12:47
qi-bot[commit] kyak: abook: wrapper to change font size prior to start http://qi-hw.com/p/openwrt-packages/6a969be12:53
wpwrakrafa: (www-data) great ! some day, wolfgang will have to set up real user accounts there. it's a bit messy if everyone shares the same account ...12:53
qi-bot[commit] kyak: forgot the wrapper file for abook... http://qi-hw.com/p/openwrt-packages/8e47e3612:55
xiangfu_kyak: work pretty good. thanks.13:07
xiangfu_time for sleep. see you.13:07
qi-bot[commit] kyak: abook: use ASCII for drawing boxes http://qi-hw.com/p/openwrt-packages/41b535614:30
drizzthum .. got to put my son back to bed, I'll be back in a minute15:35
wpwrakhow did the meeting go ?15:40
drizztwpwrak: sorry for leaving, I had to take care of my little girl. That's the price for working at home :)15:41
drizztwpwrak: quite well15:42
wpwrakhome offices are indeed more efficient without too many disturbances around :)15:42
wpwrakkewl, congrats !15:42
drizzteven better than I could have thought I learnt about this : http://pandaboard.org/content/resources/references15:43
drizzthum ... learnt may not be the right word15:43
wpwrakdoesn't seem to have PCIe, though15:45
drizztwe will have a partnership with my university (CPE-Lyon), and one of the professors pointed me this15:45
drizztI haven't looked at it yet15:46
drizztjust at this page15:46
drizztbut :15:46
drizztBoard Revisions & Documentation ... Gerber File ... Schematics ...  Bill of Materials (BOM) ... Allegro Design File ...15:47
drizztsounds quite good15:47
wpwraki see that you're aligning your requirements with reality. that's good :-)15:49
wpwrakallegro isn't so nice, though. a proprietary tool.15:54
drizztI'd rather have kicad15:55
drizztdo you know if there's a tool to convert between any of the existing CAD formats ?15:56
drizztor if some of the tools can save files in a usable format15:57
wpwrakpheeew ... there are some tools to convert some things. but i'm not sure how usable they are. i've seen a lot of footprint conversions. but then, if you convert from a proprietary system, there's a good change that these libraries will be non-free too.15:58
wpwrakso it may just be more efficient to redo everything manually and make sure you have a good review process.15:58
wpwrakdrawing schematics symbols and footprints isn't too hard for kicad (if you use fped for the footprints. the footprint editor kicad comes with isn't really usable)15:59
wpwrakfor the layout, you could actually reverse the gerbers. but i'm not sure if this catches all the details. probably not.16:00
wpwrak(reverse the gerbers) kicad can load a gerber file and convert it into a kicad board file16:00
drizztPCI-Express and Sata were kind of "why not, use them if they are here"16:12
drizztso this design is rather nice as a starting point16:13
wpwrakyeah, looks nice. if you can make your device just use the existing board, that would be even easier. that would remove a LOT of work.16:14
drizztethernet over USB is not the best, but the goal is not to build a high bandwidth web server :)16:14
drizztthe Ethernet + 2 USB connector is a bit too big16:17
drizztbut maybe we can cope with it16:17
drizztI'll have to check16:17
wpwrakwow, the connector is huge indeed16:19
wpwrakstep 1: https://cgran.org/wiki/gr-air-modes    step 2: write the corresponding sender routines ;-)17:34
qbjectwpwrak: are you close to BA?18:37
qbjectand hello. :)18:37
wpwrakqbject: more or less in the geographic middle of it :)18:37
qbjectThat's cool.18:37
qbjectJust asking because I sometimes fantasize about emigrating to Montevideo.18:37
wpwrakoh, why montevideo ? kinda boring place18:38
wpwrakwell, better government, i think. not that their argentine counterparts would make that bit particularly difficult :)18:38
drizztwpwrak: Buenos Aires ?18:38
qbjectI like meat and freedom. It sounds cheap, compared to places in the US. And I don't need fun, just a lathe, milling machine, and net connection.18:38
wpwrakdrizzt: yup18:39
wpwrakqbject: yup, argentina is cheap. uruguay probably too. you can get mills and such here too. although imported stuff tends to be more expensive than in the us. about 150-200% of what you'd pay there. for consumer electronics, often even more.18:40
qbjectwpwrak: fair enough. Ideally I'd acquire the equipment here then ship it down. Not sure how tricky the port authorities are to deal with. Then I could BUILD consumer electronics.18:41
drizztwpwrak: why are you living there ? any particular reason ?18:42
wpwrakshipping stuff when you're moving might work. otherwise, it gets very tricky.18:42
drizzt(maybe I already asked some times ago on gta02-core)18:43
qbjectwpwrak: that's probably how I'd work it. Get a container, move it all in one go.18:44
wpwrakdrizzt: because i like it here :) when i finished my phd, i considered the choices: europe - been there all my life, can go back anytime i want, not much of a challenge. switzerland - been there all my life, getting boring. us - been there for conferences and such, didn't find it too attractive. south america - been there for some conferences and vacations. like it :)18:45
drizztok, by personnal choice then :)18:46
wpwrakdrizzt: yup. for kernel hacking, internet and a pc is all you really need :) i got into playing with hardware later18:47
drizzta PC, but with two screens :)18:48
drizztwhich you can have anywhere :)18:48
drizztand playing with hardware should be no harder there :)18:49
drizztis it you who found the SMT line in a university for gta02-core ?18:50
drizztwpwrak: another question, completly independent from all others18:50
wpwrakdrizzt: (pc) three screens, actually :) it's a bit of a challenge to get this to work ...18:50
drizztwhat's your experience in kernel hacking ?18:51
wpwrakdrizzt: (university) that was jon "maddog" hall who "found" the smt line18:51
drizzter, not so much of a challenge nowdays, I sometimes have up to five, depending on the place on my desk and what I have at hand (from mimo740 to 24" screens)18:52
drizztusing synergy makes it easier18:53
wpwrakdrizzt: of the stuff that's in general use today: msdos-fs, initrd, linux-atm, diffserv, some other pieces here and there. did some projects in networking, file system, and of course embedded (linux on the psion s5, later openmoko)18:53
drizztI often use my laptop as a third screen18:54
drizztwould you perform code reviews ? (paid, but I'll need a bill)18:54
drizztI'm also doing kernel hacking, and I'm writting my first driver "from scratch", so an external look would be nice18:55
wpwrak(reviews) in what area of the kernel ?18:56
wpwrakah, first driver. always fun :)18:56
drizztI already asked Greg KH, but his current position prevents him from doing it18:57
drizztI already did modifications for specific things, but never designed from scratch18:57
wpwrakanother frequent reviewer is christoph hellwig18:58
wpwrakwhat sort of driver ?18:58
drizztthats a driver handling communication with a 8051 microcontroller over serial18:58
wpwrakUART ? or some other kind of serial ?18:58
drizztmost of it should have gone in userspace, but a few requirements and the fun of it made me do it inside of the kernel18:59
drizztpure UART18:59
wpwrakuart is one of the suckier interfaces ...19:00
wpwrak(user space) indeed, the art is avoid putting stuff into the kernel ;-)19:00
drizztbut I need to handle an interrupt, and reply in less than 50 ms when this interrupt fires19:03
wpwrakinterrupt coming over some separate line ?19:03
wpwrak50 ms seems more than generous enough for user space.19:03
drizztbut this simple part would have been possible standalone, with some protection, but I must not send the reply in the middle of a message, and not even send it if there's a message being sent19:04
drizztyep, quite generous with no system load, but the system will handle video, and the processor is not that good19:05
wpwrakand who sends the messages ? do of course have all sorts of locks in user space as well ...19:05
wpwrak(no system load) that's what real-time priorities are for ;-)19:05
drizztinterrupt over a separate line, yes (gpio)19:06
drizztyes, there's a lot of ways to do it in userspace, but as I said, it was for the fun of doing it19:06
drizztyep for priorities19:07
drizztusing it on another project19:07
wpwrakhere's an example for priorities: http://svn.openmoko.org/developers/werner/neodog/neodog.c19:07
wpwrakah, okay19:07
wpwrakinterrupts in user space are a problem, though. i wonder if there's still the little political problem that prevents us from having a general mechanism for them.19:08
drizztgot a state machine in high priority, and a Qt interface in low prio19:09
drizztand for the interrupt in userspace, I solved it on a VME board by providing a mechanism for userspace programms to register a signal that the kernel triggers when the interrupt fires19:10
wpwrakyup. but that's not something that's in mainline, is it ?19:11
drizztwith some interresting stuff depending on the signal (if I remember, RT signals can stack, thus simulating multiple interrputs)19:11
wpwrakthe political problem being that this would make it possible for a large class of closed-source drivers to be written for user space19:12
drizztno, it was a demo VME driver19:12
drizztwhen I was employee, and not as much attached to free software as I am now19:13
drizztI was following the guidlines of my employer19:14
wpwrakhehe :)19:14
wpwrakyeah, that happens. i also spent some time at ibm :)19:15
drizzt(one of the reasons that made me resign, and become my own boss :)19:15
wpwrakyup, freedom is good to have19:15
drizztso, can I forward you the email for the review ?19:15
wpwraki can have at least a quick look ...19:18
drizztok, I forwarded the original mail19:18
wpwrakbetter to avoid lines > 80 characters19:25
wpwraksee also linux-*/Documentation/CodingStyle, chapter 2, 2nd paragraph :)19:26
drizztyes, I know, but my screen are much wider than they are high19:26
wpwrakthe terminals of the readers of your code may not be, though19:28
drizztdo not take care about anything marked /* debug */19:29
drizztor lines starting with "/* */"19:29
wpwrakline 1805 s/if(/if (/19:29
drizztthose have been added afterwards to allow the author of the 8051 firmware to debug his code19:30
drizztyep, thanks19:32
wpwrakhmm, is the CRC different from the one in lib/crc-ccitt.c ?19:34
drizztmind that the conditions in the mail still apply19:34
drizztfrom my tests yes19:35
wpwrakah, conditions ... you mail is kinda hard to read too, with all the oversized lines ...19:35
drizztmaybe just a question of initial value, but I tried a lot of tools on the net before getting one wich gave me the same CRC as the 805119:36
wpwrakcrcs can be incredibly tricky, yes19:36
drizztand I'm not so good with these kind of stuff19:36
drizztso when I found one which matched, and which gave C code for the implementation ...19:37
drizztI can pay for two hours for you and two hours for another kernel developer which would be19:37
drizztinterrested, at a rate of 110 euro per hour (+ taxes) (total of 440 euros without taxes).19:37
drizztso two hours for you :)19:38
xiangfuwpwrak: Hi I got one email "fped_0.0+r5986-1_amd64.changes ACCEPTED into unstable"  :)19:39
xiangfuwpwrak: forwarding to list19:40
wpwrakxiangfu: congratulations ! now wolfgang owes you a crate of champagne  ;-)19:40
wpwrakdrizzt: kewl. the EI_KEY_... stuff wants to be an enum19:40
wpwraklinux/embedded_interface.h is missing19:41
drizztyes, it is in the userspace code, and here it will be removed19:41
drizztthe driver does not care about what the key means19:42
drizzt(they are all unused)19:42
wpwraki see19:42
drizztsame for EI_LED_*19:43
wpwrakmay be better if you do the clean up you've already planned first ? not much use to review stuff that'll go away anyway. that is, unless you want to compare coverage ;-)19:43
drizztI had not much time to do so when I sent the first request19:44
wpwrakyou have a few indentations made up of spaces. if using vi, you can find them with /^__ (where _ would be a space)19:45
drizztthe author of the 8051 firmware wanted to review my code because he said that my handling of the protocol was wrong and generated the problems I reported, such as 8051 resets, messages getting lost, crc errors, ...19:46
wpwrakheh :)19:46
drizztso I also asked for a review from someone used to kernel drivers19:46
drizztbut during the meeting with the custommer he finaly left behind the idea of reviewing my code19:47
wpwrakfirst pass is always use of whitespace :)19:47
drizztwhatever mistake I could have made, the 8051 should not reset itself ...19:48
drizztso he decided to review his code first19:48
wpwrakunless it's spec'ed to reset when confused19:48
drizztleading spaces corrected (i love vi :)19:49
drizztnot at all, rather the contrary19:50
wpwrakmore whitespace, e.g., line 425: it should be  struct message *free  not  struct message* free  (CS 3.2, 4)19:50
wpwrakyou also occasionally don't put a blank line between block-local declarations and statements, e.g., in emb_intf_put_free_message19:50
drizztthe 8051 should be the "security" part, cutting everything off in case of problem19:51
wpwrakglobal declarations between functions aren't so nice either (partial)19:51
drizztit's a medical device (not a critical one, you can keep going to the hospital)19:51
wpwrak(safety-critical) ah yes, then a reset every now and then isn't so good :)19:52
drizzt(message *free) : this is a "bad habbit" of mine19:53
drizztin fact, moving the '*' from "message *free" to "message* free" made me understand pointers19:53
wpwrak(bad habit) yup. i see it at a few places :)19:54
drizztI would say everywhere19:54
wpwrakbtw, you can save space by not indenting the "case" in a switch, see CS 3, par 219:54
drizztwhat's strange is that every time someone has difficulties with pointers, explaining with the '*' next to the type and not the name makes them understand (along with a short explanation)19:56
wpwrakhehe, that makes sense19:57
wpwrakfor commenting struct members, it improves readability dramatically if you align the comments19:57
drizzt(struct message*) free   <-- free is a (struct message*)  <-- free is a pointer to something of the type (struct message)19:57
drizztand then (*free) becomes obviously ("the something of the type (struct message)19:58
drizztthis is what had me understand pointers, and allowed me to explain to many other students19:59
drizztI sometimes wonder if the professor was that good though ...20:00
wpwrakregarding the CRC, did you check if the one in lib/crc-ccitt.c is really different ? i see that you have a funny initial value.20:00
drizzt(crc) yes, I know it may be only a matter of initial value20:01
drizztI did not chek inside of kernel, but I put the kernel version in a userspace test programm, and got different values20:02
drizztbut at that time I did not try to change the initial value20:02
drizztI had no knowledges about CRC (and I still have very few)20:03
wpwrakokay, so you looked at that one. good. i wonder where the initial value comes from. also, it's a pity the polynom isn't specified.20:03
wpwrakwhenever i've figured out crcs enough for them to make sense, i have to drink a lot afterwards, to forget all this again as quickly as possible ;-)20:04
drizztI finally found this :20:05
drizztwith the crc called "CRC-CCITT (0x1D0F)"20:05
drizztand as 1D0F was mentionned in the documentation of the 8051 programm, i gave it a try20:06
wpwrakaah ! well, your algorithm swaps the bytes. so it may be 0x0f1d.20:06
drizztmaybe that's why I never managed to used those which proposed to set the initial value, as I should have tried 0x0f1d and not 0x1d0f20:08
wpwrakthe table looks different too, though. not sure what to make of this.20:10
drizztthe table is generated in the example found on the website20:10
drizztI printed it to prevent calculation upon startup20:11
drizzt(printed and put it in my code)20:11
wpwrakyup, that's common practice. nothing wrong with that.20:12
wpwrakjust having your own private implementation of an apparently "standard" CRC in a driver is something people will complain about when you submit this for mainline inclusion20:13
drizzthum .. I can send this for mainline, but I do not see the point, it's related only to the protocol on a single product ...20:14
wpwrakyou seem to have a number of code patterns in "struct embeded_intf", such as flags_* and key_*, also with very long names. could it be cleaner to introduce a "struct ei_queue" or such for these ?20:14
wpwrakah, i see. thought you were interested in getting it into shape for mainline submission20:15
drizztit will be in the sources provided with the product (I'll put it on the internal system SD :)20:15
drizztI wanted to know if it would be OK for mainline submission20:16
wpwrakdrizzt: you could simplify things by making curr_msg_id an uint8_t20:17
drizztI can send it, as an exemple, but then it may be much more usefull on the web, outside the kernel, with the protocol and some explanations20:17
drizztthough I think the protocol is under NDA .... :(20:18
drizztbut it's quite easy to find from the sources :)20:18
drizztand it's such a bad thing that it's better not to make it public20:19
wpwrakyeah, sources tend to give such things away :)20:19
wpwrakthe messages do look a little strange indeed20:20
drizzt(the value 100 is sent as the string "100", not as 0x64)20:20
drizztso it's 3 bytes on wire, not one20:20
drizztand then, some messages have been split in three, to keep them short20:21
drizztthey told me it was for "readability"20:21
drizzta human can read it, and write it very easily ..20:22
drizztbut in fact, this is so much wrong .. I have some difficulties with the CRC calculation without a programm ...20:23
drizztso all the protocol should be rewritten .. but won't ever.20:24
drizztyep for the paterns20:26
drizztflags, but also wait queues20:26
drizzthum, your version does not have the wait_queues20:27
wpwrakhaven't gotten to the wait queues yet. btw, minor nit: you could strip one layer of parentheses in lines 516 and 52520:27
wolfspraulwpwrak: fped is in Debian unstable :-)20:27
wolfspraulso is xburst-tools20:27
drizzthad to intriduce them to put userspace programm to sleep while waiting for the replys20:28
wpwrakwolfspraul: world dominaton just got a little closer :-)20:28
wpwrakdrizzt: why, busy-waiting is so much more impressive ;-)20:28
wpwrakdrizzt: also, if (...) { single-statement; }  doesn't need the curly braces. again, some space saved :)20:29
drizztspecially when the doctor is looking at the video20:29
wpwrakdrizzt: the 'b' case in emb_intf_handle_reply could go to a separate function. this also reduces the indentation level20:30
drizzthum, i stick to these since i often add debug and forget to add the curly braces, then wonder what's going on ....20:30
drizztright for 'b' case :)20:31
wpwrak'c' case etc., the memcpys look unwieldy and they probably aren't portable either. how about emb_intf.voltages.val_5v0 = read_param(reply, 0);  with a suitable read_param function ?20:34
drizztand the switch becomes readable :)20:34
wpwrak'f' and 'g' also look as if they could be simplified20:34
drizzth too (same simplifacation in fact)20:35
wpwrakdrizzt: emb_intf_handle_received_message: if (!(msg->type & 0xf000)) { .... and then if (!(msg->type & 0xf000)) { again ?20:39
wpwrakand again, this HUGE if should be a separate function20:39
wpwrakand you could make emb_intf_handle_received_message a wrapper. spin_lock_irqsave(); if () __emb_intf_handle_received_message(); spin_unlock_irqsave();20:41
wpwrakthen you don't need to worry abou the lock inside __emb_intf_handle_received_message() and the code gets a lot easier to review20:41
wpwrakah wait .. you unlock it again inside. sigh.20:42
drizztemb_intf_send_message(request); <--- .. yep20:43
drizztthat's it20:43
drizztand there's other place where I need such things because of code that may sleep20:44
drizzt(not sleep here, but lock too)20:46
wpwraki see the lock-list_empty-del-unlock idiom quite a number of times. may be worth putting this into some helper function20:46
wpwraksee also CS 6: "Functions should be short and sweet, and do just one thing.  They should fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, as we all know), and do one thing and do that well."20:48
wpwrakperhaps the single most important rule ;-)20:48
drizztyes, I should cut in many more functions20:49
wpwrakthis often also helps to spot a pattern that leads for further simplificaiton20:49
drizztthey tend to grow and I do not split them enought20:50
wpwrakit's like peeling an onion. with each layer of complexity you remove, the layer below becomes more visible20:50
wpwrakeek. emb_intf_handle_cmd_set: case doesn't need curly braces20:53
wpwrakthe mask in ((val >> 16) & 0xffff is redundant if val is an uint32_t20:54
wpwrakdrizzt: i also see that your faith in operator precedence is weakening in this function. there's a lot of parentheses you don't need20:55
wpwraka lot of the message generation looks like stuff that could live in user space ?20:56
drizzt(the whole driver should)20:57
drizztbut this way the user space does not care about the protocol20:58
drizztelse there would be parts in the kernel and parts in the user-space20:59
drizztand it's an "arrangment" with the person who does the user-space tool21:00
drizztso he just have to use a few ioctl21:00
drizzthe wanted to kill the man who wrote the protocol when he read it21:01
wpwrakhmm. i wonder if the whole locking logic is really right. e.g., emb_intf_data_process -> emb_intf_handle_received_message -> emb_intf_handle_reply can sleep. and so can emb_intf_handle_cmd_get. yet you're using a spinlick for key_lock21:01
wpwrakor wait .. not sure about it being able to sleep ... lemme check ..21:02
wpwrakyes, it can. there's a "down" there :)21:03
drizztkey_lock ?21:03
drizztyep, but there's no function call between the lock and unlock21:05
wpwrakwhat i mean is that both are in a context where the can sleep. so why a spinlock ?21:06
drizztonly used in two places, with only tests and affectations between21:07
drizztwe can sleep outside the lock21:08
drizztbut not while holding the lock21:08
wpwraksure, but since you can sleep, you may as well use a mutex to protect the data, no ?21:08
drizzti already moved from spin_lock_irqsave to spin_lock21:09
wpwrakyou have very complex locking in that driver. almost half of the code is some spin_lock/spin_unlock. this seems unusually hard.21:10
drizztmaybe I' mistaken, but they are as good as mutexes if the code inside does not sleep21:10
wpwrakwell, you could aim for a lower density of locking operations21:11
drizztmost of the locking is around the list handling21:12
wpwrakyes. couldn't a single mutex handle this as well ? i.e., do you really need this sort of granularity21:13
drizztand i tried a version with less locking, but got problems with the lists (oops on "poison" elements)21:13
drizztand for the granularity, yes, the buffer for receiving data is very small, if i stay locked too long i loose data21:14
drizztbut a version with mutex may solve both problems21:14
drizztyou're right21:15
drizztat least some lists do not need irqsave locking21:16
wpwrakthe only obviously tricky bit seems to be davinci_gpio7_interrupt21:16
drizztand I finally removed the list from the interrupt, replacing it by a flag21:17
drizztI had list handling in there at the beginning21:17
wpwrakyup, you already decoupled this21:17
drizztbut no more21:17
wpwrak(had list handling there) hence the large number of spinlocks :)21:17
drizztthen the mutex should be better21:17
drizzthad to be irqsafe whenever handling lists21:18
wpwraki'm not enirely sure about emb_intf_tty_receive_buf ... lemme see if i can find something21:18
drizztno more now21:18
drizztthis one i must leave as is21:19
drizztit's a tty lock21:19
drizztat least it's what's done in tty drivers in the kernel (this parts has it's roots there)21:20
wpwrakokay, and it's decoupled, too. good.21:20
wpwraknow, if we could get rid of these global variables, it would be perfect21:22
drizztthese ?21:26
drizztemb_intf ?21:26
wpwraki'm not entirely sure, but my guess would be that you could put a pointer to emb_intf into driver_data21:26
drizztthere's only one, and it lives accross all the driver life21:27
wpwrakemb_intf, partial, i think there's more. you hid them well :)21:27
drizzthum, yes21:27
wpwrakhow about having multiple instances of the driver ? :)21:27
drizztonly one instance of the tty line !21:27
drizztthat's a hard limit21:28
wpwrakit's still nice to make the code "proper"21:28
drizztyou've got one point :)21:28
drizztthere's also "sending"21:28
wpwrakparticularly when submitting into mainline, there's always the risk of someone copying the example without being aware of the limitation21:29
drizztei_trace and ei_dbg, but these are for the "debug" version, and will go away with the debug code when the 8051 firmware is OK21:30
wpwrakyeah, i didn't look at the dbg things, like you said21:31
drizztnice :)21:31
wpwrakembeded_intf_remove can lose one level of indentation as well. handle the if first and return immediately. voila :)21:31
drizztho, yes :(21:32
drizztthough when development will be over this won't be a module anymore21:35
wpwrakwhy not ?21:35
drizztit's easier for development :)21:35
drizztbecause it must be here before the system finds it's init and rootfs21:36
drizztthe 8051 is sending his first interrupts long before this, and with ne answer it goes into error21:36
wpwrakoh, i see. a case for initramfs ?21:37
drizztwe cope with this for the development, but the medical persons won't like the error led and bip to turn on at every startup21:38
drizztno real incidence here (initramfs)21:39
drizztthe system boots on SD, and the module could be loader immediatly21:39
wpwrakthey're used to these devices that go BEEEEEEEP all the time :)21:40
drizztbut this is still too long, unless I manage to shorten the boot time21:40
wpwrakseems a little odd to have such a tight timeout21:40
wpwraki mean, what good is the driver if the application isn't ready ?21:41
drizzt three seconds21:41
drizztjust to reply to the "heartbeat"21:41
wpwrakseems that the application should initiate the communication. the 8051 could time out a good while later.21:41
drizzt(it's one of the purposes of the interrupt)21:42
wpwrakwell, have a longer timeout before the first heartbeat21:42
drizztthis is one of my requests to the 8051 firmware developper21:42
wpwrakah, one more ting: the params .. they're binary in the data that gets sent around, right ?21:42
drizztin the protocol ?21:43
drizztbetween 8051 and kernel ?21:43
drizztthey are ascci representations of the values21:44
drizztto send 100, you must "write" the string "100"21:44
drizztnot 0x6421:44
drizzthence the sprintf and scanf21:45
wpwrakoh, and fixed-format. not 100 but 0100,... ?21:45
drizzt"<d,01,0100>A01E" for example21:46
drizztwith even the CRC being sent "A01E" for 0xA01E !21:46
drizztand i spent a lot of time before understanding that it's "A01E" and not "a01e"21:48
wpwrakoh dear. prt[8] ;-))21:48
wpwraki mean ptr21:48
drizztprt[8] ?21:48
drizzt? ptr = ?21:49
drizztlol ?21:49
wpwrakwas this once ptr[4] ? ;-) then, when it crashed, you went to ptr[8] ? :) (in emb_intf_handle_cmd_set)21:49
drizztho, no21:50
drizztI begun with 821:50
drizztand left it so21:50
wpwrakanyway, looks correct. just very odd, the whole thing. now i also understand why you have that memcpy there21:50
wpwraki didn't spot anything evil in the rest. i'm a bit rusty on recent kernel interfaces and may have missed some atrocities, so you may want to try someone like christoph or lars for the 2nd run, after the basic cleanup21:53
drizztI'll check the mutex for the lists21:53
drizztbut thanks a lot :)21:53
wpwrakyou've logged the conversation ?21:54
drizztplease, send me a bill, or at least a way to send you the money21:54
drizztit's all saved :)21:54
drizztI'm paid for this job, so you should be for the review :)21:55
wpwrak(bill) i'll do the "paperwork" tomorrow. need to search for the stuff :)21:55
wpwrakgreat. a bit of income is always a good change from the current 100% hobby work ;-)21:56
drizztyou'll find all information there : http://www.ed3l.fr/index_en.php?page=contacts21:56
wpwrakexcellent, thanks !21:57
drizztI may have one more important review to do, for small modifications, bet for mainline submitions in the future21:58
wpwraki have an account in switzerland, so i'll deprive you of the opportunity of learning the strange rituals involved in sending money to argentina21:58
wpwrakreview of this driver ? or something else ?21:58
drizztit's for patches I made for Alcatel lucent, for usb-serial drivers21:59
drizztand modifications to ppp, to reach theorical up and download speed on 3G, HSUPA and HSDPA networks22:00
drizztthey are under testing in their labs22:00
wpwrakwow. i can review it for style and general issues. for conflicts with the existing design, you'd have to get feedback directly from the people in charge22:01
drizztmoved from 25% to near 100% of theorical rates :)22:01
wpwraknot bad :)22:01
drizztbetter than the other test suites thay had with other systems22:02
drizztand in this case, they'll even pay me for the job of submitting to the kernel22:02
wpwrakthat's how it should be22:03
drizztso, I need to get a few hours of sleep (it's 4 am here)22:03
drizztgot to get up at 822:03
wpwrakah yes. and i should get some dinner. midnight :)22:03
drizztso, "see" you tomorow :)22:04
wpwraksweet dreams ! :)22:04
wpwraki hope all the off-topic stuff didn't scare the rest of the audience too much :) since a lot of it was actually general kernel style issues, i think it may have some use here, too22:06
drizzthum, yes, we could have done it in private22:07
--- Wed Nov 10 201000:00

Generated by irclog2html.py 2.9.2 by Marius Gedminas - find it at mg.pov.lt!