All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.