| Message ID | 20200105225012.11701-15-luca@lucaceresoli.net |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | docs: i2c: rework I2C documentation, part I | expand |
On Sun, 5 Jan 2020 23:50:01 +0100, Luca Ceresoli wrote: > Hyperlinks from function names are not generated in headings. Move them in > the plain text so they are rendered as clickable hyerlinks. typo: hyperlinks > > While there also remove an unneeded colon in a heading. > > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> > --- > Documentation/i2c/smbus-protocol.rst | 64 ++++++++++++++++++---------- > 1 file changed, 42 insertions(+), 22 deletions(-) > > diff --git a/Documentation/i2c/smbus-protocol.rst b/Documentation/i2c/smbus-protocol.rst > index 10c4a989982c..997945e90419 100644 > --- a/Documentation/i2c/smbus-protocol.rst > +++ b/Documentation/i2c/smbus-protocol.rst > @@ -62,8 +62,10 @@ This sends a single bit to the device, at the place of the Rd/Wr bit:: > Functionality flag: I2C_FUNC_SMBUS_QUICK > > > -SMBus Receive Byte: i2c_smbus_read_byte() > -========================================== > +SMBus Receive Byte > +================== > + > +Implemented by i2c_smbus_read_byte() In file i2c-protocol.rst, the wording used is: This corresponds to i2c_transfer(). For consistency, can we settle for "Implemented by foo()" everywhere? Reviewed-by: Jean Delvare <jdelvare@suse.de>
Hi Jean, On 20/01/20 15:44, Jean Delvare wrote: > On Sun, 5 Jan 2020 23:50:01 +0100, Luca Ceresoli wrote: >> Hyperlinks from function names are not generated in headings. Move them in >> the plain text so they are rendered as clickable hyerlinks. > > typo: hyperlinks Ok. >> >> While there also remove an unneeded colon in a heading. >> >> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> >> --- >> Documentation/i2c/smbus-protocol.rst | 64 ++++++++++++++++++---------- >> 1 file changed, 42 insertions(+), 22 deletions(-) >> >> diff --git a/Documentation/i2c/smbus-protocol.rst b/Documentation/i2c/smbus-protocol.rst >> index 10c4a989982c..997945e90419 100644 >> --- a/Documentation/i2c/smbus-protocol.rst >> +++ b/Documentation/i2c/smbus-protocol.rst >> @@ -62,8 +62,10 @@ This sends a single bit to the device, at the place of the Rd/Wr bit:: >> Functionality flag: I2C_FUNC_SMBUS_QUICK >> >> >> -SMBus Receive Byte: i2c_smbus_read_byte() >> -========================================== >> +SMBus Receive Byte >> +================== >> + >> +Implemented by i2c_smbus_read_byte() > > In file i2c-protocol.rst, the wording used is: > > This corresponds to i2c_transfer(). > > For consistency, can we settle for "Implemented by foo()" everywhere? Good point. For v2 I added a new patch to use "Implemented by" also in i2c-protocol.rst. Thanks. By extrapolation I guess you want to review the few remaining patches. Correnct? In this case I'd wait for that before sending out v2.
On Tue, 21 Jan 2020 18:31:23 +0100, Luca Ceresoli wrote: > By extrapolation I guess you want to review the few remaining patches. > Correnct? In this case I'd wait for that before sending out v2. Yes, and I'm just done with it. Sorry it took so long but I was doing it as a background task as my work schedule allowed. One last thing you may want to fix: there's one occurrence of "stop bit" left in Documentation/i2c/writing-clients.rst. Thanks,
On Tue, 21 Jan 2020 18:31:23 +0100, Luca Ceresoli wrote: > Good point. For v2 I added a new patch to use "Implemented by" also in > i2c-protocol.rst. BTW... I don't know how Wolfram feels about it, but I don't think documentation changes need to be split to such fine-grained patches. Chances that someone will want to cherry-pick specific changes are very low in my opinion, so a large number of patches only means more paperwork with little to no benefit. Some splitting is certainly nice to make reviews easier, but only to a certain degree.
On Wed, Jan 22, 2020 at 03:26:08PM +0100, Jean Delvare wrote: > On Tue, 21 Jan 2020 18:31:23 +0100, Luca Ceresoli wrote: > > Good point. For v2 I added a new patch to use "Implemented by" also in > > i2c-protocol.rst. > > BTW... I don't know how Wolfram feels about it, but I don't think > documentation changes need to be split to such fine-grained patches. I don't mind too much. I think for a first version, fine grained can make review more easy. Maybe the second version could be less patches. Yet for me, since patchwork can handle series of patches, the amount doesn't matter too much. I am super happy that Luca did the work and you did the review!
Hi Jean, On 22/01/20 12:48, Jean Delvare wrote: > On Tue, 21 Jan 2020 18:31:23 +0100, Luca Ceresoli wrote: >> By extrapolation I guess you want to review the few remaining patches. >> Correnct? In this case I'd wait for that before sending out v2. > > Yes, and I'm just done with it. Sorry it took so long but I was doing > it as a background task as my work schedule allowed. No problem at all, it has been a background task for me as well! Thanks again for the patient work. V2 will be out soon with the many precious improvements you suggested. > One last thing you may want to fix: there's one occurrence of "stop > bit" left in Documentation/i2c/writing-clients.rst. Thanks, added.
Hi, On 22/01/20 16:37, Wolfram Sang wrote: > On Wed, Jan 22, 2020 at 03:26:08PM +0100, Jean Delvare wrote: >> On Tue, 21 Jan 2020 18:31:23 +0100, Luca Ceresoli wrote: >>> Good point. For v2 I added a new patch to use "Implemented by" also in >>> i2c-protocol.rst. >> >> BTW... I don't know how Wolfram feels about it, but I don't think >> documentation changes need to be split to such fine-grained patches. > > I don't mind too much. I think for a first version, fine grained can > make review more easy. Maybe the second version could be less patches. > Yet for me, since patchwork can handle series of patches, the amount > doesn't matter too much. I am super happy that Luca did the work and you > did the review! I initially split this work in fine-grained patches for better reviewing and also because some of the changes were not expected in the beginning: while working at an improvement I noticed an unrelated one was needed. But I agree the result is quite awkward. Coalescing some of them now would be painful, so I'm sending v2 as is. But I'm tackling the remaining sections later, and I'm going to do that work in a smaller number of patches.
diff --git a/Documentation/i2c/smbus-protocol.rst b/Documentation/i2c/smbus-protocol.rst index 10c4a989982c..997945e90419 100644 --- a/Documentation/i2c/smbus-protocol.rst +++ b/Documentation/i2c/smbus-protocol.rst @@ -62,8 +62,10 @@ This sends a single bit to the device, at the place of the Rd/Wr bit:: Functionality flag: I2C_FUNC_SMBUS_QUICK -SMBus Receive Byte: i2c_smbus_read_byte() -========================================== +SMBus Receive Byte +================== + +Implemented by i2c_smbus_read_byte() This reads a single byte from a device, without specifying a device register. Some devices are so simple that this interface is enough; for @@ -75,8 +77,10 @@ the previous SMBus command:: Functionality flag: I2C_FUNC_SMBUS_READ_BYTE -SMBus Send Byte: i2c_smbus_write_byte() -======================================== +SMBus Send Byte +=============== + +Implemented by i2c_smbus_write_byte() This operation is the reverse of Receive Byte: it sends a single byte to a device. See Receive Byte for more information. @@ -88,8 +92,10 @@ to a device. See Receive Byte for more information. Functionality flag: I2C_FUNC_SMBUS_WRITE_BYTE -SMBus Read Byte: i2c_smbus_read_byte_data() -============================================ +SMBus Read Byte +=============== + +Implemented by i2c_smbus_read_byte_data() This reads a single byte from a device, from a designated register. The register is specified through the Comm byte:: @@ -99,8 +105,10 @@ The register is specified through the Comm byte:: Functionality flag: I2C_FUNC_SMBUS_READ_BYTE_DATA -SMBus Read Word: i2c_smbus_read_word_data() -============================================ +SMBus Read Word +=============== + +Implemented by i2c_smbus_read_word_data() This operation is very like Read Byte; again, data is read from a device, from a designated register that is specified through the Comm @@ -115,8 +123,10 @@ available for reads where the two data bytes are the other way around (not SMBus compliant, but very popular.) -SMBus Write Byte: i2c_smbus_write_byte_data() -============================================== +SMBus Write Byte +================ + +Implemented by i2c_smbus_write_byte_data() This writes a single byte to a device, to a designated register. The register is specified through the Comm byte. This is the opposite of @@ -129,8 +139,10 @@ the Read Byte operation. Functionality flag: I2C_FUNC_SMBUS_WRITE_BYTE_DATA -SMBus Write Word: i2c_smbus_write_word_data() -============================================== +SMBus Write Word +================ + +Implemented by i2c_smbus_write_word_data() This is the opposite of the Read Word operation. 16 bits of data is written to a device, to the designated register that is @@ -145,8 +157,8 @@ available for writes where the two data bytes are the other way around (not SMBus compliant, but very popular.) -SMBus Process Call: -=================== +SMBus Process Call +================== This command selects a device register (through the Comm byte), sends 16 bits of data to it, and reads 16 bits of data in return:: @@ -157,8 +169,10 @@ This command selects a device register (through the Comm byte), sends Functionality flag: I2C_FUNC_SMBUS_PROC_CALL -SMBus Block Read: i2c_smbus_read_block_data() -============================================== +SMBus Block Read +================ + +Implemented by i2c_smbus_read_block_data() This command reads a block of up to 32 bytes from a device, from a designated register that is specified through the Comm byte. The amount @@ -172,8 +186,10 @@ of data is specified by the device in the Count byte. Functionality flag: I2C_FUNC_SMBUS_READ_BLOCK_DATA -SMBus Block Write: i2c_smbus_write_block_data() -================================================ +SMBus Block Write +================= + +Implemented by i2c_smbus_write_block_data() The opposite of the Block Read command, this writes up to 32 bytes to a device, to a designated register that is specified through the @@ -274,8 +290,10 @@ I2C block transactions do not limit the number of bytes transferred but the SMBus layer places a limit of 32 bytes. -I2C Block Read: i2c_smbus_read_i2c_block_data() -================================================ +I2C Block Read +============== + +Implemented by i2c_smbus_read_i2c_block_data() This command reads a block of bytes from a device, from a designated register that is specified through the Comm byte:: @@ -286,8 +304,10 @@ designated register that is specified through the Comm byte:: Functionality flag: I2C_FUNC_SMBUS_READ_I2C_BLOCK -I2C Block Write: i2c_smbus_write_i2c_block_data() -================================================== +I2C Block Write +=============== + +Implemented by i2c_smbus_write_i2c_block_data() The opposite of the Block Read command, this writes bytes to a device, to a designated register that is specified through the
Hyperlinks from function names are not generated in headings. Move them in the plain text so they are rendered as clickable hyerlinks. While there also remove an unneeded colon in a heading. Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> --- Documentation/i2c/smbus-protocol.rst | 64 ++++++++++++++++++---------- 1 file changed, 42 insertions(+), 22 deletions(-)