Re: [linux-dvb] em28xx merge process issues with linuxtv and upstream kernel

[Markus Rechberger]

On Sun, Nov 2, 2008 at 6:11 AM, Mauro Carvalho Chehab
<mchehab@xxxxxxxxxxxxx> wrote:
> On Sat, 1 Nov 2008 18:46:40 +0100
> "Markus Rechberger" <mrechberger@xxxxxxxxx> wrote:
>
>> > The last attempt was rejected because the patches were adding duplicate drivers rather than improving the existing ones.
>> > In the same project, 2 drivers managing the same hardware is not correct.
>> > Markus (or another people, why Markus may be the only person to do that?) should propose patches of the existing drivers, without breaking the v4l-dvb APIs.
>> > - First, the tuners and video decoders modifications shall be merged since they are used by several existing drivers.
>> > - Then the em28xx driver shall be improved.
>> > And this is what Markus started (thanks for this initiative) but this is hard to spend time on these minor things while supporting problems because of being out-of-kernel.
>> >
>>
>> In case of the cx25843 I discussed it with Hans Verkuil, there's more
>> or less no option since it collides with the existing inkernel driver
>> and disables support for other
>> cx25843 drivers, so as for the kernel it should be merged to the
>> existing one yes.
>
> The same kind of collision exists if we keep both the upstream and your version
> of the em28xx driver. This is also true for xc5000 and xc3028 drivers.
>

No since it doesn't use the i2c attach infrastructure. It just
directly sends the few tuner
bytes wherever it is needed. That way it didn't need any coreframework
changes either.

>> The em28xx driver, the one in the kernel has taken a few patches from
>> my repository, and it has some additional custom patches, it would
>> make more sense to work those
>> few patches into the new em28xx driver which is tested with most devices
>>
>> (compared with the driver which is in the kernel which is likely
>> tested with 5-10% of the devices which are in the cardlist).
>
> The patches should be generated against the upstream driver, not against the
> out-of-tree.
>
> You might use an strategy of first patching the out-of-tree, then patching back the
> upstream. This may eventually be used as an intermediate step, but I suspect it
> will just make things harder.
>
> Anyway, no matter what process used to generate the patchset, it should be
> generated in a way that it won't cause regressions upstream, and submitted as
> incremental patches properly describing what each patch of the series is doing
> (and not just a diff <version a> <version b>).
>
>> As for some reasons why not to merge it back then:
>> * the driver relied on reverse engineered code, which made some
>> devices extremly hot (not even xc3028/xc3028L related). Wrong gpio
>> settings also enable the device to draw more power and affect the
>> signal strength for analog TV/dvb-t, those settings can be custom for
>> every designed device. I have had one pinnacle device which had a
>> slightly melted package because of that mess.
>
> The gpio logic is just a very few lines of code. A simple patch probably can
> fix the issues.
>
>> There are additional em28xx based chipdrivers which only work with
>> em28xx based devices (eg. videology cam).
>
> A patch adding videology cam support would add this functionality upstream.
>
>> The input layer actually fully works although I disabled it because it
>> needs a redesign and shouldn't be exposed to userland like this, also
>> the polling code shouldn't be used (linux timing causes
>> alot trouble at low intervals - especially the deinitialization of
>> such timers, I sent an email to the ML about a possible race condition
>> in ir-kbd-i2c a couple of months ago.
>> netBSD developers discovered that interrupt polling works fine even
>> for remote controls. Practically since I worked alot with remote
>> controls during the last half year returning keyboard input keys
>> to userland is a mess, there was a discussion also with netbsd people
>> about a more generic interface because the IR support of the device
>> should be seen as RC5/RC6/NEC/.. protocol support
>> and not as one interface where the device is bound to a certain remote
>> and only supporting that remote control.
>> (that's just the reason why IR support is disabled :)
>
> Since it is disabled, there's no sense on currently trying to merge your IR
> code. This would be a regression.
>
>> > v4l-dvb people and Markus would be glad to see his drivers in the mainstream kernel.
>> >
>> >> I am going to ask for understanding of both the side of Markus that
>> >> worked very hard on his code, and that of the upstream developers. There
>> >> are both valid reasons on how they did there things.
>> >>
>> >> But we need a solution to get all the code back into the upstream
>> >> project so it can go into the kernel project and eventually be delivered
>> >> at the Linux distributions and all there users, so no custom compiling,
>> >> custom package install are required and non transparent bug reports can
>> >> be stopped.
>> >>
>> >> Is it possible for an upstream developer to step forward and take on the
>> >> task of merging the code of Markus back into mainstream, all questions
>> >> on the code can be discuses on several mailing-list [2] of choice.
>> >>
>> > Well, I would say: "Make it so!" ;)
>> >
>> >> Current the situation is kind of a hold-of, the issues are not being
>> >> discussed, the problem is not addressed, so no process is made and
>> >> during this time users are suffering from non working nor good supported
>> >> devices for there hybrid dvb-t/analog broadcast experiences under Linux.
>> >>
>> >> I hope this lead to a productive discussion that will get the code to
>> >> the end users through there main distribution systems.
>> >>
>> > I hope so, just to stop these useless discussions that do not discuss on patches.
>> >
>>
>> the code is there :-)
>
> If you are comfortable of having people converting your code on incremental
> patches and submitting upstream, please stop making personal attacks to the
> ones that are actually doing that ;)
>

http://fixunix.com/kernel/553062-re-patch-1-7-adding-empia-base-driver.html

I only refer to that email and the statement Hans made

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

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/