Re: [PATCH 1/7] Adding empia base driver - Kernel

This is a discussion on Re: [PATCH 1/7] Adding empia base driver - Kernel ; (Note: I'm reposting this since it bounced on several mailinglists due to "Too many recipients". I'm now only sending it to the mailinglists as I assume that most of the recipients are on at least one of these mailinglists. Apologies ...

+ Reply to Thread
Results 1 to 11 of 11

Thread: Re: [PATCH 1/7] Adding empia base driver

  1. Re: [PATCH 1/7] Adding empia base driver

    (Note: I'm reposting this since it bounced on several mailinglists due
    to "Too many recipients". I'm now only sending it to the mailinglists
    as I assume that most of the recipients are on at least one of these
    mailinglists. Apologies if this is not the case.)


    Hi Markus,

    As promised I've done a review of your empia driver and looked at what
    needs to be done to get it into the kernel.

    First of all, I've no doubt that your empia driver is better and
    supports more devices than the current em28xx driver. I also have no
    problem adding your driver separate from the current driver. It's been
    done before (certain networking drivers spring to mind) and while
    obviously not ideal I expect that the older em28xx driver can probably
    be removed after a year or something like that.

    In my opinion it's pretty much hopeless trying to convert the current
    em28xx driver into what you have. It's a huge amount of work that no
    one wants to do and (in this case) with very little benefit. Of course,
    Mauro has the final say in this.

    While there is some cleaning up to do in your code (coding style, some
    copyright/license clarifications), that is not a big deal. The coding
    style cleanups are a fair amount of work, but I think we can probably
    spread that out over several people and get that done quickly.

    What I definitely would recommend is to use video_ioctl2 rather than
    video_usercopy. The latter function will disappear in the future. I
    think the policy for new drivers is to use video_ioctl2, so this is
    probably a required task before it can be merged. Doing this will
    improve maintenance of the code as well, so it's useful to do this
    anyway. I think it's better if you do it, but I guess some volunteer
    can probably be found if needed. It's not a difficult task.

    Now the real problems are with three duplicate i2c drivers: cx25843,
    xc3028 and xc5000.

    To start with the easiest one: cx25843. There already is a driver for
    this (cx25840) and the empia driver should use that one. Since I
    maintain cx25840 the easiest approach for you is to see if you can get
    me hardware (em28xx + cx25843) so that I can test and update cx25843 to
    provide proper empia support. The alternative is that we work together
    on this, but that's probably more work for both of us. I most
    definitely do *not* want to duplicate this driver. Windows drivers
    duplicate this stuff all the time, but we're not Windows and having one
    driver ensures that fixes or new functionality become available to all
    bridge drivers that use it.

    The same reasoning is true for xc3028 and xc5000. In addition, the
    licensing of these sources is very vague. Is it even allowed to
    distribute them under a GPL license? There is no GPL license in the
    sources, yet in the module you specify GPL. This needs to be clarified
    first.

    I see two ways forward when it comes to these drivers:

    1) The empia driver switches to the current xceive drivers that are
    already in the kernel. No doubt this means that xceive driver bugs will
    surface with certain devices, but those are bugs that the xceive driver
    maintainer will have to fix. Obviously assistance would be appreciated,
    but the bottom line is that a) your driver is finally in the kernel,
    and b) there is a lot more pressure to fix bugs in the xceive drivers
    than is the case otherwise. Yes, some devices will not work initially
    with the empia driver, but I expect that to be a temporary situation.

    2) Your xceive drivers replace the current drivers. This will require
    that a) the license situation is clarified, b) the drivers are modified
    to fit the current v4l-dvb tuner architecture. This option will mean a
    lot of work for you since you are the maintainer of these drivers. In
    addition, I forsee a lot of flaming if we go this way.

    BTW, I noticed that the current xc5000 driver is very similar to yours
    but with proper copyrights/license notices and coding style. So for
    this driver option 1 is definitely the way to go.

    To be honest, I don't see option 2 as viable. I forsee too many
    inter-personal problems that will appear if we go that way. Option 1
    has the big advantage that all you need to do is to file a bug report
    if it doesn't work with a certain device. And in the meantime users can
    fallback to your stand-alone driver until it's fixed in the kernel.
    Obviously, if you can state in the bug report what the precise problem
    is, so much the better.

    So my recommendation would be to:

    1) Switch to using the current xceive drivers in your empia driver. This
    is something you have to do, I'm afraid. Unless someone would
    volunteer? I personally do not have enough experience with this to do
    it.

    2) Switch to using the cx25840 driver. If I can get hardware, then I can
    do this, otherwise we have to do this together. Initially we probably
    have to disable devices using the cx25840 until the cx25840 driver has
    been fixed for the empia driver.

    3) Switch to video_ioctl2 in the empia driver. You can do that, but we
    can probably find a volunteer as well.

    4) Conform the code to the coding style. If several people can help with
    this we can get it done pretty quickly.

    5) Merge the empia driver alongside the current em28xx driver.

    There are no doubt some things I missed, but I don't think I missed
    anything major. I've put up a hg tree here:

    http://linuxtv.org/hg/~hverkuil/v4l-dvb-em28xx/

    This allows you to compile the empia module alongside the em28xx module.
    Note that I've removed the empia cx25843 module in the last changeset
    in order to test which dependencies the driver had on that module. So
    if you want to test this tree with an empia+cx25843 device, then don't
    get the latest changeset, but the one before that.

    My tree does contain the empia xceive drivers, though. Perhaps someone
    more knowledgeable with DVB can take a look to see how much work it is
    to convert to the kernel xceive drivers? And to see if I overlooked any
    DVB-related major obstacles?

    I think this is a reasonable roadmap to finally get this in. If everyone
    can pitch in then it really shouldn't take that much work to get it
    into v4l-dvb and from there to 2.6.29.

    Regards,

    Hans Verkuil
    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [linux-dvb] [PATCH 1/7] Adding empia base driver

    On Sat, 2008-11-01 at 15:05 +0100, Hans Verkuil wrote:

    > Hi Markus,
    >
    > As promised I've done a review of your empia driver and looked at what
    > needs to be done to get it into the kernel.
    >
    > First of all, I've no doubt that your empia driver is better and
    > supports more devices than the current em28xx driver. I also have no
    > problem adding your driver separate from the current driver. It's been
    > done before (certain networking drivers spring to mind) and while
    > obviously not ideal I expect that the older em28xx driver can probably
    > be removed after a year or something like that.


    [snip]

    > So my recommendation would be to:


    [snip]

    > 3) Switch to video_ioctl2 in the empia driver. You can do that, but we
    > can probably find a volunteer as well.
    >
    > 4) Conform the code to the coding style. If several people can help with
    > this we can get it done pretty quickly.


    I can support these two portions of the effort, if what Hans' proposes
    is the agreed plan.

    Point me to the target directories in the repo, and suggest desired
    completion dates for whatever tasks.

    Standing by...

    Regards,
    Andy


    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [linux-dvb] [PATCH 1/7] Adding empia base driver

    Hello,

    I have held off on offering an opinion this to see what others
    thought, but I think now may be the time to speak up.

    First off, a disclaimer: I am a contributor to the existing in-kernel
    em28xx driver. I have spent many months working on that codebase,
    adding device support and fixing bugs. I also have a large series of
    patches queuing up that significantly improve both the codebase's
    functionality and maintainability (having recently been given access
    to some of the datasheets thanks to Empia and Pinnacle).

    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.

    Why? I've got a list of reasons, but in the interest of fairness,
    let's start with what I see to be the good things:

    # Significantly better device support - Markus has access to the
    actual hardware for many of these devices, spends time adding support
    for new devices, and since he works for the chipset vendor can in many
    cases just call the manufacturer of the product and ask them
    questions.

    # Proper tuner locking - tuner locking was one of the big issues that
    caused infighting before Markus forked off his code. Let's face it -
    it's been over a year and most of the other devices don't do any
    locking at all. His scheme, while not unified across drivers, is
    better than nothing.

    # Based on the chipset datasheets - He had the benefit of just being
    able to look up what the registers mean

    # VBI support for analog - a recent addition in the mrec driver, but
    none at all in the V4L driver

    # Support for other demods not currently in V4L - I don't think we
    have any devices yet that use the LGDT3304, but Markus's driver has
    support for devices that do.

    # More thoroughly debugged - He's working on this full time. He's
    working bugs, dealing with issues, and putting in proper fixes based
    on reliable information instead of reverse engineering.

    ========

    Now, the not so good things:

    # Doesn't leverage common infrastructure such as videobuf (resulting
    in duplicate functionality and more difficult for those who have to
    maintain multiple drivers)

    # 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)

    # 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).

    # 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.

    # 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"

    # 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"?

    ====

    I shared this list with Markus a few months ago and, the licensing
    issues aside, it was his contention that "nobody cares" about most of
    the things above. As a maintainer that wants to continue contributing
    to the codebase, *I* care. And I'm sure that anybody other than
    Markus who wants to understand the em28xx codebase and be able to fix
    bugs would also care. I'm also concerned with consistency between
    drivers. Having one driver do things differently than all the others
    is just a maintenance headache for those who have to support multiple
    drivers.

    A number of people have suggested that nobody was willing to
    incorporate Markus's changes incrementally to improve the in-kernel
    driver. This couldn't be further from the truth. I appealed to
    Markus on multiple occasions trying to find some compromise where his
    changes could be merged into the mainline em28xx driver. He outright
    refused. It was his contention that his driver was/is better than the
    in-kernel driver in every possible way, and that the existing code has
    no redeeming value. In fact, I was accused of taking his GPL'd code
    without his consent and incorporating it into the linux-dvb codebase.
    It's this "all or nothing" attitude that has prevented his work thus
    far from being incorporated, not the unwillingness of people like
    myself to do the work to merge his changes in a sane matter.

    I *really* want to see this resolved, because I recognize that I could
    be better served working on other things than duplicating efforts to
    debug issues that Markus may have already fixed in his codebase. But
    just throwing away the work of half a dozen other developers on an
    actively maintained driver is not really the sort of compromise I
    think would be best for the community.

    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.

    Devin

    --
    Devin J. Heitmueller
    http://www.devinheitmueller.com
    AIM: devinheitmueller
    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [linux-dvb] [PATCH 1/7] Adding empia base driver

    Hi Devin,

    On Sat, Nov 1, 2008 at 5:46 PM, Devin Heitmueller
    wrote:
    > A number of people have suggested that nobody was willing to
    > incorporate Markus's changes incrementally to improve the in-kernel
    > driver. This couldn't be further from the truth. I appealed to
    > Markus on multiple occasions trying to find some compromise where his
    > changes could be merged into the mainline em28xx driver. He outright
    > refused. It was his contention that his driver was/is better than the
    > in-kernel driver in every possible way, and that the existing code has
    > no redeeming value. In fact, I was accused of taking his GPL'd code
    > without his consent and incorporating it into the linux-dvb codebase.
    > It's this "all or nothing" attitude that has prevented his work thus
    > far from being incorporated, not the unwillingness of people like
    > myself to do the work to merge his changes in a sane matter.


    I'm not sure I understand how he can refuse such a thing. If the code
    is released under the GPLv2 and the author refuses to play by the well
    known rules of the kernel community, then I don't see any problem with
    taking the code and improving the current driver (as long as the
    copyright is properly attributed, of course).

    I think it's already pretty well established that we don't just take
    in shiny new drivers and trust a new maintainer to do the right thing
    because that has gotten us in such a mess so many times before. Being
    part of the community is not so much the code you write but the way
    you interact with other kernel developers.

    So, if I were you, I'd just do it.

    Pekka
    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [linux-dvb] [PATCH 1/7] Adding empia base driver

    On Sat, Nov 1, 2008 at 12:08 PM, Pekka Enberg wrote:
    > Hi Devin,
    >
    > On Sat, Nov 1, 2008 at 5:46 PM, Devin Heitmueller
    > wrote:
    >> A number of people have suggested that nobody was willing to
    >> incorporate Markus's changes incrementally to improve the in-kernel
    >> driver. This couldn't be further from the truth. I appealed to
    >> Markus on multiple occasions trying to find some compromise where his
    >> changes could be merged into the mainline em28xx driver. He outright
    >> refused. It was his contention that his driver was/is better than the
    >> in-kernel driver in every possible way, and that the existing code has
    >> no redeeming value. In fact, I was accused of taking his GPL'd code
    >> without his consent and incorporating it into the linux-dvb codebase.
    >> It's this "all or nothing" attitude that has prevented his work thus
    >> far from being incorporated, not the unwillingness of people like
    >> myself to do the work to merge his changes in a sane matter.

    >
    > I'm not sure I understand how he can refuse such a thing. If the code
    > is released under the GPLv2 and the author refuses to play by the well
    > known rules of the kernel community, then I don't see any problem with
    > taking the code and improving the current driver (as long as the
    > copyright is properly attributed, of course).
    >
    > I think it's already pretty well established that we don't just take
    > in shiny new drivers and trust a new maintainer to do the right thing
    > because that has gotten us in such a mess so many times before. Being
    > part of the community is not so much the code you write but the way
    > you interact with other kernel developers.
    >
    > So, if I were you, I'd just do it.


    Hello Pekka,

    I do not believe that he had any legal standing to prevent me from
    taking the code and incorporating it if that was my desire, given that
    he released it under the GPL. However, taking someone's code when
    they specifically said they don't want you to is kind of a crappy
    thing to do in my humble opinion, and it definitely doesn't improve
    relations with the developer.

    The reality is that from a technical standpoint I really want Markus
    to be the maintainer. He knows more about the devices than anyone,
    works for the vendor, and has access to all the datasheets. That
    said, I don't want a situation where he is the *only* one who can do
    work on the codebase because it is poorly commented and structured in
    a way where only he can understand why it works the way it does.
    Also, I believe certain design decisions should be made as a result of
    consensus with the other maintainers, taking into consideration
    consistency across drivers.

    Regards,

    Devin

    --
    Devin J. Heitmueller
    http://www.devinheitmueller.com
    AIM: devinheitmueller
    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [linux-dvb] [PATCH 1/7] Adding empia base driver

    On Sat, Nov 1, 2008 at 6:21 PM, Hans Verkuil wrote:
    > 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.


    We've done that many times and it usually ends up with us dragging the
    old driver along for a very long time (look at the eepro100 driver
    removal as an example). So you really want to just incrementally fix
    up em28xx driver to avoid a mess.
    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [linux-dvb] [PATCH 1/7] Adding empia base driver

    Hi Devin,

    Just a few quick notes:

    On Saturday 01 November 2008 16:46:03 Devin Heitmueller wrote:
    > Hello,
    >
    > I have held off on offering an opinion this to see what others
    > thought, but I think now may be the time to speak up.
    >
    > First off, a disclaimer: I am a contributor to the existing in-kernel
    > em28xx driver. I have spent many months working on that codebase,
    > adding device support and fixing bugs. I also have a large series of
    > patches queuing up that significantly improve both the codebase's
    > functionality and maintainability (having recently been given access
    > to some of the datasheets thanks to Empia and Pinnacle).
    >
    > 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.

    > Why? I've got a list of reasons, but in the interest of fairness,
    > let's start with what I see to be the good things:
    >
    > # Significantly better device support - Markus has access to the
    > actual hardware for many of these devices, spends time adding support
    > for new devices, and since he works for the chipset vendor can in
    > many cases just call the manufacturer of the product and ask them
    > questions.
    >
    > # Proper tuner locking - tuner locking was one of the big issues that
    > caused infighting before Markus forked off his code. Let's face it -
    > it's been over a year and most of the other devices don't do any
    > locking at all. His scheme, while not unified across drivers, is
    > better than nothing.
    >
    > # Based on the chipset datasheets - He had the benefit of just being
    > able to look up what the registers mean
    >
    > # VBI support for analog - a recent addition in the mrec driver, but
    > none at all in the V4L driver
    >
    > # Support for other demods not currently in V4L - I don't think we
    > have any devices yet that use the LGDT3304, but Markus's driver has
    > support for devices that do.
    >
    > # More thoroughly debugged - He's working on this full time. He's
    > working bugs, dealing with issues, and putting in proper fixes based
    > on reliable information instead of reverse engineering.
    >
    > ========
    >
    > Now, the not so good things:
    >
    > # 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).

    > # 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.

    > # 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.

    > # 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.

    > # 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'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 shared this list with Markus a few months ago and, the licensing
    > issues aside, it was his contention that "nobody cares" about most of
    > the things above. As a maintainer that wants to continue
    > contributing to the codebase, *I* care. And I'm sure that anybody
    > other than Markus who wants to understand the em28xx codebase and be
    > able to fix bugs would also care. I'm also concerned with
    > consistency between drivers. Having one driver do things differently
    > than all the others is just a maintenance headache for those who have
    > to support multiple drivers.
    >
    > A number of people have suggested that nobody was willing to
    > incorporate Markus's changes incrementally to improve the in-kernel
    > driver. This couldn't be further from the truth. I appealed to
    > Markus on multiple occasions trying to find some compromise where his
    > changes could be merged into the mainline em28xx driver. He outright
    > refused. It was his contention that his driver was/is better than
    > the in-kernel driver in every possible way, and that the existing
    > code has no redeeming value. In fact, I was accused of taking his
    > GPL'd code without his consent and incorporating it into the
    > linux-dvb codebase. It's this "all or nothing" attitude that has
    > prevented his work thus far from being incorporated, not the
    > unwillingness of people like myself to do the work to merge his
    > changes in a sane matter.
    >
    > I *really* want to see this resolved, because I recognize that I
    > could be better served working on other things than duplicating
    > efforts to debug issues that Markus may have already fixed in his
    > codebase. But just throwing away the work of half a dozen other
    > developers on an actively maintained driver is not really the sort of
    > compromise I think would be best for the community.
    >
    > 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.

    Regards,

    Hans
    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [linux-dvb] [PATCH 1/7] Adding empia base driver

    On Sat, Nov 1, 2008 at 6:19 PM, Devin Heitmueller
    wrote:
    > The reality is that from a technical standpoint I really want Markus
    > to be the maintainer. He knows more about the devices than anyone,
    > works for the vendor, and has access to all the datasheets. That
    > said, I don't want a situation where he is the *only* one who can do
    > work on the codebase because it is poorly commented and structured in
    > a way where only he can understand why it works the way it does.
    > Also, I believe certain design decisions should be made as a result of
    > consensus with the other maintainers, taking into consideration
    > consistency across drivers.


    I can understand that you want him to be the maintainer. But so far he
    hasn't really shown to be interested in doing that. It's just bit sad
    that the we don't have a proper driver given that the code exists and
    that there's a person who's willing to do the work to get it merged...
    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [linux-dvb] [PATCH 1/7] Adding empia base driver

    Hello Hans,

    Thanks for getting back to me. Responses inline:

    On Sat, Nov 1, 2008 at 12:21 PM, Hans Verkuil 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.

    >> # 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.

    >> 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.

    Devin

    --
    Devin J. Heitmueller
    http://www.devinheitmueller.com
    AIM: devinheitmueller
    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [linux-dvb] [PATCH 1/7] Adding empia base driver

    On Sat, Nov 1, 2008 at 5:29 PM, Pekka Enberg wrote:
    > On Sat, Nov 1, 2008 at 6:19 PM, Devin Heitmueller
    > wrote:
    >> The reality is that from a technical standpoint I really want Markus
    >> to be the maintainer. He knows more about the devices than anyone,
    >> works for the vendor, and has access to all the datasheets. That
    >> said, I don't want a situation where he is the *only* one who can do
    >> work on the codebase because it is poorly commented and structured in
    >> a way where only he can understand why it works the way it does.
    >> Also, I believe certain design decisions should be made as a result of
    >> consensus with the other maintainers, taking into consideration
    >> consistency across drivers.

    >
    > I can understand that you want him to be the maintainer. But so far he
    > hasn't really shown to be interested in doing that. It's just bit sad
    > that the we don't have a proper driver given that the code exists and
    > that there's a person who's willing to do the work to get it merged...
    >


    There's alot discussion and I haven't followed it yet although, the
    best way to go
    seen from my side is to take the em28xx-new code and merge the usable kernelcode
    into that one (note that only ~10% of the devices in the kernel em28xx
    driver are actually
    tested).

    I read about the License, I can say this is no issue overall, all of
    the code is available under GPL,
    and some of it is also under BSD license.

    I need to catch up the other mails before continuing...

    Markus
    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [linux-dvb] [PATCH 1/7] Adding empia base driver

    On Sat, Nov 1, 2008 at 5:51 PM, Devin Heitmueller
    wrote:
    > Hello Hans,
    >
    > Thanks for getting back to me. Responses inline:
    >
    > On Sat, Nov 1, 2008 at 12:21 PM, Hans Verkuil 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/5...9c102_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
    --
    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 http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread