Paths

Table of Contentst

fwcam: add IIDC 1394 FireWire camera driver
Needs ReviewPublic
Actions

Authored by guest-seuros on Fri, Jun 19, 8:41 PM.

Details

Reviewers

adrian
ziaee

Group Reviewers

firewire

Summary

Add fwcam(4), a driver for IIDC v1.30 (TA Document 1999023) digital
cameras over IEEE 1394.

Supports Format_0 (VGA) video modes with isochronous receive DMA,
feature control (brightness, exposure, gain, shutter, white balance,
focus, etc.), poll/kqueue, and hot-plug via bus reset handling.

Test Plan

Documetation sections are annotated.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 74032
Build 70915: arc lint + arc unit

Event Timeline

guest-seuros created this revision.Fri, Jun 19, 8:41 PM

Herald added a subscriber: ziaee. · View Herald TranscriptFri, Jun 19, 8:41 PM

guest-seuros requested review of this revision.Fri, Jun 19, 8:41 PM

Harbormaster completed remote builds in B74030: Diff 180106.Fri, Jun 19, 8:41 PM

guest-seuros added a parent revision: D57684: firewire: add shared helpers for ISO receive drivers.Fri, Jun 19, 8:42 PM

guest-seuros added reviewers: firewire, ziaee.

guest-seuros added a project: firewire.

ziaee added inline comments.Fri, Jun 19, 8:46 PM

share/man/man4/fwcam.4
2	Please prepend a blank comment line for conformance with style guides and consistency with C style comments.
12–27	I know we have widespread usage of this, but if you read mdoc(7), it is wrong.

address doc lint

Harbormaster completed remote builds in B74032: Diff 180108.Fri, Jun 19, 9:20 PM

guest-seuros marked 2 inline comments as done.Fri, Jun 19, 9:20 PM

remove bring-up debug logging.

Harbormaster completed remote builds in B74036: Diff 180112.Fri, Jun 19, 9:46 PM

in general this looks fine! thanks!

sys/dev/firewire/fwcam.c
955	why's it empty?

This revision is now accepted and ready to land.Sun, Jun 21, 8:14 PM

ziaee added inline comments.Sun, Jun 21, 8:19 PM

share/man/man4/fwcam.4
12	i dont know the international expectations, but in the US i think firewire would be a much more common and expected search term
60	tag renders really bad, please use bullet
65	please no macros in width fields

guest-seuros added inline comments.Sun, Jun 21, 9:45 PM

share/man/man4/fwcam.4
12	The how the spec spell it : https://damien.douxchamps.net/ieee1394/libdc1394/iidc/IIDC_1.31.pdf Write : `IIDC over firewire` in google, and you get Gemini explaining you IIDC 1394-based digital camera When i asked AI if we have such driver, it searched "FreeBSD IIDC 1394-based digital camera driver 2025 2026" In the driver messages i used `firewire`.
sys/dev/firewire/fwcam.c
955	The cameras I have tested so far don't need any bus reset recovery IIDC state is re-established through post_explore. I left the empty stub as scaffolding so the hook isn't forgotten when I get hardware that does need it (some cameras require re-negotiation after reset). Happy to remove it and track in my notes if you'd prefer it gone.

adrian added inline comments.Sun, Jun 21, 10:09 PM

sys/dev/firewire/fwcam.c
955	It's fine leaving it in here as scaffolding! thanks.

ziaee added inline comments.Sun, Jun 21, 10:38 PM

share/man/man4/fwcam.4
12	i want humans to be able to search the manual to find what they need. the Nd field contains the search keywords.

guest-seuros added inline comments.Sun, Jun 21, 10:41 PM

share/man/man4/fwcam.4
12	fair! So i put both ?

ziaee added inline comments.Sun, Jun 21, 10:43 PM

share/man/man4/fwcam.4
54–63
93	here too

ziaee added inline comments.Sun, Jun 21, 10:45 PM

share/man/man4/fwcam.4
12	both is fine with me, although i dont know how to word that elegantly. compare with the other firewire device driver in the tree, https://man.freebsd.org/fwohci

address reviews

This revision now requires review to proceed.Mon, Jun 22, 12:18 AM

Harbormaster completed remote builds in B74091: Diff 180259.Mon, Jun 22, 12:18 AM

ziaee added inline comments.Mon, Jun 22, 12:40 AM

share/man/man4/fwcam.4
97	can it only be set in loader.conf and not at runtime? we separate these out usually into SYSCTLS and TUNABLES, which (from ux standpoint) are the same except tunables can only be set at boot time through loader.conf or the loader prompt, whereas you can do that with sysctls but they can also be turned during runtime.