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 for Android: free password hash cracker in your pocket

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

Message-ID: <8c547bfaf0c4553760e8c4980f1d2b27d43ded3c.camel@mediatek.com>
Date: Wed, 21 May 2025 02:22:23 +0000
From: Peter Wang (王信友) <peter.wang@...iatek.com>
To: "avri.altman@....com" <avri.altman@....com>, "linux-scsi@...r.kernel.org"
	<linux-scsi@...r.kernel.org>, "tanghuan@...o.com" <tanghuan@...o.com>,
	"quic_nguyenb@...cinc.com" <quic_nguyenb@...cinc.com>, "AngeloGioacchino Del
 Regno" <angelogioacchino.delregno@...labora.com>,
	"manivannan.sadhasivam@...aro.org" <manivannan.sadhasivam@...aro.org>,
	"bvanassche@....org" <bvanassche@....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>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
	"James.Bottomley@...senPartnership.com"
	<James.Bottomley@...senPartnership.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "martin.petersen@...cle.com"
	<martin.petersen@...cle.com>
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 13:13 -0700, Bart Van Assche wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 5/20/25 6:14 AM, Peter Wang (王信友) wrote:
> > On Tue, 2025-05-20 at 17:40 +0800, Huan Tang wrote:
> > > 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);
> 
> Hi Peter,
> 
> I do not agree that this change is required. ufs_dev_info.hid_sup has
> type bool and hence the compiler will convert the result of the
> binary
> and operation implicitly into a boolean value (0/1). From the C23
> standard: "When any scalar value is converted to bool, the result is
> false if the value is a zero (for arithmetic types), null (for
> pointer
> types), or the scalar has type nullptr_t; otherwise, the result is
> true."
> 
> A double negation does not occur elsewhere in the kernel if an
> integer
> value is assigned to a boolean variable so I think that it shouldn't
> be
> added here either.
> 
> Bart.

Hi Bart,

Yes, this is not strictly necessary.
For me, this just makes it immediately clear that 
hid_sup is a bool. But I understand that you might 
think it redundant and some people will feel confused 
when reading this code.

It doesn't affect the correctness of this patch, and it is 
totally fine even if no changes are made at all. 
Hence I've added my review tag.

Thanks
Peter