public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "'Greg Kroah-Hartman'" <gregkh@linuxfoundation.org>
To: "Winkler, Tomas" <tomas.winkler@intel.com>
Cc: "Usyskin, Alexander" <alexander.usyskin@intel.com>,
	"'linux-kernel@vger.kernel.org'" <linux-kernel@vger.kernel.org>
Subject: Re: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX
Date: Wed, 30 Nov 2016 12:57:10 +0100	[thread overview]
Message-ID: <20161130115710.GB1436@kroah.com> (raw)
In-Reply-To: <5B8DA87D05A7694D9FA63FD143655C1B54333D99@hasmsx108.ger.corp.intel.com>

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

  reply	other threads:[~2016-11-30 11:57 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-19 12:16 [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX Tomas Winkler
2016-11-19 12:18 ` Greg Kroah-Hartman
2016-11-19 16:16   ` Winkler, Tomas
2016-11-28 23:03     ` Winkler, Tomas
2016-11-29 19:16       ` 'Greg Kroah-Hartman'
2016-11-29 20:09         ` Winkler, Tomas
2016-11-30 11:57           ` 'Greg Kroah-Hartman' [this message]
2016-11-30 12:00             ` Winkler, Tomas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161130115710.GB1436@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=alexander.usyskin@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tomas.winkler@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox