linux-kernel - Re: [PATCH v5 3/6] scsi: ufs: wb: Add explicit flush sysfs attribute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <82d1e6da-df4b-31bb-ec46-76099d7c3288@acm.org>
Date:   Fri, 29 Jul 2022 13:31:21 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     j-young.choi@...sung.com, ALIM AKHTAR <alim.akhtar@...sung.com>,
        "avri.altman@....com" <avri.altman@....com>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "beanhuo@...ron.com" <beanhuo@...ron.com>,
        "adrian.hunter@...el.com" <adrian.hunter@...el.com>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 3/6] scsi: ufs: wb: Add explicit flush sysfs attribute

On 7/28/22 21:56, Jinyoung CHOI wrote:
> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
> index 6b248abb1bd7..7e9e1db55d60 100644
> --- a/Documentation/ABI/testing/sysfs-driver-ufs
> +++ b/Documentation/ABI/testing/sysfs-driver-ufs
> @@ -1417,6 +1417,16 @@ Description:	This node is used to set or display whether UFS WriteBooster is
>   		platform that doesn't support UFSHCD_CAP_CLK_SCALING, we can
>   		disable/enable WriteBooster through this sysfs node.
>   
> +What:		/sys/bus/platform/drivers/ufshcd/*/wb_buf_flush_en
> +What:		/sys/bus/platform/devices/*.ufs/wb_buf_flush_en
> +Date:		July 2022
> +Contact:	Jinyoung Choi <j-young.choi@...sung.com>
> +Description:	This node is used to set or display whether WriteBooster
                      ^^^^

Please change "node" into "attribute" (here and below). Sysfs files are 
called attributes.

> +		buffer flusing is enabled. The data written in the WriteBooster
                        ^^^^^^^
                        flushing?
> +		Buffer can be flushed by an explicit host command or
> +		implicitly while in hibernate (HIBERN8) state.

The above sentence is misleading because it suggests that setting this 
attribute causes the WB buffer to be flushed in its entirety. That is 
not correct - what this attribute controls is whether or not the UFS 
device is allowed to start with flushing the WB buffer.

> +	if (!ufshcd_is_wb_allowed(hba) ||
> +	    (hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL)) {
> +		dev_warn(dev, "It is not allowed to configure WB buf flush!\n");

flush -> flushing

> +	ufshcd_rpm_get_sync(hba);
> +	res = ufshcd_wb_toggle_buf_flush(hba, wb_buf_flush_en);
> +	ufshcd_rpm_put_sync(hba);
> +out:
> +	up(&hba->host_sem);
> +	return res < 0 ? res : count;
> +}

Please leave a blank line above goto labels as requested by the kernel 
coding style guide.

Thanks,

Bart.