Re: [PATCH] Documentation/i2c: sync docs with current state of i2c-tools.

[Sam Hansen]

On Fri, Apr 13, 2018 at 5:13 AM, Jean Delvare <jdelvare@xxxxxxx> wrote:
> Hi Wolfram, Sam,
>
> On Fri, 13 Apr 2018 00:24:57 +0200, Wolfram Sang wrote:
>> On Thu, Apr 12, 2018 at 02:33:42PM -0700, Sam Hansen wrote:
>> > Currently, Documentation/i2c/dev-interface describes the use of i2c_smbus_*
>> > helper routines as static inlined functions provided by linux/i2c-dev.h.  Work
>> > has been done to refactor the linux/i2c-dev.h file in the i2c-tools project
>> > out into its own library.  As a result, these docs have become stale.
>>
>> Thanks for fixing this!
>>
>> > This patch corrects the discrepancy and directs the reader to the i2c-tools
>> > project for more information.  Additionally, some trailing-whitespace cleanups
>> > were made.
>>
>> Minor nit: Having the whitespace changes in a seperate patch is a tad
>> easier to review.
>>
>> > -  /* Using I2C Write, equivalent of
>> > +  /* Using I2C Write, equivalent of
>> >       i2c_smbus_write_word_data(file, reg, 0x6543) */
>>
>> Maybe change to Kernel coding style comments while here?
>>
>> > -  Not meant to be called  directly; instead, use the access functions
>> > -  below.
>> > +  If possible, use the provided i2c_smbus_* methods described below in favor
>> > +  of issuing direct ioctls.
>>
>> Why this change?
>
> I'm also not sure if "in favor of" is right. "instead of" would sound
> better to me, but I'm no native English speaker, I could be wrong.

Sounds good, I'll adopt "instead of".  Regarding Wolfram's earlier
comment, as an engineer, requiring an out-of-tree library to build
drivers felt a little off.  I can revert this section if you want,
just let me know.

>
>> > -The above functions are all inline functions, that resolve to calls to
>> > -the i2c_smbus_access function, that on its turn calls a specific ioctl
>> > -with the data in a specific format. Read the source code if you
>> > -want to know what happens behind the screens.
>> > +The above functions are made available by linking against the libi2c library,
>> > +which is provided by the i2c-tools project.  See:
>> > +https://git.kernel.org/pub/scm/utils/i2c-tools/i2c-tools.git/.
>>
>> This is fine with me. Maybe Jean has a comment on this?
>
> Fixing the documentation is always welcome, thanks Sam for stepping in.
> However we really want separate patches for whitespace fixes and actual
> contents change, as Wolfram already mentioned above.

Will do!  I'll send a v2 patch set, thanks.

>
> --
> Jean Delvare
> SUSE L3 Support