Re: [PATCH] mei: add timeout to send

[Greg Kroah-Hartman]

On Thu, Nov 03, 2022 at 05:55:34PM +0200, Alexander Usyskin wrote:
> When driver wakes up the firmware from the low power stand,
> it is sending a memory ready message.
> The send is done via synchronous/blocking function to ensure
> that firmware is in ready state. However firmware might be
> in unstable state and send might be block forever.
> To address this issue a timeout is added to blocking write command on
> the internal bus.
> 
> Signed-off-by: Alexander Usyskin <alexander.usyskin@xxxxxxxxx>
> ---
>  drivers/misc/mei/bus-fixup.c | 19 +++++++++++--------
>  drivers/misc/mei/bus.c       |  9 +++++----
>  drivers/misc/mei/client.c    | 21 +++++++++++++++++----
>  drivers/misc/mei/client.h    |  2 +-
>  drivers/misc/mei/main.c      |  2 +-
>  drivers/misc/mei/mei_dev.h   |  2 +-
>  6 files changed, 36 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 71fbf0bc8453..3174cad8a5cc 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -128,7 +128,7 @@ static int mei_osver(struct mei_cl_device *cldev)
>  	os_ver = (struct mei_os_ver *)fwcaps->data;
>  	os_ver->os_type = OSTYPE_LINUX;
>  
> -	return __mei_cl_send(cldev->cl, buf, size, 0, mode);
> +	return __mei_cl_send(cldev->cl, buf, size, 0, mode, 0);

Ick, adding another parameter to a function is not helpful here, what
does 0 mean unless you look it up?

> --- a/drivers/misc/mei/bus.c
> +++ b/drivers/misc/mei/bus.c
> @@ -29,11 +29,12 @@
>   * @length: buffer length
>   * @vtag: virtual tag
>   * @mode: sending mode
> + * @timeout: send timeout for blocking writes, 0 for infinite timeout

What units are these in?  Jiffies?  seconds?  hours?  Don't make us
guess :(

Why not write a function that is mei_cl_send_timeout() that has the new
option?  That way it's obvious that the other ones are blocking, right?

thanks,

greg k-h