diff mbox series

[v6,3/7] ata: libata: Rename and cleanup ata_rwcmd_protocol()

Message ID 20221108055544.1481583-4-damien.lemoal@opensource.wdc.com (mailing list archive)
State New, archived
Headers show
Series Improve libata support for FUA | expand

Commit Message

Damien Le Moal Nov. 8, 2022, 5:55 a.m. UTC 
Rename ata_rwcmd_protocol() to ata_set_rwcmd_protocol() to better
reflect the fact that this function sets a task file command and
protocol. The arguments order is also reversed and the function return
type changed to a bool to indicate if the command and protocol were set
corretly (instead of returning a completely arbitrary "-1" value.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-core.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

Comments

Johannes Thumshirn Nov. 8, 2022, 7:39 a.m. UTC | #1 
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Niklas Cassel Dec. 30, 2022, 7:42 a.m. UTC | #2 
On Tue, Nov 08, 2022 at 02:55:40PM +0900, Damien Le Moal wrote:
> Rename ata_rwcmd_protocol() to ata_set_rwcmd_protocol() to better
> reflect the fact that this function sets a task file command and
> protocol. The arguments order is also reversed and the function return
> type changed to a bool to indicate if the command and protocol were set
> corretly (instead of returning a completely arbitrary "-1" value.

correctly

> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/ata/libata-core.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 884ae73b11ea..6ee1cbac3ab0 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -574,17 +574,18 @@ static const u8 ata_rw_cmds[] = {
>  };
>  
>  /**
> - *	ata_rwcmd_protocol - set taskfile r/w commands and protocol
> - *	@tf: command to examine and configure
> - *	@dev: device tf belongs to
> + *	ata_set_rwcmd_protocol - set taskfile r/w command and protocol
> + *	@dev: target device for the taskfile
> + *	@tf: taskfile to examine and configure
>   *
> - *	Examine the device configuration and tf->flags to calculate
> - *	the proper read/write commands and protocol to use.
> + *	Examine the device configuration and tf->flags to determine
> + *	the proper read/write command and protocol to use for @tf.
>   *
>   *	LOCKING:
>   *	caller.
>   */
> -static int ata_rwcmd_protocol(struct ata_taskfile *tf, struct ata_device *dev)
> +static bool ata_set_rwcmd_protocol(struct ata_device *dev,
> +				   struct ata_taskfile *tf)
>  {
>  	u8 cmd;
>  
> @@ -607,11 +608,12 @@ static int ata_rwcmd_protocol(struct ata_taskfile *tf, struct ata_device *dev)
>  	}
>  
>  	cmd = ata_rw_cmds[index + fua + lba48 + write];
> -	if (cmd) {
> -		tf->command = cmd;
> -		return 0;
> -	}
> -	return -1;
> +	if (!cmd)
> +		return false;
> +
> +	tf->command = cmd;
> +
> +	return true;
>  }
>  
>  /**
> @@ -744,7 +746,7 @@ int ata_build_rw_tf(struct ata_queued_cmd *qc, u64 block, u32 n_block,
>  			/* request too large even for LBA48 */
>  			return -ERANGE;
>  
> -		if (unlikely(ata_rwcmd_protocol(tf, dev) < 0))
> +		if (unlikely(!ata_set_rwcmd_protocol(dev, tf)))
>  			return -EINVAL;
>  
>  		tf->nsect = n_block & 0xff;
> @@ -762,7 +764,7 @@ int ata_build_rw_tf(struct ata_queued_cmd *qc, u64 block, u32 n_block,
>  		if (!lba_28_ok(block, n_block))
>  			return -ERANGE;
>  
> -		if (unlikely(ata_rwcmd_protocol(tf, dev) < 0))
> +		if (unlikely(!ata_set_rwcmd_protocol(dev, tf)))
>  			return -EINVAL;
>  
>  		/* Convert LBA to CHS */
> -- 
> 2.38.1
> 

With or without small typo fixed:
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
diff mbox series

Patch

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 884ae73b11ea..6ee1cbac3ab0 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -574,17 +574,18 @@  static const u8 ata_rw_cmds[] = {
 };
 
 /**
- *	ata_rwcmd_protocol - set taskfile r/w commands and protocol
- *	@tf: command to examine and configure
- *	@dev: device tf belongs to
+ *	ata_set_rwcmd_protocol - set taskfile r/w command and protocol
+ *	@dev: target device for the taskfile
+ *	@tf: taskfile to examine and configure
  *
- *	Examine the device configuration and tf->flags to calculate
- *	the proper read/write commands and protocol to use.
+ *	Examine the device configuration and tf->flags to determine
+ *	the proper read/write command and protocol to use for @tf.
  *
  *	LOCKING:
  *	caller.
  */
-static int ata_rwcmd_protocol(struct ata_taskfile *tf, struct ata_device *dev)
+static bool ata_set_rwcmd_protocol(struct ata_device *dev,
+				   struct ata_taskfile *tf)
 {
 	u8 cmd;
 
@@ -607,11 +608,12 @@  static int ata_rwcmd_protocol(struct ata_taskfile *tf, struct ata_device *dev)
 	}
 
 	cmd = ata_rw_cmds[index + fua + lba48 + write];
-	if (cmd) {
-		tf->command = cmd;
-		return 0;
-	}
-	return -1;
+	if (!cmd)
+		return false;
+
+	tf->command = cmd;
+
+	return true;
 }
 
 /**
@@ -744,7 +746,7 @@  int ata_build_rw_tf(struct ata_queued_cmd *qc, u64 block, u32 n_block,
 			/* request too large even for LBA48 */
 			return -ERANGE;
 
-		if (unlikely(ata_rwcmd_protocol(tf, dev) < 0))
+		if (unlikely(!ata_set_rwcmd_protocol(dev, tf)))
 			return -EINVAL;
 
 		tf->nsect = n_block & 0xff;
@@ -762,7 +764,7 @@  int ata_build_rw_tf(struct ata_queued_cmd *qc, u64 block, u32 n_block,
 		if (!lba_28_ok(block, n_block))
 			return -ERANGE;
 
-		if (unlikely(ata_rwcmd_protocol(tf, dev) < 0))
+		if (unlikely(!ata_set_rwcmd_protocol(dev, tf)))
 			return -EINVAL;
 
 		/* Convert LBA to CHS */