linux-kernel - Re: [PATCH v6 3/7] scsi: ufs: introduce common delay function

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: <fdf91490-9c7d-df34-1c1f-e03e12855378@acm.org>
Date:   Mon, 16 Mar 2020 09:23:05 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     Stanley Chu <stanley.chu@...iatek.com>, linux-scsi@...r.kernel.org,
        "Martin K . Petersen" <martin.petersen@...cle.com>,
        avri.altman@....com, alim.akhtar@...sung.com, jejb@...ux.ibm.com
Cc:     beanhuo@...ron.com, asutoshd@...eaurora.org, cang@...eaurora.org,
        matthias.bgg@...il.com, linux-mediatek@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        kuohong.wang@...iatek.com, peter.wang@...iatek.com,
        chun-hung.wu@...iatek.com, andy.teng@...iatek.com
Subject: Re: [PATCH v6 3/7] scsi: ufs: introduce common delay function

On 3/16/20 1:52 AM, Stanley Chu wrote:
> +void ufshcd_wait_us(unsigned long us, unsigned long tolerance, bool can_sleep)
> +{
> +	if (!us)
> +		return;
> +
> +	if (us < 10 || !can_sleep)
> +		udelay(us);
> +	else
> +		usleep_range(us, us + tolerance);
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_wait_us);

I don't like this function because I think it makes the UFS code harder 
to read instead of easier. The 'can_sleep' argument is only set by one 
caller which I think is a strong argument to remove that argument again 
and to move the code that depends on that argument from the above 
function into the caller. Additionally, it is not possible to comprehend 
what a ufshcd_wait_us() call does without looking up the function 
definition to see what the meaning of the third argument is.

Please drop this patch.

Thanks,

Bart.