On Sat, Nov 1, 2008 at 5:51 PM, Devin Heitmueller <devin.heitmueller@xxxxxxxxx> wrote: > Hello Hans, > > Thanks for getting back to me. Responses inline: > > On Sat, Nov 1, 2008 at 12:21 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote: >>> As one of the half dozen people who are working on the linux-dvb >>> version of em28xx, I am against the wholesale replacement of the >>> current version with Markus's codebase. >> >> At this time I do not advocate replacing the current em28xx driver. But >> when they are both in the kernel, then I expect and hope that the best >> features of the em28xx driver are merged into the empia driver and that >> the current em28xx driver can eventually be dropped. > > I'm certainly not against this approach, and having it in the mainline > will make it easier for others to contribute and improve the codebase. > We would however have to deal with how to handle all the overlapping > product support and the increased confusion that results from users > reporting problems and figuring out which driver they are talking > about (this is already an issue today though as most users don't > understand that there are two drivers). > >>> # Doesn't leverage common infrastructure such as videobuf (resulting >>> in duplicate functionality and more difficult for those who have to >>> maintain multiple drivers) >> >> Definitely a candidate to merge into Markus' driver eventually. There >> are more drivers that do not use videobuf (my own ivtv and cx18 drivers >> spring to mind). > > Agreed. If both drivers are used in parallel than this is less of an > issue. I was just against the wholesale replacement, which would > result in moving backwards in these areas since the work was already > done in the mainline driver. > >>> # Firmware blobs embedded in source - While it's easier for the user, >>> many distributions do not allow firmware blobs in the kernel due to >>> the belief that this is not GPL compatible. We would need to get >>> permission from the vendor to redistribute the firmware as a file (in >>> the V4L driver, we extract it from the Windows driver binary) >> >> From what I saw firmware blobs were only present in the xceive drivers, >> and it is my opinion that it is not a good idea to merge these into the >> kernel. Much better to fix the existing drivers. Having the empia >> driver into the kernel will actually force those fixes to be made. > > Yes, I was referring to the Xceive drivers. I agree with what you > are saying, as long as we can agree that we should not have parallel > tuner implementations in the kernel and the changes to use the > mainline tuners should be made *before* it gets imported. > >>> # Ambigious licensing - some of the files have headers from companies >>> other than Empiatech which are very clearly not GPL compatible (like >>> the Micronas drx3973d driver). Also, it's not clear that even the >>> firmware blobs mentioned above are authorized to be redistributed by >>> their rightful owners (Xceive and Micronas). While Empiatech may be >>> ok with making a GPL driver, these parties have not consented to >>> having their intellectual property in the kernel (they may have >>> consented but the header files say just the opposite). >> >> Licensing should obviously be addressed. But such drivers (except for >> the xceive ones) are currently not used by the empia sources as >> submitted by Markus. > > I do not believe they should be included into the codebase until the > licensing issues are addressed. Having the code in the kernel is a > liability risk, even if it is not used by anything right now. > >>> # It has its own xc3028 and xc5000 tuner driver. I don't know whether >>> his driver is better than the one in V4L. Presumably he has the >>> datasheets for those parts, but on the other hand the V4L driver >>> allows loading of the firmware externally. The V4L drivers are also >>> used by devices beyond the em28xx and may have functionality required >>> by other companies products. >> >> For the record: other devs have datasheets and sources as well for these >> devices. > > Yes, I know. Markus has suggested that his versions of the drivers > are better because they are based on the reference code. The xc5000 > driver aside (where the mainline driver is also based on the Xceive > reference code with proper licensing and attribution), I do not > believe he has ever offered any technical basis for his assertion. > >>> # What I'll call "Black magic" - lots of arbitrary code without any >>> explanation as to what it is doing or why. Why does the DVB init >>> routine write 0x77 to register 0x12? What does that do? A combination >>> of poor use of constants and commented code combined with a lack of >>> access to the datasheets leaves this a mystery. You just have to >>> "trust that it's doing the right thing because it works" >> >> This is not an uncommon occurence when datasheets are not public. >> Hopefully Markus can address such problems when the driver is in the >> kernel. It's IMHO not a blocking issue. > > He has had the opportunity to do this in his own tree, and has thus > far not done it because, as he put it in email to me "nobody cares > about this". I disagree with this assertion personally as someone who > has had to fix bugs in the mainline driver and it would have been very > helpful to at least have commented what some of the code is doing. He > has the datasheets, and has made a conscious decision to not describe > what the code is doing. > You are the first person I ever saw asking for that in a driver. A short mail asking what a specific register does is the usual way how register information can be revealed. eg.: http://linuxtv.org/hg/v4l-dvb/file/55f8fcf70843/linux/drivers/media/video/sn9c102/sn9c102_ov7630.c line 36 and ongoing. >>> # He's the only one who has access to the datasheets, so there is >>> limited opportunity for peer review. The community driver is based on >>> reverse engineering, and we can pass around USB traces we collect to >>> justify/explain design decisions. How do you question a design when >>> the basis of answers is essentially "because the secret document that >>> I can't show you says so"? >> >> There are lots of drivers that are based on NDAs (e.g. my cx18 driver). >> The code is public, but the datasheets aren't. That's actually much >> better than to rely on reverse engineering. Of course, you get the best >> results if the datasheets are also public, but that's sadly not always >> possible. Often active developers can all get NDAs, so that multiple >> devs have access to datasheets (again, that's the case for the cx18 >> driver). >> >> I see this as an advantage, not a disadvantage. > > I understand the value of datasheets, as I am in this situation myself > with several devices. However, in many cases a well written driver > will have good comments as to what it is doing (super secret > algorithms aside). In fact, now that I have access to some of the > Empia datasheets, I have some patches for the mainline driver that > better document some of these cases. > Take care that you get the official agreement to publish documentation about it. >>> I'm sorry if the sharing of my views on this matter create more >>> animosity within the community, as that is the exact opposite of what >>> I want. >> >> This is I think the last chance to get Markus' driver into the kernel. >> If this fails again, then there is no other choice but to fork it all. >> But for the end-users it's so much better if Markus would maintain the >> empia driver since he has the datasheets and hardware. >> >> Forget the history, and see this as a new driver. I think I presented a >> reasonable roadmap for it to be merged. > > Sure, and if Markus is willing to compromise on things like the tuner > drivers, then this would be good for everybody. Past experience has > suggested that he was unwilling to compromise on anything (based on my > attempts in the past), so if things have changed then I would be > thrilled to work with him. > In case of the tuners I'd like to keep them the way they are *for now* - it might be changed lateron. Those things are still in progress. It doesn't interfere with other tuners in the system. The driver explicitly tells the tuner-core to not attach anything when those 2 chips are used for analog and digital TV. It's backward compatible without adding any problems. The xc5000 driver from Steven is based on reference drivers as far as I know, there have been a few updates to it and especially the xc5000 part of that device is still not in a frozen state (there are issues with the cx25843 - xc5000) All the firmware parts are moved out of the driver, frequency tables are kept inside. Markus _______________________________________________ linux-dvb mailing list linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
