Re: [PATCH v5] ufs: core: Add HID support

[Bart Van Assche <bvanassche@acm.org>]

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.