linux-kernel - Re: Re: [PATCH] ufs: core: Add HID support

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: <20250512133512.188-1-tanghuan@vivo.com>
Date: Mon, 12 May 2025 21:35:12 +0800
From: Huan Tang <tanghuan@...o.com>
To: bvanassche@....org
Cc: James.Bottomley@...senPartnership.com,
	alim.akhtar@...sung.com,
	avri.altman@....com,
	beanhuo@...ron.com,
	ebiggers@...gle.com,
	gwendal@...omium.org,
	keosung.park@...sung.com,
	linux-kernel@...r.kernel.org,
	linux-scsi@...r.kernel.org,
	luhongfei@...o.com,
	manivannan.sadhasivam@...aro.org,
	martin.petersen@...cle.com,
	minwoo.im@...sung.com,
	opensource.kernel@...o.com,
	peter.wang@...iatek.com,
	quic_cang@...cinc.com,
	quic_nguyenb@...cinc.com,
	quic_ziqichen@...cinc.com,
	tanghuan@...o.com,
	viro@...iv.linux.org.uk,
	wenxing.cheng@...o.com
Subject: Re: Re: [PATCH] ufs: core: Add HID support

Hi  Bart sir，

Sorry for not replying in time. Thank you for your comments and guidance！
Based on your, Peter's and Arvi's suggestions, I submitted the v2 patch:

https://lore.kernel.org/all/20250512131519.138-1-tanghuan@vivo.com/

> Combining HID analysis and HID defragmentation controls into a single
> sysfs attribute seems weird to me. Please replace the above two
> attributes with two different attributes: one for controlling HID
> analysis and another one for controlling HID defragmentation.

In the v2 patch,  use hid_analysis_trigger and hid_defrag_trigger

> Please change the name of this attribute into "hid_fragmented_size". I
> think that this alternative name is much more clear.

Already modified in v2 patch

> Please make the name of this attribute more clear, e.g. by renaming it
> into "hid_defrag_size". Additionally, please change "defrag" into
> "defragmentation".

Already modified in v2 patch

> Please change the format of this attribute from hexadecimal into
> decimal (64h -> 100).

Already modified in v2 patch

> Please change the format of this attribute from hexadecimal into
> decimal. Please add an additional sysfs attribute that provides the
> textual meaning of the HID state such that users don't have to look up
> the documentation of these codes.

Already modified in v2 patch

> Here and everywhere else in documentation that is intended for humans,
> please change "defrag" into "defragmentation". Please also make the
> Kconfig description more clear, e.g. by changing it into the following:

Already modified in v2 patch

> A blank line is required after the macro definition and also between the
> declaration of the static variable and the function definition.

v2 patch does not use this

> In new code, please order declarations such that the longest declaration
> occurs first ("reverse Christmas tree").

Already modified in v2 patch

> Why is auto-hibernation disabled here? Please add a comment.

Some UFS vendors say that AH8 needs to be disabled to run HID

> Please add a comment that explains why the ufshcd_auto_hibern8_update()
> call is present.

Some UFS vendors say that AH8 needs to be disabled to run HID

> Why the hexadecimal format? All other sysfs size attributes I know of
> use the decimal format.

Already modified in v2 patch

Thanks
Huan