mbox series

[v3,0/2] mei: add timeout to send

Message ID 20221116124735.2493847-1-alexander.usyskin@intel.com
Headers
Series mei: add timeout to send |

Message

Usyskin, Alexander Nov. 16, 2022, 12:47 p.m. UTC 
  When driver wakes up the firmware from the low power state,
it is sending a memory ready message.
The send is done via synchronous/blocking function to ensure
that firmware is in ready state. However, in case of firmware
undergoing reset send might be block forever.
To address this issue a timeout is added to blocking
write command on the internal bus.

Introduce the __mei_cl_send_timeout function to use instead of
__mei_cl_send in cases where timeout is required.
The mei_cl_write has only two callers and there is no need to split
it into two functions.

V2: address review comments:
 - split __mei_cl_send and __mei_cl_send_timeout
 - add units to timeout KDoc
 - use MAX_SCHEDULE_TIMEOUT to squash wait to one macro

V3: - split the state fix into separate patch
    - document define unit
    - expand timeout KDoc

Alexander Usyskin (2):
  mei: add timeout to send
  mei: bus-fixup: change pxp mode only if message was sent

 drivers/misc/mei/bus-fixup.c | 14 +++++++++-----
 drivers/misc/mei/bus.c       | 22 +++++++++++++++++++++-
 drivers/misc/mei/client.c    | 20 ++++++++++++++++----
 drivers/misc/mei/client.h    |  2 +-
 drivers/misc/mei/main.c      |  2 +-
 drivers/misc/mei/mei_dev.h   |  2 ++
 6 files changed, 50 insertions(+), 12 deletions(-)

Comments

Rodrigo Vivi Nov. 30, 2022, 2:20 p.m. UTC | #1 
On Wed, Nov 16, 2022 at 02:47:33PM +0200, Alexander Usyskin wrote:
> When driver wakes up the firmware from the low power state,
> it is sending a memory ready message.
> The send is done via synchronous/blocking function to ensure
> that firmware is in ready state. However, in case of firmware
> undergoing reset send might be block forever.
> To address this issue a timeout is added to blocking
> write command on the internal bus.
> 
> Introduce the __mei_cl_send_timeout function to use instead of
> __mei_cl_send in cases where timeout is required.
> The mei_cl_write has only two callers and there is no need to split
> it into two functions.
> 
> V2: address review comments:
>  - split __mei_cl_send and __mei_cl_send_timeout
>  - add units to timeout KDoc
>  - use MAX_SCHEDULE_TIMEOUT to squash wait to one macro
> 
> V3: - split the state fix into separate patch
>     - document define unit
>     - expand timeout KDoc

These 2 patches looks good to me now.

Greg, whenever you review it, please let me know if it is
okay to me to push these through the drm-fixes, or if you
prefer these to the mei branches.

Btw, Alexander, would we have a good "Fixes:" tag for these
patches?

Thanks,
Rodrigo.

> 
> Alexander Usyskin (2):
>   mei: add timeout to send
>   mei: bus-fixup: change pxp mode only if message was sent
> 
>  drivers/misc/mei/bus-fixup.c | 14 +++++++++-----
>  drivers/misc/mei/bus.c       | 22 +++++++++++++++++++++-
>  drivers/misc/mei/client.c    | 20 ++++++++++++++++----
>  drivers/misc/mei/client.h    |  2 +-
>  drivers/misc/mei/main.c      |  2 +-
>  drivers/misc/mei/mei_dev.h   |  2 ++
>  6 files changed, 50 insertions(+), 12 deletions(-)
> 
> -- 
> 2.34.1
>
Greg KH Nov. 30, 2022, 5:43 p.m. UTC | #2 
On Wed, Nov 30, 2022 at 09:20:28AM -0500, Rodrigo Vivi wrote:
> On Wed, Nov 16, 2022 at 02:47:33PM +0200, Alexander Usyskin wrote:
> > When driver wakes up the firmware from the low power state,
> > it is sending a memory ready message.
> > The send is done via synchronous/blocking function to ensure
> > that firmware is in ready state. However, in case of firmware
> > undergoing reset send might be block forever.
> > To address this issue a timeout is added to blocking
> > write command on the internal bus.
> > 
> > Introduce the __mei_cl_send_timeout function to use instead of
> > __mei_cl_send in cases where timeout is required.
> > The mei_cl_write has only two callers and there is no need to split
> > it into two functions.
> > 
> > V2: address review comments:
> >  - split __mei_cl_send and __mei_cl_send_timeout
> >  - add units to timeout KDoc
> >  - use MAX_SCHEDULE_TIMEOUT to squash wait to one macro
> > 
> > V3: - split the state fix into separate patch
> >     - document define unit
> >     - expand timeout KDoc
> 
> These 2 patches looks good to me now.
> 
> Greg, whenever you review it, please let me know if it is
> okay to me to push these through the drm-fixes, or if you
> prefer these to the mei branches.

These have been in my tree for over a week now, sorry.  No one told me
not to take them...

{sigh}

greg k-h
Rodrigo Vivi Nov. 30, 2022, 5:48 p.m. UTC | #3 
On Wed, 2022-11-30 at 18:43 +0100, Greg Kroah-Hartman wrote:
> On Wed, Nov 30, 2022 at 09:20:28AM -0500, Rodrigo Vivi wrote:
> > On Wed, Nov 16, 2022 at 02:47:33PM +0200, Alexander Usyskin wrote:
> > > When driver wakes up the firmware from the low power state,
> > > it is sending a memory ready message.
> > > The send is done via synchronous/blocking function to ensure
> > > that firmware is in ready state. However, in case of firmware
> > > undergoing reset send might be block forever.
> > > To address this issue a timeout is added to blocking
> > > write command on the internal bus.
> > > 
> > > Introduce the __mei_cl_send_timeout function to use instead of
> > > __mei_cl_send in cases where timeout is required.
> > > The mei_cl_write has only two callers and there is no need to
> > > split
> > > it into two functions.
> > > 
> > > V2: address review comments:
> > >  - split __mei_cl_send and __mei_cl_send_timeout
> > >  - add units to timeout KDoc
> > >  - use MAX_SCHEDULE_TIMEOUT to squash wait to one macro
> > > 
> > > V3: - split the state fix into separate patch
> > >     - document define unit
> > >     - expand timeout KDoc
> > 
> > These 2 patches looks good to me now.
> > 
> > Greg, whenever you review it, please let me know if it is
> > okay to me to push these through the drm-fixes, or if you
> > prefer these to the mei branches.
> 
> These have been in my tree for over a week now, sorry.  No one told
> me
> not to take them...
> 
> {sigh}

no worries at all. The important thing is that the users will get the
fix.
We can keep them in our topic/core-for-CI while our branches are out-
of-sync.

Thanks a lot,
Rodrigo.

> 
> greg k-h