Re: [PATCH v7 32/34] i2c: tegra: Clean up and improve comments

[Thierry Reding]

On Wed, Sep 09, 2020 at 01:40:04AM +0300, Dmitry Osipenko wrote:
> Make all comments to be consistent in regards to capitalization and
> punctuation, correct spelling and grammar errors, improve wording.
> 
> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 88 ++++++++++++++++++----------------
>  1 file changed, 47 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 558b1f2934a0..31fbc6181dd5 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -136,7 +136,7 @@
>  /* configuration load timeout in microseconds */
>  #define I2C_CONFIG_LOAD_TIMEOUT			1000000
>  
> -/* Packet header size in bytes */
> +/* packet header size in bytes */
>  #define I2C_PACKET_HEADER_SIZE			12
>  
>  /*
> @@ -148,11 +148,10 @@
>  #define I2C_PIO_MODE_PREFERRED_LEN		32
>  
>  /*
> - * msg_end_type: The bus control which need to be send at end of transfer.
> - * @MSG_END_STOP: Send stop pulse at end of transfer.
> - * @MSG_END_REPEAT_START: Send repeat start at end of transfer.
> - * @MSG_END_CONTINUE: The following on message is coming and so do not send
> - *		stop or repeat start.
> + * msg_end_type: The bus control which needs to be sent at end of transfer.
> + * @MSG_END_STOP: Send stop pulse.
> + * @MSG_END_REPEAT_START: Send repeat-start.
> + * @MSG_END_CONTINUE: Don't send stop or repeat-start.
>   */
>  enum msg_end_type {
>  	MSG_END_STOP,
> @@ -161,10 +160,10 @@ enum msg_end_type {
>  };
>  
>  /**
> - * struct tegra_i2c_hw_feature : Different HW support on Tegra
> - * @has_continue_xfer_support: Continue transfer supports.
> + * struct tegra_i2c_hw_feature : per hardware generation features

I think that space before ':' can go away. Although that's preexisting,
so could also be a separate patch, I guess.

> + * @has_continue_xfer_support: Continue-transfer supported.

This isn't a proper sentence, so I don't think it should have a
full-stop. And it shouldn't start with an uppercase letter, either.

>   * @has_per_pkt_xfer_complete_irq: Has enable/disable capability for transfer
> - *		complete interrupt per packet basis.
> + *		completion interrupt on per packet basis.
>   * @has_config_load_reg: Has the config load register to load the new
>   *		configuration.
>   * @clk_divisor_hs_mode: Clock divisor in HS mode.
> @@ -184,7 +183,7 @@ enum msg_end_type {
>   * @has_mst_fifo: The I2C controller contains the new MST FIFO interface that
>   *		provides additional features and allows for longer messages to
>   *		be transferred in one go.
> - * @quirks: i2c adapter quirks for limiting write/read transfer size and not
> + * @quirks: I2C adapter quirks for limiting write/read transfer size and not
>   *		allowing 0 length transfers.
>   * @supports_bus_clear: Bus Clear support to recover from bus hang during
>   *		SDA stuck low from device for some unknown reasons.
> @@ -245,7 +244,7 @@ struct tegra_i2c_hw_feature {
>   * @msg_err: error code for completed message
>   * @msg_buf: pointer to current message data
>   * @msg_buf_remaining: size of unsent data in the message buffer
> - * @msg_read: identifies read transfers
> + * @msg_read: indicates read direction of a transfer

Hm... "read" is the "direction". Perhaps "indicates the direction of a
transfer"? Or perhaps "indicates that the transfer is a read access"?

[...]
> @@ -1797,9 +1804,8 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev)
>  
>  	/*
>  	 * VI I2C device is attached to VE power domain which goes through
> -	 * power ON/OFF during PM runtime resume/suspend. So, controller
> -	 * should go through reset and need to re-initialize after power
> -	 * domain ON.
> +	 * power ON/OFF during of runtime PM resume/suspend, meaning that

s/during of runtime PM/during runtime PM/

Thierry

Attachment: signature.asc
Description: PGP signature