Bug 189400 - Review Request: em8300(-kmod) - Hollywood+/DXR3 hardware MPEG decoder drivers and tools
Review Request: em8300(-kmod) - Hollywood+/DXR3 hardware MPEG decoder drivers...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Thorsten Leemhuis
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-04-19 14:37 EDT by Ville Skyttä
Modified: 2007-11-30 17:11 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-07-23 05:46:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Ville Skyttä 2006-04-19 14:37:19 EDT
http://dxr3.sourceforge.net/
http://cachalot.mine.nu/5/SRPMS/em8300-0.15.3-3.src.rpm
http://cachalot.mine.nu/5/SRPMS/em8300-kmod-0.15.3-2.2.6.16_1.2080_FC5.src.rpm

These packages provide drivers and utilities for Sigma Designs Hollywood Plus/ Creative DXR3 hardware MPEG decoder cards.  The cards require a microcode (firmware) blob which is not included due to redistribution issues but can be extracted from vendor provided Windows drivers using the tools in the em8300-devel package, or downloaded from the upstream project site at http://dxr3.sourceforge.net/download/
Comment 1 Ville Skyttä 2006-04-27 14:59:03 EDT
http://cachalot.mine.nu/5/SRPMS/em8300-kmod-0.15.3-3.2.6.16_1.2096_FC5.src.rpm

* Thu Apr 27 2006 Ville Skyttä <ville.skytta at iki.fi> - 0.15.3-3
- Provide "kernel-modules" instead of "kernel-module" to match yum's config.
Comment 2 Ville Skyttä 2006-06-23 01:41:29 EDT
http://cachalot.mine.nu/5/SRPMS/em8300-kmod-0.15.3-4.2.6.16_1.2133_FC5.src.rpm

* Fri Jun 23 2006 Ville Skyttä <ville.skytta at iki.fi> - 0.15.3-4
- Invoke kmodtool with bash instead of sh.
Comment 3 Ville Skyttä 2006-06-26 13:39:25 EDT
http://cachalot.mine.nu/5/SRPMS/em8300-kmod-0.15.3-5.2.6.17_1.2139_FC5.src.rpm

* Mon Jun 26 2006 Ville Skyttä <ville.skytta at iki.fi> - 0.15.3-5
- Apply upstream patch to fix build with kernel 2.6.17, rediff WSS patch.
Comment 4 Ville Skyttä 2006-07-06 02:28:43 EDT
Per Packaging/KernelModules in the wiki, here's the additional required
explanation about the mainline merge status (from Nicolas Boullis and yours truly):

    The em8300 modules are not part of mainline kernel yet partly because
    nobody has got around to submit it, and partly because there some
    things are still thought of as unfinished in the sense that they'd
    be better off behaving and being accessible more like other related
    devices in the kernel do, requiring less manual configuration, and
    the like.  At this time, there are no specific deadlines or promises
    set for the submission, but it is expected to happen in not too
    distant future.
Comment 5 Thorsten Leemhuis 2006-07-11 14:20:37 EDT
== General ==

Review without appropriate hardware to test, but I'm sure it'll work.

>These packages provide drivers and utilities for Sigma Designs Hollywood 
> Plus/ Creative DXR3 hardware MPEG decoder cards.  The cards require a 
> microcode (firmware) blob which is not included due to redistribution
> issues

Ehh, I don't get that here. I saw a file 
 modules/em8300.uc
in the package. Is that the firmware? If yes: why it is included? At least there
is a file 
 em8300-nofirmware-0.15.3.tar.gz
on
 http://sourceforge.net/project/showfiles.php?group_id=5165&package_id=5202
and it's missing exactly that file. But from the docs it look like a file
"em8300.bin" needs to contain the firmware.
/me confused

> but can be extracted from vendor provided Windows drivers using the tools in
the em8300-devel package, or >downloaded from the upstream project site at 

Is there a good reason why it needs to be in the devel package? It's not the
obvious place where I would look out for it.

== em8300 ==

rpmlint:

SRPM:
no output

RPM:
W: em8300-devel no-documentation
-> can be ignored

* download of Source0 doesn't work -- seems to be a sourceforge problem afaics
* Open-Source-License (GPL)
* Source matches upstream
* package is named according to the guidelines
* builds in mock on ix86-FC5
* Is there a reason why the devel package doesn't require the base package?
* "Requires:       kmod-em8300 = %{version}" is wrong -- it needs to be 
  "Requires:       em8300-kmod >= %{version}"
 
== em8300-kmod ==

rpmlint:

SRPM:
W: em8300-kmod mixed-use-of-spaces-and-tabs
-> Might be a good idea to fix that in the longer term

RPMS:
W: kmod-em8300 summary-not-capitalized em8300 kernel module(s)
-> This is probably the right thing in this case

W: kmod-em8300 unstripped-binary-or-object
/lib/modules/2.6.17-1.2139_FC5/extra/em8300/adv717x.ko
W: kmod-em8300 unstripped-binary-or-object
/lib/modules/2.6.17-1.2139_FC5/extra/em8300/bt865.ko
W: kmod-em8300 unstripped-binary-or-object
/lib/modules/2.6.17-1.2139_FC5/extra/em8300/em8300.ko
-> they get stripped during build. Hmmm, this looks like a problem in rpm. Could
you look into this?

W: kmod-em8300 no-documentation
-> expected behaviour for kmod-packages

Review:
* package is named according to the guidelines
* Open-Source-License (GPL)
* Source matches upstream
* builds in mock on ix86-FC5
* download of Source0  doesn't work -- seems to be a sourceforge problem afaics

Probably needs fixing: 
- The module isn't build for smp-kernels on ppc. Please fix if there isn't a
good reason not to do so.
Comment 6 Ville Skyttä 2006-07-11 15:45:13 EDT
Thanks for the thorough review.

modules/em8300.uc is indeed the firmware blob (which should be installed as
/lib/firmware/em8300.bin), and the kmod package used accidentally the wrong
source tarball even though the firmware wasn't installed.

The microcode/firmware extractor depends on perl which was a bigger issue a long
time ago when things were packaged differently, but that's no longer the case.

Devel doesn't require the main package or -utils simply because there's nothing
in it that would require those, -devel is self contained.  Adding the dependency
would additionally inflict the need to install a kernel and a suitable em8300
module package for it which seems just useless and may be a problem in eg. some
build systems.

Strip vs not-stripped: the modules are stripped of something indeed, but
something is also left behind which causes "file" to report "not stripped" and
rpmlint uses that for its check.  Note: it's the same as for all kernel modules,
including those shipped with the kernel itself.  I'm not compentent to analyze
this, to me it's just how /usr/lib/rpm/debugedit seems to behave ;)

Other than those:

http://cachalot.mine.nu/5/SRPMS/em8300-0.15.3-4.src.rpm
* Tue Jul 11 2006 Ville Skyttä <ville.skytta at iki.fi> - 0.15.3-4
- Require >= em8300-kmod, not = kmod-em8300 (#189400).
- Move microcode extractor to -utils as em8300-mc_ex (like Debian) (#189400).
- Don't ship microcode_upload.pl, the modules and em8300setup already can
  handle that.

http://cachalot.mine.nu/5/SRPMS/em8300-kmod-0.15.3-6.2.6.17_1.2145_FC5.src.rpm
* Tue Jul 11 2006 Ville Skyttä <ville.skytta at iki.fi> - 0.15.3-6
- Enable PPC SMP builds (#189400).
- Use firmwareless tarball (#189400).
- Untabify specfile (#189400).
Comment 7 Thorsten Leemhuis 2006-07-15 14:26:48 EDT
Review:

== em8300 ==
rpmlint:

SRPM:
no output

RPM:
W: em8300-devel no-documentation
-> can be ignored

* download of Source0 doesn't work -- seems to be a sourceforge problem afaics
* Open-Source-License (GPL)
* Source matches upstream
* package is named according to the guidelines
* builds in mock on ix86-FC5
 
== em8300-kmod ==

rpmlint:

SRPM:
no output

RPMS:
W: kmod-em8300 summary-not-capitalized em8300 kernel module(s)
-> This is probably the right thing in this case

W: kmod-em8300 unstripped-binary-or-object
/lib/modules/2.6.17-1.2139_FC5/extra/em8300/adv717x.ko
W: kmod-em8300 unstripped-binary-or-object
/lib/modules/2.6.17-1.2139_FC5/extra/em8300/bt865.ko
W: kmod-em8300 unstripped-binary-or-object
/lib/modules/2.6.17-1.2139_FC5/extra/em8300/em8300.ko
-> they get stripped during build, we will ignore that

W: kmod-em8300 no-documentation
-> expected behaviour for kmod-packages

Review:
* package is named according to the guidelines
* Open-Source-License (GPL)
* Source matches upstream
* builds in mock on ix86-FC5
* matches fedora kmod scheme

== Summary ==

both nearly approved -- waiting for FESCo approval of kmod (we forgot to discuss
those in the last meeting -- sorry)
Comment 8 Ville Skyttä 2006-07-23 05:46:03 EDT
Was approved in the last FESCO meeting, and has been now imported and built for
FC-5.  Thanks.
Comment 9 David Woodhouse 2006-07-23 05:54:59 EDT
If the kernel module has a suitable licence for FC5, then it presumably has a
suitable licence for being included directly in the kernel. Thus, the correct
approach would be to get the support into the upstream kernel and therefore into
our base kernel package, rather than having a separate kmod package for it.

I strongly believe we should veto _all_ kmod packages in Core and Extras. The
only reason for kernel support not being upstream is because it's not acceptable
there.... which means it probably shouldn't be acceptable to us either.

And if it _is_ acceptable to us but isn't upstream for some reason, then we can
consider adding it to the kernel package properly. In certain exceptional cases.
You should file a separate bug (with patch) for that.
Comment 10 Thorsten Leemhuis 2006-07-23 10:46:41 EDT
(In reply to comment #9)
> If the kernel module has a suitable licence for FC5, then it 
> presumably has a suitable licence for being included directly in the 
> kernel.

Agreed.

> Thus, the correct approach would be to get the support into the 
> upstream kernel and therefore into our base kernel package, rather 
> than having a separate kmod package for it.

Also agreed, and that's why we we plan to try our best to tell upstream
maintainers "get your stuff merged". That why we require a statement
when people plan to get their stuff merged (see comment #4).

> I strongly believe we should veto _all_ kmod packages in Core and 
> Extras.

This was discussed before when we started the development of the
kmod-standard for Extras -- several people from Red Hat were involved in
that discussion and we agreed to work on it nevertheless the political
problems.

Anyway, I can really understand your point. That why I suggested in the
beginning that all kmods are only allowed for X months (X=12 or 18) in
Extras before they get dropped again and that upstream shall work on
getting its suff into the kernel in that timeframe. But that idea was
not accepted by other involved in the discussion.

Anyway, I don't think we should discuss this further here and probably
won't reply again in this bug. fedora-devel-list probably is the better
place for this sort of political discussions.


Site note: It's IMHO more a "kernel" vs. "distributions" problem:

I'm sure all those people currently listed in the CC of this bug agree
that it's important and best to get all drivers into the kernel as fast
as possible (as long as they are "clean" and even if they are far from feature
complete ) and that external drivers have many pitfalls and should be avoided as
much as possible.

But it seems some open-source-driver-developers don't feel that way and
prefer to maintain their drivers outside of the upstream kernel. They
probably fear (for no good reason) getting "Christophed". That's a
pity, but it's life in 2006.

But some distributions include these external drivers. That creates
pressure on those distributions that don't include external drivers
because people run away. E.g.: "I like Fedora. But it doesn't include
the external driver foo -- distribution bar has it already and thus is
far easier to use for me because I need that driver. That's why I
switched over to bar."

So IMHO kernel developers and the kernel-maintainers of the most
important distributions need to work together closely and create
policies like "every distribution may only contain 10 kernel-drivers that are
not yet merged into the upstream kernel". The pressure on driver developers to
get their drivers merged into linus kernel would be a lot bigger then.

Just my 2 cent.
Comment 11 Axel Thimm 2006-07-30 05:35:31 EDT
I saw a referecne to this bug about David's comment #9. I'd like to point out
that one of the oldest kernel modules packages in Fedora are Cluster/GFS bits.
Following this kind of veto, we wouldn't have GFS in Fedora.

And there isn't really a difference between externally built kernel modules and
patched in pieces that haven't been accepted upstream like for example xen.

So at the very end the xen/gfs deviation from upstream really justify most of
the external kernel module projects (keeping binary code out of the discussion,
that's very different and much uglier).

Don't get me wrong, I'm very happy with gfs/xen! And I'm also very happy with
some external kernel modules.
Comment 12 David Woodhouse 2006-07-30 05:56:38 EDT
Xen and GFS2 are in the main kernel package in rawhide/FC6. They're not in
separate packages any more.
Comment 13 Ville Skyttä 2006-07-30 06:09:43 EDT
Please take this discussion to an appropriate mailing list.
Comment 14 Thorsten Leemhuis 2006-07-30 06:20:34 EDT
(In reply to comment #13)
> Please take this discussion to an appropriate mailing list.

+1

Note You need to log in before you can comment on or make changes to this bug.