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

From: Markus Rechberger
Date: Wed Nov 26 2008 - 23:25:53 EST


On Wed, Nov 26, 2008 at 8:12 PM, Aidan Thornton <makosoft@xxxxxxxxxxxxxx> wrote:
> On 10/22/08, Markus Rechberger <mrechberger@xxxxxxxxx> wrote:
>> On Thu, Oct 23, 2008 at 12:09 AM, Greg KH <greg@xxxxxxxxx> wrote:
>>> On Wed, Oct 22, 2008 at 11:14:36PM +0200, Markus Rechberger wrote:
>>>> em2880-dvb:
>>>> * supporting the digital part of Empia based devices, which
>>>> includes ATSC, ISDB-T and DVB-T
>>>
>>> <snip>
>>>
>>> Doesn't this driver duplicate some of the existing devices we already
>>> support with the current in-kernel driver? If so, why not just add the
>>> new device support to the existing driver instead of duplicating
>>> everything?
>>>
>>> This is going to cause a big problem for distros as they will not know
>>> which to enable, so they will probably just disable this one, which is
>>> what I don't think you want to have happen :(
>>>
>>
>> the current driver doesn't support most devices which are in there,
>> also the alsa
>> audio driver can easily crash the whole system. (It's my code so I
>> know what was wrong there).
>>
>> The core video code is already too much off, the VBI code added alot
>> complexity
>> to it it does frame slicing on the fly.
>>
>> Those devices ship VBI+VIDEO within 1 datastream, VBI and Video aren't
>> that different
>> in the system. both interfaces provide framebuffers through a mmap'ed
>> interface.
>> If all the VBI buffers are filled the data has to be sliced off in any
>> case while providing
>> the same bottom data ot the Video interface
>
> Hi,
>
> Just to check, your driver does handle this correctly now, right? Last
> time I checked, it was a buggy mess with severely broken locking that
> caused a kernel panic half the time when recording whith MythTV (and
> indeed, had been for years). It looks, at a glance, like this may have
> been fixed, but since I never quite managed to pin down the exact
> cause, I can't say for sure.
>

the exact cause was that the vbi buffer handling was connected with
the video buffer handling
back then. It crashed at certain scenarios.
VBI has been reworked completely since then, also the offsets. It's a
hardcoded em28xx - videodecoder setup.

> (The code is, alas, still somewhat ugly compared to the existing
> videobuf-based code in the in-kernel driver - it should be possible to
> add VBI support to that fairly cleanly and easily. I actually
> attempted this a while back, but ran into issues due to not having
> access to the datasheets. Of course, the fact that Markus works there
> and is actively aggressive to any Linux driver development that
> doesn't go through him.)
>
>> http://mcentral.de/hg/~mrec/em28xx-new/shortlog (there are more than
>> 200 changelog entries
>> what happened in detail).
>>
>> The development has been split off (first due limitations in the
>> kernel, afterwards due ..., and finally
>> due the restriction that all that has to work without a framework
>> upgrade on the eeePC).
>
> This doesn't exactly inspire confidence; it seems to basically mean
> you forked or rewrote existing drivers rather than just using code
> that was already there.
>

well I initially worked on that driver since 2005(!) and at least
helped more than
thousand users to get their devices work.

>> diffing the 2 available drivers shows up that only the core is twice
>> as big as the one which is currently
>> in the driver (the result of 2-3 years asynchronous development).
>
> Actually, part of the reason for this is probably that there's a
> massive bunch of video buffer handling code in your driver that mostly
> repeats stuff handled by the videobuf module in the in-kernel driver.
> (Admittedly, your does slightly more in that it handles raw VBI, but
> as I've said it should be possible to do that cleanly and in a smaller
> amount of code using videobuf.)
>
>> The driver is currently also tested with signal generators (different
>> inputs, and different video standards).
>>
>> Very likely the best would be to replace the available driver with it
>> but I don't care, alot people use and have been using the driver from
>> mcentral.de for a long time, development has always been opensource
>> there too.
>
> Just not necessarily open source under a GPL compliant license, as I recall.
>

I pointed out a couple of times that the license is no issue with the
exported sources.

> As I've said before, it'd be really nice if you'd make an effort to
> actually integrate your changes, but this looks like more of the same
> old mess.
>

The driver has been exported so far, that's all I'll ever offer. In
future to avoid those clashes I'll
simply use the i2c-dev framework which is commonly available and
allows to control
those chips from userland without any extra module. We already have a
player which
is optimized for those devices and also properly supports that interface.

I simply don't have the time for those games anymore, and those people
here who have personal
issues with me and/or my work don't pay my monthly bills, neither do
they have any clue how that
driver is used with some systems of certain customers. I constantly
deliver full solutions, and not
driver code only. Neither am I limited to Linux only.

Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/