Re: [PATCH] mei: add timeout to send

From: Greg Kroah-Hartman
Date: Mon Nov 07 2022 - 03:15:58 EST


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