All of lore.kernel.org
 help / color / mirror / Atom feed
* [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX
@ 2016-11-19 12:16 Tomas Winkler
  2016-11-19 12:18 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 8+ messages in thread
From: Tomas Winkler @ 2016-11-19 12:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Alexander Usyskin, linux-kernel, Tomas Winkler

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);
 	if (ret < 0)
 		return ret;
 
@@ -272,7 +272,7 @@ static int mei_nfc_if_version(struct mei_cl *cl,
 		return -ENOMEM;
 
 	ret = 0;
-	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length);
+	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length, 0);
 	if (bytes_recv < if_version_length) {
 		dev_err(bus->dev, "Could not read IF version\n");
 		ret = -EIO;
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 2fd254ecde2f..0037153c80a6 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -98,15 +98,18 @@ ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
  * @cl: host client
  * @buf: buffer to receive
  * @length: buffer length
+ * @mode: io mode
  *
  * Return: read size in bytes of < 0 on error
  */
-ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
+ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length,
+		      unsigned int mode)
 {
 	struct mei_device *bus;
 	struct mei_cl_cb *cb;
 	size_t r_length;
 	ssize_t rets;
+	bool nonblock = !!(mode & MEI_CL_IO_RX_NONBLOCK);
 
 	if (WARN_ON(!cl || !cl->dev))
 		return -ENODEV;
@@ -127,6 +130,11 @@ ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
 	if (rets && rets != -EBUSY)
 		goto out;
 
+	if (nonblock) {
+		rets = -EAGAIN;
+		goto out;
+	}
+
 	/* wait on event only if there is no other waiter */
 	/* synchronized under device mutex */
 	if (!waitqueue_active(&cl->rx_wait)) {
@@ -192,6 +200,25 @@ ssize_t mei_cldev_send(struct mei_cl_device *cldev, u8 *buf, size_t length)
 EXPORT_SYMBOL_GPL(mei_cldev_send);
 
 /**
+ * mei_cldev_recv_nonblock - non block client receive (read)
+ *
+ * @cldev: me client device
+ * @buf: buffer to receive
+ * @length: buffer length
+ *
+ * Return: read size in bytes of < 0 on error
+ *         -EAGAIN if function will block.
+ */
+ssize_t mei_cldev_recv_nonblock(struct mei_cl_device *cldev, u8 *buf,
+				size_t length)
+{
+	struct mei_cl *cl = cldev->cl;
+
+	return __mei_cl_recv(cl, buf, length, MEI_CL_IO_RX_NONBLOCK);
+}
+EXPORT_SYMBOL_GPL(mei_cldev_recv_nonblock);
+
+/**
  * mei_cldev_recv - client receive (read)
  *
  * @cldev: me client device
@@ -204,7 +231,7 @@ ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length)
 {
 	struct mei_cl *cl = cldev->cl;
 
-	return __mei_cl_recv(cl, buf, length);
+	return __mei_cl_recv(cl, buf, length, 0);
 }
 EXPORT_SYMBOL_GPL(mei_cldev_recv);
 
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 82e69a00b7a1..f16bd1209848 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -115,10 +115,14 @@ enum mei_cb_file_ops {
  *
  * @MEI_CL_IO_TX_BLOCKING: send is blocking
  * @MEI_CL_IO_TX_INTERNAL: internal communication between driver and FW
+ *
+ * @MEI_CL_IO_RX_NONBLOCK: recv is non-blocking
  */
 enum mei_cl_io_mode {
 	MEI_CL_IO_TX_BLOCKING = BIT(0),
 	MEI_CL_IO_TX_INTERNAL = BIT(1),
+
+	MEI_CL_IO_RX_NONBLOCK = BIT(2),
 };
 
 /*
@@ -318,7 +322,8 @@ void mei_cl_bus_rescan_work(struct work_struct *work);
 void mei_cl_bus_dev_fixup(struct mei_cl_device *dev);
 ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
 		      unsigned int mode);
-ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length);
+ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length,
+		      unsigned int mode);
 bool mei_cl_bus_rx_event(struct mei_cl *cl);
 bool mei_cl_bus_notify_event(struct mei_cl *cl);
 void mei_cl_bus_remove_devices(struct mei_device *bus);
diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
index 017f5232b3de..a0d274fe08f1 100644
--- a/include/linux/mei_cl_bus.h
+++ b/include/linux/mei_cl_bus.h
@@ -75,7 +75,7 @@ void mei_cldev_driver_unregister(struct mei_cl_driver *cldrv);
 /**
  * module_mei_cl_driver - Helper macro for registering mei cl driver
  *
- * @__mei_cldrv mei_cl_driver structure
+ * @__mei_cldrv: mei_cl_driver structure
  *
  *  Helper macro for mei cl drivers which do not do anything special in module
  *  init/exit, for eliminating a boilerplate code.
@@ -86,7 +86,9 @@ void mei_cldev_driver_unregister(struct mei_cl_driver *cldrv);
 		      mei_cldev_driver_unregister)
 
 ssize_t mei_cldev_send(struct mei_cl_device *cldev, u8 *buf, size_t length);
-ssize_t  mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length);
+ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length);
+ssize_t mei_cldev_recv_nonblock(struct mei_cl_device *cldev, u8 *buf,
+				size_t length);
 
 int mei_cldev_register_rx_cb(struct mei_cl_device *cldev, mei_cldev_cb_t rx_cb);
 int mei_cldev_register_notif_cb(struct mei_cl_device *cldev,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX
  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
  0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2016-11-19 12:18 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: Alexander Usyskin, linux-kernel

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...

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX
  2016-11-19 12:18 ` Greg Kroah-Hartman
@ 2016-11-19 16:16   ` Winkler, Tomas
  2016-11-28 23:03     ` Winkler, Tomas
  0 siblings, 1 reply; 8+ messages in thread
From: Winkler, Tomas @ 2016-11-19 16:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Usyskin, Alexander, linux-kernel


> 
> 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.

Thanks
Tomas

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX
  2016-11-19 16:16   ` Winkler, Tomas
@ 2016-11-28 23:03     ` Winkler, Tomas
  2016-11-29 19:16       ` 'Greg Kroah-Hartman'
  0 siblings, 1 reply; 8+ messages in thread
From: Winkler, Tomas @ 2016-11-28 23:03 UTC (permalink / raw)
  To: 'Greg Kroah-Hartman'
  Cc: Usyskin, Alexander, 'linux-kernel@vger.kernel.org'

> 
> 
> >
> > 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 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX
  2016-11-28 23:03     ` Winkler, Tomas
@ 2016-11-29 19:16       ` 'Greg Kroah-Hartman'
  2016-11-29 20:09         ` Winkler, Tomas
  0 siblings, 1 reply; 8+ messages in thread
From: 'Greg Kroah-Hartman' @ 2016-11-29 19:16 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: Usyskin, Alexander, 'linux-kernel@vger.kernel.org'

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX
  2016-11-29 19:16       ` 'Greg Kroah-Hartman'
@ 2016-11-29 20:09         ` Winkler, Tomas
  2016-11-30 11:57           ` 'Greg Kroah-Hartman'
  0 siblings, 1 reply; 8+ messages in thread
From: Winkler, Tomas @ 2016-11-29 20:09 UTC (permalink / raw)
  To: 'Greg Kroah-Hartman'
  Cc: Usyskin, Alexander, 'linux-kernel@vger.kernel.org'


> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX
  2016-11-29 20:09         ` Winkler, Tomas
@ 2016-11-30 11:57           ` 'Greg Kroah-Hartman'
  2016-11-30 12:00             ` Winkler, Tomas
  0 siblings, 1 reply; 8+ messages in thread
From: 'Greg Kroah-Hartman' @ 2016-11-30 11:57 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: Usyskin, Alexander, 'linux-kernel@vger.kernel.org'

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [char-misc-next 4/4 V2] mei: bus: enable non-blocking RX
  2016-11-30 11:57           ` 'Greg Kroah-Hartman'
@ 2016-11-30 12:00             ` Winkler, Tomas
  0 siblings, 0 replies; 8+ messages in thread
From: Winkler, Tomas @ 2016-11-30 12:00 UTC (permalink / raw)
  To: 'Greg Kroah-Hartman'
  Cc: Usyskin, Alexander, 'linux-kernel@vger.kernel.org'


> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-11-30 12:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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'
2016-11-30 12:00             ` Winkler, Tomas

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.