Differential D5470

i2c(8) Support for I2CRDRW and change scan output
Needs ReviewPublic
Actions

Authored by manu_bidouilliste.com on Feb 27 2016, 3:12 AM.

Details

Reviewers

None

Group Reviewers

manpages

Summary

Creating a reviews based on my bugzilla entry: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=198817

This add support for I2CRDRW ioctl (with option -x)
This also change the way scan outputs its result (this was asked from loos@ on irc a long time ago)

Test Plan

Apply patch and test i2c on a system with a without I2CRDRW.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2687
Build 2707: arc lint + arc unit

Event Timeline

manu_bidouilliste.com updated this revision to Diff 13823.Feb 27 2016, 3:12 AM

manu_bidouilliste.com retitled this revision from to i2c(8) Support for I2CRDRW and change scan output.

manu_bidouilliste.com updated this object.

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

manu_bidouilliste.com set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a reviewer: manpages. · View Herald TranscriptFeb 27 2016, 3:12 AM

Herald added subscribers: Contributor Reviews (src), imp. · View Herald Transcript

wblock added a subscriber: wblock.Mar 17 2016, 10:15 PM

wblock added inline comments.

usr.sbin/i2c/i2c.8
29	Please bump this.
68	Kind of confusingly worded. How about: 8-bit hex address of the I2C device on which to operate.
94	The first sentence can be added to the end of the previous sentence: scan the bus for devices and print a table of all addresses. Then start the second sentence on a new line and s/those/these/ The content of each cell can have these meanings:
103	s/An/A/ Should that really be "if", or is it meant to be "of"?
106	"the" and "ones" are not needed: Use the I2CRDRW ioctl instead of I2CREAD/I2CWRITE.

Fixes a few sentences.
Bump .Dd

Herald edited edge metadata. · View Herald TranscriptMar 18 2016, 1:54 AM

manu_bidouilliste.com marked 5 inline comments as done.Mar 18 2016, 1:55 AM

manu_bidouilliste.com added inline comments.

usr.sbin/i2c/i2c.8
103	Both are correct for me but english isn't my native language so it might be better otherway. I've changed it, tell me if it suits you.

wblock added inline comments.Mar 18 2016, 3:09 AM

usr.sbin/i2c/i2c.8
66–68	As below, how about: 7-bit hex address of the I2C device.
68	Missed a couple of things: 8-bit hex address of the I2C device on which to operate. I'm actually not sure the last part of that is needed at all. How about: 8-bit hex address of the I2C device.
93–107	s/address/addresses/
94	s/those/these/
103	Looks good now, thanks!
106	Sorry, it needs the first "the": Use the I2CRDRW ioctl instead of I2CREAD/I2CWRITE.

Fix a few words/sentences.

Herald edited edge metadata. · View Herald TranscriptMar 18 2016, 3:22 AM

manu_bidouilliste.com marked 7 inline comments as done.Mar 18 2016, 3:22 AM

Just a couple of minor things. I will go back through and review later, but I think this is everything. Thank you!

usr.sbin/i2c/i2c.8
66–68	s/on/of/
68	s/on/of/

s/on/of

Herald edited edge metadata. · View Herald TranscriptMar 21 2016, 1:17 PM

manu_bidouilliste.com marked 2 inline comments as done.Mar 21 2016, 1:17 PM

wblock added inline comments.Mar 21 2016, 9:07 PM

usr.sbin/i2c/i2c.8
101	This should be "XX" The address was skipped .Pq see Fl n (Untested, but I think that is correct.)

I'll submit a new one after 11 is branched.

Revision Contents
Changeset List

Path

Size

usr.sbin/

i2c/

i2c.8

22 lines

i2c.c

230 lines

Diff 13823

View Options

usr.sbin/i2c/i2c.8

	# $FreeBSD$			# $FreeBSD$

	.include <src.opts.mk>			.include <src.opts.mk>

	SUBDIR=			SUBDIR=
	SUBDIR.${MK_DIALOG}+= libdialog			SUBDIR.${MK_DIALOG}+= libdialog
	SUBDIR.${MK_GCC}+= libgcov
	.if ${MK_GCC} != "no" && ${MK_OPENMP} == "no"
	SUBDIR+= libgomp
	.endif
	SUBDIR.${MK_TESTS}+= tests			SUBDIR.${MK_TESTS}+= tests

	.if ${MK_GNU_GREP} != "no" \|\| ${MK_GNU_GREP_COMPAT} != "no" \|\| \			.if ${MK_GNU_GREP} != "no" \|\| ${MK_GNU_GREP_COMPAT} != "no" \|\| \
	${MK_GDB} != "no"			${MK_GDB} != "no"
	SUBDIR+= libregex			SUBDIR+= libregex
	.endif

	# libsupc++ uses libstdc++ headers, although 'make includes' should
	# have taken care of that already.
	.if ${MK_GNUCXX} != "no"
	SUBDIR+= libstdc++ libsupc++
	SUBDIR_DEPENDS_libsupc++:= libstdc++
	.endif			.endif

	SUBDIR_PARALLEL=			SUBDIR_PARALLEL=

	.include <bsd.subdir.mk>			.include <bsd.subdir.mk>