Received-SPF: pass (google.com: domain of linux-kernel+bounces-120348-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 27 Mar 2024 06:53:49 +0100
From: Greg KH <gregkh@linuxfoundation.org>
To: Norihiko Hama <Norihiko.Hama@alpsalpine.com>
Cc: stern@rowland.harvard.edu, linux-usb@vger.kernel.org,
	usb-storage@lists.one-eyed-alien.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb-storage: Optimize scan delay more precisely
Message-ID: <2024032757-surcharge-grime-d3dd@gregkh>
References: <20240327055130.43206-1-Norihiko.Hama@alpsalpine.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240327055130.43206-1-Norihiko.Hama@alpsalpine.com>

On Wed, Mar 27, 2024 at 02:51:30PM +0900, Norihiko Hama wrote:
> Current storage scan delay is reduced by the following old commit.
> 
> a4a47bc03fe5 ("Lower USB storage settling delay to something more reasonable")
> 
> It means that delay is at least 'one second', or zero with delay_use=0.
> 'one second' is still long delay especially for embedded system but
> when delay_use is set to 0 (no delay), error still observed on some USB drives.
> 
> So delay_use should not be set to 0 but 'one second' is quite long.
> 
> This patch optimizes scan delay more precisely
> to minimize delay time but not to have any problems on USB drives
> by adding module parameter 'delay_scale' of delay-time divisor.
> By default, delay time is 'one second' for backward compatibility.
> For example, it seems to be good by changing delay_scale=100,
> that is 100 millisecond delay.
> 
> Signed-off-by: Norihiko Hama <Norihiko.Hama@alpsalpine.com>
> ---
>  drivers/usb/storage/usb.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
> index 90aa9c12ffac..f4a755e364da 100644
> --- a/drivers/usb/storage/usb.c
> +++ b/drivers/usb/storage/usb.c
> @@ -70,6 +70,9 @@ MODULE_LICENSE("GPL");
>  static unsigned int delay_use = 1;
>  module_param(delay_use, uint, S_IRUGO | S_IWUSR);
>  MODULE_PARM_DESC(delay_use, "seconds to delay before using a new device");
> +static unsigned int delay_scale = MSEC_PER_SEC;
> +module_param(delay_scale, uint, 0644);
> +MODULE_PARM_DESC(delay_scale, "time scale of delay_use");

Sorry, but module parameters are from the 1990's, we will not go back to
that if at all possible as it's not easy to maintain and will not work
properly for multiple devices.

I can understand wanting something between 1 and 0 seconds, but adding
yet-another-option isn't probably the best way, sorry.

thanks,

greg k-h