linux-kernel - Re: [PATCH v5] 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: <ab468bb26dafca673d7ffca7dff519b7cf024cdc.camel@mediatek.com>
Date: Tue, 20 May 2025 13:14:37 +0000
From: Peter Wang (王信友) <peter.wang@...iatek.com>
To: "avri.altman@....com" <avri.altman@....com>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"tanghuan@...o.com" <tanghuan@...o.com>, "quic_nguyenb@...cinc.com"
	<quic_nguyenb@...cinc.com>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>, "bvanassche@....org"
	<bvanassche@....org>, "manivannan.sadhasivam@...aro.org"
	<manivannan.sadhasivam@...aro.org>, "alim.akhtar@...sung.com"
	<alim.akhtar@...sung.com>, "luhongfei@...o.com" <luhongfei@...o.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
	"James.Bottomley@...senPartnership.com"
	<James.Bottomley@...senPartnership.com>, "martin.petersen@...cle.com"
	<martin.petersen@...cle.com>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-scsi@...r.kernel.org"
	<linux-scsi@...r.kernel.org>
CC: "opensource.kernel@...o.com" <opensource.kernel@...o.com>,
	"wenxing.cheng@...o.com" <wenxing.cheng@...o.com>
Subject: Re: [PATCH v5] ufs: core: Add HID support

On Tue, 2025-05-20 at 17:40 +0800, Huan Tang wrote:
> 
> +static const char * const hid_trigger_mode[] = {"disable",
> "enable"};
> +
> +static ssize_t analysis_trigger_store(struct device *dev,
> +               struct device_attribute *attr, const char *buf,
> size_t count)
> +{
> +       struct ufs_hba *hba = dev_get_drvdata(dev);
> +       int mode;
> +       int ret;
> +
> +       mode = sysfs_match_string(hid_trigger_mode, buf);
> +       if (mode < 0)
> +               return -EINVAL;
> 

Hi Haun,

Consider use below coding style for readability.

if (sysfs_streq(buf, "enable"))
    mode = ...;
else if (sysfs_streq(buf, "disable"))
    mode = ...;
else
    return -EINVAL;



> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 3e2097e65964..8ccd923a5761 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8390,6 +8390,10 @@ static int ufs_get_device_desc(struct ufs_hba
> *hba)
> 
>         dev_info->rtt_cap = desc_buf[DEVICE_DESC_PARAM_RTT_CAP];
> 
> +       dev_info->hid_sup = get_unaligned_be32(desc_buf +
> +                              
> DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP) &
> +                               UFS_DEV_HID_SUPPORT;
> +
> 

Could add the double negation (!!) ensures the value is exactly 0 or 1.
dev_info->hid_sup = !!(get_unaligned_be32(desc_buf +
            DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP) &
            UFS_DEV_HID_SUPPORT);

The rest of the parts looks good to me.

Reviewed-by: Peter Wang <peter.wang@...iatek.com>


Thanks
Peter