From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7BEC4C433FE for ; Mon, 7 Dec 2020 02:56:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 362C022CA0 for ; Mon, 7 Dec 2020 02:56:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728661AbgLGC4J (ORCPT ); Sun, 6 Dec 2020 21:56:09 -0500 Received: from smtprelay0214.hostedemail.com ([216.40.44.214]:56126 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726400AbgLGC4I (ORCPT ); Sun, 6 Dec 2020 21:56:08 -0500 Received: from filter.hostedemail.com (clb03-v110.bra.tucows.net [216.40.38.60]) by smtprelay04.hostedemail.com (Postfix) with ESMTP id 2B01B180A7FDB; Mon, 7 Dec 2020 02:55:27 +0000 (UTC) X-Session-Marker: 6A6F6540706572636865732E636F6D X-HE-Tag: food65_0413041273da X-Filterd-Recvd-Size: 4763 Received: from XPS-9350.home (unknown [47.151.137.21]) (Authenticated sender: joe@perches.com) by omf13.hostedemail.com (Postfix) with ESMTPA; Mon, 7 Dec 2020 02:55:24 +0000 (UTC) Message-ID: Subject: Re: [PATCH 21/22] xlink-core: add async channel and events From: Joe Perches To: mgross@linux.intel.com, markgross@kernel.org, arnd@arndb.de, bp@suse.de, damien.lemoal@wdc.com, dragan.cvetic@xilinx.com, gregkh@linuxfoundation.org, corbet@lwn.net, leonard.crestez@nxp.com, palmerdabbelt@google.com, paul.walmsley@sifive.com, peng.fan@nxp.com, robh+dt@kernel.org, shawnguo@kernel.org Cc: linux-kernel@vger.kernel.org, Seamus Kelly Date: Sun, 06 Dec 2020 18:55:23 -0800 In-Reply-To: <20201201223511.65542-22-mgross@linux.intel.com> References: <20201201223511.65542-1-mgross@linux.intel.com> <20201201223511.65542-22-mgross@linux.intel.com> Content-Type: text/plain; charset="ISO-8859-1" User-Agent: Evolution 3.38.1-1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2020-12-01 at 14:35 -0800, mgross@linux.intel.com wrote: > Enable asynchronous channel and event communication. [] > diff --git a/drivers/misc/xlink-core/xlink-core.c b/drivers/misc/xlink-core/xlink-core.c [] > + > +// sysfs attribute functions > + > +static ssize_t event0_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + struct keembay_xlink_dev *xlink_dev = dev_get_drvdata(dev); > + struct xlink_attr *a = &xlink_dev->eventx[0]; > + > + return scnprintf(buf, PAGE_SIZE, "0x%x 0x%lx\n", a->sw_dev_id, a->value); > +} > + > +static ssize_t event1_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + struct keembay_xlink_dev *xlink_dev = dev_get_drvdata(dev); > + struct xlink_attr *a = &xlink_dev->eventx[1]; > + > + return scnprintf(buf, PAGE_SIZE, "0x%x 0x%lx\n", a->sw_dev_id, a->value); > +} > + > +static ssize_t event2_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + struct keembay_xlink_dev *xlink_dev = dev_get_drvdata(dev); > + struct xlink_attr *a = &xlink_dev->eventx[2]; > + > + return scnprintf(buf, PAGE_SIZE, "0x%x 0x%lx\n", a->sw_dev_id, a->value); > +} > + > +static ssize_t event3_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + struct keembay_xlink_dev *xlink_dev = dev_get_drvdata(dev); > + struct xlink_attr *a = &xlink_dev->eventx[3]; > + > + return scnprintf(buf, PAGE_SIZE, "0x%x 0x%lx\n", a->sw_dev_id, a->value); > +} These could use the sysfs_emit function and not scnprintf and as well could use a single shared function with an index. Something like: static ssize_t eventx_show(struct device *dev, struct device_attr *attr, int index, char *buf) { struct keembay_xlink_dev *xlink_dev = dev_get_drvdata(dev); struct xlink_attr *a = &xlink_dev->eventx[index]; return sysfs_emit(buf, "0x%x 0x%lx\n", a->sw_dev_id, a->value); } static ssize_t event0_show(struct device *dev, struct device_attribute *attr, char *buf) { return eventx_show(dev, attr, 0, buf); } etc... > @@ -270,19 +353,23 @@ static long xlink_ioctl(struct file *file, unsigned int cmd, unsigned long arg) >   struct xlinkconnect con = {0}; >   struct xlinkrelease rel = {0}; >   struct xlinkstartvpu startvpu = {0}; > + struct xlinkcallback cb = {0}; >   struct xlinkgetdevicename devn = {0}; >   struct xlinkgetdevicelist devl = {0}; >   struct xlinkgetdevicestatus devs = {0}; >   struct xlinkbootdevice boot = {0}; >   struct xlinkresetdevice res = {0}; >   struct xlinkdevmode devm = {0}; > + struct xlinkregdevevent regdevevent = {0}; Perhaps significantly less stack would be used if these declarations were moved into the case blocks where used. Less initialization would be done per ioctl too. > @@ -318,6 +405,30 @@ static long xlink_ioctl(struct file *file, unsigned int cmd, unsigned long arg) >   if (copy_to_user((void __user *)op.return_code, (void *)&rc, sizeof(rc))) >   return -EFAULT; >   break; > + case XL_DATA_READY_CALLBACK: > + if (copy_from_user(&cb, (void __user *)arg, > + sizeof(struct xlinkcallback))) > + return -EFAULT; > + if (copy_from_user(&devh, (void __user *)cb.handle, > + sizeof(struct xlink_handle))) > + return -EFAULT; > + CHANNEL_SET_USER_BIT(cb.chan); // set MSbit for user space call > + rc = xlink_data_available_event(&devh, cb.chan, cb.callback); > + if (copy_to_user((void __user *)cb.return_code, (void *)&rc, sizeof(rc))) > + return -EFAULT; > + break; case XL_DATA_READY_CALLBACK: { struct xlinkcallback cb = {}; etc... break; }