Messages in this thread | | | Date | Thu, 17 Nov 2016 16:37:19 +0100 | From | Greg Kroah-Hartman <> | Subject | Re: [char-misc-next 4/4] mei: bus: enable non-blocking RX |
| |
On Wed, Nov 16, 2016 at 10:51:30PM +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. > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> > --- > drivers/misc/mei/bus-fixup.c | 4 ++-- > drivers/misc/mei/bus.c | 19 ++++++++++++++++--- > drivers/misc/mei/mei_dev.h | 7 ++++++- > drivers/nfc/mei_phy.c | 7 ++++--- > drivers/watchdog/mei_wdt.c | 2 +- > include/linux/mei_cl_bus.h | 3 ++- > 6 files changed, 31 insertions(+), 11 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..45e937937675 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)) { > @@ -197,14 +205,19 @@ EXPORT_SYMBOL_GPL(mei_cldev_send); > * @cldev: me client device > * @buf: buffer to receive > * @length: buffer length > + * @flags: read io flags [O_NONBLOCK] > * > * Return: read size in bytes of < 0 on error > */ > -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, > + unsigned int flags) > { > struct mei_cl *cl = cldev->cl; > + unsigned int mode; > + > + mode = (flags & O_NONBLOCK) ? MEI_CL_IO_RX_NONBLOCK : 0; > > - return __mei_cl_recv(cl, buf, length); > + return __mei_cl_recv(cl, buf, length, mode); > } > 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/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c > index 8a04c5e02999..0e8158c5f81b 100644 > --- a/drivers/nfc/mei_phy.c > +++ b/drivers/nfc/mei_phy.c > @@ -132,7 +132,8 @@ static int mei_nfc_if_version(struct nfc_mei_phy *phy) > if (!reply) > return -ENOMEM; > > - bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply, if_version_length); > + bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply, > + if_version_length, 0); > if (bytes_recv < 0 || bytes_recv < if_version_length) { > pr_err("Could not read IF version\n"); > r = -EIO; > @@ -193,7 +194,7 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy) > } > > bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply, > - connect_resp_length); > + connect_resp_length, 0); > if (bytes_recv < 0) { > r = bytes_recv; > pr_err("Could not read connect response %d\n", r); > @@ -279,7 +280,7 @@ static int mei_nfc_recv(struct nfc_mei_phy *phy, u8 *buf, size_t length) > struct mei_nfc_hdr *hdr; > int received_length; > > - received_length = mei_cldev_recv(phy->cldev, buf, length); > + received_length = mei_cldev_recv(phy->cldev, buf, length, 0); > if (received_length < 0) > return received_length; > > diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c > index b29c6fde7473..3e29af534f8e 100644 > --- a/drivers/watchdog/mei_wdt.c > +++ b/drivers/watchdog/mei_wdt.c > @@ -423,7 +423,7 @@ static void mei_wdt_rx(struct mei_cl_device *cldev) > const size_t res_len = sizeof(res); > int ret; > > - ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len); > + ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len, 0); > if (ret < 0) { > dev_err(&cldev->dev, "failure in recv %d\n", ret); > return; > diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h > index 017f5232b3de..c3bb565f0745 100644 > --- a/include/linux/mei_cl_bus.h > +++ b/include/linux/mei_cl_bus.h > @@ -86,7 +86,8 @@ 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, > + unsigned int flags);
Functions like this are horrid! You have to always go around and dig up what "flags" means, and if 1 means this or 0 means that or whatever.
It makes it so you can not read the code that calls this function at all and know what is going on.
Just make a new function mei_cldev_recv_async() and then call a local, static function, that does the work with the correct flag set. That way the developer always knows exactly what is going on.
thanks,
greg k-h
| |