@Forrest, I get the bug because I am on the kernel team and we get *all* of these things... ;)
I got it to compile in staging in our ubuntu-karmic git. It would not compile for 64 bit .31rc2 because the code fell into the sizeof(int) == sizeof(void *) trap. I fixed these by using the support fctns and macros. I have attached the diff of what I have done so far. This builds on amd64 now which is crucial for moving from staging to mainline. In looking at the driver style, it would appear that there is an attempt to have a common code base for a number of O/S platforms. Is this true? Is there a VIA business reason for this? I don't have a personal preference one way or the other and VIA may have a good business reasons but this can be an impediment to its acceptance. I ran checkpatch.pl against this driver and it generated ~ 20-30k errors/warnings. I've coded C for a very long time and have seen lots of "styles" come and go so I am reluctant to dictate to someone else where the curly brackets *must* go but I suggest that it would be easier on inclusion if the author or maintainer gets this more in line with the Linux coding standard. I scanned thru the checkpatch output and C99 comments and indent/tab are the bulk of the 98k+ lines of error/warning followed by typedef usage that the kernel maintainers strongly discourage. The C99 comments and indent style can be easily changed by a sed script and one of the numerous "prettyprint" formatters. The result would be able to go through more compilers and become more acceptable to the developer community. Some of these typedef'd structures mimic already defined ones in the rest of the driver tree and are therefore compatibility mis- match candidates at a later date. One of the reasons Linus and the core maintainers descourage this practice is it is a source of silent incompatibility breakage when the surrounding kernel code evolves. If these could be shifted to use the shared definitions in the same way that I made changes in the diff, the code would retain better portability as the rest of the kernel evolves. If you look closely at the definition of skb_reset_mac_header, you will see what I mean. This transparently handles a space optimization on 64 bit kernels that converts a pointer usage to an offset+base pointer. This is what broke the non-ia32 builds. There is also some dead code (for various reasons) that should probably be trimmed. If there is an engineering/business reason for accomodating more O/S environments/APIs than Linux, this could be re-factored to get the common code more in one place. The re- definition of various kernel APIs to add another level of definition re- direction to get this cross-O/S is a prime example where it would be better to refactor into O/S specific and common code modules and keep the *real* interfaces as-is. It has been my experience that using this coding style to get cross platform portability is not as successful as a re-factor. If cross-platform is not really an issue, it would be best to strip this extra out completely. I also fixed a potential suspend bug where the error return was not being propagated back. I have a FIXME comment in the resume to flag a similar problem. This is a place holder that I was going to come back to once I got the core work done. One of the biggest suspend/resume issues we encounter is breakage in wireless drivers. Having this work properly is very important. Getting the style issues resolved, or at least mitigated, helps because other developers are reluctant to plod through code that is unnecessarily different from the Linux norm and we will need their help with the inevitable new bugs once this driver goes "live". As for carrying this in Karmic, we prefer that the driver at least be "on its way" out of staging before we consider it for inclusion. This keeps the not-inconsiderable maintenance cost of an non-mainstream patch and its churn to a minimum. Drivers/modules that come into the kernel this way also don't get lost as we rebase the kernel at each release cycle. I think the best way forward is for the developer to take this input, generate patches to staging to get it moving forward into a mainline merge. We can help with that effort by providing input/review. I do not have this device so I can't really test it myself. We will obviously help shepherd the patch into Ubuntu at the appropriate time. Keep me posted on progress through this bug. I can continue helping with integration at this end. I would also encourage the developer to get a Launchpad account and be part of the conversation. Jim ** Attachment added: "vt6656.diff" http://launchpadlibrarian.net/29018484/vt6656.diff ** Changed in: linux (Ubuntu) Assignee: (unassigned) => Jim Lieb (lieb) -- VT6656 wireless chipset is unsupported https://bugs.launchpad.net/bugs/162671 You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
