Re: [PATCH] scsi: ufs: wb: Add explicit flush_threshold sysfs attribute

[Bart Van Assche]

On 6/8/23 03:16, Lu Hongfei wrote:
> +static ssize_t wb_flush_threshold_show(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buf)
> +{
> +	struct ufs_hba *hba = dev_get_drvdata(dev);
> +
> +	return sysfs_emit(buf, "%d\n", hba->vps->wb_flush_threshold);
> +}

Since wb_flush_threshold is unsigned, please change %d into %u.

> +	if (!ufshcd_is_wb_allowed(hba)) {
> +		dev_warn(dev, "It is not allowed to configure WB buf flush threshold!\n");
> +		return -EOPNOTSUPP;
> +	}

The above check prevents configuring the flush threshold before enabling 
the write booster mechanism. I don't think users will like this. Please 
leave out the above check.

> +	if (kstrtouint(buf, 0, &wb_flush_threshold))
> +		return -EINVAL;
> +
> +	/* The range of values for wb_flush_threshold is (0,10] */
> +	if (wb_flush_threshold <= 0 || wb_flush_threshold > 10) {
> +		dev_err(dev, "The value of wb_flush_threshold is invalid!\n");
> +		return -EINVAL;
> +	}

Please change '10' in the above code into UFS_WB_BUF_REMAIN_PERCENT(100) 
to make the above code easier to read.

Thanks,

Bart.