* [PATCH] scsi: ufs: core: Improve return value documentation
@ 2025-06-23 21:59 Bart Van Assche
2025-07-09 1:44 ` Martin K. Petersen
2025-07-22 3:46 ` Martin K. Petersen
0 siblings, 2 replies; 3+ messages in thread
From: Bart Van Assche @ 2025-06-23 21:59 UTC (permalink / raw)
To: Martin K . Petersen
Cc: linux-scsi, Bart Van Assche, James E.J. Bottomley, Peter Wang,
Avri Altman, Manivannan Sadhasivam, Bao D. Nguyen
Some functions return a negative value to indicate an error while other
functions return a value != 0 to indicate an error. Document the return
value behavior where this documentation is missing and fix the return
value documentation where necessary. Add warnings to detect mismatches
between documentation and implementation. This matters because several
sysfs callback functions only work correctly if a negative value is
returned upon error.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/ufs/core/ufshcd.c | 37 +++++++++++++++++++++++++++++--------
1 file changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index c2048aca09fc..b3fe4335d56c 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2566,7 +2566,7 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
* @hba: per adapter instance
* @uic_cmd: UIC command
*
- * Return: 0 only if success.
+ * Return: 0 if successful; < 0 upon failure.
*/
static int
__ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
@@ -3072,6 +3072,9 @@ static void ufshcd_setup_dev_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
hba->dev_cmd.type = cmd_type;
}
+/*
+ * Return: 0 upon success; < 0 upon failure.
+ */
static int ufshcd_compose_dev_cmd(struct ufs_hba *hba,
struct ufshcd_lrb *lrbp, enum dev_cmd_type cmd_type, int tag)
{
@@ -3184,9 +3187,13 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
break;
}
+ WARN_ONCE(err > 0, "Incorrect return value %d > 0\n", err);
return err;
}
+/*
+ * Return: 0 upon success; < 0 upon failure.
+ */
static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
struct ufshcd_lrb *lrbp, int max_timeout)
{
@@ -3261,6 +3268,7 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
}
}
+ WARN_ONCE(err > 0, "Incorrect return value %d > 0\n", err);
return err;
}
@@ -3278,6 +3286,9 @@ static void ufshcd_dev_man_unlock(struct ufs_hba *hba)
ufshcd_release(hba);
}
+/*
+ * Return: 0 upon success; < 0 upon failure.
+ */
static int ufshcd_issue_dev_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
const u32 tag, int timeout)
{
@@ -3365,6 +3376,7 @@ static int ufshcd_query_flag_retry(struct ufs_hba *hba,
dev_err(hba->dev,
"%s: query flag, opcode %d, idn %d, failed with error %d after %d retries\n",
__func__, opcode, idn, ret, retries);
+ WARN_ONCE(ret > 0, "Incorrect return value %d > 0\n", ret);
return ret;
}
@@ -3376,7 +3388,7 @@ static int ufshcd_query_flag_retry(struct ufs_hba *hba,
* @index: flag index to access
* @flag_res: the flag value after the query request completes
*
- * Return: 0 for success, non-zero in case of failure.
+ * Return: 0 for success; < 0 upon failure.
*/
int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
enum flag_idn idn, u8 index, bool *flag_res)
@@ -3432,6 +3444,7 @@ int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
out_unlock:
ufshcd_dev_man_unlock(hba);
+ WARN_ONCE(err > 0, "Incorrect return value %d > 0\n", err);
return err;
}
@@ -3444,7 +3457,7 @@ int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
* @selector: selector field
* @attr_val: the attribute value after the query request completes
*
- * Return: 0 for success, non-zero in case of failure.
+ * Return: 0 upon success; < 0 upon failure.
*/
int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
enum attr_idn idn, u8 index, u8 selector, u32 *attr_val)
@@ -3493,6 +3506,7 @@ int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
out_unlock:
ufshcd_dev_man_unlock(hba);
+ WARN_ONCE(err > 0, "Incorrect return value %d > 0\n", err);
return err;
}
@@ -3507,7 +3521,7 @@ int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
* @attr_val: the attribute value after the query request
* completes
*
- * Return: 0 for success, non-zero in case of failure.
+ * Return: 0 for success; < 0 upon failure.
*/
int ufshcd_query_attr_retry(struct ufs_hba *hba,
enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector,
@@ -3530,9 +3544,13 @@ int ufshcd_query_attr_retry(struct ufs_hba *hba,
dev_err(hba->dev,
"%s: query attribute, idn %d, failed with error %d after %d retries\n",
__func__, idn, ret, QUERY_REQ_RETRIES);
+ WARN_ONCE(ret > 0, "Incorrect return value %d > 0\n", ret);
return ret;
}
+/*
+ * Return: 0 if successful; < 0 upon failure.
+ */
static int __ufshcd_query_descriptor(struct ufs_hba *hba,
enum query_opcode opcode, enum desc_idn idn, u8 index,
u8 selector, u8 *desc_buf, int *buf_len)
@@ -3590,6 +3608,7 @@ static int __ufshcd_query_descriptor(struct ufs_hba *hba,
out_unlock:
hba->dev_cmd.query.descriptor = NULL;
ufshcd_dev_man_unlock(hba);
+ WARN_ONCE(err > 0, "Incorrect return value %d > 0\n", err);
return err;
}
@@ -3606,7 +3625,7 @@ static int __ufshcd_query_descriptor(struct ufs_hba *hba,
* The buf_len parameter will contain, on return, the length parameter
* received on the response.
*
- * Return: 0 for success, non-zero in case of failure.
+ * Return: 0 for success; < 0 upon failure.
*/
int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
enum query_opcode opcode,
@@ -3624,6 +3643,7 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
break;
}
+ WARN_ONCE(err > 0, "Incorrect return value %d > 0\n", err);
return err;
}
@@ -3636,7 +3656,7 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
* @param_read_buf: pointer to buffer where parameter would be read
* @param_size: sizeof(param_read_buf)
*
- * Return: 0 in case of success, non-zero otherwise.
+ * Return: 0 in case of success; < 0 upon failure.
*/
int ufshcd_read_desc_param(struct ufs_hba *hba,
enum desc_idn desc_id,
@@ -3703,6 +3723,7 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
out:
if (is_kmalloc)
kfree(desc_buf);
+ WARN_ONCE(ret > 0, "Incorrect return value %d > 0\n", ret);
return ret;
}
@@ -3816,7 +3837,7 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, u8 desc_index,
* @param_read_buf: pointer to buffer where parameter would be read
* @param_size: sizeof(param_read_buf)
*
- * Return: 0 in case of success, non-zero otherwise.
+ * Return: 0 in case of success; < 0 upon failure.
*/
static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
int lun,
@@ -4794,7 +4815,7 @@ static int ufshcd_complete_dev_init(struct ufs_hba *hba)
* 3. Program UTRL and UTMRL base address
* 4. Configure run-stop-registers
*
- * Return: 0 on success, non-zero value on failure.
+ * Return: 0 if successful; < 0 upon failure.
*/
int ufshcd_make_hba_operational(struct ufs_hba *hba)
{
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] scsi: ufs: core: Improve return value documentation
2025-06-23 21:59 [PATCH] scsi: ufs: core: Improve return value documentation Bart Van Assche
@ 2025-07-09 1:44 ` Martin K. Petersen
2025-07-22 3:46 ` Martin K. Petersen
1 sibling, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2025-07-09 1:44 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, linux-scsi, James E.J. Bottomley, Peter Wang,
Avri Altman, Manivannan Sadhasivam, Bao D. Nguyen
Bart,
> Some functions return a negative value to indicate an error while
> other functions return a value != 0 to indicate an error. Document the
> return value behavior where this documentation is missing and fix the
> return value documentation where necessary. Add warnings to detect
> mismatches between documentation and implementation. This matters
> because several sysfs callback functions only work correctly if a
> negative value is returned upon error.
Applied to 6.17/scsi-staging, thanks!
--
Martin K. Petersen
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] scsi: ufs: core: Improve return value documentation
2025-06-23 21:59 [PATCH] scsi: ufs: core: Improve return value documentation Bart Van Assche
2025-07-09 1:44 ` Martin K. Petersen
@ 2025-07-22 3:46 ` Martin K. Petersen
1 sibling, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2025-07-22 3:46 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, linux-scsi, James E.J. Bottomley, Peter Wang,
Avri Altman, Manivannan Sadhasivam, Bao D. Nguyen
On Mon, 23 Jun 2025 14:59:01 -0700, Bart Van Assche wrote:
> Some functions return a negative value to indicate an error while other
> functions return a value != 0 to indicate an error. Document the return
> value behavior where this documentation is missing and fix the return
> value documentation where necessary. Add warnings to detect mismatches
> between documentation and implementation. This matters because several
> sysfs callback functions only work correctly if a negative value is
> returned upon error.
>
> [...]
Applied to 6.17/scsi-queue, thanks!
[1/1] scsi: ufs: core: Improve return value documentation
https://git.kernel.org/mkp/scsi/c/cc59f3b68542
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-22 3:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-23 21:59 [PATCH] scsi: ufs: core: Improve return value documentation Bart Van Assche
2025-07-09 1:44 ` Martin K. Petersen
2025-07-22 3:46 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).