Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1531775 > unrolled thread

RE: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX

Started by"Winkler, Tomas" <tomas.winkler@intel.com>
First post2016-11-29 00:10 +0100
Last post2016-11-30 13:10 +0100
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-11-29 00:10 +0100
    Re: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org> - 2016-11-29 20:20 +0100
      RE: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-11-29 21:20 +0100
        Re: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org> - 2016-11-30 13:00 +0100
          RE: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-11-30 13:10 +0100

#1531775 — RE: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX

From"Winkler, Tomas" <tomas.winkler@intel.com>
Date2016-11-29 00:10 +0100
SubjectRE: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX
Message-ID<sIDdD-75-25@gated-at.bofh.it>
> 
> 
> >
> > On Sat, Nov 19, 2016 at 02:16:11PM +0200, Tomas Winkler wrote:
> > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > >
> > > Enable non-blocking receive for drivers on mei bus, this allows
> > > checking for data availability by mei client drivers. This is most
> > > effective for fixed address clients, that lacks flow control.
> > >
> > > This function adds new API function mei_cldev_recv_nonblock(), it
> > > retuns -EGAIN if function will block.
> > >
> > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > ---
> > > V2: use _nonblock() function suffix instead of NONBLOCK flag
> > >
> > >  drivers/misc/mei/bus-fixup.c |  4 ++--
> > >  drivers/misc/mei/bus.c       | 31 +++++++++++++++++++++++++++++--
> > >  drivers/misc/mei/mei_dev.h   |  7 ++++++-
> > >  include/linux/mei_cl_bus.h   |  6 ++++--
> > >  4 files changed, 41 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > b/drivers/misc/mei/bus-fixup.c index 7f2cef9011ae..18e05ca7584f
> > > 100644
> > > --- a/drivers/misc/mei/bus-fixup.c
> > > +++ b/drivers/misc/mei/bus-fixup.c
> > > @@ -141,7 +141,7 @@ static int mei_osver(struct mei_cl_device *cldev)
> > >  	if (ret < 0)
> > >  		return ret;
> > >
> > > -	ret = __mei_cl_recv(cldev->cl, buf, length);
> > > +	ret = __mei_cl_recv(cldev->cl, buf, length, 0);
> >
> > What is 0 here?  Again, mode...
> 
> Yes,  it means no change in behavior,  but this is an internal function.

Greg, are you okay with the changes?  We are missing this patch to complete the series. 
Thanks
Tomas

[toc] | [next] | [standalone]

#1532699

From'Greg Kroah-Hartman' <gregkh@linuxfoundation.org>
Date2016-11-29 20:20 +0100
Message-ID<sIW6C-48h-35@gated-at.bofh.it>
In reply to#1531775
On Mon, Nov 28, 2016 at 11:03:20PM +0000, Winkler, Tomas wrote:
> > 
> > 
> > >
> > > On Sat, Nov 19, 2016 at 02:16:11PM +0200, Tomas Winkler wrote:
> > > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > > >
> > > > Enable non-blocking receive for drivers on mei bus, this allows
> > > > checking for data availability by mei client drivers. This is most
> > > > effective for fixed address clients, that lacks flow control.
> > > >
> > > > This function adds new API function mei_cldev_recv_nonblock(), it
> > > > retuns -EGAIN if function will block.
> > > >
> > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > ---
> > > > V2: use _nonblock() function suffix instead of NONBLOCK flag
> > > >
> > > >  drivers/misc/mei/bus-fixup.c |  4 ++--
> > > >  drivers/misc/mei/bus.c       | 31 +++++++++++++++++++++++++++++--
> > > >  drivers/misc/mei/mei_dev.h   |  7 ++++++-
> > > >  include/linux/mei_cl_bus.h   |  6 ++++--
> > > >  4 files changed, 41 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > > b/drivers/misc/mei/bus-fixup.c index 7f2cef9011ae..18e05ca7584f
> > > > 100644
> > > > --- a/drivers/misc/mei/bus-fixup.c
> > > > +++ b/drivers/misc/mei/bus-fixup.c
> > > > @@ -141,7 +141,7 @@ static int mei_osver(struct mei_cl_device *cldev)
> > > >  	if (ret < 0)
> > > >  		return ret;
> > > >
> > > > -	ret = __mei_cl_recv(cldev->cl, buf, length);
> > > > +	ret = __mei_cl_recv(cldev->cl, buf, length, 0);
> > >
> > > What is 0 here?  Again, mode...
> > 
> > Yes,  it means no change in behavior,  but this is an internal function.

So, it makes no sense, are you not going to have to read this code again
in 10 years?  New developers?  Make the code make sense please.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1532740

From"Winkler, Tomas" <tomas.winkler@intel.com>
Date2016-11-29 21:20 +0100
Message-ID<sIX2G-4Lk-17@gated-at.bofh.it>
In reply to#1532699
> On Mon, Nov 28, 2016 at 11:03:20PM +0000, Winkler, Tomas wrote:
> > >
> > >
> > > >
> > > > On Sat, Nov 19, 2016 at 02:16:11PM +0200, Tomas Winkler wrote:
> > > > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > >
> > > > > Enable non-blocking receive for drivers on mei bus, this allows
> > > > > checking for data availability by mei client drivers. This is
> > > > > most effective for fixed address clients, that lacks flow control.
> > > > >
> > > > > This function adds new API function mei_cldev_recv_nonblock(),
> > > > > it retuns -EGAIN if function will block.
> > > > >
> > > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > > ---
> > > > > V2: use _nonblock() function suffix instead of NONBLOCK flag
> > > > >
> > > > >  drivers/misc/mei/bus-fixup.c |  4 ++--
> > > > >  drivers/misc/mei/bus.c       | 31 +++++++++++++++++++++++++++++--
> > > > >  drivers/misc/mei/mei_dev.h   |  7 ++++++-
> > > > >  include/linux/mei_cl_bus.h   |  6 ++++--
> > > > >  4 files changed, 41 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > > > b/drivers/misc/mei/bus-fixup.c index 7f2cef9011ae..18e05ca7584f
> > > > > 100644
> > > > > --- a/drivers/misc/mei/bus-fixup.c
> > > > > +++ b/drivers/misc/mei/bus-fixup.c
> > > > > @@ -141,7 +141,7 @@ static int mei_osver(struct mei_cl_device
> *cldev)
> > > > >  	if (ret < 0)
> > > > >  		return ret;
> > > > >
> > > > > -	ret = __mei_cl_recv(cldev->cl, buf, length);
> > > > > +	ret = __mei_cl_recv(cldev->cl, buf, length, 0);
> > > >
> > > > What is 0 here?  Again, mode...
> > >
> > > Yes,  it means no change in behavior,  but this is an internal function.
> 
> So, it makes no sense, are you not going to have to read this code again in 10
> years?  New developers?  Make the code make sense please.


Sorry Greg, the code does make sense to me, the whole kernel passes nonblock around as flag starting from the syscall (O_NONBLOCK)
 it doesn't make sense to write two functions that differ in one 'if' statement.
I understand that you  are in some crusade against flags, but you are  not proposing a concrete solution and I don't have one either.
I can solve it in the external wrapper, but internally it's just a same function.

Thanks
Tomas

[toc] | [prev] | [next] | [standalone]

#1533252

From'Greg Kroah-Hartman' <gregkh@linuxfoundation.org>
Date2016-11-30 13:00 +0100
Message-ID<sJbIl-5Bx-5@gated-at.bofh.it>
In reply to#1532740
On Tue, Nov 29, 2016 at 08:09:38PM +0000, Winkler, Tomas wrote:
> 
> > On Mon, Nov 28, 2016 at 11:03:20PM +0000, Winkler, Tomas wrote:
> > > >
> > > >
> > > > >
> > > > > On Sat, Nov 19, 2016 at 02:16:11PM +0200, Tomas Winkler wrote:
> > > > > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > > >
> > > > > > Enable non-blocking receive for drivers on mei bus, this allows
> > > > > > checking for data availability by mei client drivers. This is
> > > > > > most effective for fixed address clients, that lacks flow control.
> > > > > >
> > > > > > This function adds new API function mei_cldev_recv_nonblock(),
> > > > > > it retuns -EGAIN if function will block.
> > > > > >
> > > > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > > > ---
> > > > > > V2: use _nonblock() function suffix instead of NONBLOCK flag
> > > > > >
> > > > > >  drivers/misc/mei/bus-fixup.c |  4 ++--
> > > > > >  drivers/misc/mei/bus.c       | 31 +++++++++++++++++++++++++++++--
> > > > > >  drivers/misc/mei/mei_dev.h   |  7 ++++++-
> > > > > >  include/linux/mei_cl_bus.h   |  6 ++++--
> > > > > >  4 files changed, 41 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > > > > b/drivers/misc/mei/bus-fixup.c index 7f2cef9011ae..18e05ca7584f
> > > > > > 100644
> > > > > > --- a/drivers/misc/mei/bus-fixup.c
> > > > > > +++ b/drivers/misc/mei/bus-fixup.c
> > > > > > @@ -141,7 +141,7 @@ static int mei_osver(struct mei_cl_device
> > *cldev)
> > > > > >  	if (ret < 0)
> > > > > >  		return ret;
> > > > > >
> > > > > > -	ret = __mei_cl_recv(cldev->cl, buf, length);
> > > > > > +	ret = __mei_cl_recv(cldev->cl, buf, length, 0);
> > > > >
> > > > > What is 0 here?  Again, mode...
> > > >
> > > > Yes,  it means no change in behavior,  but this is an internal function.
> > 
> > So, it makes no sense, are you not going to have to read this code again in 10
> > years?  New developers?  Make the code make sense please.
> 
> 
> Sorry Greg, the code does make sense to me, the whole kernel passes nonblock around as flag starting from the syscall (O_NONBLOCK)
>  it doesn't make sense to write two functions that differ in one 'if' statement.
> I understand that you  are in some crusade against flags, but you are  not proposing a concrete solution and I don't have one either.
> I can solve it in the external wrapper, but internally it's just a same function.

What is wrong with your email client that it doesn't wrap things
properly?

Anyway, I don't remember anymore, please resend and I will review it
then.

greg k-h

[toc] | [prev] | [next] | [standalone]

#1533259

From"Winkler, Tomas" <tomas.winkler@intel.com>
Date2016-11-30 13:10 +0100
Message-ID<sJbS2-5UT-11@gated-at.bofh.it>
In reply to#1533252
> On Tue, Nov 29, 2016 at 08:09:38PM +0000, Winkler, Tomas wrote:
> >
> > > On Mon, Nov 28, 2016 at 11:03:20PM +0000, Winkler, Tomas wrote:
> > > > >
> > > > >
> > > > > >
> > > > > > On Sat, Nov 19, 2016 at 02:16:11PM +0200, Tomas Winkler wrote:
> > > > > > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > > > >
> > > > > > > Enable non-blocking receive for drivers on mei bus, this
> > > > > > > allows checking for data availability by mei client drivers.
> > > > > > > This is most effective for fixed address clients, that lacks flow
> control.
> > > > > > >
> > > > > > > This function adds new API function
> > > > > > > mei_cldev_recv_nonblock(), it retuns -EGAIN if function will block.
> > > > > > >
> > > > > > > Signed-off-by: Alexander Usyskin
> > > > > > > <alexander.usyskin@intel.com>
> > > > > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > > > > ---
> > > > > > > V2: use _nonblock() function suffix instead of NONBLOCK flag
> > > > > > >
> > > > > > >  drivers/misc/mei/bus-fixup.c |  4 ++--
> > > > > > >  drivers/misc/mei/bus.c       | 31 +++++++++++++++++++++++++++++-
> -
> > > > > > >  drivers/misc/mei/mei_dev.h   |  7 ++++++-
> > > > > > >  include/linux/mei_cl_bus.h   |  6 ++++--
> > > > > > >  4 files changed, 41 insertions(+), 7 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > > > > > b/drivers/misc/mei/bus-fixup.c index
> > > > > > > 7f2cef9011ae..18e05ca7584f
> > > > > > > 100644
> > > > > > > --- a/drivers/misc/mei/bus-fixup.c
> > > > > > > +++ b/drivers/misc/mei/bus-fixup.c
> > > > > > > @@ -141,7 +141,7 @@ static int mei_osver(struct
> > > > > > > mei_cl_device
> > > *cldev)
> > > > > > >  	if (ret < 0)
> > > > > > >  		return ret;
> > > > > > >
> > > > > > > -	ret = __mei_cl_recv(cldev->cl, buf, length);
> > > > > > > +	ret = __mei_cl_recv(cldev->cl, buf, length, 0);
> > > > > >
> > > > > > What is 0 here?  Again, mode...
> > > > >
> > > > > Yes,  it means no change in behavior,  but this is an internal function.
> > >
> > > So, it makes no sense, are you not going to have to read this code
> > > again in 10 years?  New developers?  Make the code make sense please.
> >
> >
> > Sorry Greg, the code does make sense to me, the whole kernel passes
> > nonblock around as flag starting from the syscall (O_NONBLOCK)  it doesn't
> make sense to write two functions that differ in one 'if' statement.
> > I understand that you  are in some crusade against flags, but you are  not
> proposing a concrete solution and I don't have one either.
> > I can solve it in the external wrapper, but internally it's just a same function.
> 
> What is wrong with your email client that it doesn't wrap things properly?

MS Outlook + Exchange. 

> Anyway, I don't remember anymore, please resend and I will review it then.

Okay
Tomas

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web