Add USB Mass Storage CTL frontend.
ClosedPublic
Actions

Authored by trasz on Dec 14 2016, 8:41 AM.

Details

Reviewers

emaste
wblock

Group Reviewers

manpages

Commits

rS313959: Add USB Mass Storage CTL frontend. This makes it possible

Summary

Add USB Mass Storage CTL frontend. Note that this is not the latest
version of the code; I'll update it after I have tested my changes.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

trasz updated this revision to Diff 22903.Dec 14 2016, 8:41 AM

trasz retitled this revision from to Add USB Mass Storage CTL frontend..

trasz updated this object.

trasz edited the test plan for this revision. (Show Details)

Herald added a reviewer: manpages. · View Herald TranscriptDec 14 2016, 8:41 AM

Herald added a subscriber: imp. · View Herald Transcript

bjk added a subscriber: bjk.Dec 15 2016, 12:01 AM

bjk added inline comments.

share/man/man4/cfumass.4
71 ↗	(On Diff #22903)	This could probably be another .Xr ctl 4. Also there's the Dq macro for putting quotes around things, though one could argue that .Pa is more appropriate for umassX.

emaste added a subscriber: emaste.Jan 6 2017, 2:00 AM

emaste added inline comments.

sys/dev/usb/storage/cfumass.c
389–390 ↗	(On Diff #22903)	How do we resolve these questions?
849–854 ↗	(On Diff #22903)	What do we do here?

emaste requested changes to this revision.Jan 24 2017, 9:22 PM

emaste added a reviewer: emaste.

emaste added inline comments.

sys/dev/usb/storage/cfumass.c
403–404 ↗	(On Diff #22903)	Needs rebasing after rS312651

This revision now requires changes to proceed.Jan 24 2017, 9:22 PM

emaste mentioned this in D8788: Document USB Mass Storage CTL frontend..Jan 24 2017, 10:03 PM

emaste added inline comments.Jan 26 2017, 2:33 AM

sys/dev/usb/storage/cfumass.c

836–837 ↗

(On Diff #22903)

Compiling on sparc64:

/scratch/tmp/emaste/freebsd/sys/modules/usb/cfumass/../../../dev/usb/storage/cfumass.c: In function 'cfumass_t_data_in_callback':
/scratch/tmp/emaste/freebsd/sys/modules/usb/cfumass/../../../dev/usb/storage/cfumass.c:834: warning: format '%zd' expects type 'signed size_t', but argument 4 has type 'int' [-Wformat]
/scratch/tmp/emaste/freebsd/sys/modules/usb/cfumass/../../../dev/usb/storage/cfumass.c:834: warning: format '%zd' expects type 'signed size_t', but argument 5 has type 'uint32_t' [-Wformat]

Remember to update .Dd in the man pages where a .Xr has been added to this new man page.

Attached is a version of cfumass.4 with the changes mentioned above plus some other edits for clarity, markup, and corrected dates.

cfumass.43 KBDownload

share/man/man4/cfumass.4

47 ↗

(On Diff #22903)

The driver can be loaded as a module at boot by placing this line in

Yes, I know the old line has been reused so many times it is dogma. It's still halting and redundant. We can and should do better.

62 ↗

(On Diff #22903)

This is a long way of saying "To use", but this is a one-sentence paragraph with asides and several conditions.

Using
.Nm
requires
.Bl -bullet
.It
.Xr usb_template 4
must be loaded.
.It
The USB OTG port must be working in USB device-side mode. This happens
automatically upon connection to a USB host.
.It
The USB Mass Storage template must be chosen by setting the
.Va hw.usb.template
sysctl to 0.
.El

Updated version with numerous small improvements here and there.
I've incorporated the man page updated with wblocked@, but did some
more changes; please tell me what you think.

Note that there's one important change in behaviour: the CTL port
is created on load, not on attach. This is nicer from ctld point
of view.

Herald edited edge metadata. · View Herald TranscriptFeb 11 2017, 12:38 PM

trasz added inline comments.Feb 11 2017, 12:42 PM

sys/dev/usb/storage/cfumass.c
389–390 ↗	(On Diff #22903)	I've left the "XXX KDM" question here; it exists in (I believe literally) all CTL frontend drivers. The real answer would be to just get rid of the number altogether; it's not currently used. I need to discuss this with mav@, though.

Fix build on sparc64.

Herald edited edge metadata. · View Herald TranscriptFeb 14 2017, 1:35 PM

Note to self: tinderboxed.

Man page changes look good to me. Thanks!

emaste added inline comments.Feb 17 2017, 9:49 PM

sys/dev/usb/storage/cfumass.c
2 ↗	(On Diff #25150)	Year update?
560–561 ↗	(On Diff #25150)	this is just for the the start_stop ignore switch, correct? it's not clear from the name

trasz added inline comments.Feb 17 2017, 10:02 PM

sys/dev/usb/storage/cfumass.c
560–561 ↗	(On Diff #25150)	For now it's just for this single command, yeah, but we might grow something else in the future, thus the generic name. That's also why the command detection is inside the function.